diff --git a/contracts/src/L1/rollup/ScrollChain.sol b/contracts/src/L1/rollup/ScrollChain.sol index b32f778eac..dd6c2db50b 100644 --- a/contracts/src/L1/rollup/ScrollChain.sol +++ b/contracts/src/L1/rollup/ScrollChain.sol @@ -360,6 +360,10 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { /// @notice Add an account to the sequencer list. /// @param _account The address of account to add. function addSequencer(address _account) external onlyOwner { + // @note Currently many external services rely on EOA sequencer to decode metadata directly from tx.calldata. + // So we explicitly make sure the account is EOA. + require(_account.code.length == 0, "not EOA"); + isSequencer[_account] = true; emit UpdateSequencer(_account, true); @@ -376,6 +380,9 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { /// @notice Add an account to the prover list. /// @param _account The address of account to add. function addProver(address _account) external onlyOwner { + // @note Currently many external services rely on EOA prover to decode metadata directly from tx.calldata. + // So we explicitly make sure the account is EOA. + require(_account.code.length == 0, "not EOA"); isProver[_account] = true; emit UpdateProver(_account, true); diff --git a/contracts/src/test/L1GatewayTestBase.t.sol b/contracts/src/test/L1GatewayTestBase.t.sol index 368f1c5015..2d7f2c8adc 100644 --- a/contracts/src/test/L1GatewayTestBase.t.sol +++ b/contracts/src/test/L1GatewayTestBase.t.sol @@ -106,8 +106,8 @@ abstract contract L1GatewayTestBase is DSTestPlus { } function prepareL2MessageRoot(bytes32 messageHash) internal { - rollup.addSequencer(address(this)); - rollup.addProver(address(this)); + rollup.addSequencer(address(0)); + rollup.addProver(address(0)); // import genesis batch bytes memory batchHeader0 = new bytes(89); @@ -122,7 +122,9 @@ abstract contract L1GatewayTestBase is DSTestPlus { bytes memory chunk0 = new bytes(1 + 60); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); bytes memory batchHeader1 = new bytes(89); assembly { @@ -134,6 +136,7 @@ abstract contract L1GatewayTestBase is DSTestPlus { mstore(add(batchHeader1, add(0x20, 57)), batchHash0) // parentBatchHash } + hevm.startPrank(address(0)); rollup.finalizeBatchWithProof( batchHeader1, bytes32(uint256(1)), @@ -141,5 +144,6 @@ abstract contract L1GatewayTestBase is DSTestPlus { messageHash, new bytes(0) ); + hevm.stopPrank(); } } diff --git a/contracts/src/test/ScrollChain.t.sol b/contracts/src/test/ScrollChain.t.sol index 4ae25f5be4..a3b07b2934 100644 --- a/contracts/src/test/ScrollChain.t.sol +++ b/contracts/src/test/ScrollChain.t.sol @@ -67,30 +67,40 @@ contract ScrollChainTest is DSTestPlus { hevm.expectRevert("caller not sequencer"); rollup.commitBatch(0, batchHeader0, new bytes[](0), new bytes(0)); - rollup.addSequencer(address(this)); + rollup.addSequencer(address(0)); // invalid version, revert + hevm.startPrank(address(0)); hevm.expectRevert("invalid version"); rollup.commitBatch(1, batchHeader0, new bytes[](0), new bytes(0)); + hevm.stopPrank(); // batch is empty, revert + hevm.startPrank(address(0)); hevm.expectRevert("batch is empty"); rollup.commitBatch(0, batchHeader0, new bytes[](0), new bytes(0)); + hevm.stopPrank(); // batch header length too small, revert + hevm.startPrank(address(0)); hevm.expectRevert("batch header length too small"); rollup.commitBatch(0, new bytes(88), new bytes[](1), new bytes(0)); + hevm.stopPrank(); // wrong bitmap length, revert + hevm.startPrank(address(0)); hevm.expectRevert("wrong bitmap length"); rollup.commitBatch(0, new bytes(90), new bytes[](1), new bytes(0)); + hevm.stopPrank(); // incorrect parent batch hash, revert assembly { mstore(add(batchHeader0, add(0x20, 25)), 2) // change data hash for batch0 } + hevm.startPrank(address(0)); hevm.expectRevert("incorrect parent batch hash"); rollup.commitBatch(0, batchHeader0, new bytes[](1), new bytes(0)); + hevm.stopPrank(); assembly { mstore(add(batchHeader0, add(0x20, 25)), 1) // change back } @@ -101,15 +111,19 @@ contract ScrollChainTest is DSTestPlus { // no block in chunk, revert chunk0 = new bytes(1); chunks[0] = chunk0; + hevm.startPrank(address(0)); hevm.expectRevert("no block in chunk"); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); // invalid chunk length, revert chunk0 = new bytes(1); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); hevm.expectRevert("invalid chunk length"); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); // cannot skip last L1 message, revert chunk0 = new bytes(1 + 60); @@ -119,8 +133,10 @@ contract ScrollChainTest is DSTestPlus { chunk0[60] = bytes1(uint8(1)); // numL1Messages = 1 bitmap[31] = bytes1(uint8(1)); chunks[0] = chunk0; + hevm.startPrank(address(0)); hevm.expectRevert("cannot skip last L1 message"); rollup.commitBatch(0, batchHeader0, chunks, bitmap); + hevm.stopPrank(); // num txs less than num L1 msgs, revert chunk0 = new bytes(1 + 60); @@ -130,26 +146,34 @@ contract ScrollChainTest is DSTestPlus { chunk0[60] = bytes1(uint8(3)); // numL1Messages = 3 bitmap[31] = bytes1(uint8(3)); chunks[0] = chunk0; + hevm.startPrank(address(0)); hevm.expectRevert("num txs less than num L1 msgs"); rollup.commitBatch(0, batchHeader0, chunks, bitmap); + hevm.stopPrank(); // incomplete l2 transaction data, revert chunk0 = new bytes(1 + 60 + 1); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); hevm.expectRevert("incomplete l2 transaction data"); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); // commit batch with one chunk, no tx, correctly chunk0 = new bytes(1 + 60); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); assertGt(uint256(rollup.committedBatches(1)), 0); // batch is already committed, revert + hevm.startPrank(address(0)); hevm.expectRevert("batch already committed"); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); } function testFinalizeBatchWithProof() public { @@ -157,8 +181,8 @@ contract ScrollChainTest is DSTestPlus { hevm.expectRevert("caller not prover"); rollup.finalizeBatchWithProof(new bytes(0), bytes32(0), bytes32(0), bytes32(0), new bytes(0)); - rollup.addProver(address(this)); - rollup.addSequencer(address(this)); + rollup.addProver(address(0)); + rollup.addSequencer(address(0)); bytes memory batchHeader0 = new bytes(89); @@ -176,7 +200,9 @@ contract ScrollChainTest is DSTestPlus { chunk0 = new bytes(1 + 60); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); assertGt(uint256(rollup.committedBatches(1)), 0); bytes memory batchHeader1 = new bytes(89); @@ -190,12 +216,15 @@ contract ScrollChainTest is DSTestPlus { } // incorrect batch hash, revert - hevm.expectRevert("incorrect batch hash"); batchHeader1[0] = bytes1(uint8(1)); // change version to 1 + hevm.startPrank(address(0)); + hevm.expectRevert("incorrect batch hash"); rollup.finalizeBatchWithProof(batchHeader1, bytes32(uint256(1)), bytes32(uint256(2)), bytes32(0), new bytes(0)); + hevm.stopPrank(); batchHeader1[0] = bytes1(uint8(0)); // change back // batch header length too small, revert + hevm.startPrank(address(0)); hevm.expectRevert("batch header length too small"); rollup.finalizeBatchWithProof( new bytes(88), @@ -204,8 +233,10 @@ contract ScrollChainTest is DSTestPlus { bytes32(0), new bytes(0) ); + hevm.stopPrank(); // wrong bitmap length, revert + hevm.startPrank(address(0)); hevm.expectRevert("wrong bitmap length"); rollup.finalizeBatchWithProof( new bytes(90), @@ -214,13 +245,17 @@ contract ScrollChainTest is DSTestPlus { bytes32(0), new bytes(0) ); + hevm.stopPrank(); // incorrect previous state root, revert + hevm.startPrank(address(0)); hevm.expectRevert("incorrect previous state root"); rollup.finalizeBatchWithProof(batchHeader1, bytes32(uint256(2)), bytes32(uint256(2)), bytes32(0), new bytes(0)); + hevm.stopPrank(); // verify success assertBoolEq(rollup.isBatchFinalized(1), false); + hevm.startPrank(address(0)); rollup.finalizeBatchWithProof( batchHeader1, bytes32(uint256(1)), @@ -228,12 +263,14 @@ contract ScrollChainTest is DSTestPlus { bytes32(uint256(3)), new bytes(0) ); + hevm.stopPrank(); assertBoolEq(rollup.isBatchFinalized(1), true); assertEq(rollup.finalizedStateRoots(1), bytes32(uint256(2))); assertEq(rollup.withdrawRoots(1), bytes32(uint256(3))); assertEq(rollup.lastFinalizedBatchIndex(), 1); // batch already verified, revert + hevm.startPrank(address(0)); hevm.expectRevert("batch already verified"); rollup.finalizeBatchWithProof( batchHeader1, @@ -242,11 +279,12 @@ contract ScrollChainTest is DSTestPlus { bytes32(uint256(3)), new bytes(0) ); + hevm.stopPrank(); } function testCommitAndFinalizeWithL1Messages() public { - rollup.addSequencer(address(this)); - rollup.addProver(address(this)); + rollup.addSequencer(address(0)); + rollup.addProver(address(0)); // import 300 L1 messages for (uint256 i = 0; i < 300; i++) { @@ -307,14 +345,17 @@ contract ScrollChainTest is DSTestPlus { chunks = new bytes[](1); chunks[0] = chunk0; bitmap = new bytes(32); + hevm.startPrank(address(0)); hevm.expectEmit(true, true, false, true); emit CommitBatch(1, bytes32(0x00847173b29b238cf319cde79512b7c213e5a8b4138daa7051914c4592b6dfc7)); rollup.commitBatch(0, batchHeader0, chunks, bitmap); + hevm.stopPrank(); assertBoolEq(rollup.isBatchFinalized(1), false); bytes32 batchHash1 = rollup.committedBatches(1); assertEq(batchHash1, bytes32(0x00847173b29b238cf319cde79512b7c213e5a8b4138daa7051914c4592b6dfc7)); // finalize batch1 + hevm.startPrank(address(0)); hevm.expectEmit(true, true, false, true); emit FinalizeBatch(1, batchHash1, bytes32(uint256(2)), bytes32(uint256(3))); rollup.finalizeBatchWithProof( @@ -324,6 +365,7 @@ contract ScrollChainTest is DSTestPlus { bytes32(uint256(3)), new bytes(0) ); + hevm.stopPrank(); assertBoolEq(rollup.isBatchFinalized(1), true); assertEq(rollup.finalizedStateRoots(1), bytes32(uint256(2))); assertEq(rollup.withdrawRoots(1), bytes32(uint256(3))); @@ -431,21 +473,28 @@ contract ScrollChainTest is DSTestPlus { // too many txs in one chunk, revert rollup.updateMaxNumTxInChunk(2); // 3 - 1 + hevm.startPrank(address(0)); hevm.expectRevert("too many txs in one chunk"); rollup.commitBatch(0, batchHeader1, chunks, bitmap); // first chunk with too many txs + hevm.stopPrank(); rollup.updateMaxNumTxInChunk(185); // 5+10+300 - 2 - 127 + hevm.startPrank(address(0)); hevm.expectRevert("too many txs in one chunk"); rollup.commitBatch(0, batchHeader1, chunks, bitmap); // second chunk with too many txs + hevm.stopPrank(); rollup.updateMaxNumTxInChunk(186); + hevm.startPrank(address(0)); hevm.expectEmit(true, true, false, true); emit CommitBatch(2, bytes32(0x03a9cdcb9d582251acf60937db006ec99f3505fd4751b7c1f92c9a8ef413e873)); rollup.commitBatch(0, batchHeader1, chunks, bitmap); + hevm.stopPrank(); assertBoolEq(rollup.isBatchFinalized(2), false); bytes32 batchHash2 = rollup.committedBatches(2); assertEq(batchHash2, bytes32(0x03a9cdcb9d582251acf60937db006ec99f3505fd4751b7c1f92c9a8ef413e873)); // verify committed batch correctly + hevm.startPrank(address(0)); hevm.expectEmit(true, true, false, true); emit FinalizeBatch(2, batchHash2, bytes32(uint256(4)), bytes32(uint256(5))); rollup.finalizeBatchWithProof( @@ -455,6 +504,7 @@ contract ScrollChainTest is DSTestPlus { bytes32(uint256(5)), new bytes(0) ); + hevm.stopPrank(); assertBoolEq(rollup.isBatchFinalized(2), true); assertEq(rollup.finalizedStateRoots(2), bytes32(uint256(4))); assertEq(rollup.withdrawRoots(2), bytes32(uint256(5))); @@ -489,7 +539,7 @@ contract ScrollChainTest is DSTestPlus { rollup.revertBatch(new bytes(89), 1); hevm.stopPrank(); - rollup.addSequencer(address(this)); + rollup.addSequencer(address(0)); bytes memory batchHeader0 = new bytes(89); @@ -507,7 +557,9 @@ contract ScrollChainTest is DSTestPlus { chunk0 = new bytes(1 + 60); chunk0[0] = bytes1(uint8(1)); // one block in this chunk chunks[0] = chunk0; + hevm.startPrank(address(0)); rollup.commitBatch(0, batchHeader0, chunks, new bytes(0)); + hevm.stopPrank(); bytes memory batchHeader1 = new bytes(89); assembly { @@ -520,7 +572,9 @@ contract ScrollChainTest is DSTestPlus { } // commit another batch + hevm.startPrank(address(0)); rollup.commitBatch(0, batchHeader1, chunks, new bytes(0)); + hevm.stopPrank(); // count must be nonzero, revert hevm.expectRevert("count must be nonzero"); @@ -563,7 +617,11 @@ contract ScrollChainTest is DSTestPlus { rollup.removeSequencer(_sequencer); hevm.stopPrank(); - // change to random operator + hevm.expectRevert("not EOA"); + rollup.addSequencer(address(this)); + hevm.assume(_sequencer.code.length == 0); + + // change to random EOA operator hevm.expectEmit(true, false, false, true); emit UpdateSequencer(_sequencer, true); @@ -586,7 +644,11 @@ contract ScrollChainTest is DSTestPlus { rollup.removeProver(_prover); hevm.stopPrank(); - // change to random operator + hevm.expectRevert("not EOA"); + rollup.addProver(address(this)); + hevm.assume(_prover.code.length == 0); + + // change to random EOA operator hevm.expectEmit(true, false, false, true); emit UpdateProver(_prover, true); @@ -601,8 +663,8 @@ contract ScrollChainTest is DSTestPlus { } function testSetPause() external { - rollup.addSequencer(address(this)); - rollup.addProver(address(this)); + rollup.addSequencer(address(0)); + rollup.addProver(address(0)); // not owner, revert hevm.startPrank(address(1)); @@ -614,10 +676,12 @@ contract ScrollChainTest is DSTestPlus { rollup.setPause(true); assertBoolEq(true, rollup.paused()); + hevm.startPrank(address(0)); hevm.expectRevert("Pausable: paused"); rollup.commitBatch(0, new bytes(0), new bytes[](0), new bytes(0)); hevm.expectRevert("Pausable: paused"); rollup.finalizeBatchWithProof(new bytes(0), bytes32(0), bytes32(0), bytes32(0), new bytes(0)); + hevm.stopPrank(); // unpause rollup.setPause(false);