Improve prover setup#112
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the Ethscriptions prover system from immediate proving to a batched approach. The change improves efficiency by queueing ethscriptions during block operations and flushing them in batches at block boundaries.
- Replaces immediate
proveEthscriptionDatacalls with a queue-and-flush mechanism - Integrates L1Block contract to trigger batch flushes at the start of each new block
- Updates test suite to verify batched proving behavior and removes failure handling tests that are no longer applicable
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/src/EthscriptionsProver.sol | Implements batched proving with queue management and L1Block-triggered flushing |
| contracts/src/Ethscriptions.sol | Switches from immediate proving to queueing ethscriptions for batch processing |
| contracts/src/L2/L1Block.sol | Adds flush trigger at block boundaries to process queued proofs |
| contracts/test/EthscriptionsProver.t.sol | Updates tests to verify batched proving functionality |
| contracts/test/EthscriptionsFailureHandling.t.sol | Removes failure handling tests as they're no longer applicable with the new design |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| delete queuedProofInfo[txHash]; | ||
| } | ||
|
|
||
| emit ProofBatchFlushed(count, block.number - 1); |
There was a problem hiding this comment.
The event emits block.number - 1 which could underflow if block.number is 0. Consider using a safer approach or documenting why this subtraction is safe in this context.
| emit ProofBatchFlushed(count, block.number - 1); | |
| emit ProofBatchFlushed(count, block.number > 0 ? block.number - 1 : 0); |
| // Decode the data to get the count (first uint256 in data) | ||
| proofCount = abi.decode(logs[i].data, (uint256)); |
There was a problem hiding this comment.
The comment indicates this decodes the count as the first uint256 in data, but the ProofBatchFlushed event signature shows it has two parameters (count, blockNumber). This should decode both values or clarify which specific parameter is being extracted.
| // Decode the data to get the count (first uint256 in data) | |
| proofCount = abi.decode(logs[i].data, (uint256)); | |
| // Decode the data to get the count (first uint256 in data, second is blockNumber) | |
| (uint256 count, ) = abi.decode(logs[i].data, (uint256, uint256)); | |
| proofCount = count; |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /// @param ethscriptionTxHash The transaction hash of the ethscription | ||
| function proveEthscriptionData(bytes32 ethscriptionTxHash) external virtual { | ||
| /// @param proofInfo The queued proof info containing block data | ||
| function _createAndSendProof(bytes32 ethscriptionTxHash, QueuedProof storage proofInfo) internal { |
There was a problem hiding this comment.
The proofInfo parameter should be passed by value (memory) instead of by reference (storage) since it's only being read and doesn't need to be modified. Using storage unnecessarily keeps a reference to storage which is less gas efficient for read-only operations.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.