CCIP-3736 FeeQuoter gas optimizations and redundancy/sanity checks (#…

…14805)

* gas optimizations and redundancy/sanity checks

* Update gethwrappers

* formatting, shapshotting, and changesetting

* CCIP-3736 update foundry and snapshot again

* [Bot] Update changeset file with jira issues

* Update gethwrappers

* snapshot fix and missing chageset file

* snapshot CI tests are the bane of my existence

* fill in coverage gap

* forge fmt

* Update gethwrappers

* CI checks are murphy's law, anything can happen at any time for no reason

* Update gethwrappers

* gas fix

* i will never understand why the gas changes on every single run.

* Update FeeQuoter.sol

* [Bot] Update changeset file with jira issues

* gas snapshot update after merge

* Update gethwrappers

* rm changeset

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
Co-authored-by: Rens Rooimans <[email protected]>

contracts/.changeset/happy-crabs-rescue.md

@@ -0,0 +1,10 @@
+---
+'@chainlink/contracts': patch
+---
+
+Gas optimizations and comment cleanup #internal
+
+
+PR issue: CCIP-3736
+
+Solidity Review issue: CCIP-3966

contracts/gas-snapshots/ccip.gas-snapshot

@@ -19,7 +19,7 @@ BurnWithFromMintTokenPool_lockOrBurn:test_ChainNotAllowed_Revert() (gas: 28842)
 BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas: 55271)
 BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurn_Success() (gas: 244092)
 BurnWithFromMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 24189)
-CCIPClientExample_sanity:test_ImmutableExamples_Success() (gas: 2108381)
+CCIPClientExample_sanity:test_ImmutableExamples_Success() (gas: 2108425)
 CCIPHome__validateConfig:test__validateConfigLessTransmittersThanSigners_Success() (gas: 332601)
 CCIPHome__validateConfig:test__validateConfigSmallerFChain_Success() (gas: 458549)
 CCIPHome__validateConfig:test__validateConfig_ABIEncodedAddress_OfframpAddressCannotBeZero_Reverts() (gas: 289173)
@@ -67,7 +67,7 @@ CCIPHome_setCandidate:test_setCandidate_ConfigDigestMismatch_reverts() (gas: 139
 CCIPHome_setCandidate:test_setCandidate_success() (gas: 1365381)
 DefensiveExampleTest:test_HappyPath_Success() (gas: 200048)
 DefensiveExampleTest:test_Recovery() (gas: 424306)
-E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1516795)
+E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1516297)
 EtherSenderReceiverTest_ccipReceive:test_ccipReceive_fallbackToWethTransfer() (gas: 96909)
 EtherSenderReceiverTest_ccipReceive:test_ccipReceive_happyPath() (gas: 49796)
 EtherSenderReceiverTest_ccipReceive:test_ccipReceive_wrongToken() (gas: 17435)
@@ -101,14 +101,14 @@ FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_OnlyCallableByOwnerOrAdmin
 FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_applyPremiumMultiplierWeiPerEthUpdatesMultipleTokens_Success() (gas: 54728)
 FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_applyPremiumMultiplierWeiPerEthUpdatesSingleToken_Success() (gas: 45219)
 FeeQuoter_applyPremiumMultiplierWeiPerEthUpdates:test_applyPremiumMultiplierWeiPerEthUpdatesZeroInput() (gas: 12376)
-FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeConfig_Success() (gas: 87721)
+FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeConfig_Success() (gas: 87827)
 FeeQuoter_applyTokenTransferFeeConfigUpdates:test_ApplyTokenTransferFeeZeroInput() (gas: 13233)
-FeeQuoter_applyTokenTransferFeeConfigUpdates:test_InvalidDestBytesOverhead_Revert() (gas: 17344)
+FeeQuoter_applyTokenTransferFeeConfigUpdates:test_InvalidDestBytesOverhead_Revert() (gas: 17397)
 FeeQuoter_applyTokenTransferFeeConfigUpdates:test_OnlyCallableByOwnerOrAdmin_Revert() (gas: 12330)
-FeeQuoter_constructor:test_InvalidLinkTokenEqZeroAddress_Revert() (gas: 106689)
-FeeQuoter_constructor:test_InvalidMaxFeeJuelsPerMsg_Revert() (gas: 111039)
-FeeQuoter_constructor:test_InvalidStalenessThreshold_Revert() (gas: 111092)
-FeeQuoter_constructor:test_Setup_Success() (gas: 5064266)
+FeeQuoter_constructor:test_InvalidLinkTokenEqZeroAddress_Revert() (gas: 106738)
+FeeQuoter_constructor:test_InvalidMaxFeeJuelsPerMsg_Revert() (gas: 111088)
+FeeQuoter_constructor:test_InvalidStalenessThreshold_Revert() (gas: 111141)
+FeeQuoter_constructor:test_Setup_Success() (gas: 5086711)
 FeeQuoter_convertTokenAmount:test_ConvertTokenAmount_Success() (gas: 68361)
 FeeQuoter_convertTokenAmount:test_LinkTokenNotSupported_Revert() (gas: 29234)
 FeeQuoter_getDataAvailabilityCost:test_EmptyMessageCalculatesDataAvailabilityCost_Success() (gas: 96069)
@@ -129,7 +129,7 @@ FeeQuoter_getTokenTransferCost:test_MixedTokenTransferFee_Success() (gas: 97630)
 FeeQuoter_getTokenTransferCost:test_NoTokenTransferChargesZeroFee_Success() (gas: 20506)
 FeeQuoter_getTokenTransferCost:test_SmallTokenTransferChargesMinFeeAndGas_Success() (gas: 27823)
 FeeQuoter_getTokenTransferCost:test_ZeroAmountTokenTransferChargesMinFeeAndGas_Success() (gas: 27846)
-FeeQuoter_getTokenTransferCost:test_ZeroFeeConfigChargesMinFee_Success() (gas: 40415)
+FeeQuoter_getTokenTransferCost:test_ZeroFeeConfigChargesMinFee_Success() (gas: 40609)
 FeeQuoter_getTokenTransferCost:test_getTokenTransferCost_selfServeUsesDefaults_Success() (gas: 29542)
 FeeQuoter_getValidatedFee:test_DestinationChainNotEnabled_Revert() (gas: 18377)
 FeeQuoter_getValidatedFee:test_EmptyMessage_Success() (gas: 82916)
@@ -141,7 +141,7 @@ FeeQuoter_getValidatedFee:test_MessageTooLarge_Revert() (gas: 100353)
 FeeQuoter_getValidatedFee:test_MessageWithDataAndTokenTransfer_Success() (gas: 142587)
 FeeQuoter_getValidatedFee:test_NotAFeeToken_Revert() (gas: 21234)
 FeeQuoter_getValidatedFee:test_SingleTokenMessage_Success() (gas: 113938)
-FeeQuoter_getValidatedFee:test_TooManyTokens_Revert() (gas: 22752)
+FeeQuoter_getValidatedFee:test_TooManyTokens_Revert() (gas: 23409)
 FeeQuoter_getValidatedFee:test_ZeroDataAvailabilityMultiplier_Success() (gas: 63711)
 FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Above18Decimals_Success() (gas: 1974510)
 FeeQuoter_getValidatedTokenPrice:test_GetValidatedTokenPriceFromFeedErc20Below18Decimals_Success() (gas: 1974468)
@@ -168,15 +168,16 @@ FeeQuoter_parseEVMExtraArgsFromBytes:test_EVMExtraArgsGasLimitTooHigh_Revert() (
 FeeQuoter_parseEVMExtraArgsFromBytes:test_EVMExtraArgsInvalidExtraArgsTag_Revert() (gas: 18198)
 FeeQuoter_parseEVMExtraArgsFromBytes:test_EVMExtraArgsV1_Success() (gas: 18557)
 FeeQuoter_parseEVMExtraArgsFromBytes:test_EVMExtraArgsV2_Success() (gas: 18680)
+FeeQuoter_processMessageArgs:test_applyTokensTransferFeeConfigUpdates_InvalidFeeRange_Revert() (gas: 21372)
 FeeQuoter_processMessageArgs:test_processMessageArgs_InvalidEVMAddressDestToken_Revert() (gas: 44681)
 FeeQuoter_processMessageArgs:test_processMessageArgs_InvalidExtraArgs_Revert() (gas: 19914)
 FeeQuoter_processMessageArgs:test_processMessageArgs_MalformedEVMExtraArgs_Revert() (gas: 20311)
 FeeQuoter_processMessageArgs:test_processMessageArgs_MessageFeeTooHigh_Revert() (gas: 17882)
-FeeQuoter_processMessageArgs:test_processMessageArgs_SourceTokenDataTooLarge_Revert() (gas: 122599)
+FeeQuoter_processMessageArgs:test_processMessageArgs_SourceTokenDataTooLarge_Revert() (gas: 122505)
 FeeQuoter_processMessageArgs:test_processMessageArgs_TokenAmountArraysMismatching_Revert() (gas: 42010)
 FeeQuoter_processMessageArgs:test_processMessageArgs_WitEVMExtraArgsV2_Success() (gas: 28534)
 FeeQuoter_processMessageArgs:test_processMessageArgs_WithConvertedTokenAmount_Success() (gas: 29927)
-FeeQuoter_processMessageArgs:test_processMessageArgs_WithCorrectPoolReturnData_Success() (gas: 76123)
+FeeQuoter_processMessageArgs:test_processMessageArgs_WithCorrectPoolReturnData_Success() (gas: 75957)
 FeeQuoter_processMessageArgs:test_processMessageArgs_WithEVMExtraArgsV1_Success() (gas: 28132)
 FeeQuoter_processMessageArgs:test_processMessageArgs_WithEmptyEVMExtraArgs_Success() (gas: 26003)
 FeeQuoter_processMessageArgs:test_processMessageArgs_WithLinkTokenAmount_Success() (gas: 19501)
@@ -519,11 +520,11 @@ OnRamp_forwardFromRouter:test_ShouldIncrementNonceOnlyOnOrdered_Success() (gas:
 OnRamp_forwardFromRouter:test_ShouldIncrementSeqNumAndNonce_Success() (gas: 212648)
 OnRamp_forwardFromRouter:test_ShouldStoreLinkFees() (gas: 146934)
 OnRamp_forwardFromRouter:test_ShouldStoreNonLinkFees() (gas: 161127)
-OnRamp_forwardFromRouter:test_SourceTokenDataTooLarge_Revert() (gas: 3615161)
+OnRamp_forwardFromRouter:test_SourceTokenDataTooLarge_Revert() (gas: 3615067)
 OnRamp_forwardFromRouter:test_UnAllowedOriginalSender_Revert() (gas: 24016)
 OnRamp_forwardFromRouter:test_UnsupportedToken_Revert() (gas: 75850)
 OnRamp_forwardFromRouter:test_forwardFromRouter_UnsupportedToken_Revert() (gas: 38583)
-OnRamp_forwardFromRouter:test_forwardFromRouter_WithInterception_Success() (gas: 280235)
+OnRamp_forwardFromRouter:test_forwardFromRouter_WithInterception_Success() (gas: 280047)
 OnRamp_getFee:test_EmptyMessage_Success() (gas: 98689)
 OnRamp_getFee:test_EnforceOutOfOrder_Revert() (gas: 65468)
 OnRamp_getFee:test_GetFeeOfZeroForTokenMessage_Success() (gas: 87080)
@@ -612,7 +613,7 @@ Router_applyRampUpdates:test_OffRampUpdatesWithRouting() (gas: 10663332)
 Router_applyRampUpdates:test_OnRampDisable() (gas: 56007)
 Router_applyRampUpdates:test_OnlyOwner_Revert() (gas: 12334)
 Router_ccipSend:test_CCIPSendLinkFeeNoTokenSuccess_gas() (gas: 131252)
-Router_ccipSend:test_CCIPSendLinkFeeOneTokenSuccess_gas() (gas: 220952)
+Router_ccipSend:test_CCIPSendLinkFeeOneTokenSuccess_gas() (gas: 220974)
 Router_ccipSend:test_FeeTokenAmountTooLow_Revert() (gas: 71675)
 Router_ccipSend:test_InvalidMsgValue() (gas: 32267)
 Router_ccipSend:test_NativeFeeTokenInsufficientValue() (gas: 69363)
@@ -624,7 +625,7 @@ Router_ccipSend:test_UnsupportedDestinationChain_Revert() (gas: 24900)
 Router_ccipSend:test_WhenNotHealthy_Revert() (gas: 44902)
 Router_ccipSend:test_WrappedNativeFeeToken_Success() (gas: 193844)
 Router_ccipSend:test_ccipSend_nativeFeeNoTokenSuccess_gas() (gas: 140513)
-Router_ccipSend:test_ccipSend_nativeFeeOneTokenSuccess_gas() (gas: 230126)
+Router_ccipSend:test_ccipSend_nativeFeeOneTokenSuccess_gas() (gas: 230148)
 Router_constructor:test_Constructor_Success() (gas: 13076)
 Router_getArmProxy:test_getArmProxy() (gas: 10573)
 Router_getFee:test_GetFeeSupportedChain_Success() (gas: 51716)

contracts/src/v0.8/ccip/FeeQuoter.sol

@@ -42,7 +42,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
   error MessageFeeTooHigh(uint256 msgFeeJuels, uint256 maxFeeJuelsPerMsg);
   error InvalidStaticConfig();
   error MessageTooLarge(uint256 maxSize, uint256 actualSize);
-  error UnsupportedNumberOfTokens();
+  error UnsupportedNumberOfTokens(uint256 numberOfTokens, uint256 maxNumberOfTokensPerMsg);
+  error InvalidFeeRange(uint256 minFeeUSDCents, uint256 maxFeeUSDCents);
 
   event FeeTokenAdded(address indexed feeToken);
   event FeeTokenRemoved(address indexed feeToken);
@@ -522,6 +523,7 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
         revert StaleKeystoneUpdate(feeds[i].token, feeds[i].timestamp, s_usdPerToken[feeds[i].token].timestamp);
       }
 
+      // Update the token price with the new value and timestamp
       s_usdPerToken[feeds[i].token] =
         Internal.TimestampedPackedUint224({value: rebasedValue, timestamp: feeds[i].timestamp});
       emit UsdPerTokenUpdated(feeds[i].token, rebasedValue, feeds[i].timestamp);
@@ -586,8 +588,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     // We then multiply this gas total with the gas multiplier and gas price, converting it into USD with 36 decimals.
     // uint112(packedGasPrice) = executionGasPrice
 
-    // NOTE: when supporting non-EVM chains, revisit how generic this fee logic can be
-    // NOTE: revisit parsing non-EVM args
+    // NOTE: Fee logic is currently only supported for EVM-Chains, and the gas price is assumed to be in wei.
+    // fee logic for other chains should be implemented in the future.
     uint256 executionCost = uint112(packedGasPrice)
       * (
         destChainConfig.destGasOverhead + (message.data.length * destChainConfig.destGasPerPayloadByte) + tokenTransferGas
@@ -804,6 +806,10 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
           tokenTransferFeeConfigArg.tokenTransferFeeConfigs[j].tokenTransferFeeConfig;
         address token = tokenTransferFeeConfigArg.tokenTransferFeeConfigs[j].token;
 
+        if (tokenTransferFeeConfig.minFeeUSDCents >= tokenTransferFeeConfig.maxFeeUSDCents) {
+          revert InvalidFeeRange(tokenTransferFeeConfig.minFeeUSDCents, tokenTransferFeeConfig.maxFeeUSDCents);
+        }
+
         if (tokenTransferFeeConfig.destBytesOverhead < Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES) {
           revert InvalidDestBytesOverhead(token, tokenTransferFeeConfig.destBytesOverhead);
         }
@@ -849,6 +855,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
       _parseUnvalidatedEVMExtraArgsFromBytes(extraArgs, destChainConfig.defaultTxGasLimit);
 
     if (evmExtraArgs.gasLimit > uint256(destChainConfig.maxPerMsgGasLimit)) revert MessageGasLimitTooHigh();
+
+    // If the chain enforces out of order execution, the extra args must allow it, otherwise revert
     if (destChainConfig.enforceOutOfOrder && !evmExtraArgs.allowOutOfOrderExecution) {
       revert ExtraArgOutOfOrderExecutionMustBeTrue();
     }
@@ -899,7 +907,9 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     if (dataLength > uint256(destChainConfig.maxDataBytes)) {
       revert MessageTooLarge(uint256(destChainConfig.maxDataBytes), dataLength);
     }
-    if (numberOfTokens > uint256(destChainConfig.maxNumberOfTokensPerMsg)) revert UnsupportedNumberOfTokens();
+    if (numberOfTokens > uint256(destChainConfig.maxNumberOfTokensPerMsg)) {
+      revert UnsupportedNumberOfTokens(numberOfTokens, destChainConfig.maxNumberOfTokensPerMsg);
+    }
     _validateDestFamilyAddress(destChainConfig.chainFamilySelector, receiver);
   }
 
@@ -932,8 +942,10 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
     if (msgFeeJuels > i_maxFeeJuelsPerMsg) revert MessageFeeTooHigh(msgFeeJuels, i_maxFeeJuelsPerMsg);
 
     uint64 defaultTxGasLimit = s_destChainConfigs[destChainSelector].defaultTxGasLimit;
-    // NOTE: when supporting non-EVM chains, revisit this and parse non-EVM args.
-    // We can parse unvalidated args since this message is called after getFee (which will already validate the params)
+
+    // NOTE: Only EVM chains are supported for now, additional validation logic will be added when supporting other
+    // chain families to parse non-EVM args
+    // Since the message is called after getFee, which will already validate the params, no validation is necessary
     Client.EVMExtraArgsV2 memory parsedExtraArgs = _parseUnvalidatedEVMExtraArgsFromBytes(extraArgs, defaultTxGasLimit);
     isOutOfOrderExecution = parsedExtraArgs.allowOutOfOrderExecution;
     destExecDataPerToken = _processPoolReturnData(destChainSelector, onRampTokenTransfers, sourceTokenAmounts);
@@ -969,10 +981,12 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
       _validateDestFamilyAddress(chainFamilySelector, onRampTokenTransfers[i].destTokenAddress);
       FeeQuoter.TokenTransferFeeConfig memory tokenTransferFeeConfig =
         s_tokenTransferFeeConfig[destChainSelector][sourceToken];
-      uint32 defaultGasOverhead = s_destChainConfigs[destChainSelector].defaultTokenDestGasOverhead;
-      // NOTE: Revisit this when adding new non-EVM chain family selector support
-      uint32 destGasAmount =
-        tokenTransferFeeConfig.isEnabled ? tokenTransferFeeConfig.destGasOverhead : defaultGasOverhead;
+
+      // NOTE: Only EVM chains' gas model is supported for now, additional fee logic for non-EVM chains will
+      // be required in the future with parsing based on chain family selector.
+      uint32 destGasAmount = tokenTransferFeeConfig.isEnabled
+        ? tokenTransferFeeConfig.destGasOverhead
+        : s_destChainConfigs[destChainSelector].defaultTokenDestGasOverhead;
 
       // The user will be billed either the default or the override, so we send the exact amount that we billed for
       // to the destination chain to be used for the token releaseOrMint and transfer.
@@ -1011,7 +1025,8 @@ contract FeeQuoter is AuthorizedCallers, IFeeQuoter, ITypeAndVersion, IReceiver,
       uint64 destChainSelector = destChainConfigArgs[i].destChainSelector;
       DestChainConfig memory destChainConfig = destChainConfigArg.destChainConfig;
 
-      // NOTE: when supporting non-EVM chains, update chainFamilySelector validations
+      // destChainSelector must be non-zero, defaultTxGasLimit must be set, and must be less than maxPerMsgGasLimit
+      // Only EVM chains are supported for now, additional validation logic will be added when supporting other chain families
       if (
         destChainSelector == 0 || destChainConfig.defaultTxGasLimit == 0
           || destChainConfig.chainFamilySelector != Internal.CHAIN_FAMILY_SELECTOR_EVM

contracts/src/v0.8/ccip/interfaces/IFeeQuoter.sol

@@ -17,8 +17,8 @@ interface IFeeQuoter is IPriceRegistry {
 
   /// @notice Converts the extraArgs to the latest version and returns the converted message fee in juels
   /// @notice Validates pool return data
-  /// @param destChainSelector destination chain selector to process
-  /// @param feeToken Fee token address used to pay for message fees
+  /// @param destChainSelector destination chain selector to process, must be a configured valid chain
+  /// @param feeToken token address used to pay for message fees, must be a configured valid fee token
   /// @param feeTokenAmount Fee token amount
   /// @param extraArgs Message extra args that were passed in by the client
   /// @param onRampTokenTransfers Token amounts with populated pool return data