feat: add VerifyDER, test vector generator, and snap planning docs#176
feat: add VerifyDER, test vector generator, and snap planning docs#176
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive implementation plan for non-custodial signing via MetaMask Snaps, alongside a new utility for generating deterministic crypto test vectors to ensure cross-platform compatibility with TypeScript. The changes also include the addition of a VerifyDER function in the keys package to validate DER-encoded ECDSA signatures. Review feedback suggests improving the VerifyDER implementation by using standard library constants for the secp256k1 curve order and adopting more efficient, idiomatic signature verification methods instead of public key recovery.
| } | ||
|
|
||
| // Verify low-S normalization (BIP-62): s must be <= n/2 | ||
| secp256k1N, _ := new(big.Int).SetString("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16) |
There was a problem hiding this comment.
Instead of hardcoding the secp256k1 curve order as a string, use the constant provided by the crypto package. This improves maintainability and reduces the risk of typos.
| secp256k1N, _ := new(big.Int).SetString("fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141", 16) | |
| secp256k1N := crypto.S256().Params().N |
| // Try recovery IDs 0 and 1 to find which one recovers the correct public key | ||
| for v := byte(0); v <= 1; v++ { | ||
| recoverSig := make([]byte, 65) | ||
| copy(recoverSig, rawSig) | ||
| recoverSig[64] = v | ||
|
|
||
| recoveredPub, recErr := crypto.SigToPub(hash, recoverSig) | ||
| if recErr != nil { | ||
| continue | ||
| } | ||
| if crypto.PubkeyToAddress(*recoveredPub) == crypto.PubkeyToAddress(*pubKey) { | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("signature verification failed") |
There was a problem hiding this comment.
Using a recovery loop (SigToPub) to verify a signature when the public key is already known is inefficient. It is more idiomatic and performant to use crypto.VerifySignature directly. Additionally, comparing public keys directly is more robust than comparing their Ethereum addresses, as it avoids the overhead of hashing and the theoretical (though negligible) risk of collisions.
// Verify the signature against the public key
if !crypto.VerifySignature(crypto.FromECDSAPub(pubKey), hash, rawSig) {
return fmt.Errorf("signature verification failed")
}
return nilAdd VerifyDER to pkg/keys for server-side validation of client-provided DER signatures before forwarding to Canton. This is needed for the non-custodial transfer execute endpoint. Add cmd/generate-test-vectors to produce deterministic crypto test vectors (SPKI DER, fingerprints, DER signatures) for cross-validation with the canton-snap TypeScript implementation. Add planning docs for the MetaMask Snap non-custodial signing approach.
3b19432 to
d2060f8
Compare
Integration test that proves Canton accepts signatures from test vector keys — the same keys validated by canton-snap TypeScript cross-validation tests (T3). Test flow: register external party with test vector key → mint → prepare transfer → sign with test vector key → execute → verify balances. Run with: go run scripts/testing/test-snap-crypto.go Requires local Canton environment (bootstrap-local.sh).
Summary
Phase 1 of the MetaMask Snap non-custodial signing effort (see
docs/non-custodial-snap-plan.md):VerifyDER(pkg/keys/canton_keys.go) — Server-side verification of DER-encoded ECDSA signatures against a compressed public key and pre-hashed digest. Validates DER structure, low-S normalization, and signature correctness. Needed by the transfer execute endpoint to validate client-provided signatures before forwarding to Canton.cmd/generate-test-vectors— Generates deterministic crypto test vectors (SPKI DER, fingerprints, DER signatures) from hardcoded private keys. Output is a JSON file for cross-validation with thecanton-snapTypeScript implementation. Run viamake test-vectors.docs/non-custodial-snap-plan.md(full hybrid architecture plan) anddocs/phase1-crypto-compatibility.md(detailed Phase 1 task breakdown).Test plan
TestVerifyDER— valid sig, wrong hash, wrong pubkey, malformed DER, wrong hash length, trailing bytesTestVerifyDERWithKnownVector— deterministic key produces verifiable signaturecmd/generate-test-vectorsself-verifies all generated signatures viaVerifyDERmake lintpassesgo test ./pkg/keys/passes