Add admin reg to offramp (#990)

## Motivation
We want token pools to be upgradable. Having messages containing the
token pool address would result in sub-par upgradability.

## Solution

Replace the pool address in the msg with the token address. This means
we have to do an extra call to the token admin registry on the offRamp,
but it gives both upgradability and extra security.

.changeset/few-apples-speak.md

@@ -0,0 +1,5 @@
+---
+"ccip": patch
+---
+
+#changed add token admin registry to offRamp

contracts/.changeset/lovely-pumas-happen.md

@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts-ccip": patch
+---
+
+#changed add token admin registry to offRamp

contracts/gas-snapshots/liquiditymanager.gas-snapshot

@@ -1,11 +1,11 @@
 LiquidityManager__report:test_EmptyReportReverts() (gas: 11159)
-LiquidityManager_addLiquidity:test_addLiquiditySuccess() (gas: 279136)
+LiquidityManager_addLiquidity:test_addLiquiditySuccess() (gas: 279154)
 LiquidityManager_rebalanceLiquidity:test_InsufficientLiquidityReverts() (gas: 19585)
 LiquidityManager_rebalanceLiquidity:test_InvalidRemoteChainReverts() (gas: 190304)
 LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess() (gas: 8809026)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8667885)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8663093)
-LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8589907)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPoolsSuccess_AlreadyFinalized() (gas: 8757871)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_MultiStageFinalization() (gas: 8753079)
+LiquidityManager_rebalanceLiquidity:test_rebalanceBetweenPools_NativeRewrap() (gas: 8679893)
 LiquidityManager_rebalanceLiquidity:test_rebalanceLiquiditySuccess() (gas: 379308)
 LiquidityManager_removeLiquidity:test_InsufficientLiquidityReverts() (gas: 184481)
 LiquidityManager_removeLiquidity:test_OnlyOwnerReverts() (gas: 10967)
@@ -16,7 +16,7 @@ LiquidityManager_setCrossChainRebalancer:test_ZeroChainSelectorReverts() (gas: 1
 LiquidityManager_setCrossChainRebalancer:test_setCrossChainRebalancerSuccess() (gas: 162208)
 LiquidityManager_setLocalLiquidityContainer:test_OnlyOwnerReverts() (gas: 11030)
 LiquidityManager_setLocalLiquidityContainer:test_ReverstWhen_CalledWithTheZeroAddress() (gas: 10621)
-LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3374339)
+LiquidityManager_setLocalLiquidityContainer:test_setLocalLiquidityContainerSuccess() (gas: 3464367)
 LiquidityManager_setMinimumLiquidity:test_OnlyOwnerReverts() (gas: 10925)
 LiquidityManager_setMinimumLiquidity:test_setMinimumLiquiditySuccess() (gas: 36434)
 LiquidityManager_withdrawNative:test_OnlyOwnerReverts() (gas: 13115)

contracts/scripts/native_solc_compile_all_ccip

@@ -8,7 +8,7 @@ echo " └───────────────────────
 
 SOLC_VERSION="0.8.24"
 OPTIMIZE_RUNS=26000
-OPTIMIZE_RUNS_OFFRAMP=21000
+OPTIMIZE_RUNS_OFFRAMP=19500
 OPTIMIZE_RUNS_ONRAMP=3600
 
 

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

@@ -55,8 +55,15 @@ library Internal {
   }
 
   struct SourceTokenData {
+    // The source pool address, abi encoded. This value is trusted as it was obtained through the onRamp. It can be
+    // relied upon by the destination pool to validate the source pool.
     bytes sourcePoolAddress;
-    bytes destPoolAddress;
+    // The address of the destination token pool, 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
+    // 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;
   }
 

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

@@ -9,7 +9,7 @@ library Pool {
 
   // The number of bytes in the return data for a pool v1 releaseOrMint call.
   // This should match the size of the ReleaseOrMintOutV1 struct.
-  uint16 public constant CCIP_POOL_V1_RET_BYTES = 2 * 32;
+  uint16 public constant CCIP_POOL_V1_RET_BYTES = 32;
 
   // 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
@@ -25,15 +25,22 @@ library Pool {
   }
 
   struct LockOrBurnOutV1 {
-    bytes destPoolAddress;
+    // The address of the destination token pool, 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
+    // 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 destPoolData;
   }
 
   struct ReleaseOrMintInV1 {
     bytes originalSender; //          The original sender of the tx on the source chain
     uint64 remoteChainSelector; // ─╮ The chain ID of the source chain
-    address receiver; // ───────────╯ The recipient of the tokens on the destination chain
+    address receiver; // ───────────╯ The recipient of the tokens on the destination chain. This is *NOT* the address to
+    // send the tokens to, but the address that will receive the tokens via the offRamp.
     uint256 amount; //                The amount of tokens to release or mint, denominated in the source token's decimals
+    address localToken; //            The address on this chain of the token to release or mint
     /// @dev WARNING: sourcePoolAddress should be checked prior to any processing of funds. Make sure it matches the
     /// expected pool address for the given remoteChainSelector.
     bytes sourcePoolAddress; //       The address of the source pool, abi encoded in the case of EVM chains
@@ -43,7 +50,6 @@ library Pool {
   }
 
   struct ReleaseOrMintOutV1 {
-    address localToken; // The address of the local token
     // The number of tokens released or minted on the destination chain, denominated in  the local token's  decimals.
     // This value is expected to be equal to the ReleaseOrMintInV1.amount in the case where the source and destination
     // chain have the same number of decimals

contracts/src/v0.8/ccip/ocr/OCR2BaseNoChecks.sol

@@ -10,13 +10,19 @@ import {OCR2Abstract} from "./OCR2Abstract.sol";
 /// @dev This contract does ***NOT*** check the supplied signatures on `transmit`
 /// This is intentional.
 abstract contract OCR2BaseNoChecks is OwnerIsCreator, OCR2Abstract {
-  error InvalidConfig(string message);
+  error InvalidConfig(InvalidConfigErrorType errorType);
   error WrongMessageLength(uint256 expected, uint256 actual);
   error ConfigDigestMismatch(bytes32 expected, bytes32 actual);
   error ForkedChain(uint256 expected, uint256 actual);
   error UnauthorizedTransmitter();
   error OracleCannotBeZeroAddress();
 
+  enum InvalidConfigErrorType {
+    F_MUST_BE_POSITIVE,
+    TOO_MANY_TRANSMITTERS,
+    REPEATED_ORACLE_ADDRESS
+  }
+
   // Packing these fields used on the hot path in a ConfigInfo variable reduces the
   // retrieval of all of them to a minimum number of SLOADs.
   struct ConfigInfo {
@@ -76,8 +82,8 @@ abstract contract OCR2BaseNoChecks is OwnerIsCreator, OCR2Abstract {
 
   // Reverts transaction if config args are invalid
   modifier checkConfigValid(uint256 numTransmitters, uint256 f) {
-    if (numTransmitters > MAX_NUM_ORACLES) revert InvalidConfig("too many transmitters");
-    if (f == 0) revert InvalidConfig("f must be positive");
+    if (numTransmitters > MAX_NUM_ORACLES) revert InvalidConfig(InvalidConfigErrorType.TOO_MANY_TRANSMITTERS);
+    if (f == 0) revert InvalidConfig(InvalidConfigErrorType.F_MUST_BE_POSITIVE);
     _;
   }
 
@@ -101,15 +107,19 @@ abstract contract OCR2BaseNoChecks is OwnerIsCreator, OCR2Abstract {
     bytes memory offchainConfig
   ) external override checkConfigValid(transmitters.length, f) onlyOwner {
     _beforeSetConfig(onchainConfig);
-    uint256 oldTransmitterLength = s_transmitters.length;
-    for (uint256 i = 0; i < oldTransmitterLength; ++i) {
-      delete s_oracles[s_transmitters[i]];
+    // Scoped to reduce contract size
+    {
+      uint256 oldTransmitterLength = s_transmitters.length;
+      for (uint256 i = 0; i < oldTransmitterLength; ++i) {
+        delete s_oracles[s_transmitters[i]];
+      }
     }
-
     uint256 newTransmitterLength = transmitters.length;
     for (uint256 i = 0; i < newTransmitterLength; ++i) {
       address transmitter = transmitters[i];
-      if (s_oracles[transmitter].role != Role.Unset) revert InvalidConfig("repeated transmitter address");
+      if (s_oracles[transmitter].role != Role.Unset) {
+        revert InvalidConfig(InvalidConfigErrorType.REPEATED_ORACLE_ADDRESS);
+      }
       if (transmitter == address(0)) revert OracleCannotBeZeroAddress();
       s_oracles[transmitter] = Oracle(uint8(i), Role.Transmitter);
     }
@@ -178,10 +188,7 @@ abstract contract OCR2BaseNoChecks is OwnerIsCreator, OCR2Abstract {
     bytes32 configDigest = reportContext[0];
     bytes32 latestConfigDigest = s_configInfo.latestConfigDigest;
     if (latestConfigDigest != configDigest) revert ConfigDigestMismatch(latestConfigDigest, configDigest);
-    // If the cached chainID at time of deployment doesn't match the current chainID, we reject all signed reports.
-    // This avoids a (rare) scenario where chain A forks into chain A and A', A' still has configDigest
-    // calculated from chain A and so OCR reports will be valid on both forks.
-    if (i_chainID != block.chainid) revert ForkedChain(i_chainID, block.chainid);
+    _checkChainForked();
 
     emit Transmitted(configDigest, uint32(uint256(reportContext[1]) >> 8));
 
@@ -200,6 +207,13 @@ abstract contract OCR2BaseNoChecks is OwnerIsCreator, OCR2Abstract {
     if (msg.data.length != expectedDataLength) revert WrongMessageLength(expectedDataLength, msg.data.length);
   }
 
+  function _checkChainForked() internal view {
+    // If the cached chainID at time of deployment doesn't match the current chainID, we reject all signed reports.
+    // This avoids a (rare) scenario where chain A forks into chain A and A', A' still has configDigest
+    // calculated from chain A and so OCR reports will be valid on both forks.
+    if (i_chainID != block.chainid) revert ForkedChain(i_chainID, block.chainid);
+  }
+
   /// @notice information about current offchain reporting protocol configuration
   /// @return configCount ordinal number of current config, out of all configs applied to this contract so far
   /// @return blockNumber block at which this config was set

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

@@ -10,6 +10,7 @@ import {IMultiCommitStore} from "../interfaces/IMultiCommitStore.sol";
 import {IPool} from "../interfaces/IPool.sol";
 import {IRMN} from "../interfaces/IRMN.sol";
 import {IRouter} from "../interfaces/IRouter.sol";
+import {ITokenAdminRegistry} from "../interfaces/ITokenAdminRegistry.sol";
 
 import {CallWithExactGas} from "../../shared/call/CallWithExactGas.sol";
 import {EnumerableMapAddresses} from "../../shared/enumerable/EnumerableMapAddresses.sol";
@@ -55,7 +56,6 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
   error NotACompatiblePool(address notPool);
   error InvalidDataLength(uint256 expected, uint256 got);
   error InvalidNewState(uint64 sourceChainSelector, uint64 sequenceNumber, Internal.MessageExecutionState newState);
-  error IndexOutOfRange();
   error InvalidStaticConfig(uint64 sourceChainSelector);
 
   /// @dev Atlas depends on this event, if changing, please notify Atlas.
@@ -80,6 +80,7 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
     address commitStore; // ────────╮  CommitStore address on the destination chain
     uint64 chainSelector; // ───────╯  Destination chainSelector
     address rmnProxy; //               RMN proxy address
+    address tokenAdminRegistry; //     Token admin registry address
   }
 
   /// @notice Per-chain source config (defining a lane from a Source Chain -> Dest OffRamp)
@@ -130,6 +131,8 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
   uint64 internal immutable i_chainSelector;
   /// @dev The address of the RMN proxy
   address internal immutable i_rmnProxy;
+  /// @dev The address of the token admin registry
+  address internal immutable i_tokenAdminRegistry;
 
   // DYNAMIC CONFIG
   DynamicConfig internal s_dynamicConfig;
@@ -151,11 +154,14 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
     s_executionStates;
 
   constructor(StaticConfig memory staticConfig, SourceChainConfigArgs[] memory sourceChainConfigs) MultiOCR3Base() {
-    if (staticConfig.commitStore == address(0)) revert ZeroAddressNotAllowed();
+    if (staticConfig.commitStore == address(0) || staticConfig.tokenAdminRegistry == address(0)) {
+      revert ZeroAddressNotAllowed();
+    }
 
     i_commitStore = staticConfig.commitStore;
     i_chainSelector = staticConfig.chainSelector;
     i_rmnProxy = staticConfig.rmnProxy;
+    i_tokenAdminRegistry = staticConfig.tokenAdminRegistry;
 
     _applySourceChainConfigUpdates(sourceChainConfigs);
   }
@@ -590,7 +596,12 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
   /// @dev This function will always return the same struct as the contents is static and can never change.
   /// RMN depends on this function, if changing, please notify the RMN maintainers.
   function getStaticConfig() external view returns (StaticConfig memory) {
-    return StaticConfig({commitStore: i_commitStore, chainSelector: i_chainSelector, rmnProxy: i_rmnProxy});
+    return StaticConfig({
+      commitStore: i_commitStore,
+      chainSelector: i_chainSelector,
+      rmnProxy: i_rmnProxy,
+      tokenAdminRegistry: i_tokenAdminRegistry
+    });
   }
 
   /// @notice Returns the current dynamic config.
@@ -667,7 +678,13 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
 
     // TODO: contract size golfing - is StaticConfig needed in the event?
     emit ConfigSet(
-      StaticConfig({chainSelector: i_chainSelector, rmnProxy: i_rmnProxy, commitStore: i_commitStore}), dynamicConfig
+      StaticConfig({
+        commitStore: i_commitStore,
+        chainSelector: i_chainSelector,
+        rmnProxy: i_rmnProxy,
+        tokenAdminRegistry: i_tokenAdminRegistry
+      }),
+      dynamicConfig
     );
   }
 
@@ -698,12 +715,14 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
         abi.decode(encodedSourceTokenData[i], (Internal.SourceTokenData));
       // We need to safely decode the pool address from the sourceTokenData, as it could be wrong,
       // in which case it doesn't have to be a valid EVM address.
-      address localPoolAddress = Internal._validateEVMAddress(sourceTokenData.destPoolAddress);
+      address localToken = Internal._validateEVMAddress(sourceTokenData.destTokenAddress);
+      // We check with the token admin registry if the token has a pool on this chain.
+      address localPoolAddress = ITokenAdminRegistry(i_tokenAdminRegistry).getPool(localToken);
       // This will call the supportsInterface through the ERC165Checker, and not directly on the pool address.
       // This is done to prevent a pool from reverting the entire transaction if it doesn't support the interface.
       // The call gets a max or 30k gas per instance, of which there are three. This means gas estimations should
       // account for 90k gas overhead due to the interface check.
-      if (!localPoolAddress.supportsInterface(Pool.CCIP_POOL_V1)) {
+      if (localPoolAddress == address(0) || !localPoolAddress.supportsInterface(Pool.CCIP_POOL_V1)) {
         revert NotACompatiblePool(localPoolAddress);
       }
 
@@ -719,6 +738,7 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
             originalSender: messageRoute.sender,
             receiver: messageRoute.receiver,
             amount: sourceTokenAmounts[i].amount,
+            localToken: localToken,
             remoteChainSelector: messageRoute.sourceChainSelector,
             sourcePoolAddress: sourceTokenData.sourcePoolAddress,
             sourcePoolData: sourceTokenData.extraData,
@@ -738,24 +758,19 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
       if (returnData.length != Pool.CCIP_POOL_V1_RET_BYTES) {
         revert InvalidDataLength(Pool.CCIP_POOL_V1_RET_BYTES, returnData.length);
       }
-      (uint256 decodedAddress, uint256 amount) = abi.decode(returnData, (uint256, uint256));
-      address destTokenAddress = Internal._validateEVMAddressFromUint256(decodedAddress);
+      uint256 amount = abi.decode(returnData, (uint256));
 
       (success, returnData,) = CallWithExactGas._callWithExactGasSafeReturnData(
         abi.encodeWithSelector(IERC20.transfer.selector, messageRoute.receiver, amount),
-        destTokenAddress,
+        localToken,
         s_dynamicConfig.maxTokenTransferGas,
         Internal.GAS_FOR_CALL_EXACT_CHECK,
         Internal.MAX_RET_BYTES
       );
 
-      // This is the same check SafeERC20 does. We validate the optional boolean return value of the transfer function.
-      // If nothing is returned, we assume success, if something is returned, it should be `true`.
-      if (!success || (returnData.length > 0 && !abi.decode(returnData, (bool)))) {
-        revert TokenHandlingError(returnData);
-      }
+      if (!success) revert TokenHandlingError(returnData);
 
-      destTokenAmounts[i].token = destTokenAddress;
+      destTokenAmounts[i].token = localToken;
       destTokenAmounts[i].amount = amount;
     }
 
@@ -768,8 +783,7 @@ contract EVM2EVMMultiOffRamp is IAny2EVMMultiOffRamp, ITypeAndVersion, MultiOCR3
 
   /// @notice Reverts as this contract should not access CCIP messages
   function ccipReceive(Client.Any2EVMMessage calldata) external pure {
-    /* solhint-disable */
+    // solhint-disable-next-line
     revert();
-    /* solhint-enable*/
   }
 }