Fix early revert in `verify` for accurate gas estimation #70

amiecorso · 2025-10-17T20:48:59Z

The verify function in WebAuthn-sol checks the validity of a passkey signature. It’s called during the validation phase of an ERC-4337 flow for passkey-signed userOps. When preparing a userOp, the wallet client must estimate the “verification gas limit” (VGL) for the userOp, which is the estimation of how much gas will be consumed during the onchain validation of the userOp before the execution of it’s actual payload. There is some tolerance for error in this estimation, but using a value that’s too high or too low will cause the bundler to reject the userOp.

When the verification flow is simulated without a valid passkey signature, the verify function was returning much earlier in its execution flow than it would when executing with a valid passkey signature during the actual onchain verification. This was leading to an underestimation of simulated verification gas.

To resolve this, we change the verify function so that it does not return early and always executes the heavy path (computing the SHA‑256 hashes and attempting signature verification) regardless of whether the inputs are ultimately valid. With these changes, valid and invalid inputs now cost roughly the same gas, so simulations without a final signature approximate the true cost of a later, valid verification.

There is one trade‑off: genuinely invalid signatures onchain are now slightly more expensive than before because we no longer revert early. In practice, this is a rare scenario as it will usually be caught by simulation, and if not, the gas cost is still relatively minor.

cb-heimdall · 2025-10-17T20:49:03Z

✅ Heimdall Review Status

Requirement Status More Info

Reviews

✅ 1/1

Denominator calculation

Show calculation

1 if user is bot 0

1 if user is external 0

2 if repo is sensitive 0

From .codeflow.yml 1

Additional review requirements

Show calculation

Max	0

0

From CODEOWNERS 0

Global minimum 0

Max 1

1

1 if commit is unverified 0

Sum 1

src/WebAuthn.sol

stevieraykatz · 2025-10-20T23:18:06Z

src/WebAuthn.sol

        returns (bool)
    {
+        bool hasFailedChecks = false;


nit: Could use hasFailedChecks as return param and accumulate sig checks into it

the actual return is an expression that incorporates multiple booleans

test/BaseWebAuthnTest.t.sol

stevieraykatz · 2025-10-20T23:22:32Z

test/WebAuthn.fork.t.sol

+        baseFork = vm.createFork("https://mainnet.base.org");
+        ethFork = vm.createFork("https://ethereum-rpc.publicnode.com");
+        arbFork = vm.createFork("https://arb1.arbitrum.io/rpc");


We should opt to use env based rpc urls so that CI/CD is more reliable

stevieraykatz · 2025-10-20T23:24:40Z

test/WebAuthn.fork.t.sol

What's the motivation for fork testing on arb, base and eth specifically? How were these networks chosen?

originally this was to get a sense of the gas difference between the early-return v.s. the heavy path on chains that do/don't implement RIP-7212 (precompile for secp256r1 ec i.e. passkey signatures). Ethereum mainnet does not implement this, but Base and Arbitrum should (among other rollups). we're not actually able to see a difference in fork testing, so I'm not sure if foundry's fork testing environment incorporates precompiles. Would be open to removing arbitrum to make it somewhat less arbitrary (i.e. one 7212-compatible chain and one non-7212 chain)

we're not actually able to see a difference in fork testing, so I'm not sure if foundry's fork testing environment incorporates precompiles. Would be open to removing arbitrum to make it somewhat less arbitrary (i.e. one 7212-compatible chain and one non-7212 chain)

Think this would be nice for clarity

ilikesymmetry

Looks good, thanks for the additional tests

ilikesymmetry · 2025-10-21T16:01:08Z

src/WebAuthn.sol

+    /// @dev Verifies a P256 signature using the precompiled contract or FCL.
+    /// @param messageHash The hash of the message to verify.
+    /// @param r The r value of the signature.
+    /// @param s The s value of the signature.
+    /// @param x The x coordinate of the public key.
+    /// @param y The y coordinate of the public key.
+    /// @return True if the signature is valid, false otherwise.


nit: separate tag types with empty line

ilikesymmetry · 2025-10-21T16:16:58Z

test/WebAuthn.fork.t.sol

we're not actually able to see a difference in fork testing, so I'm not sure if foundry's fork testing environment incorporates precompiles. Would be open to removing arbitrum to make it somewhat less arbitrary (i.e. one 7212-compatible chain and one non-7212 chain)

Think this would be nice for clarity

ilikesymmetry · 2025-10-21T16:18:02Z

.gas-snapshot-with-early-return

+WebAuthnForkTest:test_verify_fullPath_arb() (gas: 253734)
+WebAuthnForkTest:test_verify_fullPath_base() (gas: 253756)
+WebAuthnForkTest:test_verify_fullPath_eth() (gas: 253757)
+WebAuthnForkTest:test_verify_invalidChallenge_arb() (gas: 43717)
+WebAuthnForkTest:test_verify_invalidChallenge_base() (gas: 43740)
+WebAuthnForkTest:test_verify_invalidChallenge_eth() (gas: 43739)


wow crazy difference 😮

cb-heimdall · 2025-10-21T16:18:19Z

Review Error for ilikesymmetry @ 2025-10-21 16:18:19 UTC
User must have write permissions to review

cb-heimdall · 2025-10-21T17:18:52Z

Review Error for stevieraykatz @ 2025-10-21 17:18:52 UTC
User must have write permissions to review

stevieraykatz · 2025-10-22T18:51:59Z

Ci/Cd is failing because we include snapshot checking:

      - name: Check snapshot
        run: |
          forge snapshot --check
        id: snapshot

We can safely merge with failed ci/cd in this instance.

amiecorso added 5 commits May 9, 2025 13:47

gas benchmarks for early failure and late failure cases

4a711a6

fork tests

9d2776a

proof that evm_version is incorporated but 7212 doesn't seem active

f2cb9e8

track invalid state through verify function

13cc1a7

helper function for stack too deep

78d74ed

amiecorso added 2 commits October 17, 2025 14:16

clean up tests

c97c9ed

remove specific evm version

4f17a2a

amiecorso commented Oct 17, 2025

View reviewed changes

src/WebAuthn.sol Show resolved Hide resolved

amiecorso commented Oct 17, 2025

View reviewed changes

src/WebAuthn.sol Show resolved Hide resolved

amiecorso added 2 commits October 17, 2025 14:26

remove extra space

5a06e86

add snapshot with benchmarks against early return case

03e0711

stevieraykatz reviewed Oct 17, 2025

View reviewed changes

src/WebAuthn.sol Outdated Show resolved Hide resolved

invert flag

89eddf6

amiecorso requested review from stevieraykatz October 20, 2025 22:53

amiecorso marked this pull request as ready for review October 20, 2025 22:54

stevieraykatz reviewed Oct 20, 2025

View reviewed changes

src/WebAuthn.sol Outdated Show resolved Hide resolved

stevieraykatz reviewed Oct 20, 2025

View reviewed changes

test/BaseWebAuthnTest.t.sol Outdated Show resolved Hide resolved

stevieraykatz reviewed Oct 20, 2025

View reviewed changes

review comments

151a415

amiecorso requested a review from stevieraykatz October 20, 2025 23:39

ilikesymmetry approved these changes Oct 21, 2025

View reviewed changes

stevieraykatz approved these changes Oct 21, 2025

View reviewed changes

stevieraykatz approved these changes Oct 22, 2025

View reviewed changes

Fix early revert in verify for accurate gas estimation #70

Are you sure you want to change the base?

Fix early revert in verify for accurate gas estimation #70

Uh oh!

Conversation

amiecorso commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cb-heimdall commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Heimdall Review Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilikesymmetry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cb-heimdall commented Oct 21, 2025

Uh oh!

cb-heimdall commented Oct 21, 2025

Uh oh!

stevieraykatz commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix early revert in `verify` for accurate gas estimation #70

Fix early revert in `verify` for accurate gas estimation #70

amiecorso commented Oct 17, 2025 •

edited

Loading

cb-heimdall commented Oct 17, 2025 •

edited

Loading