Add ETH crypto - #1843
Conversation
secp256k1 (vendored libsecp256k1 v0.8.0, recoverable ECDSA), BIP-39 mnemonics, BIP-32 derivation, Keccak-256, EIP-55 addresses and EIP-712 typed data hashing. Client-side signing only: no RLP, no transaction construction, no chain writes. The resolver path stays read-only. Verified against published vectors — the 24 official BIP-39 English vectors, BIP-32 spec vectors 1 and 2, the EIP-55 spec addresses and the EIP-712 Mail example. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the whole branch: the secp256k1 FFI bindings, BIP-32/BIP-39, Keccak, EIP-55 addresses, EIP-712 hashing, ERC-5564 stealth addresses, the test suite, and the cabal/submodule wiring.
This is well-structured, and the safety-critical decisions are the right ones and are documented where it matters:
- Trust boundaries are parsed, not validated.
PrivateKey,PublicKey,Address,Mnemonicare all abstract with private constructors reachable only through validating smart constructors, so an invalid key/address is unrepresentable downstream.mkPrivateKeyrejects zero and>= n, which is exactly what makespublicKey/signRecoverabletotal. - Secrets carry redacting
Show(PrivateKey,ExtendedKey,Mnemonic) andPrivateKeyuses constant-timeEq. - Keccak-256 vs SHA3-256 is isolated behind one module with a warning, and the ECDH point is exposed raw specifically because
secp256k1_ecdhwould hash with SHA-256 instead of keccak256 — the ERC-5564 interop trap is avoided deliberately. - Signing is RFC-6979 deterministic and low-s (EIP-2), and
isLowS' half-order constant is correct (n/2 for secp256k1). - The shared context is randomized once at CAF init and only read afterward, so the pure-over-
unsafePerformIOAPI and cross-thread sharing are sound.
I verified the numeric paths by hand against the specs: BIP-39 checksum bit extraction for all five strengths, mnemonicToEntropy bit arithmetic (totalBits*32 div 33), EIP-712 int256/uint256 range checks and two's-complement encoding, EIP-55 nibble/case logic, and the stealth ECDH/tweak algebra (sender P_spend + s_h·G, recipient p_spend + s_h). All consistent. The tests pin published vectors (Trezor BIP-39, BIP-32 vectors 1/2, EIP-55, the EIP-712 Mail example) plus an independent Python cross-check for stealth, rather than the code's own output — the right way to test crypto.
FFI signatures, context flags (CONTEXT_NONE=1, EC_COMPRESSED=258), and the ENABLE_MODULE_RECOVERY build flag all line up with the vendored libsecp256k1.
No blocking issues found. Minor, non-blocking notes:
- Private key material lives in ordinary
ByteStrings that are never zeroized. This matches the rest of the codebase and is hard to change under GHC's GC, so it is a known limitation rather than a regression — worth a note only. - Small duplication (
beToIntegerin bothSecp256k1andBIP39, local hex helpers inAddress) is fine at this size; not worth extracting. secp256k1Ctxcan, under a rare CAF-init race, create two contexts and leak one. Harmless.
Approving.
In order to support in-app name purchases, we need ETH crypto primitives available. This PR adds them
Tested against a simple use case in simplex-chat:
https://github.com/simplex-chat/simplex-chat/blob/ab/eth-crypto-test/src/Simplex/Chat/Names/Wallet.hs