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

perf: optimize solidity verification contract #720

Closed
wants to merge 4 commits into from

Conversation

xBA5ED
Copy link

@xBA5ED xBA5ED commented May 12, 2024

When I was looking at the sp1-project-template I spotted some ways to optimize the contract a bit more and simplify it a bit at the same time. I followed the source of the contract back to this repo.

The following is the gas change for the tests when the changes are applied to the template repository:

Ran 2 tests for test/Fibonacci.t.sol:FibonacciTest
[PASS] testFail_InvalidFibonacciProof() (gas: 27543)
[PASS] test_ValidFibonacciProof() (gas: 348368)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 7.88ms (7.57ms CPU time)

Ran 1 test suite in 11.28ms (7.88ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)
test_ValidFibonacciProof() (gas: -3458 (-0.983%))
testFail_InvalidFibonacciProof() (gas: -641 (-2.274%))
Overall gas change: -4099 (-1.079%)

jtguibas and others added 4 commits May 10, 2024 11:23
Co-authored-by: Tamir Hemo <tamir@succinct.xyz>
Co-authored-by: Ubuntu <ubuntu@ip-172-31-74-90.ec2.internal>
Co-authored-by: Kevin Jue <kjue235@gmail.com>
Co-authored-by: Tarik Moon <tarik@tarikmoon.com>
Co-authored-by: Matthias Görgens <matthias.goergens@gmail.com>
Co-authored-by: Eugene Rabinovich <eugene@succinct.xyz>
@jtguibas jtguibas changed the base branch from main to dev May 13, 2024 01:50
@jtguibas
Copy link
Contributor

Great! Thank you :) We definitely haven't optimized the contract yet.

@jtguibas
Copy link
Contributor

It might sense to merge this PR once we add foundry testing to CI.

@puma314
Copy link
Contributor

puma314 commented May 30, 2024

We switched to a PLONK prover, which no longer requires this!

@puma314 puma314 closed this May 30, 2024
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.

3 participants