privacy-scaling-explorations / zkevm-circuits

https://privacy-scaling-explorations.github.io/zkevm-circuits/
Other
818 stars 854 forks source link

MPT: Testing & go library fix #1757

Closed adria0 closed 8 months ago

adria0 commented 8 months ago

Description

This PR:

Type of change

Contents

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.

lispc commented 8 months ago

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 commented 8 months ago

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!

https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/e3ffe4450c75dc5bac3dc4f09d8524337038f7b7/bin/mpt-test/Cargo.toml#L26

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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!

https://github.com/privacy-scaling-explorations/zkevm-circuits/blob/e3ffe4450c75dc5bac3dc4f09d8524337038f7b7/bin/mpt-test/Cargo.toml#L26

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!