Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion common/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions contracts/scripts/foundry/InitializeL1BridgeContracts.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions contracts/src/L1/rollup/IScrollChain.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions contracts/src/L1/rollup/MultipleVersionRollupVerifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 *
***************/
Expand All @@ -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 *
*************************/
Expand Down Expand Up @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/L1/rollup/ScrollChain.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
34 changes: 33 additions & 1 deletion contracts/src/test/MultipleVersionRollupVerifier.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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));

Expand All @@ -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));

Expand Down
15 changes: 2 additions & 13 deletions contracts/src/test/mocks/MockScrollChain.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
*/
}