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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from
Sep 1, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d15a5c7
check actual number of transactions in each chunk
zimpha Aug 30, 2023
8b6976f
chore: auto version bump [bot]
zimpha Aug 30, 2023
83a712c
rename maxNumL2TxInChunk to maxNumTxInChunk
zimpha Sep 1, 2023
c9effa3
merge with develop branch
zimpha Sep 1, 2023
02bce20
chore: auto version bump [bot]
zimpha Sep 1, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
55 changes: 0 additions & 55 deletions contracts/docs/apis/L2ScrollMessenger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
65 changes: 35 additions & 30 deletions contracts/src/L1/rollup/ScrollChain.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/*************************
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
colinlyguo marked this conversation as resolved.
Show resolved Hide resolved
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
}

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand All @@ -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");
}

Expand Down
23 changes: 16 additions & 7 deletions contracts/src/test/ScrollChain.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down