feat(protocol): pacaya fork with simplified based rollup protocol #18535
Conversation
dantaik
added
option.pinned
feat(protocol): pacaya fork with simplified based rollup protocol
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
…n SignalService (#18763)
| __Taiko_init(_owner, _rollupResolver, _genesisBlockHash); | ||
| } | ||
|
|
||
| /// @notice Proposes multiple batches. |
There was a problem hiding this comment.
@dantaik I don't think this allows for proposing multiple batches, does it?
| /// @param blockHash The hash of the verified batch. | ||
| event BatchesVerified(uint64 batchId, bytes32 blockHash); | ||
|
|
||
| error AnchorBlockIdSmallerThanParent(); |
There was a problem hiding this comment.
@dantaik How about we add error selectors as comments for each error definition? It is very helpful to read and debug. If you agree, I can submit a PR later to do that.
…ed by the same verifier. (#18767)
| /// @param blockHash The hash of the verified batch. | ||
| event BatchesVerified(uint64 batchId, bytes32 blockHash); | ||
|
|
||
| error AnchorBlockIdSmallerThanParent(); |
There was a problem hiding this comment.
| error AnchorBlockIdSmallerThanParent(); | |
| // @dev Error selectors: 0xfe1698b2 | |
| error AnchorBlockIdSmallerThanParent(); |
Here's what it looks like
| state.stats1.genesisHeight = uint64(block.number); | ||
|
|
||
| state.stats2.lastProposedIn = uint56(block.number); | ||
| state.stats2.numBatches = 1; |
There was a problem hiding this comment.
lastVerifiedBatchId also needs to start from 1 ?
| uint256 stopBatchId = (_config.maxBatchesToVerify * _length + _stats2.lastVerifiedBatchId) | ||
| .min(_stats2.numBatches); | ||
|
|
||
| for (++batchId; batchId < stopBatchId; ++batchId) { |
There was a problem hiding this comment.
The numBatches already += 1 before this function, so _config.maxBatchesToVerify * _length + _stats2.lastVerifiedBatchId also needs to +1 to keep aligned with numBatches.
| /// | ||
| /// @dev Registered in the address resolver as "taiko". | ||
| /// @custom:security-contact security@taiko.xyz | ||
| abstract contract TaikoInbox is EssentialContract, ITaikoInbox, ITaiko, IFork { |
There was a problem hiding this comment.
Do we have any test about the contract size after moving such much code into one contract from lib contracts?
Co-authored-by: Daniel Wang <dan@taiko.xyz>
|
|
||
| // SSTORE #3 {{ | ||
| if (stats2.numBatches == config.forkHeights.pacaya) { | ||
| batch.lastBlockId = batch.batchId + uint8(params.blocks.length) - 1; |
There was a problem hiding this comment.
| batch.lastBlockId = batch.batchId + uint8(params.blocks.length) - 1; | |
| batch.lastBlockId = batch.batchId + uint64(params.blocks.length) - 1; |
There was a problem hiding this comment.
The params.blocks length limitation is maxBlocksPerBatch(768).
| if (stats2.numBatches == config.forkHeights.pacaya) { | ||
| batch.lastBlockId = batch.batchId + uint8(params.blocks.length) - 1; | ||
| } else { | ||
| batch.lastBlockId = lastBatch.lastBlockId + uint8(params.blocks.length); |
There was a problem hiding this comment.
| batch.lastBlockId = lastBatch.lastBlockId + uint8(params.blocks.length); | |
| batch.lastBlockId = lastBatch.lastBlockId + uint64(params.blocks.length); |
| bytes32 _genesisBlockHash | ||
| ) | ||
| external | ||
| initializer |
There was a problem hiding this comment.
Shouldn't this be a reinitializer?
|
|
||
| /// @notice Initializes the contract. | ||
| /// @param _owner The owner of this contract. msg.sender will be used if this value is zero. | ||
| /// @param _resolver The address of the {DefaultResolver} contract. |
There was a problem hiding this comment.
The comment states that _resolver is the DefaultResolver contract, but I see that this initialiser either uses RollupResolver or SharedResolver, and neither of these resolvers implement DefaultResolver, they only inherit from ResolverBase
| require(!stats2.paused, ContractPaused()); | ||
|
|
||
| Config memory config = getConfig(); | ||
| require(stats2.numBatches >= config.forkHeights.pacaya, InvalidForkHeight()); |
There was a problem hiding this comment.
At the first call of this proposeBatch function, the inited value of numBatches(1) can't match the pacaya height, which means that the first call will never succeed.
The two variables in this judgment have nothing to do with the calling parameters of the function, which means that whether the requirements are met is certain and cannot be modified in the current call.
|
|
||
| unchecked { | ||
| require( | ||
| stats2.numBatches < stats2.lastVerifiedBatchId + config.maxBatchProposals, |
Summary
This PR introduces significant changes, focusing on the following improvements:
Simplified Taiko Rollup Protocol:
The protocol has been streamlined by removing tier proofs and contestation mechanisms. The updated assumption is straightforward: the validity proof is trusted. Existing
TaikoL1tests have been removed, and only a limited number of new tests have been added, resulting in low overall test coverage.Support smaller more frequent L2 blocks (replacing soft blocks)
See feat(protocol): support smaller more frequent L2 blocks (replacing soft blocks) #18743
Enhanced Address Lookup:
A new interface,
IResolver, has been introduced to improve how addresses are resolved within the system.