Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

Adding the 6th,7th and 8th precompiles and its tests, with the change… #1284

Closed
wants to merge 1 commit into from

Conversation

madiazp
Copy link

@madiazp madiazp commented Oct 5, 2019

…s of the

#1283 PR.

Signed-off-by: matias diaz [email protected]

Go-ethereum use a Cloudflare and a deprecated google package for bn256 modfied to work with libsnark. The cloudflare one have a BSD-3 license so is safe to use and modify.

With this package is possible to code the 6th,7th and 8th precompile contract.

I also notice the changes introduced in the #1283 PR. So I add some of them alongside with the precompile functions to make more easy to work with this PR (no credit stealing intended).

@madiazp
Copy link
Author

madiazp commented Oct 5, 2019

Also add some test for the 5,6,7 and 8th precompiles functions. For some reason the 5th one doesn't work with the changes of the #1283 PR. When the #1283 PR get merged I can add a ZK-SNARK proof in the evm_test that will test the 5,6,7 and 8th at once with a real case use.

Copy link
Contributor

@silasdavis silasdavis 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 this Matias!

I think Clearmatics maintains this forked version of bn256 here: https://github.com/clearmatics/bn256. Let's use that as dependency rather than copying in the one from go-ethereum (though as you note there is no licensing issue with that, I'd just prefer to use a somewhat maintained fork).

If you rebase on master you'll get the other changes I have made, then I will give this another review and get it merged.

go.mod Outdated
@@ -11,10 +11,12 @@ require (
github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf // indirect
github.com/btcsuite/btcd v0.0.0-20190523000118-16327141da8c
github.com/cep21/xdgbasedir v0.0.0-20170329171747-21470bfc93b9
github.com/cloudflare/bn256 v0.0.0-20190523220833-828ba4f91854 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

needs to be removed

go.mod Outdated
github.com/eapache/channels v1.1.0
github.com/eapache/queue v1.1.0 // indirect
github.com/elgs/gojq v0.0.0-20160421194050-81fa9a608a13
github.com/elgs/gosplitargs v0.0.0-20161028071935-a491c5eeb3c8 // indirect
github.com/ethereum/go-ethereum v1.9.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Also needs to be removed :)

@madiazp madiazp force-pushed the master branch 2 times, most recently from bd1e5ef to 9e7b87e Compare October 10, 2019 18:54
@madiazp
Copy link
Author

madiazp commented Oct 10, 2019

Done ;)

Also fixed a problem with expMod. And added a proof snark test in evm_test.go

Your test of expMod on evm_test failed but the test of it in precompile_test passed and the snark_proof also passed so you may want to check it.

@gregdhill
Copy link
Contributor

@madiazp if you wouldn't mind re-basing this PR, I would be keen to merge this into master!

@silasdavis
Copy link
Contributor

@gregdhill perhaps you could pick up integrating this and maybe some of the other EVM updates we need.

@madiazp madiazp requested a review from a team March 19, 2020 03:15
@madiazp
Copy link
Author

madiazp commented Mar 19, 2020

@silasdavis Uh I don't know why the docker check failed. But again the test of expMod failed, you may want to check that personally.
Sorry the delay

@silasdavis silasdavis closed this Mar 19, 2021
@silasdavis silasdavis deleted the branch hyperledger-archives:master March 19, 2021 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants