feat: add mina-bip32 and ledger-test-vectors for Ledger wallet validation #3388

dannywillems · 2025-12-16T18:14:48Z

Summary

Add mina-bip32 crate for BIP32 hierarchical deterministic key derivation as used by Ledger hardware wallets
Add ledger-test-vectors tool for generating JSON test vectors for Mina Ledger hardware wallet signing validation

mina-bip32 crate

Master key derivation using HMAC-SHA512 with "Bitcoin seed"
Child key derivation with secp256k1 curve order (matching Ledger's os_perso_derive_node_bip32(CX_CURVE_256K1))
Mina BIP44 path derivation: m/44'/12586'/account'/0/0
Bit masking for valid Pallas scalar field elements
Comprehensive documentation for external implementers

ledger-test-vectors tool

BIP32-derived keys using mina-bip32 crate
Payment and delegation transaction types
Coverage for mainnet/testnet, multiple accounts
CLI: cargo run --package ledger-test-vectors

Test plan

cargo test --package mina-bip32 - all 16 tests pass
make lint - no warnings or errors
cargo run --package ledger-test-vectors - generates valid JSON test vectors
Validate test vectors against actual Ledger hardware wallet

Notes

Transaction types in ledger-test-vectors will be updated when Extract transaction types into a crate mina-rust#1665 is implemented
Regression test addresses should be validated against actual Ledger devices

querolita

Comments for commit dcf9885

querolita · 2025-12-17T17:36:19Z

bip32/src/lib.rs

+///
+/// BIP32 specifies using secp256k1 for key derivation even when the
+/// target curve is different (like Pallas for Mina).
+const SECP256K1_ORDER: [u8; 32] = [


querolita · 2025-12-17T17:46:25Z

bip32/src/lib.rs

+
+    for i in (0..32).rev() {
+        let sum = a[i] as u16 + b[i] as u16 + carry;
+        result[i + 1] = sum as u8;


Just a note. So here, if sum is >255, what will the casting do? Will it perform the mod operation with 256? Meaning, will it discard the 9th bit?

querolita · 2025-12-17T17:46:50Z

bip32/src/lib.rs

+    for i in (0..32).rev() {
+        let sum = a[i] as u16 + b[i] as u16 + carry;
+        result[i + 1] = sum as u8;
+        carry = sum >> 8;


Here it's clear that it shifts 8 positions to get the 9th bit (potential carry bit)

querolita · 2025-12-17T17:53:49Z

bip32/src/lib.rs

+                    break;
+                }
+                core::cmp::Ordering::Less => {
+                    break;


efficient, good

querolita · 2025-12-17T17:56:12Z

bip32/src/lib.rs

+        for i in (0..32).rev() {
+            let diff = result[i + 1] as i16 - SECP256K1_ORDER[i] as i16 - borrow;
+            if diff < 0 {
+                result[i + 1] = (diff + 256) as u8;


safe type, also the 256 comes from the borrow bit corresponding to the next significant byte which is 2^8 in this step. Just noting for correctness check.

querolita · 2025-12-17T18:19:24Z

bip32/src/lib.rs

+            .expect("HMAC can take key of any size");
+
+        // Construct the HMAC data based on derivation type
+        if index >= HARDENED_OFFSET {


I find this confusing, the fact that both branches do the same.

querolita · 2025-12-17T18:20:32Z

bip32/src/lib.rs

+            mac.update(&self.private_key);
+        }
+        // Append the index in big-endian format
+        mac.update(&index.to_be_bytes());


everything big endian so far

querolita · 2025-12-17T18:21:52Z

bip32/src/lib.rs

+
+        // Compute child_private_key = (parent_private_key + child_key_material) mod n
+        // where n is the secp256k1 curve order (standard BIP32)
+        let final_key = add_mod_secp256k1(&self.private_key, &child_key_material);


why call it final_key in the variable but child_private_key in the comment?

querolita · 2025-12-17T18:25:12Z

bip32/src/lib.rs

+    /// # Algorithm
+    ///
+    /// 1. Copy the 32-byte private key
+    /// 2. Mask the top 2 bits of the first byte to ensure the value


Are we ok with collisions?

querolita · 2025-12-17T18:26:22Z

bip32/src/lib.rs

+        key_bytes.reverse();
+
+        // Convert to scalar field element
+        ScalarField::from_le_bytes_mod_order(&key_bytes)


querolita

Comments for commit c7b7918

querolita · 2025-12-17T18:48:34Z

signer/ledger-test-vectors/src/transaction.rs

+        fee: String,
+        /// Account nonce
+        nonce: u32,
+        /// Valid until slot (null for no expiry)


null or None?

querolita · 2025-12-17T18:49:01Z

signer/ledger-test-vectors/src/transaction.rs

+impl Hashable for Transaction {
+    type D = NetworkId;
+
+    fn to_roinput(&self) -> ROInput {


Why this order? It's not the same as the order in the struct fields.

querolita · 2025-12-17T18:51:15Z

signer/ledger-test-vectors/src/vectors.rs

+/// on the Pallas curve.
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub struct PublicKeyJson {
+    /// X-coordinate of the public key point (hex-encoded, little-endian)


here starts little-endian world

querolita · 2025-12-17T18:53:39Z

signer/ledger-test-vectors/src/vectors.rs

+    ///
+    /// After BIP32 derivation and bit masking, this is the scalar
+    /// value used for signing.
+    pub private_key: String,


Do you mean private_key is the sk in the formula above s = k + e * sk?

Add a new crate implementing BIP32 hierarchical deterministic key derivation for Mina, as used by Ledger hardware wallets. Key features: - Master key derivation using HMAC-SHA512 with "Bitcoin seed" - Child key derivation with secp256k1 curve order (matching Ledger) - Mina BIP44 path: m/44'/12586'/account'/0/0 - Bit masking for valid Pallas scalar field elements - Comprehensive documentation for external implementers Note: Test vectors should be validated against actual Ledger devices. The Ledger uses proprietary os_perso_derive_node_bip32(CX_CURVE_256K1).

Add a tool to generate JSON test vectors for validating Mina Ledger hardware wallet signing implementations. Features: - BIP32-derived keys using mina-bip32 crate - Payment and delegation transaction types - Coverage for mainnet/testnet, multiple accounts - CLI: cargo run --package ledger-test-vectors Test vectors include: - Seed, private key, public key coordinates, address - Transaction details (type, to, amount, fee, nonce, memo) - Signature (rx, s) Note: Transaction types will be updated when o1-labs/mina-rust#1665 is implemented.

…ion) Add test vectors from ledger-mina repository for validation: - Test mnemonic and expected addresses for accounts 0, 1, 2, 3, 49370, 12586 - Tests are marked as ignored because our derivation produces different addresses than expected - requires further investigation Also removes deny(missing_docs) attribute and adds pbkdf2 dev-dependency.

querolita

Approving for now, but let's keep in mind the mismatch between Ledger's implementation and this one.

dannywillems marked this pull request as draft December 16, 2025 18:39

querolita self-assigned this Dec 17, 2025

querolita reviewed Dec 17, 2025

View reviewed changes

dannywillems added 4 commits December 18, 2025 08:57

fix(bip32): remove deny(missing_docs) attribute

ad4764d

dannywillems force-pushed the dw/mina-signer-c-external branch from 8ad9f4f to 4f8422f Compare December 18, 2025 16:57

dannywillems marked this pull request as ready for review December 18, 2025 17:00

querolita approved these changes Dec 22, 2025

View reviewed changes

dannywillems mentioned this pull request Dec 30, 2025

feat: add BIP32 derivation support dannywillems/mina-web-wallet#2

Open

7 tasks

feat: add mina-bip32 and ledger-test-vectors for Ledger wallet validation #3388

Are you sure you want to change the base?

feat: add mina-bip32 and ledger-test-vectors for Ledger wallet validation #3388

Uh oh!

Conversation

dannywillems commented Dec 16, 2025

Summary

mina-bip32 crate

ledger-test-vectors tool

Test plan

Notes

Uh oh!

querolita left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

querolita left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

querolita left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants