fix: left-pad r/s in Signature.toCompactBytes/toRecoveredBytes

[gomesalexandre]

tl;dr

Signature.toCompactBytes and Signature.toRecoveredBytes right-pad r/s to 32 bytes instead of left-padding. Since r/s are big-endian integers, right-padding a value shorter than 32 bytes changes its numeric value — for a hand-built { r, s, yParity } literal (fully type-legal, no length constraint on Hex.Hex) whose r or s happens to have a leading zero byte, this makes Secp256k1.recoverPublicKey silently return the wrong public key, with no error, roughly 60% of the time it fires (the rest of the time it throws instead).

Where

src/core/Signature.ts:565-574 (toCompactBytes) and :623-633 (toRecoveredBytes), both via Bytes.fromHex(v, { size: 32 }), which right-pads by construction (src/core/Bytes.ts:223-232, own comment: "Right-pad the hex string before parsing").

The same file's own normalizer does this correctly: Signature.from (:314-317) left-pads via Hex.padLeft after assert.

Reproduction

Over 3000 real secp256k1 signatures (varying the private key against a fixed payload), 10 had a leading-zero r byte (~1/256 expected rate). Of those 10, feeding each through toRecoveredBytes → fromRecoveredBytes → recoverPublicKey with r in minimal (non-zero-padded) hex form — the shape you'd get reading r back out of an RPC response, subgraph, or DB that stores minimal-form quantities — produced:

leading-zero-r signatures found: 10
  threw:               4
  silently wrong key:  6   <- no error, just a different address
  accidentally correct: 0

Concrete example:

correct pubkey.x: 0xd14ea6b0d28249f7404018b97aef6fab03642ba2b27e3b5214d996b48c4de730
full (left-padded) r:    0x00f7eb915663884a849bc182207d6090f97d48c3782ee520829a272d37344a39
minimal-form (trimmed) r:  0xf7eb915663884a849bc182207d6090f97d48c3782ee520829a272d37344a39

toRecoveredBytes(trimmed) r-half (CURRENT/buggy):
  0xf7eb915663884a849bc182207d6090f97d48c3782ee520829a272d37344a3900
                                                                  ^^ garbage byte shifted in

recoverPublicKey on that round-trip -> THREW "bad point: is not on curve" in this instance
(6/10 sampled cases instead silently returned a different, wrong pubkey.x)

Scope — please read before assuming this is worse than it is

All entry points that construct a Signature through ox's own APIs (Secp256k1.sign, Signature.from, Signature.fromHex, etc.) already pad r/s to 32 bytes before returning, per the design invariant established in #247. I confirmed this myself — Secp256k1.sign returns pre-padded r/s, so this bug does not fire through the normal signing path. It fires specifically when a caller builds { r, s, yParity } by hand from a source that stores r/s in minimal (non-zero-padded) hex — no downstream code doing exactly that was found in this repo or observed by me. So: a real, silent, wrong-answer bug on a public, untested API — not "signature verification is broken" in general.

Fix

Left-pad instead of right-pad in both functions, matching Signature.from's existing pattern:

-  bytes.set(Bytes.fromHex(signature.r, { size: 32 }), 0)
-  bytes.set(Bytes.fromHex(signature.s, { size: 32 }), 32)
+  bytes.set(Bytes.fromHex(Hex.padLeft(signature.r, 32)), 0)
+  bytes.set(Bytes.fromHex(Hex.padLeft(signature.s, 32)), 32)

Bundled in the same patch (same root cause class — these functions previously accepted malformed input silently): fromCompactBytes/fromRecoveredBytes used Uint8Array#subarray, which clamps instead of throwing — fromCompactBytes(new Uint8Array(20)) previously returned a bogus signature instead of throwing, and fromRecoveredBytes never validated yParity ∈ {0,1}. Both now throw Signature.InvalidSerializedSizeError / Signature.InvalidYParityError (reusing the errors the sibling fromHex already throws for the same conditions).

One honest side effect worth flagging: oversized r/s (>32 bytes) now throws Hex.SizeExceedsPaddingSizeError via Hex.padLeft instead of the previous Hex.SizeOverflowError via Bytes.fromHex's internal assertSize. Both are real, correctly-typed errors for the same underlying problem; no existing test asserted on the old error type for these two functions (there were zero prior tests for any of the four functions touched here — confirmed via the exports key-list test, which only listed their names). Happy to adjust if you'd prefer the old error type preserved.

receipts

$ npx tsc -b
(clean, no output)

$ npx vp test src/core/_test/Signature.test.ts
 Test Files  1 passed (1)
      Tests  40 passed (40)

$ npx vp test src/core/_test/Signature.test.ts src/core/_test/Signature.fuzz.ts src/core/_test/Secp256k1.test.ts src/core/_test/P256.test.ts
 Test Files  3 passed (3)
      Tests  134 passed (134)

$ npx vp check --fix
Found 0 errors and 2 warnings in 783 files   # both pre-existing, unrelated (site/Landing.tsx)

Red-before/green-after confirmed directly: stashed the fix and re-ran Signature.test.ts — the 6 new tests exercising the bug (padding-direction assertions + the two malformed-input preconditions) failed against unfixed code with the exact wrong values shown above; the other 34 (including the two default round-trip tests that don't exercise the bug) still passed. Restored the fix, all 40 pass.

Codex adversarial review didn't return in 5 minutes — killed it and did a thorough self-review instead (that's how the error-type change above got caught and written up honestly rather than glossed over).

risk

Low. toCompactBytes/toRecoveredBytes/fromCompactBytes/fromRecoveredBytes had no prior test coverage and no internal callers in this repo — the diff only changes the pad direction and adds input validation, both matching patterns already established elsewhere in the same file.

[vercel]

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

A member of the Team first needs to authorize it.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/ox@435

commit: a41e551