add l1 origin check + unit tests in sequencer solo client

Author: RnkSngh

bindings/go/sequencersoloclient/SequencerSoloClient.go

@@ -23,30 +23,43 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
 import {AppStateVerifier} from "../base/AppStateVerifier.sol";
 
 /**
- * @title OptimisticProofVerifier
- * @notice Verifies proofs related to Optimistic Rollup state updates
+ * @title SequencerSignatureVerifier
+ * @notice Verifies ECDSA signatures from a sequencer for client updates. Is used by the SequencerSoloClient to verify
+ * signatures on client updates.
  * @author Polymer Labs
  */
 contract SequencerSignatureVerifier is AppStateVerifier, ISignatureVerifier {
     using RLPReader for RLPReader.RLPItem;
     using RLPReader for bytes;
 
-    // @notice known L2 Output Oracle contract address to verify state update proofs against
-    address public immutable SEQUENCER;
+    address public immutable SEQUENCER; // The trusted sequencer address that polymer p2p signer holds the private key
+        // to
     bytes32 public immutable CHAIN_ID; // Chain ID of the L2 chain for which the sequencer signs over
 
     constructor(address sequencer_, bytes32 chainId_) {
         SEQUENCER = sequencer_;
         CHAIN_ID = chainId_;
     }
 
+    /**
+     * @notice Verifies that the sequencer signature is valid for a given l1 origin. This is used by the
+     * SequencerSoloClient update client method.
+     * @param l2BlockNumber The block number of the L2 block that the state update is for
+     * @param appHash The app hash of the state update to be saved in the parent soloClient contract
+     * @param l1BlockHash The hash of the L1 origin that the peptide height corresponds to
+     * @param signature The sequencer's ECDSA over the state update
+     */
     function verifyStateUpdate(uint256 l2BlockNumber, bytes32 appHash, bytes32 l1BlockHash, bytes calldata signature)
         external
         view
     {
         _verifySequencerSignature(l2BlockNumber, appHash, l1BlockHash, signature);
     }
 
+    /**
+     * @notice Verify the ECDSA signature of the sequencer over the given l2BLockNumber, peptideAppHash, and origin
+     * l1BlockHash
+     */
     function _verifySequencerSignature(
         uint256 l2BlockNumber,
         bytes32 appHash,

contracts/core/SequencerSignatureVerifier.sol

@@ -20,45 +20,51 @@ pragma solidity 0.8.15;
 import {Ics23Proof} from "../interfaces/IProofVerifier.sol";
 import {ISignatureVerifier} from "../interfaces/ISignatureVerifier.sol";
 import {ILightClient, LightClientType} from "../interfaces/ILightClient.sol";
+import {L1Block} from "optimism/L2/L1Block.sol";
 
 /**
- * @title SubfinalityLightClient
+ * @title SequencerSoloClient
  * @author Polymer Labs
- * @dev This specific light client implementation uses the same client that is used in the op-stack
+ * @dev This light client implementation verifies a single signature by the polymer p2p-signer, and updates the light
+ * client if valid signatures are provided.
  */
 contract SequencerSoloClient is ILightClient {
     LightClientType public constant LIGHT_CLIENT_TYPE = LightClientType.SequencerLightClient; // Stored as a constant
         // for cheap on-chain use
+
+    ISignatureVerifier public immutable verifier;
+    L1Block public immutable l1BlockProvider;
+
     uint8 private _LightClientType = uint8(LIGHT_CLIENT_TYPE); // Also redundantly stored as a private mutable type in
         // case it needs to be accessed in any proofs
 
-    // consensusStates maps from the height to the appHash.
+    // consensusStates maps from the height to the appHash, and is modified during state updates if it is a valid state
+    // update.
     mapping(uint256 => uint256) public consensusStates;
 
-    ISignatureVerifier public immutable verifier;
-
-    error CannotUpdatePendingOptimisticConsensusState();
+    error InvalidL1Origin();
+    error CannotUpdateClientWithDifferentAppHash();
     error AppHashHasNotPassedFraudProofWindow();
     error NonMembershipProofsNotYetImplemented();
     error NoConsensusStateAtHeight(uint256 height);
 
-    constructor(ISignatureVerifier verifier_) {
+    constructor(ISignatureVerifier verifier_, L1Block _l1BlockProvider) {
         verifier = verifier_;
+        l1BlockProvider = _l1BlockProvider;
     }
 
     /**
      * @inheritdoc ILightClient
      * @param proof An array of bytes that contain the l1blockhash and the sequencer's signature. The first 32 bytes of
-     * this argument should be
-     * the l1BlockHash, and the remaining bytes should be the sequencer signature which attests to the peptide AppHash
+     * this argument should be the l1BlockHash, and the remaining bytes should be the sequencer signature which attests
+     * to the peptide AppHash
      * for that l1BlockHash
      */
     function updateClient(bytes calldata proof, uint256 peptideHeight, uint256 peptideAppHash) external override {
-        if (consensusStates[peptideHeight] != 0) {
-            return;
+        if (l1BlockProvider.hash() != bytes32(proof[:32])) {
+            revert InvalidL1Origin();
         }
-        verifier.verifyStateUpdate(peptideHeight, bytes32(peptideAppHash), bytes32(proof[:32]), proof[32:]);
-        consensusStates[peptideHeight] = peptideAppHash;
+        _updateClient(proof, peptideHeight, peptideAppHash);
     }
 
     /**
@@ -67,10 +73,10 @@ contract SequencerSoloClient is ILightClient {
     function getState(uint256 height) external view returns (uint256 appHash) {
         return _getState(height);
     }
+
     /**
      * @inheritdoc ILightClient
      */
-
     function verifyMembership(Ics23Proof calldata proof, bytes calldata key, bytes calldata expectedValue)
         external
         view
@@ -88,6 +94,20 @@ contract SequencerSoloClient is ILightClient {
         revert NonMembershipProofsNotYetImplemented();
     }
 
+    function _updateClient(bytes calldata proof, uint256 peptideHeight, uint256 peptideAppHash) internal {
+        if (consensusStates[peptideHeight] != 0) {
+            // Note: we don't cache consensusStates[peptideHeight] in mem for gas savings here because this if statement
+            // would not be triggered too often, and would make the more frequent branch more expensive due to mem
+            // allocation.
+            if (consensusStates[peptideHeight] != peptideAppHash) {
+                revert CannotUpdateClientWithDifferentAppHash();
+            }
+            return;
+        }
+        verifier.verifyStateUpdate(peptideHeight, bytes32(peptideAppHash), bytes32(proof[:32]), proof[32:]);
+        consensusStates[peptideHeight] = peptideAppHash;
+    }
+
     /**
      * @dev Returns the internal state of the light client at a given height.
      */

contracts/core/SequencerSoloClient.sol

@@ -1,6 +1,6 @@
 {
   "name": "@open-ibc/vibc-core-smart-contracts",
-  "version": "4.0.4",
+  "version": "4.0.5",
   "main": "dist/index.js",
   "bin": {
     "verify-vibc-core-smart-contracts": "./dist/scripts/verify-contract-script.js",

package.json

@@ -65,6 +65,7 @@
   deployer: 'KEY_DEPLOYER'
   deployArgs:
     - '{{SequencerSigVerifier}}'
+    - '{{L1BlockAddress}}'
 
 - name: DummyLightClient
   description: 'DummyLightClient'

specs/update.spec.yaml

@@ -62,6 +62,7 @@ export interface SequencerSoloClientInterface extends Interface {
       | "LIGHT_CLIENT_TYPE"
       | "consensusStates"
       | "getState"
+      | "l1BlockProvider"
       | "updateClient"
       | "verifier"
       | "verifyMembership"
@@ -80,6 +81,10 @@ export interface SequencerSoloClientInterface extends Interface {
     functionFragment: "getState",
     values: [BigNumberish]
   ): string;
+  encodeFunctionData(
+    functionFragment: "l1BlockProvider",
+    values?: undefined
+  ): string;
   encodeFunctionData(
     functionFragment: "updateClient",
     values: [BytesLike, BigNumberish, BigNumberish]
@@ -103,6 +108,10 @@ export interface SequencerSoloClientInterface extends Interface {
     data: BytesLike
   ): Result;
   decodeFunctionResult(functionFragment: "getState", data: BytesLike): Result;
+  decodeFunctionResult(
+    functionFragment: "l1BlockProvider",
+    data: BytesLike
+  ): Result;
   decodeFunctionResult(
     functionFragment: "updateClient",
     data: BytesLike
@@ -167,6 +176,8 @@ export interface SequencerSoloClient extends BaseContract {
 
   getState: TypedContractMethod<[height: BigNumberish], [bigint], "view">;
 
+  l1BlockProvider: TypedContractMethod<[], [string], "view">;
+
   updateClient: TypedContractMethod<
     [
       proof: BytesLike,
@@ -204,6 +215,9 @@ export interface SequencerSoloClient extends BaseContract {
   getFunction(
     nameOrSignature: "getState"
   ): TypedContractMethod<[height: BigNumberish], [bigint], "view">;
+  getFunction(
+    nameOrSignature: "l1BlockProvider"
+  ): TypedContractMethod<[], [string], "view">;
   getFunction(
     nameOrSignature: "updateClient"
   ): TypedContractMethod<