Add Scroll integration #42
Open
+1,580
−93
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add indentation Co-authored-by: Nick Johnson <arachnid@notdot.net>
* remove log result util * bump server-analytics version, update types and functions * bypass frozen-lockfile flag for deps update * bypass frozen-lockfile flag for deps update * lint * revert frozen-lockfile flag * keep rozen-lockfile disabled * lint
Co-authored-by: Nick Johnson <arachnid@notdot.net>
* move propsDecoder * Add Request as type * Update workers-types * Fix lint error
ScrollVerifier
Gas estimate 2418085
✔ simple proofs for fixed values (2652ms)
Gas estimate 2419066
✔ simple proofs for dynamic values (1641ms)
Gas estimate 2282506
✔ nested proofs for dynamic values (1800ms)
Gas estimate 4773362
✔ nested proofs for long dynamic values (1960ms)
Gas estimate 3607123
✔ nested proofs with lookbehind (2488ms)
Gas estimate 3527831
✔ nested proofs with lookbehind for dynamic values (1810ms)
Gas estimate 2353134
✔ mappings with variable-length keys (1721ms)
Gas estimate 3608890
✔ nested proofs of mappings with variable-length keys (2086ms)
Gas estimate 2223797
✔ treats uninitialized storage elements as zeroes (1534ms)
Gas estimate 2228241
✔ treats uninitialized dynamic values as empty strings (1800ms)
Arachnid
requested changes
May 14, 2024
| * @dev Returns an object representing a block whose state can be proven on L1. | ||
| */ | ||
| public async getProvableBlock(): Promise<ScrollProvableBlock> { | ||
| const block = await this.l2Provider.send("eth_getBlockByNumber", ["finalized", false]); |
There was a problem hiding this comment.
I don't think it means finalized to be false
According to the doc (https://www.ankr.com/docs/rpc-service/chains/chains-api/scroll/#eth_getblockbynumber) ,
<boolean>: if true it returns the full transaction objects,
if false — only the hashes of the transactions.
both L1ProofService and OPProofService set the second value as false as well FYI
| bytes memory proof | ||
| ) external view returns (bytes[] memory values) { | ||
| (ScrollWitnessData memory scrollData, StateProof memory stateProof) = abi.decode(proof, (ScrollWitnessData, StateProof)); | ||
| bytes32 expectedStateRoot = IScrollChain(verifier.rollup()).finalizedStateRoots(scrollData.batchIndex); |
There was a problem hiding this comment.
We need to add support for checking the block is the right age.
There was a problem hiding this comment.
How did you handle on L1 and OP Verifier? I didn't find any right age check on these verifiers.
Co-authored-by: Nick Johnson <arachnid@notdot.net>
Co-authored-by: Nick Johnson <arachnid@notdot.net>
zimpha
reviewed
May 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
List of changes
StateProofinterface fromstateTrieWitness: string[];, storageProofs: string[][];tostateTrieWitness: string;, storageProofs: string[];getStorageValuesto take a proof verification function which isn't dependent on SecureMerkleTriegetStorageRootfromEVMHelper.soltoMerkleTrieProofHelper.solGas comparison
Arbitrum
Scroll
Things to improve
ScrollProofServiceis currently fetching thebatch_indexvia centralised API server. Should replace it with l1 smart contract call once Scroll team provides the functionality.verifier.verifyZkTrieProofcurrently verifies account proof every time it tries to verify storage proof which is unnecessary. We are currently requesting Scroll team to provide function that can only verify account proof or storage proof, not both at the same time.