Remove nops and weights from multi onramp (#912)

## Motivation

Remove nops and weights logic from the `EVM2EVMMultiOnRamp` and
outsource the logic to an external fee aggregator contract.

## Solution

All fee tokens are now transferred to the configured `feeAggregator`
address which will handle payment logic.

The below state variables have been removed:
- `s_nops`
- `s_nopWeightsTotal`
- `s_nopFeesJuels`: replaced by events
- `i_maxNopFeesJuels`: replaced by `i_maxFeeJuelsPerMsg` which applies a
check at the message level

Out of scope:
- Full coverage

.changeset/quick-kangaroos-tell.md

@@ -0,0 +1,5 @@
+---
+"ccip": minor
+---
+
+#removed nops and weights logic from multi onramp

contracts/.changeset/tall-toys-attack.md

@@ -0,0 +1,5 @@
+---
+"@chainlink/contracts-ccip": minor
+---
+
+remove nops and weights logic from multi onramp

contracts/src/v0.8/ccip/PriceRegistry.sol

@@ -237,8 +237,7 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator, ITypeAndVersion {
   // │                         Fee tokens                           │
   // ================================================================
 
-  /// @notice Get the list of fee tokens.
-  /// @return The tokens set as fee tokens.
+  /// @inheritdoc IPriceRegistry
   function getFeeTokens() external view returns (address[] memory) {
     return s_feeTokens.values();
   }

contracts/src/v0.8/ccip/interfaces/IPriceRegistry.sol

@@ -67,4 +67,8 @@ interface IPriceRegistry {
     uint256 fromTokenAmount,
     address toToken
   ) external view returns (uint256 toTokenAmount);
+
+  /// @notice Get the list of fee tokens.
+  /// @return The tokens set as fee tokens.
+  function getFeeTokens() external view returns (address[] memory);
 }

contracts/src/v0.8/ccip/test/BaseTest.t.sol

@@ -15,6 +15,7 @@ contract BaseTest is Test {
   address internal constant DUMMY_CONTRACT_ADDRESS = 0x1111111111111111111111111111111111111112;
   address internal constant ON_RAMP_ADDRESS = 0x11118e64e1FB0c487f25dD6D3601FF6aF8d32E4e;
   address internal constant ZERO_ADDRESS = address(0);
+  address internal constant FEE_AGGREGATOR = 0xa33CDB32eAEce34F6affEfF4899cef45744EDea3;
 
   address internal constant USER_1 = address(1);
   address internal constant USER_2 = address(2);
@@ -32,6 +33,7 @@ contract BaseTest is Test {
 
   // Onramp
   uint96 internal constant MAX_NOP_FEES_JUELS = 1e27;
+  uint96 internal constant MAX_MSG_FEES_JUELS = 1e18;
   uint32 internal constant DEST_GAS_OVERHEAD = 350_000;
   uint16 internal constant DEST_GAS_PER_PAYLOAD_BYTE = 16;
 

contracts/src/v0.8/ccip/test/helpers/EVM2EVMMultiOnRampHelper.sol

@@ -11,17 +11,15 @@ contract EVM2EVMMultiOnRampHelper is EVM2EVMMultiOnRamp, IgnoreContractSize {
     DestChainConfigArgs[] memory destChainConfigs,
     RateLimiter.Config memory rateLimiterConfig,
     PremiumMultiplierWeiPerEthArgs[] memory premiumMultiplierWeiPerEthArgs,
-    TokenTransferFeeConfigArgs[] memory tokenTransferFeeConfigArgs,
-    NopAndWeight[] memory nopsAndWeights
+    TokenTransferFeeConfigArgs[] memory tokenTransferFeeConfigArgs
   )
     EVM2EVMMultiOnRamp(
       staticConfig,
       dynamicConfig,
       destChainConfigs,
       rateLimiterConfig,
       premiumMultiplierWeiPerEthArgs,
-      tokenTransferFeeConfigArgs,
-      nopsAndWeights
+      tokenTransferFeeConfigArgs
     )
   {}
 

contracts/src/v0.8/ccip/test/onRamp/EVM2EVMMultiOnRamp.t.sol

@@ -22,7 +22,7 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup {
     EVM2EVMMultiOnRamp.StaticConfig memory staticConfig = EVM2EVMMultiOnRamp.StaticConfig({
       linkToken: s_sourceTokens[0],
       chainSelector: SOURCE_CHAIN_SELECTOR,
-      maxNopFeesJuels: MAX_NOP_FEES_JUELS,
+      maxFeeJuelsPerMsg: MAX_MSG_FEES_JUELS,
       rmnProxy: address(s_mockRMN),
       tokenAdminRegistry: address(s_tokenAdminRegistry)
     });
@@ -94,35 +94,44 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup {
       EVM2EVMMultiOnRamp.StaticConfig({
         linkToken: address(0),
         chainSelector: SOURCE_CHAIN_SELECTOR,
-        maxNopFeesJuels: MAX_NOP_FEES_JUELS,
+        maxFeeJuelsPerMsg: MAX_NOP_FEES_JUELS,
         rmnProxy: address(s_mockRMN),
         tokenAdminRegistry: address(s_tokenAdminRegistry)
       }),
       _generateDynamicMultiOnRampConfig(address(s_sourceRouter), address(s_priceRegistry)),
       _generateDestChainConfigArgs(),
       getOutboundRateLimiterConfig(),
       s_premiumMultiplierWeiPerEthArgs,
-      s_tokenTransferFeeConfigArgs,
-      _getMultiOnRampNopsAndWeights()
+      s_tokenTransferFeeConfigArgs
     );
   }
 
-  function test_Constructor_InvalidConfigChainSelectorEqZero_Revert() public {
+  function test_Constructor_InvalidConfigLinkChainSelectorEqZero_Revert() public {
+    EVM2EVMMultiOnRamp.DynamicConfig memory dynamicConfig =
+      _generateDynamicMultiOnRampConfig(address(s_sourceRouter), address(s_priceRegistry));
+    EVM2EVMMultiOnRamp.DestChainConfigArgs[] memory destChainConfigArgs = _generateDestChainConfigArgs();
+    EVM2EVMMultiOnRamp.StaticConfig memory staticConfig = EVM2EVMMultiOnRamp.StaticConfig({
+      linkToken: s_sourceTokens[0],
+      chainSelector: 0,
+      maxFeeJuelsPerMsg: MAX_NOP_FEES_JUELS,
+      rmnProxy: address(s_mockRMN),
+      tokenAdminRegistry: address(s_tokenAdminRegistry)
+    });
+
     vm.expectRevert(EVM2EVMMultiOnRamp.InvalidConfig.selector);
     new EVM2EVMMultiOnRampHelper(
       EVM2EVMMultiOnRamp.StaticConfig({
         linkToken: s_sourceTokens[0],
         chainSelector: 0,
-        maxNopFeesJuels: MAX_NOP_FEES_JUELS,
+        maxFeeJuelsPerMsg: MAX_NOP_FEES_JUELS,
         rmnProxy: address(s_mockRMN),
         tokenAdminRegistry: address(s_tokenAdminRegistry)
       }),
       _generateDynamicMultiOnRampConfig(address(s_sourceRouter), address(s_priceRegistry)),
       _generateDestChainConfigArgs(),
       getOutboundRateLimiterConfig(),
       s_premiumMultiplierWeiPerEthArgs,
-      s_tokenTransferFeeConfigArgs,
-      _getMultiOnRampNopsAndWeights()
+      s_tokenTransferFeeConfigArgs
     );
   }
 
@@ -132,16 +141,15 @@ contract EVM2EVMMultiOnRamp_constructor is EVM2EVMMultiOnRampSetup {
       EVM2EVMMultiOnRamp.StaticConfig({
         linkToken: s_sourceTokens[0],
         chainSelector: SOURCE_CHAIN_SELECTOR,
-        maxNopFeesJuels: MAX_NOP_FEES_JUELS,
+        maxFeeJuelsPerMsg: MAX_NOP_FEES_JUELS,
         rmnProxy: address(0),
         tokenAdminRegistry: address(s_tokenAdminRegistry)
       }),
       _generateDynamicMultiOnRampConfig(address(s_sourceRouter), address(s_priceRegistry)),
       _generateDestChainConfigArgs(),
       getOutboundRateLimiterConfig(),
       s_premiumMultiplierWeiPerEthArgs,
-      s_tokenTransferFeeConfigArgs,
-      _getMultiOnRampNopsAndWeights()
+      s_tokenTransferFeeConfigArgs
     );
   }
 }
@@ -316,7 +324,7 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
   }
 
   function setUp() public virtual override {
-    EVM2EVMMultiOnRampSetup.setUp();
+    super.setUp();
 
     address[] memory feeTokens = new address[](1);
     feeTokens[0] = s_sourceTokens[1];
@@ -405,10 +413,11 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
     uint256 feeAmount = 1234567890;
     IERC20(s_sourceFeeToken).transferFrom(OWNER, address(s_onRamp), feeAmount);
 
+    vm.expectEmit();
+    emit EVM2EVMMultiOnRamp.FeePaid(s_sourceFeeToken, feeAmount);
     s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER);
 
     assertEq(IERC20(s_sourceFeeToken).balanceOf(address(s_onRamp)), feeAmount);
-    assertEq(s_onRamp.getNopFeesJuels(), feeAmount);
   }
 
   function test_ShouldStoreNonLinkFees() public {
@@ -418,17 +427,17 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
     uint256 feeAmount = 1234567890;
     IERC20(s_sourceTokens[1]).transferFrom(OWNER, address(s_onRamp), feeAmount);
 
-    s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER);
-
-    assertEq(IERC20(s_sourceTokens[1]).balanceOf(address(s_onRamp)), feeAmount);
-
     // Calculate conversion done by prices contract
     uint256 feeTokenPrice = s_priceRegistry.getTokenPrice(s_sourceTokens[1]).value;
     uint256 linkTokenPrice = s_priceRegistry.getTokenPrice(s_sourceFeeToken).value;
     uint256 conversionRate = (feeTokenPrice * 1e18) / linkTokenPrice;
     uint256 expectedJuels = (feeAmount * conversionRate) / 1e18;
 
-    assertEq(s_onRamp.getNopFeesJuels(), expectedJuels);
+    vm.expectEmit();
+    emit EVM2EVMMultiOnRamp.FeePaid(s_sourceTokens[1], expectedJuels);
+    s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER);
+
+    assertEq(IERC20(s_sourceTokens[1]).balanceOf(address(s_onRamp)), feeAmount);
   }
 
   // Make sure any valid sender, receiver and feeAmount can be handled.
@@ -440,7 +449,7 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
     // To avoid RouterMustSetOriginalSender
     vm.assume(originalSender != address(0));
     vm.assume(uint160(receiver) >= Internal.PRECOMPILE_SPACE);
-    vm.assume(feeTokenAmount <= MAX_NOP_FEES_JUELS);
+    feeTokenAmount = uint96(bound(feeTokenAmount, 0, MAX_MSG_FEES_JUELS));
 
     Client.EVM2AnyMessage memory message = _generateEmptyMessage();
     message.receiver = abi.encode(receiver);
@@ -450,15 +459,15 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
 
     Internal.EVM2EVMMessage memory expectedEvent = _messageToEvent(message, 1, 1, feeTokenAmount, originalSender);
 
+    vm.expectEmit();
+    emit EVM2EVMMultiOnRamp.FeePaid(s_sourceFeeToken, feeTokenAmount);
     vm.expectEmit(false, false, false, true);
     emit EVM2EVMMultiOnRamp.CCIPSendRequested(DEST_CHAIN_SELECTOR, expectedEvent);
 
     // Assert the message Id is correct
     assertEq(
       expectedEvent.messageId, s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeTokenAmount, originalSender)
     );
-    // Assert the fee token amount is correctly assigned to the nop fee pool
-    assertEq(feeTokenAmount, s_onRamp.getNopFeesJuels());
   }
 
   function test_OverValueWithARLOff_Success() public {
@@ -670,12 +679,14 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter is EVM2EVMMultiOnRampSetup {
     }
   }
 
-  function test_MaxFeeBalanceReached_Revert() public {
+  function test_MesssageFeeTooHigh_Revert() public {
     Client.EVM2AnyMessage memory message = _generateEmptyMessage();
 
-    vm.expectRevert(EVM2EVMMultiOnRamp.MaxFeeBalanceReached.selector);
+    vm.expectRevert(
+      abi.encodeWithSelector(EVM2EVMMultiOnRamp.MessageFeeTooHigh.selector, MAX_MSG_FEES_JUELS + 1, MAX_MSG_FEES_JUELS)
+    );
 
-    s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, MAX_NOP_FEES_JUELS + 1, OWNER);
+    s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, MAX_MSG_FEES_JUELS + 1, OWNER);
   }
 
   function test_InvalidChainSelector_Revert() public {
@@ -847,16 +858,15 @@ contract EVM2EVMMultiOnRamp_forwardFromRouter_upgrade is EVM2EVMMultiOnRampSetup
       EVM2EVMMultiOnRamp.StaticConfig({
         linkToken: s_sourceTokens[0],
         chainSelector: SOURCE_CHAIN_SELECTOR,
-        maxNopFeesJuels: MAX_NOP_FEES_JUELS,
+        maxFeeJuelsPerMsg: MAX_NOP_FEES_JUELS,
         rmnProxy: address(s_mockRMN),
         tokenAdminRegistry: address(s_tokenAdminRegistry)
       }),
       _generateDynamicMultiOnRampConfig(address(s_sourceRouter), address(s_priceRegistry)),
       destChainConfigArgs,
       getOutboundRateLimiterConfig(),
       s_premiumMultiplierWeiPerEthArgs,
-      s_tokenTransferFeeConfigArgs,
-      _getMultiOnRampNopsAndWeights()
+      s_tokenTransferFeeConfigArgs
     );
 
     s_metadataHash = keccak256(
@@ -938,7 +948,7 @@ contract EVM2EVMMultiOnRamp_getFeeSetup is EVM2EVMMultiOnRampSetup {
   address internal s_selfServeTokenDefaultPricing = makeAddr("self-serve-token-default-pricing");
 
   function setUp() public virtual override {
-    EVM2EVMMultiOnRampSetup.setUp();
+    super.setUp();
 
     // Add additional pool addresses for test tokens to mark them as supported
     s_tokenAdminRegistry.registerAdministratorPermissioned(s_sourceRouter.getWrappedNative(), OWNER);
@@ -1651,8 +1661,11 @@ contract EVM2EVMMultiOnRamp_getTokenTransferCost is EVM2EVMMultiOnRamp_getFeeSet
 contract EVM2EVMMultiOnRamp_setDynamicConfig is EVM2EVMMultiOnRampSetup {
   function test_SetDynamicConfig_Success() public {
     EVM2EVMMultiOnRamp.StaticConfig memory staticConfig = s_onRamp.getStaticConfig();
-    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig =
-      EVM2EVMMultiOnRamp.DynamicConfig({router: address(2134), priceRegistry: address(23423)});
+    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig = EVM2EVMMultiOnRamp.DynamicConfig({
+      router: address(2134),
+      priceRegistry: address(23423),
+      feeAggregator: FEE_AGGREGATOR
+    });
 
     vm.expectEmit();
     emit EVM2EVMMultiOnRamp.ConfigSet(staticConfig, newConfig);
@@ -1666,17 +1679,37 @@ contract EVM2EVMMultiOnRamp_setDynamicConfig is EVM2EVMMultiOnRampSetup {
 
   // Reverts
 
+  function test_SetConfigInvalidConfigPriceRegistryEqAddressZero_Revert() public {
+    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig = EVM2EVMMultiOnRamp.DynamicConfig({
+      router: address(2134),
+      priceRegistry: address(0),
+      feeAggregator: FEE_AGGREGATOR
+    });
+
+    vm.expectRevert(EVM2EVMMultiOnRamp.InvalidConfig.selector);
+    s_onRamp.setDynamicConfig(newConfig);
+  }
+
   function test_SetConfigInvalidConfig_Revert() public {
-    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig =
-      EVM2EVMMultiOnRamp.DynamicConfig({router: address(1), priceRegistry: address(23423)});
+    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig = EVM2EVMMultiOnRamp.DynamicConfig({
+      router: address(1),
+      priceRegistry: address(23423),
+      feeAggregator: FEE_AGGREGATOR
+    });
 
     // Invalid price reg reverts.
     newConfig.priceRegistry = address(0);
     vm.expectRevert(EVM2EVMMultiOnRamp.InvalidConfig.selector);
     s_onRamp.setDynamicConfig(newConfig);
+  }
 
-    // Succeeds if valid
-    newConfig.priceRegistry = address(23423);
+  function test_SetConfigInvalidConfigFeeAggregatorEqAddressZero_Revert() public {
+    EVM2EVMMultiOnRamp.DynamicConfig memory newConfig = EVM2EVMMultiOnRamp.DynamicConfig({
+      router: address(2134),
+      priceRegistry: address(23423),
+      feeAggregator: address(0)
+    });
+    vm.expectRevert(EVM2EVMMultiOnRamp.InvalidConfig.selector);
     s_onRamp.setDynamicConfig(newConfig);
   }
 
@@ -1690,7 +1723,66 @@ contract EVM2EVMMultiOnRamp_setDynamicConfig is EVM2EVMMultiOnRampSetup {
   }
 }
 
-contract EVM2EVMMultiOnRamp_applyPremiumMultiplierWeiPerEthUpdates is EVM2EVMMultiOnRampSetup {
+contract EVM2EVMMultiOnRamp_withdrawFeeTokens is EVM2EVMMultiOnRampSetup {
+  mapping(address => uint256) internal s_nopFees;
+
+  function setUp() public virtual override {
+    super.setUp();
+
+    // Since we'll mostly be testing for valid calls from the router we'll
+    // mock all calls to be originating from the router and re-mock in
+    // tests that require failure.
+    vm.startPrank(address(s_sourceRouter));
+
+    uint256 feeAmount = 1234567890;
+
+    // Send a bunch of messages, increasing the juels in the contract
+    for (uint256 i = 0; i < s_sourceFeeTokens.length; ++i) {
+      Client.EVM2AnyMessage memory message = _generateEmptyMessage();
+      message.feeToken = s_sourceFeeTokens[i % s_sourceFeeTokens.length];
+      uint256 newFeeTokenBalance = IERC20(message.feeToken).balanceOf(address(s_onRamp)) + feeAmount;
+      deal(message.feeToken, address(s_onRamp), newFeeTokenBalance);
+      s_nopFees[message.feeToken] = newFeeTokenBalance;
+      s_onRamp.forwardFromRouter(DEST_CHAIN_SELECTOR, message, feeAmount, OWNER);
+    }
+  }
+
+  function test_Fuzz_WithdrawFeeTokens_Success(uint256[5] memory amounts) public {
+    vm.startPrank(OWNER);
+    address[] memory feeTokens = new address[](amounts.length);
+    for (uint256 i = 0; i < amounts.length; ++i) {
+      vm.assume(amounts[i] > 0);
+      feeTokens[i] = _deploySourceToken("", amounts[i], 18);
+      IERC20(feeTokens[i]).transfer(address(s_onRamp), amounts[i]);
+    }
+
+    s_priceRegistry.applyFeeTokensUpdates(feeTokens, new address[](0));
+
+    for (uint256 i = 0; i < feeTokens.length; ++i) {
+      vm.expectEmit();
+      emit EVM2EVMMultiOnRamp.FeeTokenWithdrawn(FEE_AGGREGATOR, feeTokens[i], amounts[i]);
+    }
+
+    s_onRamp.withdrawFeeTokens();
+
+    for (uint256 i = 0; i < feeTokens.length; ++i) {
+      assertEq(IERC20(feeTokens[i]).balanceOf(FEE_AGGREGATOR), amounts[i]);
+      assertEq(IERC20(feeTokens[i]).balanceOf(address(s_onRamp)), 0);
+    }
+  }
+
+  function test_WithdrawFeeTokens_Success() public {
+    vm.expectEmit();
+    emit EVM2EVMMultiOnRamp.FeeTokenWithdrawn(FEE_AGGREGATOR, s_sourceFeeToken, s_nopFees[s_sourceFeeToken]);
+
+    s_onRamp.withdrawFeeTokens();
+
+    assertEq(IERC20(s_sourceFeeToken).balanceOf(FEE_AGGREGATOR), s_nopFees[s_sourceFeeToken]);
+    assertEq(IERC20(s_sourceFeeToken).balanceOf(address(s_onRamp)), 0);
+  }
+}
+
+contract EVM2EVMOnRamp_applyPremiumMultiplierWeiPerEthUpdates is EVM2EVMMultiOnRampSetup {
   function test_Fuzz_applyPremiumMultiplierWeiPerEthUpdates_Success(
     EVM2EVMMultiOnRamp.PremiumMultiplierWeiPerEthArgs memory premiumMultiplierWeiPerEthArg
   ) public {