From 0fc3c24ca8778d8a1cc11f2ec1a49c89abf06b69 Mon Sep 17 00:00:00 2001 From: Ryan Hall Date: Fri, 2 Aug 2024 12:12:46 -0400 Subject: [PATCH] change address => IRouter in OnRamp; add getRouter() func --- .../v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol | 21 +++++----- .../src/v0.8/ccip/test/NonceManager.t.sol | 2 +- .../v0.8/ccip/test/e2e/MultiRampsEnd2End.sol | 2 +- .../ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol | 38 +++++++++---------- .../test/onRamp/EVM2EVMMultiOnRampSetup.t.sol | 7 ++-- 5 files changed, 36 insertions(+), 34 deletions(-) diff --git a/contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol b/contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol index fd0d7d93a6..bf550e69f3 100644 --- a/contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol +++ b/contracts/src/v0.8/ccip/onRamp/EVM2EVMMultiOnRamp.sol @@ -8,6 +8,7 @@ import {INonceManager} from "../interfaces/INonceManager.sol"; import {IPoolV1} from "../interfaces/IPool.sol"; import {IPriceRegistry} from "../interfaces/IPriceRegistry.sol"; import {IRMN} from "../interfaces/IRMN.sol"; +import {IRouter} from "../interfaces/IRouter.sol"; import {ITokenAdminRegistry} from "../interfaces/ITokenAdminRegistry.sol"; import {OwnerIsCreator} from "../../shared/access/OwnerIsCreator.sol"; @@ -72,7 +73,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre uint64 sequenceNumber; // This is the local router address that is allowed to send messages to the destination chain. // This is NOT the receiving router address on the destination chain. - address router; + IRouter router; } /// @dev Same as DestChainConfig but with the destChainSelector so that an array of these @@ -80,7 +81,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre //solhint-disable gas-struct-packing struct DestChainConfigArgs { uint64 destChainSelector; // Destination chain selector - address router; // Source router address + IRouter router; // Source router address } // STATIC CONFIG @@ -155,7 +156,7 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre // Validate message sender is set and allowed. Not validated in `getFee` since it is not user-driven. if (originalSender == address(0)) revert RouterMustSetOriginalSender(); // Router address may be zero intentionally to pause. - if (msg.sender != destChainConfig.router) revert MustBeCalledByRouter(); + if (msg.sender != address(destChainConfig.router)) revert MustBeCalledByRouter(); { // scoped to reduce stack usage @@ -296,6 +297,13 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre _setDynamicConfig(dynamicConfig); } + /// @notice Gets the source router for a destination chain + /// @param destChainSelector The destination chain selector + /// @return router the router for the provided destination chain + function getRouter(uint64 destChainSelector) external view returns (IRouter) { + return s_destChainConfigs[destChainSelector].router; + } + /// @notice Internal version of setDynamicConfig to allow for reuse in the constructor. function _setDynamicConfig(DynamicConfig memory dynamicConfig) internal { if (dynamicConfig.priceRegistry == address(0) || dynamicConfig.feeAggregator == address(0)) revert InvalidConfig(); @@ -313,13 +321,6 @@ contract EVM2EVMMultiOnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCre ); } - /// @notice Gets the source router for a destination chain - /// @param destChainSelector The destination chain selector - /// @return destChainConfig the config for the provided destination chain - function getDestChainConfig(uint64 destChainSelector) external view returns (DestChainConfig memory) { - return s_destChainConfigs[destChainSelector]; - } - /// @notice Updates the destination chain specific config. /// @param destChainConfigArgs Array of source chain specific configs. function applyDestChainConfigUpdates(DestChainConfigArgs[] memory destChainConfigArgs) external onlyOwner { diff --git a/contracts/src/v0.8/ccip/test/NonceManager.t.sol b/contracts/src/v0.8/ccip/test/NonceManager.t.sol index 82bb15e4ff..b65f37f1f2 100644 --- a/contracts/src/v0.8/ccip/test/NonceManager.t.sol +++ b/contracts/src/v0.8/ccip/test/NonceManager.t.sol @@ -245,7 +245,7 @@ contract NonceManager_OnRampUpgrade is EVM2EVMMultiOnRampSetup { s_outboundNonceManager.applyPreviousRampsUpdates(previousRamps); (s_onRamp, s_metadataHash) = _deployOnRamp( - SOURCE_CHAIN_SELECTOR, address(s_sourceRouter), address(s_outboundNonceManager), address(s_tokenAdminRegistry) + SOURCE_CHAIN_SELECTOR, s_sourceRouter, address(s_outboundNonceManager), address(s_tokenAdminRegistry) ); vm.startPrank(address(s_sourceRouter)); diff --git a/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol b/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol index f812614d57..52436503d2 100644 --- a/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol +++ b/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol @@ -66,7 +66,7 @@ contract MultiRampsE2E is EVM2EVMMultiOnRampSetup, EVM2EVMMultiOffRampSetup { s_onRamp2, s_metadataHash2 ) = _deployOnRamp( - SOURCE_CHAIN_SELECTOR + 1, address(s_sourceRouter2), address(s_nonceManager2), address(s_tokenAdminRegistry2) + SOURCE_CHAIN_SELECTOR + 1, s_sourceRouter2, address(s_nonceManager2), address(s_tokenAdminRegistry2) ); address[] memory authorizedCallers = new address[](1); diff --git a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol index cd4ffc54ce..42286bc2bc 100644 --- a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol +++ b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol @@ -31,12 +31,10 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { emit EVM2EVMMultiOnRamp.ConfigSet(staticConfig, dynamicConfig); vm.expectEmit(); emit EVM2EVMMultiOnRamp.DestChainConfigSet( - DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, address(s_sourceRouter)) + DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, s_sourceRouter) ); - _deployOnRamp( - SOURCE_CHAIN_SELECTOR, address(s_sourceRouter), address(s_outboundNonceManager), address(s_tokenAdminRegistry) - ); + _deployOnRamp(SOURCE_CHAIN_SELECTOR, s_sourceRouter, address(s_outboundNonceManager), address(s_tokenAdminRegistry)); EVM2EVMMultiOnRamp.StaticConfig memory gotStaticConfig = s_onRamp.getStaticConfig(); _assertStaticConfigsEqual(staticConfig, gotStaticConfig); @@ -48,7 +46,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { assertEq("EVM2EVMMultiOnRamp 1.6.0-dev", s_onRamp.typeAndVersion()); assertEq(OWNER, s_onRamp.owner()); assertEq(1, s_onRamp.getExpectedNextSequenceNumber(DEST_CHAIN_SELECTOR)); - assertEq(address(s_sourceRouter), s_onRamp.getDestChainConfig(DEST_CHAIN_SELECTOR).router); + assertEq(address(s_sourceRouter), address(s_onRamp.getRouter(DEST_CHAIN_SELECTOR))); } function test_Constructor_InvalidConfigChainSelectorEqZero_Revert() public { @@ -61,7 +59,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { tokenAdminRegistry: address(s_tokenAdminRegistry) }), _generateDynamicMultiOnRampConfig(address(s_priceRegistry)), - _generateDestChainConfigArgs(address(0)) + _generateDestChainConfigArgs(IRouter(address(0))) ); } @@ -75,7 +73,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { tokenAdminRegistry: address(s_tokenAdminRegistry) }), _generateDynamicMultiOnRampConfig(address(s_priceRegistry)), - _generateDestChainConfigArgs(address(0)) + _generateDestChainConfigArgs(IRouter(address(0))) ); } @@ -89,7 +87,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { tokenAdminRegistry: address(s_tokenAdminRegistry) }), _generateDynamicMultiOnRampConfig(address(s_priceRegistry)), - _generateDestChainConfigArgs(address(0)) + _generateDestChainConfigArgs(IRouter(address(0))) ); } @@ -103,7 +101,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup { tokenAdminRegistry: address(0) }), _generateDynamicMultiOnRampConfig(address(s_priceRegistry)), - _generateDestChainConfigArgs(address(0)) + _generateDestChainConfigArgs(IRouter(address(0))) ); } } @@ -146,7 +144,7 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup { IERC20(s_sourceFeeToken).transferFrom(OWNER, address(s_onRamp), feeAmount); // Change the source router for this lane - address newRouter = makeAddr("NEW ROUTER"); + IRouter newRouter = IRouter(makeAddr("NEW ROUTER")); vm.stopPrank(); vm.prank(OWNER); s_onRamp.applyDestChainConfigUpdates(_generateDestChainConfigArgs(newRouter)); @@ -157,7 +155,7 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup { s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); // forward succeeds from correct router - vm.prank(newRouter); + vm.prank(address(newRouter)); s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER); } @@ -752,24 +750,26 @@ contract EVM2EVMMultiOnRamp_applyDestChainConfigUpdates is EVM2EVMMultiOnRampSet // supports disabling a lane by setting a router to zero vm.expectEmit(); - emit EVM2EVMMultiOnRamp.DestChainConfigSet(DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, address(0))); + emit EVM2EVMMultiOnRamp.DestChainConfigSet( + DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, IRouter(address(0))) + ); s_onRamp.applyDestChainConfigUpdates(configArgs); - assertEq(address(0), s_onRamp.getDestChainConfig(DEST_CHAIN_SELECTOR).router); + assertEq(address(0), address(s_onRamp.getRouter(DEST_CHAIN_SELECTOR))); // supports updating and adding lanes simultaneously configArgs = new EVM2EVMMultiOnRamp.DestChainConfigArgs[](2); configArgs[0] = - EVM2EVMMultiOnRamp.DestChainConfigArgs({destChainSelector: DEST_CHAIN_SELECTOR, router: address(s_sourceRouter)}); - configArgs[1] = EVM2EVMMultiOnRamp.DestChainConfigArgs({destChainSelector: 9999, router: address(9999)}); + EVM2EVMMultiOnRamp.DestChainConfigArgs({destChainSelector: DEST_CHAIN_SELECTOR, router: s_sourceRouter}); + configArgs[1] = EVM2EVMMultiOnRamp.DestChainConfigArgs({destChainSelector: 9999, router: IRouter(address(9999))}); vm.expectEmit(); emit EVM2EVMMultiOnRamp.DestChainConfigSet( - DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, address(s_sourceRouter)) + DEST_CHAIN_SELECTOR, EVM2EVMMultiOnRamp.DestChainConfig(0, s_sourceRouter) ); vm.expectEmit(); - emit EVM2EVMMultiOnRamp.DestChainConfigSet(9999, EVM2EVMMultiOnRamp.DestChainConfig(0, address(9999))); + emit EVM2EVMMultiOnRamp.DestChainConfigSet(9999, EVM2EVMMultiOnRamp.DestChainConfig(0, IRouter(address(9999)))); s_onRamp.applyDestChainConfigUpdates(configArgs); - assertEq(address(s_sourceRouter), s_onRamp.getDestChainConfig(DEST_CHAIN_SELECTOR).router); - assertEq(address(9999), s_onRamp.getDestChainConfig(9999).router); + assertEq(address(s_sourceRouter), address(s_onRamp.getRouter(DEST_CHAIN_SELECTOR))); + assertEq(address(9999), address(s_onRamp.getRouter(9999))); // handles empty list uint256 numLogs = vm.getRecordedLogs().length; diff --git a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRampSetup.t.sol b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRampSetup.t.sol index f2a7317eec..d237a23e8f 100644 --- a/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRampSetup.t.sol +++ b/contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRampSetup.t.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.24; import {IPoolV1} from "../../interfaces/IPool.sol"; +import {IRouter} from "../../interfaces/IRouter.sol"; import {AuthorizedCallers} from "../../../shared/access/AuthorizedCallers.sol"; import {NonceManager} from "../../NonceManager.sol"; @@ -38,7 +39,7 @@ contract EVM2EVMMultiOnRampSetup is TokenSetup, PriceRegistryFeeSetup { s_outboundMessageValidator = new MessageInterceptorHelper(); s_outboundNonceManager = new NonceManager(new address[](0)); (s_onRamp, s_metadataHash) = _deployOnRamp( - SOURCE_CHAIN_SELECTOR, address(s_sourceRouter), address(s_outboundNonceManager), address(s_tokenAdminRegistry) + SOURCE_CHAIN_SELECTOR, s_sourceRouter, address(s_outboundNonceManager), address(s_tokenAdminRegistry) ); s_offRamps = new address[](2); @@ -111,7 +112,7 @@ contract EVM2EVMMultiOnRampSetup is TokenSetup, PriceRegistryFeeSetup { return result; } - function _generateDestChainConfigArgs(address router) + function _generateDestChainConfigArgs(IRouter router) internal pure returns (EVM2EVMMultiOnRamp.DestChainConfigArgs[] memory) @@ -124,7 +125,7 @@ contract EVM2EVMMultiOnRampSetup is TokenSetup, PriceRegistryFeeSetup { function _deployOnRamp( uint64 sourceChainSelector, - address router, + IRouter router, address nonceManager, address tokenAdminRegistry ) internal returns (EVM2EVMMultiOnRampHelper, bytes32 metadataHash) {