Make onRamp mutable in OffRamp (#1422)

## Motivation
The goal of this PR is to reduce the misconfig risk on the `OffRamp`
with the `onRamp` address. Currently, if we accidentally configure the
wrong `onRamp` address on an existing `OffRamp`, the fix would involve a
full redeployment + reconfig.

## Solution

- Make the `onRamp` mutable in the `OffRamp`.
- Add `onRamp` address verification in the `commit()` function
- Only allow `onRamp` address modification when `minSeqNr == 1`

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>

contracts/foundry.toml

@@ -26,7 +26,7 @@ single_line_statement_blocks = "preserve"
 solc_version = '0.8.24'
 src = 'src/v0.8/ccip'
 test = 'src/v0.8/ccip/test'
-optimizer_runs = 1_925
+optimizer_runs = 1_650 
 evm_version = 'paris'
 
 [profile.functions]

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

@@ -57,13 +57,14 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
   error NotACompatiblePool(address notPool);
   error InvalidDataLength(uint256 expected, uint256 got);
   error InvalidNewState(uint64 sourceChainSelector, uint64 sequenceNumber, Internal.MessageExecutionState newState);
-  error InvalidStaticConfig(uint64 sourceChainSelector);
   error StaleCommitReport();
   error InvalidInterval(uint64 sourceChainSelector, uint64 min, uint64 max);
   error ZeroAddressNotAllowed();
   error InvalidMessageDestChainSelector(uint64 messageDestChainSelector);
   error SourceChainSelectorMismatch(uint64 reportSourceChainSelector, uint64 messageSourceChainSelector);
   error SignatureVerificationDisabled();
+  error CommitOnRampMismatch(bytes reportOnRamp, bytes configOnRamp);
+  error InvalidOnRampUpdate(uint64 sourceChainSelector);
 
   /// @dev Atlas depends on this event, if changing, please notify Atlas.
   event StaticConfigSet(StaticConfig staticConfig);
@@ -140,6 +141,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
 
   // STATIC CONFIG
   string public constant override typeAndVersion = "OffRamp 1.6.0-dev";
+  /// @dev Hash of encoded address(0) used for empty address checks
+  bytes32 internal constant EMPTY_ENCODED_ADDRESS_HASH = keccak256(abi.encode(address(0)));
   /// @dev ChainSelector of this chain
   uint64 internal immutable i_chainSelector;
   /// @dev The RMN verification contract
@@ -655,6 +658,10 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
 
       SourceChainConfig storage sourceChainConfig = _getEnabledSourceChainConfig(sourceChainSelector);
 
+      if (keccak256(root.onRampAddress) != keccak256(sourceChainConfig.onRamp)) {
+        revert CommitOnRampMismatch(root.onRampAddress, sourceChainConfig.onRamp);
+      }
+
       if (sourceChainConfig.minSeqNr != root.minSeqNr || root.minSeqNr > root.maxSeqNr) {
         revert InvalidInterval(root.sourceChainSelector, root.minSeqNr, root.maxSeqNr);
       }
@@ -775,24 +782,27 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
       }
 
       SourceChainConfig storage currentConfig = s_sourceChainConfigs[sourceChainSelector];
-      bytes memory currentOnRamp = currentConfig.onRamp;
       bytes memory newOnRamp = sourceConfigUpdate.onRamp;
 
-      // OnRamp can never be zero - if it is, then the source chain has been added for the first time
-      if (currentOnRamp.length == 0) {
-        if (newOnRamp.length == 0) {
-          revert ZeroAddressNotAllowed();
-        }
-
-        currentConfig.onRamp = newOnRamp;
+      if (currentConfig.onRamp.length == 0) {
         currentConfig.minSeqNr = 1;
         emit SourceChainSelectorAdded(sourceChainSelector);
-      } else if (keccak256(currentOnRamp) != keccak256(newOnRamp)) {
-        revert InvalidStaticConfig(sourceChainSelector);
+      } else if (currentConfig.minSeqNr != 1) {
+        // OnRamp updates should only happens due to a misconfiguration
+        // If an OnRamp is misconfigured not reports should have been committed and no messages should have been executed
+        // This is enforced byt the onRamp address check in the commit function
+        revert InvalidOnRampUpdate(sourceChainSelector);
       }
 
+      // OnRamp can never be zero - if it is, then the source chain has been added for the first time
+      if (newOnRamp.length == 0 || keccak256(newOnRamp) == EMPTY_ENCODED_ADDRESS_HASH) {
+        revert ZeroAddressNotAllowed();
+      }
+
+      currentConfig.onRamp = newOnRamp;
       currentConfig.isEnabled = sourceConfigUpdate.isEnabled;
       currentConfig.router = sourceConfigUpdate.router;
+
       emit SourceChainConfigSet(sourceChainSelector, currentConfig);
     }
   }

contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol

@@ -162,7 +162,7 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
       });
       roots[1] = Internal.MerkleRoot({
         sourceChainSelector: SOURCE_CHAIN_SELECTOR + 1,
-        onRampAddress: abi.encode(address(s_onRamp)),
+        onRampAddress: abi.encode(address(s_onRamp2)),
         minSeqNr: messages2[0].header.sequenceNumber,
         maxSeqNr: messages2[0].header.sequenceNumber,
         merkleRoot: merkleRoots[1]

contracts/src/v0.8/ccip/test/offRamp/OffRamp.t.sol

@@ -814,24 +814,6 @@ contract OffRamp_executeSingleReport is OffRampSetup {
     s_offRamp.executeSingleReport(executionReport, new OffRamp.GasLimitOverride[](0));
   }
 
-  function test_MismatchingOnRampRoot_Revert() public {
-    s_offRamp.setVerifyOverrideResult(SOURCE_CHAIN_SELECTOR_1, 0);
-
-    Internal.Any2EVMRampMessage[] memory messages =
-      _generateSingleBasicMessage(SOURCE_CHAIN_SELECTOR_1, ON_RAMP_ADDRESS_1);
-
-    OffRamp.CommitReport memory commitReport = _constructCommitReport(
-      // Root against mismatching on ramp
-      Internal._hash(messages[0], ON_RAMP_ADDRESS_3)
-    );
-    _commit(commitReport, s_latestSequenceNumber);
-
-    Internal.ExecutionReportSingleChain memory executionReport =
-      _generateReportFromMessages(SOURCE_CHAIN_SELECTOR_1, messages);
-    vm.expectRevert(abi.encodeWithSelector(OffRamp.RootNotCommitted.selector, SOURCE_CHAIN_SELECTOR_1));
-    s_offRamp.executeSingleReport(executionReport, new OffRamp.GasLimitOverride[](0));
-  }
-
   function test_UnhealthySingleChainCurse_Revert() public {
     _setMockRMNChainCurse(SOURCE_CHAIN_SELECTOR_1, true);
     vm.expectEmit();
@@ -3187,6 +3169,27 @@ contract OffRamp_applySourceChainConfigUpdates is OffRampSetup {
     );
   }
 
+  function test_ReplaceExistingChainOnRamp_Success() public {
+    OffRamp.SourceChainConfigArgs[] memory sourceChainConfigs = new OffRamp.SourceChainConfigArgs[](1);
+    sourceChainConfigs[0] = OffRamp.SourceChainConfigArgs({
+      router: s_destRouter,
+      sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
+      onRamp: ON_RAMP_ADDRESS_1,
+      isEnabled: true
+    });
+
+    s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
+
+    sourceChainConfigs[0].onRamp = ON_RAMP_ADDRESS_2;
+
+    vm.expectEmit();
+    emit OffRamp.SourceChainConfigSet(
+      SOURCE_CHAIN_SELECTOR_1,
+      OffRamp.SourceChainConfig({router: s_destRouter, isEnabled: true, minSeqNr: 1, onRamp: ON_RAMP_ADDRESS_2})
+    );
+    s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
+  }
+
   // Reverts
 
   function test_ZeroOnRampAddress_Revert() public {
@@ -3200,6 +3203,10 @@ contract OffRamp_applySourceChainConfigUpdates is OffRampSetup {
 
     vm.expectRevert(OffRamp.ZeroAddressNotAllowed.selector);
     s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
+
+    sourceChainConfigs[0].onRamp = abi.encode(address(0));
+    vm.expectRevert(OffRamp.ZeroAddressNotAllowed.selector);
+    s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
   }
 
   function test_RouterAddress_Revert() public {
@@ -3228,7 +3235,7 @@ contract OffRamp_applySourceChainConfigUpdates is OffRampSetup {
     s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
   }
 
-  function test_ReplaceExistingChainOnRamp_Revert() public {
+  function test_InvalidOnRampUpdate_Revert() public {
     OffRamp.SourceChainConfigArgs[] memory sourceChainConfigs = new OffRamp.SourceChainConfigArgs[](1);
     sourceChainConfigs[0] = OffRamp.SourceChainConfigArgs({
       router: s_destRouter,
@@ -3239,9 +3246,30 @@ contract OffRamp_applySourceChainConfigUpdates is OffRampSetup {
 
     s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
 
+    Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
+    roots[0] = Internal.MerkleRoot({
+      sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
+      onRampAddress: ON_RAMP_ADDRESS_1,
+      minSeqNr: 1,
+      maxSeqNr: 2,
+      merkleRoot: "test #2"
+    });
+
+    _commit(
+      OffRamp.CommitReport({
+        priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
+        merkleRoots: roots,
+        rmnSignatures: s_rmnSignatures
+      }),
+      s_latestSequenceNumber
+    );
+
+    vm.stopPrank();
+    vm.startPrank(OWNER);
+
     sourceChainConfigs[0].onRamp = ON_RAMP_ADDRESS_2;
 
-    vm.expectRevert(abi.encodeWithSelector(OffRamp.InvalidStaticConfig.selector, SOURCE_CHAIN_SELECTOR_1));
+    vm.expectRevert(abi.encodeWithSelector(OffRamp.InvalidOnRampUpdate.selector, SOURCE_CHAIN_SELECTOR_1));
     s_offRamp.applySourceChainConfigUpdates(sourceChainConfigs);
   }
 }
@@ -3278,7 +3306,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: max1,
       merkleRoot: root
@@ -3307,7 +3335,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: maxSeq,
       merkleRoot: "stale report 1"
@@ -3458,7 +3486,7 @@ contract OffRamp_commit is OffRampSetup {
     roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: maxSeq,
       merkleRoot: "stale report"
@@ -3564,7 +3592,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: 4,
       merkleRoot: bytes32(0)
@@ -3580,7 +3608,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 2,
       maxSeqNr: 2,
       merkleRoot: bytes32(0)
@@ -3601,7 +3629,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: 0,
       merkleRoot: bytes32(0)
@@ -3666,7 +3694,7 @@ contract OffRamp_commit is OffRampSetup {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: 2,
       merkleRoot: "Only a single root"
@@ -3684,11 +3712,20 @@ contract OffRamp_commit is OffRampSetup {
     _commit(commitReport, ++s_latestSequenceNumber);
   }
 
+  function test_CommitOnRampMismatch_Revert() public {
+    OffRamp.CommitReport memory commitReport = _constructCommitReport();
+
+    commitReport.merkleRoots[0].onRampAddress = ON_RAMP_ADDRESS_2;
+
+    vm.expectRevert(abi.encodeWithSelector(OffRamp.CommitOnRampMismatch.selector, ON_RAMP_ADDRESS_2, ON_RAMP_ADDRESS_1));
+    _commit(commitReport, s_latestSequenceNumber);
+  }
+
   function _constructCommitReport() internal view returns (OffRamp.CommitReport memory) {
     Internal.MerkleRoot[] memory roots = new Internal.MerkleRoot[](1);
     roots[0] = Internal.MerkleRoot({
       sourceChainSelector: SOURCE_CHAIN_SELECTOR_1,
-      onRampAddress: abi.encode(ON_RAMP_ADDRESS_1),
+      onRampAddress: ON_RAMP_ADDRESS_1,
       minSeqNr: 1,
       maxSeqNr: s_maxInterval,
       merkleRoot: "test #2"

core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -23,7 +23,7 @@ multi_aggregate_rate_limiter: ../../../contracts/solc/v0.8.24/MultiAggregateRate
 multi_ocr3_helper: ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.abi ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.bin 6b56e0114a4d50797d30a34aecc2641ef340451d0c3fcb9d729bba4df2435122
 nonce_manager: ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.abi ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.bin 6f64e1083b356c06ee66b9138e398b9c97a4cd3e8c9ec38cf3010cebc79af536
 ocr3_config_encoder: ../../../contracts/solc/v0.8.24/IOCR3ConfigEncoder/IOCR3ConfigEncoder.abi ../../../contracts/solc/v0.8.24/IOCR3ConfigEncoder/IOCR3ConfigEncoder.bin 9254b35a86f00fde7b7193a033ca58f6521a66e87b9cf9da6ce5660082e79f5d
-offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin d7f33e895b1b282930c81ac4e19ec51f6a7900fe6dddda3a7ba1b2796eacff32
+offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 56945d1dff7a37553ad6db6b0701159af4ad2d85fb49acf368f66881c382f08a
 onramp: ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.abi ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.bin 594439983f963f4158f9c5009dee7cba4ee56be61900bb1d5b9108eaeac3d6a6
 ping_pong_demo: ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.abi ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.bin c1c2f8a65c7ffd971899cae7fe62f2da57d09e936151e2b92163c4bebe699d6b
 price_registry: ../../../contracts/solc/v0.8.24/PriceRegistry/PriceRegistry.abi ../../../contracts/solc/v0.8.24/PriceRegistry/PriceRegistry.bin e7781d600c1bb7aa4620106af7f6e146a109b97f4cb6a7d06c9e15773340ecb2