Skip to content

fix(tempo): verify allowAccessKey for secp256k1 keychain#4596

Open
cmende wants to merge 1 commit into
wevm:mainfrom
cmende:tempo-allow-accesskey-secp256k1-verifyhash
Open

fix(tempo): verify allowAccessKey for secp256k1 keychain#4596
cmende wants to merge 1 commit into
wevm:mainfrom
cmende:tempo-allow-accesskey-secp256k1-verifyhash

Conversation

@cmende
Copy link
Copy Markdown

@cmende cmende commented May 11, 2026

What is this PR solving?

This PR fixes an incomplete allowAccessKey verification path in tempo verifyHash.

In the keychain branch, the current logic derives the access key address only from envelope.inner.publicKey.
For secp256k1 access keys, the inner envelope may not include publicKey, which can cause thrown errors or false-negative verification for otherwise valid signatures.

This change adds a fallback path:

  • If envelope.inner.publicKey exists, keep current behavior.
  • If it is missing and envelope.inner.type === 'secp256k1', recover the access key address from the inner signature + payload.
  • If address derivation fails, return false instead of throwing.

This preserves the existing metadata checks (authorized, not revoked, not expired) and only broadens support for valid secp256k1 keychain envelopes.

Alternatives explored

  • Require publicKey for all key types in keychain envelopes.
    • Rejected: not compatible with existing secp256k1 envelope shape.
  • Skip local envelope verification and always fall back to generic verifyHash.
    • Rejected: would bypass the intended access-key metadata validation path in allowAccessKey mode.
  • Current approach: minimal fallback only for missing-publicKey secp256k1 inner signatures.
    • Chosen for minimal surface area and behavior preservation.

Reviewer attention

Please focus on:

  • The new fallback in src/tempo/chainConfig.ts for keychain + allowAccessKey.
  • innerPayload usage consistency for v1/v2 keychain envelopes.
  • Error handling behavior (false return instead of throw) in malformed/missing-key scenarios.

Tests

Added:

  • accessKey: secp256k1 valid signature in src/tempo/chainConfig.test.ts.

The new test reproduces the previously unhandled secp256k1 keychain path and validates the fix.

Docs

No public API changes. No docs update required.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 11, 2026

⚠️ No Changeset found

Latest commit: d8e2142

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented May 11, 2026

@cmende is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

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.

1 participant