From e9da1dfb1ce3eaae573afe44b8714410e67d7cf1 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 10 Oct 2024 11:49:55 -0400 Subject: [PATCH] rework lock-release pool deployment to be more stringent --- .../tokenAdminRegistry/TokenPoolFactory.t.sol | 89 ++++++++----------- .../TokenPoolFactory/FactoryBurnMintERC20.sol | 5 +- .../TokenPoolFactory/TokenPoolFactory.sol | 9 +- 3 files changed, 45 insertions(+), 58 deletions(-) diff --git a/contracts/src/v0.8/ccip/test/tokenAdminRegistry/TokenPoolFactory.t.sol b/contracts/src/v0.8/ccip/test/tokenAdminRegistry/TokenPoolFactory.t.sol index b7e9adf599..ccf8f342b1 100644 --- a/contracts/src/v0.8/ccip/test/tokenAdminRegistry/TokenPoolFactory.t.sol +++ b/contracts/src/v0.8/ccip/test/tokenAdminRegistry/TokenPoolFactory.t.sol @@ -20,6 +20,8 @@ import {TokenAdminRegistrySetup} from "./TokenAdminRegistry.t.sol"; import {Create2} from "../../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/Create2.sol"; +import {console2 as console} from "forge-std/console2.sol"; + contract TokenPoolFactorySetup is TokenAdminRegistrySetup { using Create2 for bytes32; @@ -98,8 +100,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { new TokenPoolFactory.RemoteTokenPoolInfo[](0), s_tokenInitCode, s_poolInitCode, - FAKE_SALT, - TokenPoolFactory.PoolType.BURN_MINT + FAKE_SALT ); assertNotEq(address(0), tokenAddress, "Token Address should not be 0"); @@ -160,8 +161,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { remoteTokenPools, // No existing remote pools s_tokenInitCode, // Token Init Code s_poolInitCode, // Pool Init Code - FAKE_SALT, // Salt - TokenPoolFactory.PoolType.BURN_MINT // Pool Type + FAKE_SALT // Salt ); // Ensure that the remote Token was set to the one we predicted @@ -199,8 +199,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { new TokenPoolFactory.RemoteTokenPoolInfo[](0), s_tokenInitCode, s_poolInitCode, - FAKE_SALT, - TokenPoolFactory.PoolType.BURN_MINT + FAKE_SALT ); assertEq( @@ -289,7 +288,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { // Since the remote chain information was provided, we should be able to get the information from the newly // deployed token pool using the available getter functions (address tokenAddress, address poolAddress) = s_tokenPoolFactory.deployTokenAndTokenPool( - remoteTokenPools, s_tokenInitCode, s_poolInitCode, FAKE_SALT, TokenPoolFactory.PoolType.BURN_MINT + remoteTokenPools, s_tokenInitCode, s_poolInitCode, FAKE_SALT ); assertEq(address(TokenPool(poolAddress).getToken()), tokenAddress, "Token Address should have been set locally"); @@ -365,7 +364,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { ); (address tokenAddress, address poolAddress) = s_tokenPoolFactory.deployTokenAndTokenPool( - remoteTokenPools, s_tokenInitCode, s_poolInitCode, FAKE_SALT, TokenPoolFactory.PoolType.BURN_MINT + remoteTokenPools, s_tokenInitCode, s_poolInitCode, FAKE_SALT ); assertNotEq(address(0), tokenAddress, "Token Address should not be 0"); @@ -394,7 +393,7 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { assertEq(IOwner(poolAddress).owner(), OWNER, "Token should be owned by the owner"); } - function test_createTokenPoolLockRelease_NoExistingToken_predict_Success() public { + function test_createTokenPoolLockRelease_ExistingToken_predict_Success() public { vm.startPrank(OWNER); bytes32 dynamicSalt = keccak256(abi.encodePacked(FAKE_SALT, OWNER)); @@ -411,6 +410,12 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { TokenPoolFactory.RemoteChainConfig memory remoteChainConfig = TokenPoolFactory.RemoteChainConfig(address(newTokenPoolFactory), address(s_destRouter), address(s_rmnProxy)); + FactoryBurnMintERC20 newLocalToken = + new FactoryBurnMintERC20("TestToken", "TEST", 18, type(uint256).max, PREMINT_AMOUNT, OWNER); + + FactoryBurnMintERC20 newRemoteToken = + new FactoryBurnMintERC20("TestToken", "TEST", 18, type(uint256).max, PREMINT_AMOUNT, OWNER); + // Create an array of remote pools where nothing exists yet, but we want to predict the address for // the new pool and token on DEST_CHAIN_SELECTOR TokenPoolFactory.RemoteTokenPoolInfo[] memory remoteTokenPools = new TokenPoolFactory.RemoteTokenPoolInfo[](1); @@ -423,74 +428,58 @@ contract TokenPoolFactoryTests is TokenPoolFactorySetup { type(LockReleaseTokenPool).creationCode, // remotePoolInitCode remoteChainConfig, // remoteChainConfig TokenPoolFactory.PoolType.LOCK_RELEASE, // poolType - "", // remoteTokenAddress + abi.encode(address(newRemoteToken)), // remoteTokenAddress s_tokenInitCode, // remoteTokenInitCode RateLimiter.Config(false, 0, 0) ); - - // Predict the address of the token and pool on the DESTINATION chain - address predictedTokenAddress = dynamicSalt.computeAddress(keccak256(s_tokenInitCode), address(newTokenPoolFactory)); - + // Since the remote chain information was provided, we should be able to get the information from the newly // deployed token pool using the available getter functions - (, address poolAddress) = s_tokenPoolFactory.deployTokenAndTokenPool( + address poolAddress = s_tokenPoolFactory.deployTokenPoolWithExistingToken( + address(newLocalToken), remoteTokenPools, - s_tokenInitCode, type(LockReleaseTokenPool).creationCode, FAKE_SALT, TokenPoolFactory.PoolType.LOCK_RELEASE ); - // Ensure that the remote Token was set to the one we predicted - assertEq( - abi.encode(predictedTokenAddress), - TokenPool(poolAddress).getRemoteToken(DEST_CHAIN_SELECTOR), - "Token Address should have been predicted" - ); - - { - // Create the constructor params for the predicted pool - // The predictedTokenAddress is NOT abi-encoded since the raw evm-address - // is used in the constructor params - bytes memory predictedPoolCreationParams = - abi.encode(predictedTokenAddress, new address[](0), s_rmnProxy, true, address(s_destRouter)); + // Check that the pool was correctly deployed on the local chain first - // Take the init code and concat the destination params to it, the initCode shouldn't change - bytes memory predictedPoolInitCode = - abi.encodePacked(type(LockReleaseTokenPool).creationCode, predictedPoolCreationParams); + // Accept the ownership which was transfered + OwnerIsCreator(poolAddress).acceptOwnership(); - // Predict the address of the pool on the DESTINATION chain - address predictedPoolAddress = - dynamicSalt.computeAddress(keccak256(predictedPoolInitCode), address(newTokenPoolFactory)); + // Ensure that the remote Token was set to the one we predicted + assertEq(address(LockReleaseTokenPool(poolAddress).getToken()), address(newLocalToken), "Token Address should have been set"); - // Assert that the address set for the remote pool is the same as the predicted address - assertEq( - abi.encode(predictedPoolAddress), - TokenPool(poolAddress).getRemotePool(DEST_CHAIN_SELECTOR), - "Pool Address should have been predicted" - ); - } + LockReleaseTokenPool(poolAddress).setRebalancer(OWNER); + assertEq(OWNER, LockReleaseTokenPool(poolAddress).getRebalancer(), "Rebalancer should be set"); - // On the new token pool factory, representing a destination chain, - // deploy a new token and a new pool - (address newTokenAddress, address newPoolAddress) = newTokenPoolFactory.deployTokenAndTokenPool( + // Deploy the Lock-Release Token Pool on the destination chain with the existing remote token + (address newPoolAddress) = newTokenPoolFactory.deployTokenPoolWithExistingToken( + address(newRemoteToken), new TokenPoolFactory.RemoteTokenPoolInfo[](0), // No existing remote pools - s_tokenInitCode, // Token Init Code type(LockReleaseTokenPool).creationCode, // Pool Init Code FAKE_SALT, // Salt - TokenPoolFactory.PoolType.LOCK_RELEASE // Pool Type + TokenPoolFactory.PoolType.LOCK_RELEASE ); assertEq( - TokenPool(poolAddress).getRemotePool(DEST_CHAIN_SELECTOR), + LockReleaseTokenPool(poolAddress).getRemotePool(DEST_CHAIN_SELECTOR), abi.encode(newPoolAddress), "New Pool Address should have been deployed correctly" ); assertEq( - TokenPool(poolAddress).getRemoteToken(DEST_CHAIN_SELECTOR), - abi.encode(newTokenAddress), + LockReleaseTokenPool(poolAddress).getRemoteToken(DEST_CHAIN_SELECTOR), + abi.encode(address(newRemoteToken)), "New Token Address should have been deployed correctly" ); + + assertEq( + address(LockReleaseTokenPool(newPoolAddress).getToken()), + address(newRemoteToken), + "New Remote Token should be set correctly" + ); + } } diff --git a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/FactoryBurnMintERC20.sol b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/FactoryBurnMintERC20.sol index c59be5775d..e22aa339c6 100644 --- a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/FactoryBurnMintERC20.sol +++ b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/FactoryBurnMintERC20.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity 0.8.24; import {IGetCCIPAdmin} from "../../../ccip/interfaces/IGetCCIPAdmin.sol"; import {IOwnable} from "../../../shared/interfaces/IOwnable.sol"; @@ -70,9 +70,10 @@ contract FactoryBurnMintERC20 is IBurnMintERC20, IGetCCIPAdmin, IERC165, ERC20Bu grantBurnRole(newOwner_); } + /// @inheritdoc IERC165 function supportsInterface(bytes4 interfaceId) public pure virtual override returns (bool) { return interfaceId == type(IERC20).interfaceId || interfaceId == type(IBurnMintERC20).interfaceId - || interfaceId == type(IERC165).interfaceId || interfaceId == type(IOwnable).interfaceId; + || interfaceId == type(IERC165).interfaceId || interfaceId == type(IOwnable).interfaceId || interfaceId == type(IGetCCIPAdmin).interfaceId; } // ================================================================ diff --git a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/TokenPoolFactory.sol b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/TokenPoolFactory.sol index b545a601e2..7be091d5d2 100644 --- a/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/TokenPoolFactory.sol +++ b/contracts/src/v0.8/ccip/tokenAdminRegistry/TokenPoolFactory/TokenPoolFactory.sol @@ -6,7 +6,6 @@ import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol"; import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol"; import {ITokenAdminRegistry} from "../../interfaces/ITokenAdminRegistry.sol"; -import {OwnerIsCreator} from "../../../shared/access/OwnerIsCreator.sol"; import {RateLimiter} from "../../libraries/RateLimiter.sol"; import {TokenPool} from "../../pools/TokenPool.sol"; import {RegistryModuleOwnerCustom} from "../RegistryModuleOwnerCustom.sol"; @@ -18,7 +17,7 @@ import {Create2} from "../../../vendor/openzeppelin-solidity/v5.0.2/contracts/ut /// ownership transfer in a separate transaction. /// @dev The address prediction mechanism is only capable of deploying and predicting addresses for EVM based chains. /// adding compatibility for other chains will require additional offchain computation. -contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { +contract TokenPoolFactory is ITypeAndVersion { using Create2 for bytes32; event RemoteChainConfigUpdated(uint64 indexed remoteChainSelector, RemoteChainConfig remoteChainConfig); @@ -95,15 +94,13 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { /// @param tokenInitCode The creation code for the token, which includes the constructor parameters already appended /// @param tokenPoolInitCode The creation code for the token pool, without the constructor parameters appended /// @param salt The salt to be used in the create2 deployment of the token and token pool to ensure a unique address - /// @param poolType The type of pool to deploy, either Burn/Mint or Lock/Release /// @return token The address of the token that was deployed /// @return pool The address of the token pool that was deployed function deployTokenAndTokenPool( RemoteTokenPoolInfo[] calldata remoteTokenPools, bytes memory tokenInitCode, bytes calldata tokenPoolInitCode, - bytes32 salt, - PoolType poolType + bytes32 salt ) external returns (address, address) { // Ensure a unique deployment between senders even if the same input parameter is used to prevent // DOS/Frontrunning attacks @@ -113,7 +110,7 @@ contract TokenPoolFactory is OwnerIsCreator, ITypeAndVersion { address token = Create2.deploy(0, salt, tokenInitCode); // Deploy the token pool - address pool = _createTokenPool(token, remoteTokenPools, tokenPoolInitCode, salt, poolType); + address pool = _createTokenPool(token, remoteTokenPools, tokenPoolInitCode, salt, PoolType.BURN_MINT); // Grant the mint and burn roles to the pool for the token IBurnMintERC20(token).grantMintAndBurnRoles(pool);