Skip to content

Add ETH crypto - #1843

Open
brenzi wants to merge 5 commits into
masterfrom
ab/eth-crypto
Open

Add ETH crypto#1843
brenzi wants to merge 5 commits into
masterfrom
ab/eth-crypto

Conversation

@brenzi

@brenzi brenzi commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

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

claude and others added 4 commits August 18, 2026 13:47
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>

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, Mnemonic are all abstract with private constructors reachable only through validating smart constructors, so an invalid key/address is unrepresentable downstream. mkPrivateKey rejects zero and >= n, which is exactly what makes publicKey/signRecoverable total.
  • Secrets carry redacting Show (PrivateKey, ExtendedKey, Mnemonic) and PrivateKey uses constant-time Eq.
  • Keccak-256 vs SHA3-256 is isolated behind one module with a warning, and the ECDH point is exposed raw specifically because secp256k1_ecdh would 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-unsafePerformIO API 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 (beToInteger in both Secp256k1 and BIP39, local hex helpers in Address) is fine at this size; not worth extracting.
  • secp256k1Ctx can, under a rare CAF-init race, create two contexts and leak one. Harmless.

Approving.

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.

2 participants