fix(contracts): check actual number of transactions in each chunk (#887)

Co-authored-by: zimpha <zimpha@users.noreply.github.com>
This commit is contained in:
Xi Lin
2023-09-01 11:28:54 +08:00
committed by GitHub
parent 3958e8bd86
commit 8daa5d5496
4 changed files with 52 additions and 93 deletions

View File

@@ -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 {

View File

@@ -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

View File

@@ -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");
}

View File

@@ -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 {