Merge 49a66fb into 2804858

smartcontractkit · Sep 16, 2024 · 27e5fbf · 27e5fbf
2 parents 2804858 + 49a66fb
commit 27e5fbf
Show file tree

Hide file tree

Showing 13 changed files with 393 additions and 356 deletions.
diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot
diff --git a/contracts/src/v0.8/ccip/interfaces/IRMNV2.sol b/contracts/src/v0.8/ccip/interfaces/IRMNV2.sol
@@ -19,7 +19,8 @@ interface IRMNV2 {
   function verify(
     address offRampAddress,
     Internal.MerkleRoot[] memory destLaneUpdates,
-    Signature[] memory signatures
+    Signature[] memory signatures,
+    uint256 rawVs
   ) external view;
 
   /// @notice gets the current set of cursed subjects
@@ -32,6 +33,6 @@ interface IRMNV2 {
 
   /// @notice If there is an active global curse, or an active curse for `subject`, this function returns true.
   /// @param subject To check whether a particular chain is cursed, set to bytes16(uint128(chainSelector)).
-  /// @return bool true if the profived subject is cured *or* if there is an active global curse
+  /// @return bool true if the provided subject is cured *or* if there is an active global curse
   function isCursed(bytes16 subject) external view returns (bool);
 }
diff --git a/contracts/src/v0.8/ccip/offRamp/OffRamp.sol b/contracts/src/v0.8/ccip/offRamp/OffRamp.sol
@@ -131,6 +131,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     Internal.PriceUpdates priceUpdates; // Collection of gas and price updates to commit
     Internal.MerkleRoot[] merkleRoots; // Collection of merkle roots per source chain to commit
     IRMNV2.Signature[] rmnSignatures; // RMN signatures on the merkle roots
+    uint256 rmnRawVs; // Raw v values of the RMN signatures
   }
 
   struct GasLimitOverride {
@@ -620,13 +621,13 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
     bytes calldata report,
     bytes32[] calldata rs,
     bytes32[] calldata ss,
-    bytes32 rawVs // signatures
+    bytes32 rawVs
   ) external {
     CommitReport memory commitReport = abi.decode(report, (CommitReport));
 
     // Verify RMN signatures
     if (commitReport.merkleRoots.length > 0) {
-      i_rmn.verify(address(this), commitReport.merkleRoots, commitReport.rmnSignatures);
+      i_rmn.verify(address(this), commitReport.merkleRoots, commitReport.rmnSignatures, commitReport.rmnRawVs);
     }
 
     // Check if the report contains price updates

diff --git a/contracts/src/v0.8/ccip/rmn/RMNRemote.sol b/contracts/src/v0.8/ccip/rmn/RMNRemote.sol
@@ -94,11 +94,13 @@ contract RMNRemote is OwnerIsCreator, ITypeAndVersion, IRMNV2 {
   function verify(
     address offrampAddress,
     Internal.MerkleRoot[] memory destLaneUpdates,
-    Signature[] memory signatures
+    Signature[] memory signatures,
+    uint256 rawVs
   ) external view {
     if (s_configCount == 0) {
       revert ConfigNotSet();
     }
+    if (signatures.length < s_config.minSigners) revert ThresholdNotMet();
 
     bytes32 signedHash = keccak256(
       abi.encode(
@@ -114,18 +116,15 @@ contract RMNRemote is OwnerIsCreator, ITypeAndVersion, IRMNV2 {
       )
     );
 
-    uint256 numSigners = 0;
     address prevAddress = address(0);
     for (uint256 i = 0; i < signatures.length; ++i) {
       Signature memory sig = signatures[i];
-      address signerAddress = ecrecover(signedHash, 27, sig.r, sig.s);
-      if (signerAddress == address(0)) revert InvalidSignature();
-      if (!(prevAddress < signerAddress)) revert OutOfOrderSignatures();
+      address signerAddress = ecrecover(signedHash, 27 + uint8(rawVs & 0x01 << i), sig.r, sig.s);
+      // check for duplicates and also validates that no signature is 0
+      if (prevAddress >= signerAddress) revert OutOfOrderSignatures();
       if (!s_signers[signerAddress]) revert UnexpectedSigner();
       prevAddress = signerAddress;
-      ++numSigners;
     }
-    if (numSigners < s_config.minSigners) revert ThresholdNotMet();
   }
 
   // ================================================================
@@ -134,7 +133,7 @@ contract RMNRemote is OwnerIsCreator, ITypeAndVersion, IRMNV2 {
 
   /// @notice Sets the configuration of the contract
   /// @param newConfig the new configuration
-  /// @dev setting congig is atomic; we delete all pre-existing config and set everything from scratch
+  /// @dev setting config is atomic; we delete all pre-existing config and set everything from scratch
   function setConfig(Config calldata newConfig) external onlyOwner {
     // sanity checks
     {

diff --git a/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol b/contracts/src/v0.8/ccip/test/e2e/MultiRampsEnd2End.sol
@@ -168,8 +168,12 @@ contract MultiRampsE2E is OnRampSetup, OffRampSetup {
         merkleRoot: merkleRoots[1]
       });
 
-      OffRamp.CommitReport memory report =
-        OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: rmnSignatures});
+      OffRamp.CommitReport memory report = OffRamp.CommitReport({
+        priceUpdates: _getEmptyPriceUpdates(),
+        merkleRoots: roots,
+        rmnSignatures: rmnSignatures,
+        rawVs: 0
+      });
 
       vm.resumeGasMetering();
       _commit(report, ++s_latestSequenceNumber);

diff --git a/contracts/src/v0.8/ccip/test/offRamp/OffRamp.t.sol b/contracts/src/v0.8/ccip/test/offRamp/OffRamp.t.sol
@@ -995,7 +995,8 @@ contract OffRamp_executeSingleReport is OffRampSetup {
     return OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
   }
 }
@@ -3259,7 +3260,8 @@ contract OffRamp_applySourceChainConfigUpdates is OffRampSetup {
       OffRamp.CommitReport({
         priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
         merkleRoots: roots,
-        rmnSignatures: s_rmnSignatures
+        rmnSignatures: s_rmnSignatures,
+        rawVs: 0
       }),
       s_latestSequenceNumber
     );
@@ -3312,8 +3314,12 @@ contract OffRamp_commit is OffRampSetup {
       merkleRoot: root
     });
 
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectEmit();
     emit OffRamp.CommitReportAccepted(commitReport.merkleRoots, commitReport.priceUpdates);
@@ -3340,8 +3346,12 @@ contract OffRamp_commit is OffRampSetup {
       maxSeqNr: maxSeq,
       merkleRoot: "stale report 1"
     });
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectEmit();
     emit OffRamp.CommitReportAccepted(commitReport.merkleRoots, commitReport.priceUpdates);
@@ -3379,7 +3389,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectEmit();
@@ -3401,7 +3412,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectEmit();
@@ -3419,7 +3431,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectEmit();
@@ -3471,7 +3484,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, tokenPrice1),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectEmit();
@@ -3581,8 +3595,12 @@ contract OffRamp_commit is OffRampSetup {
       onRampAddress: abi.encode(ON_RAMP_ADDRESS_1)
     });
 
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectRevert(abi.encodeWithSelector(OffRamp.CursedByRMN.selector, roots[0].sourceChainSelector));
     _commit(commitReport, s_latestSequenceNumber);
@@ -3597,8 +3615,12 @@ contract OffRamp_commit is OffRampSetup {
       maxSeqNr: 4,
       merkleRoot: bytes32(0)
     });
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectRevert(OffRamp.InvalidRoot.selector);
     _commit(commitReport, s_latestSequenceNumber);
@@ -3613,8 +3635,12 @@ contract OffRamp_commit is OffRampSetup {
       maxSeqNr: 2,
       merkleRoot: bytes32(0)
     });
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectRevert(
       abi.encodeWithSelector(
@@ -3634,8 +3660,12 @@ contract OffRamp_commit is OffRampSetup {
       maxSeqNr: 0,
       merkleRoot: bytes32(0)
     });
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectRevert(
       abi.encodeWithSelector(
@@ -3650,7 +3680,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectRevert(OffRamp.StaleCommitReport.selector);
@@ -3662,7 +3693,8 @@ contract OffRamp_commit is OffRampSetup {
     OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
 
     vm.expectEmit();
@@ -3683,8 +3715,12 @@ contract OffRamp_commit is OffRampSetup {
       merkleRoot: "Only a single root"
     });
 
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     vm.expectRevert(abi.encodeWithSelector(OffRamp.SourceChainNotEnabled.selector, 0));
     _commit(commitReport, s_latestSequenceNumber);
@@ -3699,8 +3735,12 @@ contract OffRamp_commit is OffRampSetup {
       maxSeqNr: 2,
       merkleRoot: "Only a single root"
     });
-    OffRamp.CommitReport memory commitReport =
-      OffRamp.CommitReport({priceUpdates: _getEmptyPriceUpdates(), merkleRoots: roots, rmnSignatures: s_rmnSignatures});
+    OffRamp.CommitReport memory commitReport = OffRamp.CommitReport({
+      priceUpdates: _getEmptyPriceUpdates(),
+      merkleRoots: roots,
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
+    });
 
     _commit(commitReport, s_latestSequenceNumber);
     commitReport.merkleRoots[0].minSeqNr = 3;
@@ -3734,7 +3774,8 @@ contract OffRamp_commit is OffRampSetup {
     return OffRamp.CommitReport({
       priceUpdates: _getSingleTokenPriceUpdateStruct(s_sourceFeeToken, 4e18),
       merkleRoots: roots,
-      rmnSignatures: s_rmnSignatures
+      rmnSignatures: s_rmnSignatures,
+      rawVs: 0
     });
   }
 }