diff --git a/common/version/version.go b/common/version/version.go index 2e7b43810..0349de375 100644 --- a/common/version/version.go +++ b/common/version/version.go @@ -7,7 +7,7 @@ import ( "strings" ) -var tag = "v4.2.9" +var tag = "v4.2.10" var commit = func() string { if info, ok := debug.ReadBuildInfo(); ok { diff --git a/contracts/docs/apis/L2ScrollMessenger.md b/contracts/docs/apis/L2ScrollMessenger.md index 6a21e6308..8d682ccc2 100644 --- a/contracts/docs/apis/L2ScrollMessenger.md +++ b/contracts/docs/apis/L2ScrollMessenger.md @@ -104,45 +104,6 @@ Mapping from L2 message hash to sent status. |---|---|---| | _0 | bool | undefined | -### l1MessageFailedTimes - -```solidity -function l1MessageFailedTimes(bytes32) external view returns (uint256) -``` - -Mapping from L1 message hash to the number of failure times. - - - -#### Parameters - -| Name | Type | Description | -|---|---|---| -| _0 | bytes32 | undefined | - -#### Returns - -| Name | Type | Description | -|---|---|---| -| _0 | uint256 | undefined | - -### maxFailedExecutionTimes - -```solidity -function maxFailedExecutionTimes() external view returns (uint256) -``` - -The maximum number of times each L1 message can fail on L2. - - - - -#### Returns - -| Name | Type | Description | -|---|---|---| -| _0 | uint256 | undefined | - ### messageQueue ```solidity @@ -329,22 +290,6 @@ Update fee vault contract. |---|---|---| | _newFeeVault | address | The address of new fee vault contract. | -### updateMaxFailedExecutionTimes - -```solidity -function updateMaxFailedExecutionTimes(uint256 _newMaxFailedExecutionTimes) external nonpayable -``` - -Update max failed execution times. - -*This function can only called by contract owner.* - -#### Parameters - -| Name | Type | Description | -|---|---|---| -| _newMaxFailedExecutionTimes | uint256 | The new max failed execution times. | - ### updateRateLimiter ```solidity diff --git a/contracts/src/L1/rollup/ScrollChain.sol b/contracts/src/L1/rollup/ScrollChain.sol index 57f429607..b254304f0 100644 --- a/contracts/src/L1/rollup/ScrollChain.sol +++ b/contracts/src/L1/rollup/ScrollChain.sol @@ -36,10 +36,10 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { /// @param newVerifier The address of new rollup verifier. event UpdateVerifier(address indexed oldVerifier, address indexed newVerifier); - /// @notice Emitted when the value of `maxNumL2TxInChunk` is updated. - /// @param oldMaxNumL2TxInChunk The old value of `maxNumL2TxInChunk`. - /// @param newMaxNumL2TxInChunk The new value of `maxNumL2TxInChunk`. - event UpdateMaxNumL2TxInChunk(uint256 oldMaxNumL2TxInChunk, uint256 newMaxNumL2TxInChunk); + /// @notice Emitted when the value of `maxNumTxInChunk` is updated. + /// @param oldMaxNumTxInChunk The old value of `maxNumTxInChunk`. + /// @param newMaxNumTxInChunk The new value of `maxNumTxInChunk`. + event UpdateMaxNumTxInChunk(uint256 oldMaxNumTxInChunk, uint256 newMaxNumTxInChunk); /************* * Constants * @@ -53,7 +53,7 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { *************/ /// @notice The maximum number of transactions allowed in each chunk. - uint256 public maxNumL2TxInChunk; + uint256 public maxNumTxInChunk; /// @notice The address of L1MessageQueue. address public messageQueue; @@ -107,16 +107,16 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { function initialize( address _messageQueue, address _verifier, - uint256 _maxNumL2TxInChunk + uint256 _maxNumTxInChunk ) public initializer { OwnableUpgradeable.__Ownable_init(); messageQueue = _messageQueue; verifier = _verifier; - maxNumL2TxInChunk = _maxNumL2TxInChunk; + maxNumTxInChunk = _maxNumTxInChunk; emit UpdateVerifier(address(0), _verifier); - emit UpdateMaxNumL2TxInChunk(0, _maxNumL2TxInChunk); + emit UpdateMaxNumTxInChunk(0, _maxNumTxInChunk); } /************************* @@ -398,13 +398,13 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { emit UpdateVerifier(_oldVerifier, _newVerifier); } - /// @notice Update the value of `maxNumL2TxInChunk`. - /// @param _maxNumL2TxInChunk The new value of `maxNumL2TxInChunk`. - function updateMaxNumL2TxInChunk(uint256 _maxNumL2TxInChunk) external onlyOwner { - uint256 _oldMaxNumL2TxInChunk = maxNumL2TxInChunk; - maxNumL2TxInChunk = _maxNumL2TxInChunk; + /// @notice Update the value of `maxNumTxInChunk`. + /// @param _maxNumTxInChunk The new value of `maxNumTxInChunk`. + function updateMaxNumTxInChunk(uint256 _maxNumTxInChunk) external onlyOwner { + uint256 _oldMaxNumTxInChunk = maxNumTxInChunk; + maxNumTxInChunk = _maxNumTxInChunk; - emit UpdateMaxNumL2TxInChunk(_oldMaxNumL2TxInChunk, _maxNumL2TxInChunk); + emit UpdateMaxNumTxInChunk(_oldMaxNumTxInChunk, _maxNumTxInChunk); } /// @notice Pause the contract @@ -462,19 +462,26 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { uint256 _numBlocks = ChunkCodec.validateChunkLength(chunkPtr, _chunk.length); - // concatenate block contexts - uint256 _totalTransactionsInChunk; - for (uint256 i = 0; i < _numBlocks; i++) { - dataPtr = ChunkCodec.copyBlockContext(chunkPtr, dataPtr, i); - uint256 _numTransactionsInBlock = ChunkCodec.numTransactions(blockPtr); - unchecked { - _totalTransactionsInChunk += _numTransactionsInBlock; - blockPtr += ChunkCodec.BLOCK_CONTEXT_LENGTH; + // concatenate block contexts, use scope to avoid stack too deep + { + uint256 _totalTransactionsInChunk; + for (uint256 i = 0; i < _numBlocks; i++) { + dataPtr = ChunkCodec.copyBlockContext(chunkPtr, dataPtr, i); + uint256 _numTransactionsInBlock = ChunkCodec.numTransactions(blockPtr); + unchecked { + _totalTransactionsInChunk += _numTransactionsInBlock; + blockPtr += ChunkCodec.BLOCK_CONTEXT_LENGTH; + } + } + assembly { + mstore(0x40, add(dataPtr, mul(_totalTransactionsInChunk, 0x20))) // reserve memory for tx hashes } } + // It is used to compute the actual number of transactions in chunk. + uint256 txHashStartDataPtr; assembly { - mstore(0x40, add(dataPtr, mul(_totalTransactionsInChunk, 0x20))) // reserve memory for tx hashes + txHashStartDataPtr := dataPtr blockPtr := add(chunkPtr, 1) // reset block ptr } @@ -513,11 +520,8 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { } } - // check the number of L2 transactions in the chunk - require( - _totalTransactionsInChunk - _totalNumL1MessagesInChunk <= maxNumL2TxInChunk, - "too many L2 txs in one chunk" - ); + // check the actual number of transactions in the chunk + require((dataPtr - txHashStartDataPtr) / 32 <= maxNumTxInChunk, "too many txs in one chunk"); // check chunk has correct length require(l2TxPtr - chunkPtr == _chunk.length, "incomplete l2 transaction data"); @@ -550,9 +554,10 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { unchecked { uint256 _bitmap; + uint256 rem; for (uint256 i = 0; i < _numL1Messages; i++) { uint256 quo = _totalL1MessagesPoppedInBatch >> 8; - uint256 rem = _totalL1MessagesPoppedInBatch & 0xff; + rem = _totalL1MessagesPoppedInBatch & 0xff; // load bitmap every 256 bits if (i == 0 || rem == 0) { @@ -574,7 +579,7 @@ contract ScrollChain is OwnableUpgradeable, PausableUpgradeable, IScrollChain { } // check last L1 message is not skipped, _totalL1MessagesPoppedInBatch must > 0 - uint256 rem = (_totalL1MessagesPoppedInBatch - 1) & 0xff; + rem = (_totalL1MessagesPoppedInBatch - 1) & 0xff; require(((_bitmap >> rem) & 1) == 0, "cannot skip last L1 message"); } diff --git a/contracts/src/test/ScrollChain.t.sol b/contracts/src/test/ScrollChain.t.sol index af934f0ac..8eb30ce91 100644 --- a/contracts/src/test/ScrollChain.t.sol +++ b/contracts/src/test/ScrollChain.t.sol @@ -19,7 +19,7 @@ contract ScrollChainTest is DSTestPlus { event UpdateSequencer(address indexed account, bool status); event UpdateProver(address indexed account, bool status); event UpdateVerifier(address indexed oldVerifier, address indexed newVerifier); - event UpdateMaxNumL2TxInChunk(uint256 oldMaxNumL2TxInChunk, uint256 newMaxNumL2TxInChunk); + event UpdateMaxNumTxInChunk(uint256 oldMaxNumTxInChunk, uint256 newMaxNumTxInChunk); event CommitBatch(uint256 indexed batchIndex, bytes32 indexed batchHash); event FinalizeBatch(uint256 indexed batchIndex, bytes32 indexed batchHash, bytes32 stateRoot, bytes32 withdrawRoot); @@ -429,6 +429,15 @@ contract ScrollChainTest is DSTestPlus { mstore(add(bitmap, add(0x20, 32)), 42) // bitmap1 } + // too many txs in one chunk, revert + rollup.updateMaxNumTxInChunk(2); // 3 - 1 + hevm.expectRevert("too many txs in one chunk"); + rollup.commitBatch(0, batchHeader1, chunks, bitmap); // first chunk with too many txs + rollup.updateMaxNumTxInChunk(185); // 5+10+300 - 2 - 127 + hevm.expectRevert("too many txs in one chunk"); + rollup.commitBatch(0, batchHeader1, chunks, bitmap); // second chunk with too many txs + + rollup.updateMaxNumTxInChunk(186); hevm.expectEmit(true, true, false, true); emit CommitBatch(2, bytes32(0x03a9cdcb9d582251acf60937db006ec99f3505fd4751b7c1f92c9a8ef413e873)); rollup.commitBatch(0, batchHeader1, chunks, bitmap); @@ -631,20 +640,20 @@ contract ScrollChainTest is DSTestPlus { assertEq(rollup.verifier(), _newVerifier); } - function testUpdateMaxNumL2TxInChunk(uint256 _maxNumL2TxInChunk) public { + function testUpdateMaxNumTxInChunk(uint256 _maxNumTxInChunk) public { // set by non-owner, should revert hevm.startPrank(address(1)); hevm.expectRevert("Ownable: caller is not the owner"); - rollup.updateMaxNumL2TxInChunk(_maxNumL2TxInChunk); + rollup.updateMaxNumTxInChunk(_maxNumTxInChunk); hevm.stopPrank(); // change to random operator hevm.expectEmit(false, false, false, true); - emit UpdateMaxNumL2TxInChunk(100, _maxNumL2TxInChunk); + emit UpdateMaxNumTxInChunk(100, _maxNumTxInChunk); - assertEq(rollup.maxNumL2TxInChunk(), 100); - rollup.updateMaxNumL2TxInChunk(_maxNumL2TxInChunk); - assertEq(rollup.maxNumL2TxInChunk(), _maxNumL2TxInChunk); + assertEq(rollup.maxNumTxInChunk(), 100); + rollup.updateMaxNumTxInChunk(_maxNumTxInChunk); + assertEq(rollup.maxNumTxInChunk(), _maxNumTxInChunk); } function testImportGenesisBlock() public {