fix(signers): await the post-sign signature verification

[eastagiletracker]

This PR proposes awaiting the post-sign signature verification in the Bitcoin, Ethereum and Polkadot signers so the UnableToVerify guard actually runs, and fixing the two places where that guard could never have passed. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/364. You can sign in with your GitHub ID to claim ownership of the project.

What's wrong

All three signers end sign() with a self-check that re-verifies the signature they are about to return. verify() is async, and none of the three call sites awaits it, so the check evaluates a Promise:

// packages/signers/bitcoin/src/index.ts
if (!this.verify(...)) { throw new Error(Errors.SigningErrors.UnableToVerify); }   // !promise === false

// packages/signers/ethereum/src/index.ts
if (!this.verify(bufferToHex(msgHashBuffer), rpcSig, keyPair.publicKey)) { ... }   // same

// packages/signers/polkadot/src/index.ts  (x3, one per SignerType)
assert(this.verify(...), Errors.SigningErrors.UnableToVerify);                     // assert(promise) always passes

!promise is always false and assert(promise) always passes, so a signature that does not verify is handed back to the caller instead of raising. KeyRing.sign() in packages/keyring/src/index.ts dispatches straight into these three classes, so this is the extension's signing path for every secp256k1, ecdsa, ed25519, sr25519 and Bitcoin account.

Two further defects were sitting behind the dead guard, and both had to be fixed for it to be usable:

1. Bitcoin passed the 65-byte recoverable signature (64-byte compact signature plus recovery id) to verify(), which only accepts the 64-byte compact form. Awaiting alone would have rejected every valid signature. It now verifies rsig[0], the compact signature, and still returns the 65-byte value unchanged.
2. Ethereum compared the recovered 64-byte public key against the supplied one verbatim, so it returned false for the SEC1 compressed keys the keyring stores for imported private keys — exactly the format in your own packages/keyring/tests/sign.test.ts fixtures (0x03330102...). verify() now brings both sides to the 64-byte form with importPublic before comparing, which leaves 64-byte callers untouched.

Reproducing it on current main

Signing with a key pair whose public key does not match the private key returns a signature instead of throwing, on b9ba802 (current main):

$ cd packages/signers/ethereum && yarn vitest run tests/sign.test.ts
 × it should verify against a compressed public key
 × it should reject a signature that does not verify
AssertionError: promise resolved "'0x99e71a99cb2270b8cac5254f9e99b6210c6…'" instead of rejecting

The same two tests, plus the Bitcoin and the three Polkadot ones, are red on the unpatched tree and green with the change: 5 rejection tests, 1 compressed-key test, 1 Bitcoin happy-path test. The clearest confirmation that this code is live came from your own suite — with the await added but before the compressed-key fix, packages/keyring/tests/sign.test.ts went red on keyring should sign ethereum messages and keyring should sign raw keypairs, because the guard was finally running and correctly rejecting the compressed public keys those two tests import.

Verification

yarn build:all && yarn test was run on b9ba802 before any change and again with the change applied, under Node 22.18.0 as pinned in .nvmrc. The baseline is fully green and stays fully green; the only difference is the 7 added tests (signer-bitcoin 3 to 5, signer-ethereum 5 to 7, signer-polkadot 6 to 9), with keyring at 19 passed in both runs. yarn lint reports no changes in the three touched packages.

Nothing about the signatures produced for a correct key pair changes — the existing byte-for-byte signature assertions in all three packages pass untouched. EthereumSigner.verify is widened, not narrowed: an unparseable public key falls back to the old comparison rather than throwing, so a caller that got false before still gets false.

How this was managed

This work was tracked as https://eastagiletracker.com/projects/364/stories/223034 on a board imported from this repository's own issues and pull requests (770 stories, 9 labels), which you can browse at https://eastagiletracker.com/projects/364.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

Summary by CodeRabbit

• Bug Fixes

  • Improved signature verification for Bitcoin, Ethereum, and Polkadot signing.
  • Signing now correctly rejects when the public key does not match the private key.
  • Ethereum signing now supports compressed and uncompressed public keys.
  • Invalid signatures consistently return the configured signing error.

• Tests

  • Added coverage for valid signatures, invalid key pairs, and supported signature types.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e971d384-489a-44b3-b85f-4a998ea89d05

📥 Commits

Reviewing files that changed from the base of the PR and between b9ba802 and 0a8834a.

📒 Files selected for processing (6)

• packages/signers/bitcoin/src/index.ts
• packages/signers/bitcoin/tests/sign.test.ts
• packages/signers/ethereum/src/index.ts
• packages/signers/ethereum/tests/sign.test.ts
• packages/signers/polkadot/src/index.ts
• packages/signers/polkadot/tests/sign.test.ts

Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review.

---

Walkthrough

Bitcoin, Ethereum, and Polkadot signers now await signature verification before returning signatures. Ethereum normalizes compressed and uncompressed public keys. Tests cover valid signatures and mismatched key failures.

Changes

Signer verification

Layer / File(s)	Summary
Bitcoin signature verification packages/signers/bitcoin/src/index.ts, packages/signers/bitcoin/tests/sign.test.ts	Bitcoin validates the compact 64-byte signature without the recovery ID. Tests cover valid signing and mismatched public keys.
Ethereum key normalization and verification packages/signers/ethereum/src/index.ts, packages/signers/ethereum/tests/sign.test.ts	Ethereum normalizes SEC1 public keys before comparison and awaits signature verification. Tests cover compressed keys and verification failure.
Polkadot signature verification packages/signers/polkadot/src/index.ts, packages/signers/polkadot/tests/sign.test.ts	ECDSA, Ed25519, and Sr25519 signing now awaits verification. Tests cover mismatched public keys for each signer type.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 0a883

The change enforces post-signature verification across the affected signers while preserving valid signature formats and compatibility; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: awaiting post-signature verification in signers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

Warning

⚠️ This pull request has been flagged as potential spam (promotional) by CodeRabbit slop detection and should be reviewed carefully.