fix(contracts): OZ-L07 Potentially Misleading Verifier Event (#849)

Co-authored-by: zimpha <zimpha@users.noreply.github.com>
Co-authored-by: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com>

common/version/version.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 )
 
-var tag = "v4.1.108"
+var tag = "v4.1.109"
 
 var commit = func() string {
 	if info, ok := debug.ReadBuildInfo(); ok {

contracts/scripts/foundry/InitializeL1BridgeContracts.s.sol

@@ -12,6 +12,7 @@ import {L1ScrollMessenger} from "../../src/L1/L1ScrollMessenger.sol";
 import {L1StandardERC20Gateway} from "../../src/L1/gateways/L1StandardERC20Gateway.sol";
 import {L1WETHGateway} from "../../src/L1/gateways/L1WETHGateway.sol";
 import {L1DAIGateway} from "../../src/L1/gateways/L1DAIGateway.sol";
+import {MultipleVersionRollupVerifier} from "../../src/L1/rollup/MultipleVersionRollupVerifier.sol";
 import {ScrollChain} from "../../src/L1/rollup/ScrollChain.sol";
 import {L1MessageQueue} from "../../src/L1/rollup/L1MessageQueue.sol";
 import {L2GasPriceOracle} from "../../src/L1/rollup/L2GasPriceOracle.sol";
@@ -71,6 +72,9 @@ contract InitializeL1BridgeContracts is Script {
         ScrollChain(L1_SCROLL_CHAIN_PROXY_ADDR).addSequencer(L1_COMMIT_SENDER_ADDRESS);
         ScrollChain(L1_SCROLL_CHAIN_PROXY_ADDR).addProver(L1_FINALIZE_SENDER_ADDRESS);
 
+        // initialize MultipleVersionRollupVerifier
+        MultipleVersionRollupVerifier(L1_MULTIPLE_VERSION_ROLLUP_VERIFIER_ADDR).initialize(L1_SCROLL_CHAIN_PROXY_ADDR);
+
         // initialize L2GasPriceOracle
         L2GasPriceOracle(L2_GAS_PRICE_ORACLE_PROXY_ADDR).initialize(
             21000, // _txGas

contracts/src/L1/rollup/IScrollChain.sol

@@ -28,6 +28,9 @@ interface IScrollChain {
      * Public View Functions *
      *************************/
 
+    /// @notice The latest finalized batch index.
+    function lastFinalizedBatchIndex() external view returns (uint256);
+
     /// @notice Return the batch hash of a committed batch.
     /// @param batchIndex The index of the batch.
     function committedBatches(uint256 batchIndex) external view returns (bytes32);

contracts/src/L1/rollup/MultipleVersionRollupVerifier.sol

@@ -4,6 +4,7 @@ pragma solidity =0.8.16;
 
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 
+import {IScrollChain} from "./IScrollChain.sol";
 import {IRollupVerifier} from "../../libraries/verifier/IRollupVerifier.sol";
 import {IZkEvmVerifier} from "../../libraries/verifier/IZkEvmVerifier.sol";
 
@@ -38,6 +39,9 @@ contract MultipleVersionRollupVerifier is IRollupVerifier, Ownable {
     /// @notice The lastest used zkevm verifier.
     Verifier public latestVerifier;
 
+    /// @notice The address of ScrollChain contract.
+    address public scrollChain;
+
     /***************
      * Constructor *
      ***************/
@@ -48,6 +52,12 @@ contract MultipleVersionRollupVerifier is IRollupVerifier, Ownable {
         latestVerifier.verifier = _verifier;
     }
 
+    function initialize(address _scrollChain) external onlyOwner {
+        require(scrollChain == address(0), "initialized");
+
+        scrollChain = _scrollChain;
+    }
+
     /*************************
      * Public View Functions *
      *************************/
@@ -101,6 +111,8 @@ contract MultipleVersionRollupVerifier is IRollupVerifier, Ownable {
     /// @param _startBatchIndex The start batch index when the verifier will be used.
     /// @param _verifier The address of new verifier.
     function updateVerifier(uint64 _startBatchIndex, address _verifier) external onlyOwner {
+        require(_startBatchIndex > IScrollChain(scrollChain).lastFinalizedBatchIndex(), "start batch index finalized");
+
         Verifier memory _latestVerifier = latestVerifier;
         require(_startBatchIndex >= _latestVerifier.startBatchIndex, "start batch index too small");
         require(_verifier != address(0), "zero verifier address");

contracts/src/L1/rollup/ScrollChain.sol

@@ -67,8 +67,8 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain {
     /// @notice Whether an account is a prover.
     mapping(address => bool) public isProver;
 
-    /// @notice The latest finalized batch index.
-    uint256 public lastFinalizedBatchIndex;
+    /// @inheritdoc IScrollChain
+    uint256 public override lastFinalizedBatchIndex;
 
     /// @inheritdoc IScrollChain
     mapping(uint256 => bytes32) public override committedBatches;

contracts/src/test/MultipleVersionRollupVerifier.t.sol

@@ -7,6 +7,7 @@ import {DSTestPlus} from "solmate/test/utils/DSTestPlus.sol";
 import {L1MessageQueue} from "../L1/rollup/L1MessageQueue.sol";
 import {MultipleVersionRollupVerifier} from "../L1/rollup/MultipleVersionRollupVerifier.sol";
 
+import {MockScrollChain} from "./mocks/MockScrollChain.sol";
 import {MockZkEvmVerifier} from "./mocks/MockZkEvmVerifier.sol";
 
 contract MultipleVersionRollupVerifierTest is DSTestPlus {
@@ -17,27 +18,54 @@ contract MultipleVersionRollupVerifierTest is DSTestPlus {
     MockZkEvmVerifier private v0;
     MockZkEvmVerifier private v1;
     MockZkEvmVerifier private v2;
+    MockScrollChain private chain;
 
     function setUp() external {
         v0 = new MockZkEvmVerifier();
         v1 = new MockZkEvmVerifier();
         v2 = new MockZkEvmVerifier();
+        chain = new MockScrollChain();
 
         verifier = new MultipleVersionRollupVerifier(address(v0));
     }
 
+    function testInitialize(address _chain) external {
+        hevm.assume(_chain != address(0));
+
+        // set by non-owner, should revert
+        hevm.startPrank(address(1));
+        hevm.expectRevert("Ownable: caller is not the owner");
+        verifier.initialize(_chain);
+        hevm.stopPrank();
+
+        // succeed
+        assertEq(verifier.scrollChain(), address(0));
+        verifier.initialize(_chain);
+        assertEq(verifier.scrollChain(), _chain);
+
+        // initialized, revert
+        hevm.expectRevert("initialized");
+        verifier.initialize(_chain);
+    }
+
     function testUpdateVerifier(address _newVerifier) external {
         hevm.assume(_newVerifier != address(0));
 
+        verifier.initialize(address(chain));
+
         // set by non-owner, should revert
         hevm.startPrank(address(1));
         hevm.expectRevert("Ownable: caller is not the owner");
         verifier.updateVerifier(0, address(0));
         hevm.stopPrank();
 
+        // start batch index finalized, revert
+        hevm.expectRevert("start batch index finalized");
+        verifier.updateVerifier(0, address(1));
+
         // zero verifier address, revert
         hevm.expectRevert("zero verifier address");
-        verifier.updateVerifier(0, address(0));
+        verifier.updateVerifier(1, address(0));
 
         // change to random operator
         assertEq(verifier.legacyVerifiersLength(), 0);
@@ -65,6 +93,8 @@ contract MultipleVersionRollupVerifierTest is DSTestPlus {
     }
 
     function testGetVerifier() external {
+        verifier.initialize(address(chain));
+
         verifier.updateVerifier(100, address(v1));
         verifier.updateVerifier(300, address(v2));
 
@@ -80,6 +110,8 @@ contract MultipleVersionRollupVerifierTest is DSTestPlus {
     }
 
     function testVerifyAggregateProof() external {
+        verifier.initialize(address(chain));
+
         verifier.updateVerifier(100, address(v1));
         verifier.updateVerifier(300, address(v2));
 

contracts/src/test/mocks/MockScrollChain.sol

@@ -7,18 +7,7 @@ import {ScrollChain} from "../../L1/rollup/ScrollChain.sol";
 contract MockScrollChain is ScrollChain {
     constructor() ScrollChain(0) {}
 
-    /*
-    function computePublicInputHash(uint64 accTotalL1Messages, Batch memory batch)
-        external
-        view
-        returns (
-            bytes32,
-            uint64,
-            uint64,
-            uint64
-        )
-    {
-        return _computePublicInputHash(accTotalL1Messages, batch);
+    function setLastFinalizedBatchIndex(uint256 _lastFinalizedBatchIndex) external {
+        lastFinalizedBatchIndex = _lastFinalizedBatchIndex;
     }
-    */
 }