Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

MPT: Testing & go library fix #1757

Merged
merged 25 commits into from
Feb 8, 2024
Merged

MPT: Testing & go library fix #1757

merged 25 commits into from
Feb 8, 2024

Conversation

adria0
Copy link
Member

@adria0 adria0 commented Feb 5, 2024

Description

This PR:

  • Removes the light client (that now is deprecated) and moves its StateUpdate circuit into bin/mpt-test.
  • bin/mpt-test is a tool to prove mainnet tests, see the README inside for more information.
  • Is not possible to link to external golang libraries in the same binary, so this PR also moves golang MPT code inside /geth-utils

Type of change

  • New feature (non-breaking change which adds functionality)

Contents

  • Removal if light-client-poc folder
  • Move mpt-witness-generator into geth-utils, this includes also renaming the git action workflow file to geth utils.
  • Create new bin/mpt-folder
    • access-lists contains JSON files of accesslists of mainnet blocks
    • src/circuit contains a circuit and the witness generator to test chained MPT proofs. Well, there's some stuff that is not strictly mandatory to be there - it's taken from light client circuit - but it can be done later.
    • src/cache.rs it's a proxy server with a cache file. There's a lot of RPC calls to execute this tests and this allows to cache them all into a file, this makes the testing being possible without accessing any RPC server and speeding-up it.

How Has This Been Tested?

For the moment, some of the tests are not passing, but the idea is to integrate into CI when tests passes successfully.

@github-actions github-actions bot added crate-zkevm-circuits Issues related to the zkevm-circuits workspace member crate-geth-utils Issues related to the geth-utils workspace member labels Feb 5, 2024
@adria0 adria0 changed the title MPT Testing MPT: Testing & go library fix Feb 5, 2024
@github-actions github-actions bot added the CI Issues related to the Continuous Integration mechanisms of the repository. label Feb 5, 2024
@adria0 adria0 force-pushed the mpt-testing branch 3 times, most recently from 752d895 to b9a9bb4 Compare February 5, 2024 15:44
@adria0 adria0 marked this pull request as ready for review February 5, 2024 15:49
@adria0 adria0 force-pushed the mpt-testing branch 2 times, most recently from fa47682 to 872b3cf Compare February 5, 2024 16:27
@github-actions github-actions bot added T-bench Type: benchmark improvements crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member labels Feb 5, 2024
Copy link
Collaborator

@miha-stopar miha-stopar left a comment

Choose a reason for hiding this comment

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

Nice! Only one comment: it would be good to have instructions how to run the tests (how to ignore keccak ...) in bin/mpt-test/README.md.

@lispc
Copy link
Collaborator

lispc commented Feb 7, 2024

one comment: "not possible to link to external golang libraries in the same binary". Not sure about this, in our repo we use dynamic library (.so), so we can put 2 golang libs inside 1 rust main bin.

@adria0
Copy link
Member Author

adria0 commented Feb 7, 2024

Nice! Only one comment: it would be good to have instructions how to run the tests (how to ignore keccak ...) in bin/mpt-test/README.md.

Keccak is ignored by default!

default = ["disable-keccak"]

So probally it will be nice to explain HOW to run them with keccak 👍 I will add a comment on this.

The idea is to run the tests by using test_mainnet_blocks.sh that automatically downloads the caché file and runs the tests, as is explained in the README.md, do you think that needs to be somehow improved?

Edit: added 0f6d620

@adria0
Copy link
Member Author

adria0 commented Feb 7, 2024

one comment: "not possible to link to external golang libraries in the same binary". Not sure about this, in our repo we use dynamic library (.so), so we can put 2 golang libs inside 1 rust main bin.

If I understand correctly, you linked two .so without problems? Btw, is not an .rlib instead a .so?

Happened to @rrtoledo that when trying to integrate MPT circuit go library in bus-mapping the it crashed really badly with minpc= 0x105e6dc20 min= 0x105e6dc20 maxpc= 0x1064282c0 max= 0x1064283b0 fatal error: minpc or maxpc invalid runtime: panic before malloc heap initialized.

When we joined the two golang libraries everything worked ok, so we just glued together the libraries.

@rrtoledo
Copy link
Contributor

rrtoledo commented Feb 7, 2024

The code looks good to me! Only problem is I cannot run the script test_mainnet_blocks. I have been looking at it since yesterday with Adria.

@miha-stopar
Copy link
Collaborator

Nice! Only one comment: it would be good to have instructions how to run the tests (how to ignore keccak ...) in bin/mpt-test/README.md.

Keccak is ignored by default!

default = ["disable-keccak"]

So probally it will be nice to explain HOW to run them with keccak 👍 I will add a comment on this.

The idea is to run the tests by using test_mainnet_blocks.sh that automatically downloads the caché file and runs the tests, as is explained in the README.md, do you think that needs to be somehow improved?

Edit: added 0f6d620

Yep, sorry, meant with keccak. Thanks!

Copy link
Contributor

@rrtoledo rrtoledo left a comment

Choose a reason for hiding this comment

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

Thanks for the work!

@adria0 adria0 added this pull request to the merge queue Feb 8, 2024
Merged via the queue into main with commit 3151efd Feb 8, 2024
15 checks passed
@adria0 adria0 deleted the mpt-testing branch February 8, 2024 10:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI Issues related to the Continuous Integration mechanisms of the repository. crate-circuit-benchmarks Issues related to the circuit-benchmarks workspace member crate-geth-utils Issues related to the geth-utils workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member T-bench Type: benchmark improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants