From e398344c45d10ee950f2f58a71d4aec28f892f11 Mon Sep 17 00:00:00 2001 From: DhairyaSethi <55102840+DhairyaSethi@users.noreply.github.com> Date: Thu, 17 Oct 2024 16:38:31 +0530 Subject: [PATCH] fix: use chain agnostic proxypool --- .../ccip/pools/GHO/UpgradeableTokenPool.sol | 24 ++++++-------- .../test/pools/GHO/GhoTokenPoolRemote.t.sol | 33 +++++-------------- .../TokenPoolsUpgrade.t.sol | 4 +-- 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/contracts/src/v0.8/ccip/pools/GHO/UpgradeableTokenPool.sol b/contracts/src/v0.8/ccip/pools/GHO/UpgradeableTokenPool.sol index f1dccc2249..a8e91549a9 100644 --- a/contracts/src/v0.8/ccip/pools/GHO/UpgradeableTokenPool.sol +++ b/contracts/src/v0.8/ccip/pools/GHO/UpgradeableTokenPool.sol @@ -257,7 +257,7 @@ abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 { /// is a permissioned onRamp for the given chain on the Router. modifier onlyOnRamp(uint64 remoteChainSelector) { if (!isSupportedChain(remoteChainSelector)) revert ChainNotAllowed(remoteChainSelector); - if (!(msg.sender == getProxyPool(remoteChainSelector) || msg.sender == s_router.getOnRamp(remoteChainSelector))) + if (!(msg.sender == getProxyPool() || msg.sender == s_router.getOnRamp(remoteChainSelector))) revert CallerIsNotARampOnRouter(msg.sender); _; } @@ -327,29 +327,25 @@ abstract contract UpgradeableTokenPool is IPool, OwnerIsCreator, IERC165 { } /// @notice Getter for proxy pool address. - /// @param remoteChainSelector The remote chain selector for which the proxy pool is being retrieved. /// @return proxyPool The proxy pool address for the given remoteChainSelector - function getProxyPool(uint64 remoteChainSelector) public view returns (address proxyPool) { + function getProxyPool() public view returns (address proxyPool) { assembly ("memory-safe") { - mstore(0, PROXY_POOL_SLOT) - mstore(32, remoteChainSelector) - proxyPool := shr(96, shl(96, sload(keccak256(0, 64)))) + proxyPool := shr(96, shl(96, sload(PROXY_POOL_SLOT))) } } /// @notice Setter for proxy pool address, only callable by the DAO. - /// @param remoteChainSelector The remote chain selector for which the proxy pool is being set. + /// @dev This router is currently set for the Eth/Arb lane, and this pool is not expected + /// to support any other lanes in the future - hence can be stored agnostic to chain selector. /// @param proxyPool The address of the proxy pool. - function setProxyPool(uint64 remoteChainSelector, address proxyPool) external onlyOwner { - _setPoolProxy(remoteChainSelector, proxyPool); + function setProxyPool(address proxyPool) external onlyOwner { + _setPoolProxy(proxyPool); } - function _setPoolProxy(uint64 remoteChainSelector, address proxyPool) internal { - if (!isSupportedChain(remoteChainSelector)) revert ChainNotAllowed(remoteChainSelector); + function _setPoolProxy(address proxyPool) internal { + if (proxyPool == address(0)) revert ZeroAddressNotAllowed(); assembly ("memory-safe") { - mstore(0, PROXY_POOL_SLOT) - mstore(32, remoteChainSelector) - sstore(keccak256(0, 64), proxyPool) + sstore(PROXY_POOL_SLOT, proxyPool) } } } diff --git a/contracts/src/v0.8/ccip/test/pools/GHO/GhoTokenPoolRemote.t.sol b/contracts/src/v0.8/ccip/test/pools/GHO/GhoTokenPoolRemote.t.sol index eeb7eaa87f..deba3b745b 100644 --- a/contracts/src/v0.8/ccip/test/pools/GHO/GhoTokenPoolRemote.t.sol +++ b/contracts/src/v0.8/ccip/test/pools/GHO/GhoTokenPoolRemote.t.sol @@ -381,37 +381,20 @@ contract GhoTokenPoolRemote_proxyPool is GhoTokenPoolRemoteSetup { function testSetProxyPoolAdminReverts() public { vm.startPrank(STRANGER); vm.expectRevert("Only callable by owner"); - s_pool.setProxyPool(DEST_CHAIN_SELECTOR, makeAddr("proxyPool")); + s_pool.setProxyPool(makeAddr("proxyPool")); } - function testSetProxyPoolInvalidChainReverts(uint64 nonExistentChainSelector) public { - vm.assume(nonExistentChainSelector != DEST_CHAIN_SELECTOR); - changePrank(AAVE_DAO); - vm.expectRevert(abi.encodeWithSelector(UpgradeableTokenPool.ChainNotAllowed.selector, nonExistentChainSelector)); - s_pool.setProxyPool(nonExistentChainSelector, makeAddr("proxyPool")); + function testSetProxyPoolZeroAddressReverts() public { + vm.startPrank(AAVE_DAO); + vm.expectRevert(UpgradeableTokenPool.ZeroAddressNotAllowed.selector); + s_pool.setProxyPool(address(0)); } function testSetProxyPoolSuccess(address proxyPool) public { + vm.assume(proxyPool != address(0)); changePrank(AAVE_DAO); - s_pool.setProxyPool(DEST_CHAIN_SELECTOR, proxyPool); - - assertEq(s_pool.getProxyPool(DEST_CHAIN_SELECTOR), proxyPool); - } - - function testFuzzGetProxyPool(uint64 chainSelector, address proxyPool) public { - vm.assume(chainSelector != DEST_CHAIN_SELECTOR); - UpgradeableTokenPool.ChainUpdate[] memory chains = new UpgradeableTokenPool.ChainUpdate[](1); - chains[0] = UpgradeableTokenPool.ChainUpdate({ - remoteChainSelector: chainSelector, - allowed: true, - outboundRateLimiterConfig: getOutboundRateLimiterConfig(), - inboundRateLimiterConfig: getInboundRateLimiterConfig() - }); - - changePrank(AAVE_DAO); - s_pool.applyChainUpdates(chains); // more robust than modifying `s_remoteChainSelectors` set storage - s_pool.setProxyPool(chainSelector, proxyPool); + s_pool.setProxyPool(proxyPool); - assertEq(s_pool.getProxyPool(chainSelector), proxyPool); + assertEq(s_pool.getProxyPool(), proxyPool); } } diff --git a/contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrate1_4To1_5/TokenPoolsUpgrade.t.sol b/contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrate1_4To1_5/TokenPoolsUpgrade.t.sol index 6d142fc7e9..f363c728e5 100644 --- a/contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrate1_4To1_5/TokenPoolsUpgrade.t.sol +++ b/contracts/src/v0.8/ccip/test/pools/GHO/fork/GhoTokenPoolMigrate1_4To1_5/TokenPoolsUpgrade.t.sol @@ -23,11 +23,11 @@ contract ForkPoolUpgradeAfterMigration is ForkBase { // #2: setProxyPool vm.selectFork(l1.forkId); vm.prank(l1.tokenPool.owner()); - l1.tokenPool.setProxyPool(l2.chainSelector, l1.proxyPool); + l1.tokenPool.setProxyPool(l1.proxyPool); vm.selectFork(l2.forkId); vm.prank(l2.tokenPool.owner()); - l2.tokenPool.setProxyPool(l1.chainSelector, l2.proxyPool); + l2.tokenPool.setProxyPool(l2.proxyPool); } function test_lockOrBurnViaLegacyRouterL1() public {