Misc onchain fixes (#1212)

Various minor onchain fixes

- improve accounting for token payload data
- rm old check in usdc pool
- fix comment on precompile range
- add transferLiquidity function to LockReleaseTokenPool
- move rate limit admin logic from lockRelease pool to TokenPool

New changes

- removed defaultTokenDestBytesOverhead and used
CCIP_LOCK_OR_BURN_V1_RET_BYTES as default value
- Add uint32 destGasAmount to the SourceTokenData struct to allow us to
send the amount we billed on source to dest
- This single value is used for the releaseOrMint and what is left after
that call is used for transfer
- Removed the defaults for releaseOrMint and transfer from the offRamp

---------

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

contracts/gas-snapshots/liquiditymanager.gas-snapshot

@@ -3,9 +3,9 @@ LiquidityManager_addLiquidity:test_addLiquiditySuccess() (gas: 279154)
 LiquidityManager_rebalanceLiquidity:test_InsufficientLiquidityReverts() (gas: 206745)
 LiquidityManager_rebalanceLiquidity:test_InvalidRemoteChainReverts() (gas: 192319)
 LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess() (gas: 9141768)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8898695)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8893901)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8821699)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8957594)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8952800)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8880598)
 LiquidityManager_rebalanceLiquidity:test_rebalanceLiquiditySuccess() (gas: 382897)
 LiquidityManager_receive:test_receive_success() (gas: 21182)
 LiquidityManager_removeLiquidity:test_InsufficientLiquidityReverts() (gas: 184869)
@@ -19,7 +19,7 @@ LiquidityManager_setFinanceRole:test_OnlyOwnerReverts() (gas: 10987)
 LiquidityManager_setFinanceRole:test_setFinanceRoleSuccess() (gas: 21836)
 LiquidityManager_setLocalLiquidityContainer:test_OnlyOwnerReverts() (gas: 11052)
 LiquidityManager_setLocalLiquidityContainer:test_ReverstWhen_CalledWithTheZeroAddress() (gas: 10643)
-LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3436651)
+LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3495598)
 LiquidityManager_setMinimumLiquidity:test_OnlyOwnerReverts() (gas: 10925)
 LiquidityManager_setMinimumLiquidity:test_setMinimumLiquiditySuccess() (gas: 36389)
 LiquidityManager_withdrawERC20:test_withdrawERC20Reverts() (gas: 180359)

contracts/src/v0.8/ccip/libraries/Internal.sol

@@ -65,6 +65,7 @@ library Internal {
     // CCIP_LOCK_OR_BURN_V1_RET_BYTES bytes. If more data is required, the TokenTransferFeeConfig.destBytesOverhead
     // has to be set for the specific token.
     bytes extraData;
+    uint32 destGasAmount; // The amount of gas available for the releaseOrMint and transfer calls on the offRamp
   }
 
   /// @notice Report that is submitted by the execution DON at the execution phase. (including chain selector data)
@@ -91,19 +92,19 @@ library Internal {
   /// @notice The cross chain message that gets committed to EVM chains.
   /// @dev RMN depends on this struct, if changing, please notify the RMN maintainers.
   struct EVM2EVMMessage {
-    uint64 sourceChainSelector; // ───────────╮ the chain selector of the source chain, note: not chainId
-    address sender; // ───────────────────────╯ sender address on the source chain
-    address receiver; // ─────────────────────╮ receiver address on the destination chain
-    uint64 sequenceNumber; // ────────────────╯ sequence number, not unique across lanes
-    uint256 gasLimit; //                        user supplied maximum gas amount available for dest chain execution
-    bool strict; // ──────────────────────────╮ DEPRECATED
-    uint64 nonce; //                          │ nonce for this lane for this sender, not unique across senders/lanes
-    address feeToken; // ─────────────────────╯ fee token
-    uint256 feeTokenAmount; //                  fee token amount
-    bytes data; //                              arbitrary data payload supplied by the message sender
-    Client.EVMTokenAmount[] tokenAmounts; //    array of tokens and amounts to transfer
-    bytes[] sourceTokenData; //                 array of token data, one per token
-    bytes32 messageId; //                       a hash of the message data
+    uint64 sourceChainSelector; // ────────╮ the chain selector of the source chain, note: not chainId
+    address sender; // ────────────────────╯ sender address on the source chain
+    address receiver; // ──────────────────╮ receiver address on the destination chain
+    uint64 sequenceNumber; // ─────────────╯ sequence number, not unique across lanes
+    uint256 gasLimit; //                     user supplied maximum gas amount available for dest chain execution
+    bool strict; // ───────────────────────╮ DEPRECATED
+    uint64 nonce; //                       │ nonce for this lane for this sender, not unique across senders/lanes
+    address feeToken; // ──────────────────╯ fee token
+    uint256 feeTokenAmount; //               fee token amount
+    bytes data; //                           arbitrary data payload supplied by the message sender
+    Client.EVMTokenAmount[] tokenAmounts; // array of tokens and amounts to transfer
+    bytes[] sourceTokenData; //              array of token data, one per token
+    bytes32 messageId; //                    a hash of the message data
   }
 
   /// @dev EVM2EVMMessage struct has 13 fields, including 3 variable arrays.
@@ -113,9 +114,20 @@ library Internal {
   /// For structs that contain arrays, 1 more slot is added to the front, reaching a total of 17.
   uint256 public constant MESSAGE_FIXED_BYTES = 32 * 17;
 
-  /// @dev Each token transfer adds 1 EVMTokenAmount and 1 bytes.
-  /// When abiEncoded, each EVMTokenAmount takes 2 slots, each bytes takes 2 slots, excl bytes contents
-  uint256 public constant MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * 4;
+  /// @dev Each token transfer adds 1 EVMTokenAmount and 3 bytes at 3 slots each and one slot for the destGasAmount.
+  /// When abi encoded, each EVMTokenAmount takes 2 slots, each bytes takes 1 slot for length, one slot of data and one
+  /// slot for the offset. This results in effectively 3*3 slots per SourceTokenData.
+  /// 0x20
+  /// destGasAmount
+  /// sourcePoolAddress_offset
+  /// destTokenAddress_offset
+  /// extraData_offset
+  /// sourcePoolAddress_length
+  /// sourcePoolAddress_content // assume 1 slot
+  /// destTokenAddress_length
+  /// destTokenAddress_content // assume 1 slot
+  /// extraData_length // contents billed separately
+  uint256 public constant MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * ((1 + 3 * 3) + 2);
 
   /// @dev Any2EVMRampMessage struct has 10 fields, including 3 variable unnested arrays (data, receiver and tokenAmounts).
   /// Each variable array takes 1 more slot to store its length.
@@ -127,9 +139,9 @@ library Internal {
 
   /// @dev Each token transfer adds 1 RampTokenAmount
   /// RampTokenAmount has 4 fields, including 3 bytes.
-  /// Each bytes takes 1 more slot to store its length.
-  /// When abi encoded, each token transfer takes up 7 slots, excl bytes contents.
-  uint256 public constant ANY_2_EVM_MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * 7;
+  /// Each bytes takes 1 more slot to store its length, and one slot to store the offset.
+  /// When abi encoded, each token transfer takes up 10 slots, excl bytes contents.
+  uint256 public constant ANY_2_EVM_MESSAGE_FIXED_BYTES_PER_TOKEN = 32 * 10;
 
   bytes32 internal constant EVM_2_EVM_MESSAGE_HASH = keccak256("EVM2EVMMessageHashV2");
 
@@ -226,8 +238,12 @@ library Internal {
     );
   }
 
-  /// @dev We disallow the first 1024 addresses to never allow calling precompiles. It is extremely unlikely that
-  /// anyone would ever be able to generate an address in this range.
+  /// @dev We disallow the first 1024 addresses to avoid calling into a range known for hosting precompiles. Calling
+  /// into precompiles probably won't cause any issues, but to be safe we can disallow this range. It is extremely
+  /// unlikely that anyone would ever be able to generate an address in this range. There is no official range of
+  /// precompiles, but EIP-7587 proposes to reserve the range 0x100 to 0x1ff. Our range is more conservative, even
+  /// though it might not be exhaustive for all chains, which is OK. We also disallow the zero address, which is a
+  /// common practice.
   uint256 public constant PRECOMPILE_SPACE = 1024;
 
   /// @notice This methods provides validation for parsing abi encoded addresses by ensuring the
@@ -282,10 +298,10 @@ library Internal {
   /// The messageId is not expected to match hash(message), since it may originate from another ramp family
   struct RampMessageHeader {
     bytes32 messageId; // Unique identifier for the message, generated with the source chain's encoding scheme (i.e. not necessarily abi.encoded)
-    uint64 sourceChainSelector; // ───────╮ the chain selector of the source chain, note: not chainId
-    uint64 destChainSelector; //          | the chain selector of the destination chain, note: not chainId
-    uint64 sequenceNumber; //             │ sequence number, not unique across lanes
-    uint64 nonce; // ─────────────────────╯ nonce for this lane for this sender, not unique across senders/lanes
+    uint64 sourceChainSelector; // ──╮ the chain selector of the source chain, note: not chainId
+    uint64 destChainSelector; //     | the chain selector of the destination chain, note: not chainId
+    uint64 sequenceNumber; //        │ sequence number, not unique across lanes
+    uint64 nonce; // ────────────────╯ nonce for this lane for this sender, not unique across senders/lanes
   }
 
   /// @notice Family-agnostic message routed to an OffRamp

contracts/src/v0.8/ccip/libraries/Pool.sol

@@ -14,7 +14,7 @@ library Pool {
   // The default max number of bytes in the return data for a pool v1 lockOrBurn call.
   // This data can be used to send information to the destination chain token pool. Can be overwritten
   // in the TokenTransferFeeConfig.destBytesOverhead if more data is required.
-  uint256 public constant CCIP_LOCK_OR_BURN_V1_RET_BYTES = 32;
+  uint32 public constant CCIP_LOCK_OR_BURN_V1_RET_BYTES = 32;
 
   struct LockOrBurnInV1 {
     bytes receiver; //  The recipient of the tokens on the destination chain, abi encoded
@@ -25,7 +25,7 @@ library Pool {
   }
 
   struct LockOrBurnOutV1 {
-    // The address of the destination token pool, abi encoded in the case of EVM chains
+    // The address of the destination token, abi encoded in the case of EVM chains
     // This value is UNTRUSTED as any pool owner can return whatever value they want.
     bytes destTokenAddress;
     // Optional pool data to be transferred to the destination chain. Be default this is capped at

contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol

@@ -90,9 +90,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
     uint32 maxDataBytes; //                             │ Maximum payload data size in bytes
     uint16 maxNumberOfTokensPerMsg; //                  │ Maximum number of ERC20 token transfers that can be included per message
     address router; // ─────────────────────────────────╯ Router address
-    address priceRegistry; // ──────────╮ Price registry address
-    uint32 maxPoolReleaseOrMintGas; //  │ Maximum amount of gas passed on to token pool `releaseOrMint` call
-    uint32 maxTokenTransferGas; // ─────╯ Maximum amount of gas passed on to token `transfer` call
+    address priceRegistry; //                             Price registry address
   }
 
   /// @notice RateLimitToken struct containing both the source and destination token addresses
@@ -616,7 +614,8 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
     // contains a contract. If it doesn't it reverts with a known error, which we catch gracefully.
     // We call the pool with exact gas to increase resistance against malicious tokens or token pools.
     // We protects against return data bombs by capping the return data size at MAX_RET_BYTES.
-    (bool success, bytes memory returnData,) = CallWithExactGas._callWithExactGasSafeReturnData(
+    (bool success, bytes memory returnData, uint256 gasUsedReleaseOrMint) = CallWithExactGas
+      ._callWithExactGasSafeReturnData(
       abi.encodeCall(
         IPoolV1.releaseOrMint,
         Pool.ReleaseOrMintInV1({
@@ -631,7 +630,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
         })
       ),
       localPoolAddress,
-      s_dynamicConfig.maxPoolReleaseOrMintGas,
+      sourceTokenData.destGasAmount,
       Internal.GAS_FOR_CALL_EXACT_CHECK,
       Internal.MAX_RET_BYTES
     );
@@ -650,7 +649,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
     (success, returnData,) = CallWithExactGas._callWithExactGasSafeReturnData(
       abi.encodeCall(IERC20.transfer, (receiver, localAmount)),
       localToken,
-      s_dynamicConfig.maxTokenTransferGas,
+      sourceTokenData.destGasAmount - gasUsedReleaseOrMint,
       Internal.GAS_FOR_CALL_EXACT_CHECK,
       Internal.MAX_RET_BYTES
     );

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

@@ -96,8 +96,6 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
     // The following three properties are defaults, they can be overridden by setting the TokenTransferFeeConfig for a token
     uint16 defaultTokenFeeUSDCents; // ──────────╮ Default token fee charged per token transfer
     uint32 defaultTokenDestGasOverhead; //       │ Default gas charged to execute the token transfer on the destination chain
-    //                                           │ Default data availability bytes that are returned from the source pool and sent
-    uint32 defaultTokenDestBytesOverhead; //     | to the destination pool. Must be >= Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES
     bool enforceOutOfOrder; // ──────────────────╯ Whether to enforce the allowOutOfOrderExecution extraArg value to be true.
   }
 
@@ -304,11 +302,15 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
     }
     if (s_nopFeesJuels > i_maxNopFeesJuels) revert MaxFeeBalanceReached();
 
-    if (i_prevOnRamp != address(0)) {
-      if (s_senderNonce[originalSender] == 0) {
-        // If this is first time send for a sender in new OnRamp, check if they have a nonce
-        // from the previous OnRamp and start from there instead of zero.
-        s_senderNonce[originalSender] = IEVM2AnyOnRamp(i_prevOnRamp).getSenderNonce(originalSender);
+    // Get the current nonce if the message is an ordered message. If it's not ordered, we don't have to make the
+    // external call.
+    if (!extraArgs.allowOutOfOrderExecution) {
+      if (i_prevOnRamp != address(0)) {
+        if (s_senderNonce[originalSender] == 0) {
+          // If this is first time send for a sender in new OnRamp, check if they have a nonce
+          // from the previous OnRamp and start from there instead of zero.
+          s_senderNonce[originalSender] = IEVM2AnyOnRamp(i_prevOnRamp).getSenderNonce(originalSender);
+        }
       }
     }
 
@@ -359,18 +361,27 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
       // extraData. This prevents gas bomb attacks on the NOPs. As destBytesOverhead accounts for both
       // extraData and offchainData, this caps the worst case abuse to the number of bytes reserved for offchainData.
       if (poolReturnData.destPoolData.length > Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES) {
+        // If TokenTransferFeeConfig.enabled is false, there is no config. That means destBytesOverhead is zero and
+        // this check is always true. That ensures that a pool without config cannot send more than
+        // Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES bytes of data.
         if (poolReturnData.destPoolData.length > s_tokenTransferFeeConfig[tokenAndAmount.token].destBytesOverhead) {
           revert SourceTokenDataTooLarge(tokenAndAmount.token);
         }
       }
+
       // We validate the token address to ensure it is a valid EVM address
       Internal._validateEVMAddress(poolReturnData.destTokenAddress);
 
       newMessage.sourceTokenData[i] = abi.encode(
         Internal.SourceTokenData({
           sourcePoolAddress: abi.encode(sourcePool),
           destTokenAddress: poolReturnData.destTokenAddress,
-          extraData: poolReturnData.destPoolData
+          extraData: poolReturnData.destPoolData,
+          // 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.
+          destGasAmount: s_tokenTransferFeeConfig[tokenAndAmount.token].isEnabled
+            ? s_tokenTransferFeeConfig[tokenAndAmount.token].destGasOverhead
+            : s_dynamicConfig.defaultTokenDestGasOverhead
         })
       );
     }
@@ -464,10 +475,6 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
   function _setDynamicConfig(DynamicConfig memory dynamicConfig) internal {
     // We permit router to be set to zero as a way to pause the contract.
     if (dynamicConfig.priceRegistry == address(0)) revert InvalidConfig();
-    if (dynamicConfig.defaultTokenDestBytesOverhead < Pool.CCIP_LOCK_OR_BURN_V1_RET_BYTES) {
-      revert InvalidDestBytesOverhead(address(0), dynamicConfig.defaultTokenDestBytesOverhead);
-    }
-
     s_dynamicConfig = dynamicConfig;
 
     emit ConfigSet(
@@ -635,7 +642,7 @@ contract EVM2EVMOnRamp is IEVM2AnyOnRamp, ILinkAvailable, AggregateRateLimiter,
       if (!transferFeeConfig.isEnabled) {
         tokenTransferFeeUSDWei += uint256(s_dynamicConfig.defaultTokenFeeUSDCents) * 1e16;
         tokenTransferGas += s_dynamicConfig.defaultTokenDestGasOverhead;
-        tokenTransferBytesOverhead += s_dynamicConfig.defaultTokenDestBytesOverhead;
+        tokenTransferBytesOverhead += Pool.CCIP_POOL_V1_RET_BYTES;
         continue;
       }