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.

Sign up for GitHub

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

Jump to bottom

add gas benchmark suite and minor edit to verifier #14

Merged
merged 2 commits into from
Sep 22, 2023
Merged

add gas benchmark suite and minor edit to verifier #14

merged 2 commits into from
Sep 22, 2023

Conversation

nalinbhardwaj
Copy link
Member

@nalinbhardwaj nalinbhardwaj commented Sep 21, 2023 •
edited
Loading

Closes #10.

Running 2 tests for test/GasBenchmark.t.sol:GasBenchmarkTest
[PASS] testBenchmarkRandomGasUsage() (gas: 682790212)
Logs:
  Our verifier gas usage  avg:  341287
  Our verifier gas usage  min:  327588
  Our verifier gas usage  max:  354643
  

  FCL verifier gas usage  avg:  269825
  FCL verifier gas usage  min:  256961
  FCL verifier gas usage  max:  282294
  


[PASS] testBenchmarkWycheProofGasUsage() (gas: 438096793)
Logs:
  Our verifier gas usage  avg:  274581
  Our verifier gas usage  min:  999
  Our verifier gas usage  max:  386445
  

  FCL verifier gas usage  avg:  217123
  FCL verifier gas usage  min:  1645
  FCL verifier gas usage  max:  310435

dcposch
dcposch approved these changes Sep 21, 2023
View reviewed changes
Copy link
Member

@dcposch dcposch left a comment

Choose a reason for hiding this comment

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

LGreatTM

.vscode/settings.json
@@ -9,5 +9,6 @@
"[solidity]": {
"editor.defaultFormatter": "JuanBlanco.solidity"
},
"coverage-gutters.coverageFileNames": ["lcov.info"]
"coverage-gutters.coverageFileNames": ["lcov.info"],
"solidity.compileUsingRemoteVersion": "v0.8.21+commit.d9974bed"
Copy link
Member

Choose a reason for hiding this comment

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

i recommend installing svm

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 installed!
I think might still be good to pin a version here since CREATE2 addresses depend on compiler versions.

src/P256Verifier.sol
@@ -88,7 +88,7 @@ contract P256Verifier {
uint256 scalar_u = mulmod(uint256(message_hash), sInv, n); // (h * s^-1) in scalar field
uint256 scalar_v = mulmod(r, sInv, n); // (r * s^-1) in scalar field

(uint256 r_x, bool mulmuladd_success) = ecZZ_mulmuladd_S_asm(
(uint256 r_x, bool mulmuladd_success) = ecZZ_mulmuladd(
Copy link
Member

Choose a reason for hiding this comment

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

cosmetic diff from as-audited but LGTM

test-vectors/generate_random.ts Outdated Show resolved Hide resolved
test-vectors/generate_wycheproof.ts Outdated Show resolved Hide resolved
test-vectors/test.ts
.split("\n")
.map((line) => JSON.parse(line) as Vector);

const vectors = [...wycheproofVectors, ...randomVectors];
Copy link
Member

Choose a reason for hiding this comment

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

👍

@nalinbhardwaj
Copy link
Member Author

Post enabling optimiser-runs, seems our verifier takes ~340k gas vs. FCL's ~270k. Seems worth it for readability/other improvements still.

@nalinbhardwaj nalinbhardwaj merged commit e450bca into master Sep 22, 2023
@nalinbhardwaj nalinbhardwaj deleted the nibnalin/gas-benchmark branch September 22, 2023 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcposch dcposch dcposch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gas measurement test
2 participants
@nalinbhardwaj @dcposch