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.

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

Add Scroll integration #42

Merged
merged 129 commits into from
May 27, 2024
Merged

Add Scroll integration #42

merged 129 commits into from
May 27, 2024

Conversation

makoto
Copy link
Member

@makoto makoto commented May 8, 2024

List of changes

  • Change StateProof interface from stateTrieWitness: string[];, storageProofs: string[][]; to stateTrieWitness: string;, storageProofs: string[];
  • Modify getStorageValues to take a proof verification function which isn't dependent on SecureMerkleTrie
  • Move getStorageRoot from EVMHelper.sol to MerkleTrieProofHelper.sol

Gas comparison

Arbitrum

  ArbVerifier
Gas estimate 770439
    ✔ simple proofs for fixed values (2154ms)
Gas estimate 722247
    ✔ simple proofs for dynamic values (1314ms)
Gas estimate 777244
    ✔ nested proofs for dynamic values (1361ms)
Gas estimate 1045721
    ✔ nested proofs for long dynamic values (1437ms)
Gas estimate 887013
    ✔ nested proofs with lookbehind (1206ms)
Gas estimate 940514
    ✔ nested proofs with lookbehind for dynamic values (1631ms)
Gas estimate 726250
    ✔ mappings with variable-length keys (1337ms)
Gas estimate 840324
    ✔ nested proofs of mappings with variable-length keys (1479ms)
Gas estimate 679429
    ✔ treats uninitialized storage elements as zeroes (1040ms)
Gas estimate 683462
    ✔ treats uninitialized dynamic values as empty strings (1426ms)

Scroll

  ScrollVerifier
Gas estimate 1187799
    ✔ simple proofs for fixed values (2016ms)
Gas estimate 1188778
    ✔ simple proofs for dynamic values (1545ms)
Gas estimate 1128781
    ✔ nested proofs for dynamic values (1849ms)
Gas estimate 3391568
    ✔ nested proofs for long dynamic values (1673ms)
Gas estimate 2294214
    ✔ nested proofs with lookbehind (1449ms)
Gas estimate 2231340
    ✔ nested proofs with lookbehind for dynamic values (1838ms)
Gas estimate 1160883
    ✔ mappings with variable-length keys (1364ms)
Gas estimate 2295970
    ✔ nested proofs of mappings with variable-length keys (1510ms)
Gas estimate 1093162
    ✔ treats uninitialized storage elements as zeroes (1357ms)
Gas estimate 1097201
    ✔ treats uninitialized dynamic values as empty strings (1488ms)

Things to improve

  • ScrollProofService is currently fetching the batch_index via centralised API server. Should replace it with l1 smart contract call once Scroll team provides the functionality.
  • verifier.verifyZkTrieProof currently 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.

makoto and others added 18 commits April 26, 2024 10:37
Co-authored-by: Nick Johnson <[email protected]>
* 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)
@makoto makoto requested a review from Arachnid May 8, 2024 15:10
evm-gateway/src/utils.ts Outdated Show resolved Hide resolved
evm-verifier/contracts/MerkleTrieProofHelper.sol Outdated Show resolved Hide resolved
scroll-gateway/src/ScrollProofService.ts Outdated Show resolved Hide resolved
* @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]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't finalized be true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

scroll-verifier/contracts/ScrollVerifier.sol Outdated Show resolved Hide resolved
scroll-verifier/contracts/ScrollVerifier.sol Show resolved Hide resolved
@makoto makoto merged commit 14ebd3f into main May 27, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants