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

Add Gas Benchmarking Tests for WebAuthn Signer #324

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

nlordell
Copy link
Collaborator

@nlordell nlordell commented Mar 12, 2024

This PR adds gas benchmarking tests for the WebAuthn signer. Note that we got rid of the MultipleVerifiers.spec.ts file, as it is essentially the new gas benchmarking tests, and there was no need to duplicate the work.

The benchmarks look like this:

  Gas Benchmarking
    WebAuthnSigner
      ⛽ deployment: 612123
      ✔ Benchmark signer deployment cost (694ms)
      ⛽ verification (FreshCryptoLib): 219365
      ✔ Benchmark signer verification cost with FreshCryptoLib verifier (172ms)
      ⛽ verification (daimo-eth): 351273
      ✔ Benchmark signer verification cost with daimo-eth verifier (201ms)
      ⛽ verification (Dummy): 13835
      ✔ Benchmark signer verification cost with Dummy verifier

Note that we include a "dummy" benchmark, this is because gas consumption of P-256 signature verification is unstable, and this allows us to better compare gas characteristics of the WebAuthn signing message computation overhead.

Furthermore, we remove the check on whether or not the P-256 verifier has code. This is important as precompiles have no code, so our WebAuthn stuff wouldn't work with precompiles without this change.

@nlordell nlordell requested a review from a team as a code owner March 12, 2024 16:04
@nlordell nlordell requested review from akshay-ap, mmv08 and remedcu and removed request for a team March 12, 2024 16:04
@coveralls
Copy link

coveralls commented Mar 12, 2024

Pull Request Test Coverage Report for Build 8338690017

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+62.2%) to 76.923%

Totals Coverage Status
Change from base Build 8328723798: 62.2%
Covered Lines: 131
Relevant Lines: 155

💛 - Coveralls

Copy link
Member

@remedcu remedcu left a comment

Choose a reason for hiding this comment

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

One doubt, rest LGTM 👍🏾

Base automatically changed from refactor-webauthn-signing to main March 18, 2024 14:53
@nlordell nlordell merged commit 1e96cdc into main Mar 19, 2024
14 checks passed
@nlordell nlordell deleted the chore/add-gas-benchmark branch March 19, 2024 07:24
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants