Add tests for BIP86

We also fix a bug about withdrawing bitcoi. There are a few gross things that need to fixed at this point which is the handle_memleak stuff.

Author: sangbida

bitcoin/tx.c

@@ -940,6 +940,7 @@ size_t bitcoin_tx_input_witness_weight(enum utxotype utxotype)
 		/* In practice, these predate anchors, so: */
 		return 1 + 1 + bitcoin_tx_input_sig_weight();
 	case UTXO_P2TR:
+	case UTXO_P2TR_BIP86:
 		return 1 + 64;
 	}
 	abort();

bitcoin/tx.h

@@ -57,6 +57,8 @@ enum utxotype {
 	UTXO_P2WSH_FROM_CLOSE = 3,
 	/* "p2tr" addresses. */
 	UTXO_P2TR = 4,
+	/* "bip86" addresses (P2TR with BIP86 derivation). */
+	UTXO_P2TR_BIP86 = 5,
 };
 
 struct bitcoin_tx_output *new_tx_output(const tal_t *ctx,

common/hsm_secret.c

@@ -189,7 +189,8 @@ static struct hsm_secret *extract_plain_secret(const tal_t *ctx,
 
 	hsms->type = HSM_SECRET_PLAIN;
 	hsms->mnemonic = NULL;
-	memcpy(&hsms->secret, hsm_secret, sizeof(hsms->secret));
+	hsms->secret_len = HSM_SECRET_PLAIN_SIZE;
+	hsms->secret_data = tal_dup_arr(hsms, u8, hsm_secret, HSM_SECRET_PLAIN_SIZE, 0);
 
 	*err = HSM_SECRET_OK;
 	return hsms;
@@ -215,22 +216,28 @@ static struct hsm_secret *extract_encrypted_secret(const tal_t *ctx,
 		return tal_free(hsms);
 	}
 
-	/* Clear secret data first in case of partial decryption */
-	memset(&hsms->secret, 0, sizeof(hsms->secret));
+	/* Prepare for legacy encrypted secret (decrypted to 32 bytes) */
+	hsms->secret_len = HSM_SECRET_PLAIN_SIZE;
+	hsms->secret_data = tal_arr(hsms, u8, HSM_SECRET_PLAIN_SIZE);
+	memset(hsms->secret_data, 0, HSM_SECRET_PLAIN_SIZE);
 
 	/* Attempt decryption */
-	decrypt_success = decrypt_hsm_secret(encryption_key, hsm_secret, &hsms->secret);
+	struct secret temp_secret;
+	decrypt_success = decrypt_hsm_secret(encryption_key, hsm_secret, &temp_secret);
 
 	/* Clear encryption key immediately after use */
 	destroy_secret(encryption_key);
 
 	if (!decrypt_success) {
 		/* Clear any partial decryption data */
-		memset(&hsms->secret, 0, sizeof(hsms->secret));
+		memset(hsms->secret_data, 0, HSM_SECRET_PLAIN_SIZE);
 		*err = HSM_SECRET_ERR_WRONG_PASSPHRASE;
 		return tal_free(hsms);
 	}
 
+	/* Copy decrypted secret */
+	memcpy(hsms->secret_data, temp_secret.data, HSM_SECRET_PLAIN_SIZE);
+	
 	hsms->type = HSM_SECRET_ENCRYPTED;
 	hsms->mnemonic = NULL;
 
@@ -298,11 +305,9 @@ static struct hsm_secret *extract_mnemonic_secret(const tal_t *ctx,
 		return tal_free(hsms);
 	}
 
-	/* Store the full 64-byte BIP32 seed for BIP86 derivation */
-	hsms->bip32_seed = tal_dup_arr(hsms, u8, bip32_seed, bip32_seed_len, 0);
-	
-	/* We also keep the first 32 bytes for legacy compatibility */
-	memcpy(hsms->secret.data, bip32_seed, sizeof(hsms->secret.data));
+	/* Store the full BIP32 seed for mnemonic-based secrets */
+	hsms->secret_len = HSM_SECRET_MNEMONIC_SIZE;  /* Should be 64 bytes */
+	hsms->secret_data = tal_dup_arr(hsms, u8, bip32_seed, HSM_SECRET_MNEMONIC_SIZE, 0);
 
 	*err = HSM_SECRET_OK;
 	return hsms;

common/hsm_secret.h

@@ -14,6 +14,7 @@
 #define ENCRYPTED_HSM_SECRET_LEN (HS_HEADER_LEN + HS_CIPHERTEXT_LEN)
 #define PASSPHRASE_HASH_LEN 32
 #define HSM_SECRET_PLAIN_SIZE 32
+#define HSM_SECRET_MNEMONIC_SIZE 64
 
 enum hsm_secret_type {
 	HSM_SECRET_PLAIN = 0,           /* Legacy 32-byte format */
@@ -40,8 +41,8 @@ enum hsm_secret_error {
  * Represents the content of the hsm_secret file, either a raw seed or a mnemonic.
  */
 struct hsm_secret {
-	struct secret secret;      /* 32-byte secret (legacy compatibility) */
-	u8 *bip32_seed;           /* 64-byte BIP32 seed (NULL for legacy) */
+	u8 *secret_data;          /* Variable length: 32 bytes (legacy) or 64 bytes (mnemonic) */
+	size_t secret_len;        /* Length of secret_data: 32 or 64 bytes */
 	char *mnemonic;           /* NULL if not derived from mnemonic */
     enum hsm_secret_type type;
 };

common/utxo.c

@@ -47,6 +47,8 @@ const char *utxotype_to_str(enum utxotype utxotype)
 		return "p2wsh_from_close";
 	case UTXO_P2TR:
 		return "p2tr";
+	case UTXO_P2TR_BIP86:
+		return "p2tr_bip86";
 	}
 	abort();
 }

contrib/msggen/msggen/schema.json

@@ -25994,12 +25994,13 @@
           "addresstype": {
             "type": "string",
             "description": [
-              "It specifies the type of address wanted; currently *bech32* (e.g. `tb1qu9j4lg5f9rgjyfhvfd905vw46eg39czmktxqgg` on bitcoin testnet or `bc1qwqdg6squsna38e46795at95yu9atm8azzmyvckulcc7kytlcckxswvvzej` on bitcoin mainnet), or *p2tr* taproot addresses. The special value *all* generates all known address types for the same underlying key."
+              "It specifies the type of address wanted; currently *bech32* (e.g. `tb1qu9j4lg5f9rgjyfhvfd905vw46eg39czmktxqgg` on bitcoin testnet or `bc1qwqdg6squsna38e46795at95yu9atm8azzmyvckulcc7kytlcckxswvvzej` on bitcoin mainnet), *p2tr* taproot addresses, or *bip86* for BIP86-derived taproot addresses. The special value *all* generates all known address types for the same underlying key."
             ],
             "default": "*bech32* address",
             "enum": [
               "bech32",
               "p2tr",
+              "bip86",
               "all"
             ]
           }
@@ -26013,7 +26014,7 @@
             "added": "v23.08",
             "type": "string",
             "description": [
-              "The taproot address."
+              "The taproot address (returned for both 'p2tr' and 'bip86' addresstype)."
             ]
           },
           "bech32": {
@@ -26061,6 +26062,18 @@
           "response": {
             "p2tr": "bcrt1p2gppccw6ywewmg74qqxxmqfdpjds3rpr0mf22y9tm9xcc0muggwsea9nkf"
           }
+        },
+        {
+          "request": {
+            "id": "example:newaddr#3",
+            "method": "newaddr",
+            "params": {
+              "addresstype": "bip86"
+            }
+          },
+          "response": {
+            "p2tr": "bcrt1p2gppccw6ywewmg74qqxxmqfdpjds3rpr0mf22y9tm9xcc0muggwsea9nkf"
+          }
         }
       ]
     },

doc/schemas/newaddr.json

@@ -17,12 +17,13 @@
       "addresstype": {
         "type": "string",
         "description": [
-          "It specifies the type of address wanted; currently *bech32* (e.g. `tb1qu9j4lg5f9rgjyfhvfd905vw46eg39czmktxqgg` on bitcoin testnet or `bc1qwqdg6squsna38e46795at95yu9atm8azzmyvckulcc7kytlcckxswvvzej` on bitcoin mainnet), or *p2tr* taproot addresses. The special value *all* generates all known address types for the same underlying key."
+          "It specifies the type of address wanted; currently *bech32* (e.g. `tb1qu9j4lg5f9rgjyfhvfd905vw46eg39czmktxqgg` on bitcoin testnet or `bc1qwqdg6squsna38e46795at95yu9atm8azzmyvckulcc7kytlcckxswvvzej` on bitcoin mainnet), *p2tr* taproot addresses, or *bip86* for BIP86-derived taproot addresses. The special value *all* generates all known address types for the same underlying key."
         ],
         "default": "*bech32* address",
         "enum": [
           "bech32",
           "p2tr",
+          "bip86",
           "all"
         ]
       }
@@ -36,7 +37,7 @@
         "added": "v23.08",
         "type": "string",
         "description": [
-          "The taproot address."
+          "The taproot address (returned for both 'p2tr' and 'bip86' addresstype)."
         ]
       },
       "bech32": {
@@ -84,6 +85,18 @@
       "response": {
         "p2tr": "bcrt1p2gppccw6ywewmg74qqxxmqfdpjds3rpr0mf22y9tm9xcc0muggwsea9nkf"
       }
+    },
+    {
+      "request": {
+        "id": "example:newaddr#3",
+        "method": "newaddr",
+        "params": {
+          "addresstype": "bip86"
+        }
+      },
+      "response": {
+        "p2tr": "bcrt1p2gppccw6ywewmg74qqxxmqfdpjds3rpr0mf22y9tm9xcc0muggwsea9nkf"
+      }
     }
   ]
 }

hsmd/hsmd.c

@@ -18,6 +18,7 @@
 #include <common/hsm_secret.h>
 #include <common/memleak.h>
 #include <common/status.h>
+#include <hsmd/libhsmd.h>
 #include <common/status_wiregen.h>
 #include <common/subdaemon.h>
 #include <errno.h>
@@ -40,7 +41,7 @@
 #define REQ_FD 3
 
 /* Temporary storage for the secret until we pass it to `hsmd_init` */
-struct hsm_secret hsm_secret = { .bip32_seed = NULL, .mnemonic = NULL };
+struct hsm_secret hsm_secret = { .secret_data = NULL, .secret_len = 0, .mnemonic = NULL, .type = HSM_SECRET_INVALID };
 
 /*~ We keep track of clients, but there's not much to keep. */
 struct client {
@@ -369,11 +370,9 @@ static void create_hsm(int fd, const char *passphrase)
 	}
 	status_debug("HSM: Derived BIP32 seed from mnemonic");
 
-	/* Store the full 64-byte BIP32 seed for BIP86 derivation */
-	hsm_secret.bip32_seed = tal_dup_arr(tmpctx, u8, bip32_seed, bip32_seed_len, 0);
-	
-	/* Also store first 32 bytes for legacy compatibility */
-	memcpy(&hsm_secret.secret, bip32_seed, sizeof(hsm_secret.secret));
+	/* Store the full BIP32 seed */
+	hsm_secret.secret_data = tal_dup_arr(tmpctx, u8, bip32_seed, bip32_seed_len, 0);
+	hsm_secret.secret_len = bip32_seed_len;
 
 	/* Set the type based on whether passphrase was used */
 	hsm_secret.type = passphrase ? HSM_SECRET_MNEMONIC_WITH_PASS : HSM_SECRET_MNEMONIC_NO_PASS;
@@ -478,8 +477,19 @@ static void load_hsm(const char *passphrase)
 			                        "Failed to load hsm_secret: %s", hsm_secret_error_str(err));
 	}
 
-	/* Copy the extracted secret to our global hsm_secret */
-	hsm_secret = *hsms;
+	/* Deep copy the extracted secret to our global hsm_secret */
+	hsm_secret = *hsms;  /* Copy the basic fields */
+	
+	/* Deep copy the secret data if it exists */
+	if (hsms->secret_data) {
+		hsm_secret.secret_data = tal_dup_arr(NULL, u8, hsms->secret_data, hsms->secret_len, 0);
+		hsm_secret.secret_len = hsms->secret_len;
+	}
+	
+	/* Deep copy the mnemonic if it exists */
+	if (hsms->mnemonic) {
+		hsm_secret.mnemonic = tal_strdup(NULL, hsms->mnemonic);
+	}
 }
 
 /*~ We have a pre-init call in developer mode, to set dev flags */
@@ -558,7 +568,9 @@ static struct io_plan *init_hsm(struct io_conn *conn,
 		hsm_passphrase = (const char *)tlvs->hsm_passphrase;
 
 	/*~ Don't swap this. */
-	sodium_mlock(hsm_secret.secret.data, sizeof(hsm_secret.secret.data));
+	if (hsm_secret.secret_data && hsm_secret.secret_len > 0) {
+		sodium_mlock(hsm_secret.secret_data, hsm_secret.secret_len);
+	}
 
 	if (!developer) {
 		assert(!dev_force_privkey);
@@ -578,7 +590,7 @@ static struct io_plan *init_hsm(struct io_conn *conn,
 
 	/* This was tallocated off NULL, and memleak complains if we don't free it */
 	tal_free(tlvs);
-	return req_reply(conn, c, hsmd_init(hsm_secret.secret, hsmd_mutual_version,
+	return req_reply(conn, c, hsmd_init(hsm_secret.secret_data, hsm_secret.secret_len, hsmd_mutual_version,
 					    bip32_key_version));
 }
 
@@ -662,6 +674,11 @@ static struct io_plan *handle_memleak(struct io_conn *conn,
 
 	memleak_ptr(memtable, dev_force_privkey);
 	memleak_ptr(memtable, dev_force_bip32_seed);
+	
+	/* Question for Rusty/ Reviewer: Is this a suss pattern? */ 
+	memleak_ptr(memtable, hsm_secret.secret_data);
+	memleak_ptr(memtable, hsm_secret.mnemonic);
+	memleak_ptr(memtable, get_secretstuff_bip32_seed());
 
 	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	reply = towire_hsmd_dev_memleak_reply(NULL, found_leak);
@@ -706,46 +723,17 @@ void hsmd_status_failed(enum status_failreason reason, const char *fmt, ...)
 	status_send_fatal(take(towire_status_fail(NULL, reason, str)));
 }
 
-
-/* Helper function to derive the BIP86 base key (m/86'/0'/0') */
-static void derive_bip86_base_key(struct ext_key *bip86_base)
-{
-	/* Check if we have the full BIP32 seed available */
-	if (!hsm_secret.bip32_seed) {
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "BIP86 derivation requires full BIP32 seed (not available in legacy format)");
-	}
-
-	/* First create the master key from the seed */
-	struct ext_key master_key;
-	if (bip32_key_from_seed(hsm_secret.bip32_seed, 64, BIP32_VER_MAIN_PRIVATE, 0, &master_key) != WALLY_OK) {
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Failed to create master key from BIP32 seed");
-	}
-
-	/* Set up the BIP86 base path: m/86'/0'/0' */
-	u32 base_path[3];
-	base_path[0] = 86 | 0x80000000;  /* 86' */
-	base_path[1] = 0x80000000;       /* 0' */
-	base_path[2] = 0x80000000;       /* 0' */
-
-	/* Derive the BIP86 base key */
-	if (bip32_key_from_parent_path(&master_key, base_path, 3, BIP32_FLAG_KEY_PRIVATE, bip86_base) != WALLY_OK) {
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Failed to derive BIP86 base key");
-	}
-}
-
 /* Handle BIP86 key derivation request */
 static struct io_plan *handle_derive_bip86_key(struct io_conn *conn,
 					       struct client *c,
 					       const u8 *msg_in)
 {
 	u8 *reply;
-	struct pubkey pubkey;
+	u32 index;
+	bool is_change;
 
-	/* Validate the incoming message format */
-	if (!fromwire_hsmd_derive_bip86_key(msg_in, NULL, NULL))
+	/* Extract parameters from the wire message */
+	if (!fromwire_hsmd_derive_bip86_key(msg_in, &index, &is_change))
 		return bad_req(conn, c, msg_in);
 
 	/* Check if we have a mnemonic-based HSM secret */
@@ -755,57 +743,15 @@ static struct io_plan *handle_derive_bip86_key(struct io_conn *conn,
 				   "BIP86 derivation requires mnemonic-based HSM secret");
 	}
 
-	/* Derive the BIP86 base key using the helper function */
+	/* Derive only the BIP86 base key (m/86'/0'/0') */
 	struct ext_key bip86_base;
 	derive_bip86_base_key(&bip86_base);
 
-	/* Return the BIP86 base key as a pubkey */
-	if (!secp256k1_ec_pubkey_parse(secp256k1_ctx, &pubkey.pubkey,
-				       bip86_base.pub_key, sizeof(bip86_base.pub_key))) {
-		return bad_req_fmt(conn, c, msg_in,
-				   "Failed to parse BIP86 base pubkey");
-	}
-
-	reply = towire_hsmd_derive_bip86_key_reply(NULL, &pubkey);
+	/* Return the full BIP86 base extended key */
+	reply = towire_hsmd_derive_bip86_key_reply(NULL, &bip86_base);
 	return req_reply(conn, c, take(reply));
 }
 
-/*~ Get the BIP86 keys for this given index: if privkey is NULL, we
- * don't fill it in. This derives the full path: m/86'/0'/0'/0/index */
-static void bip86_key(struct privkey *privkey, struct pubkey *pubkey,
-		      u32 index)
-{
-	struct privkey unused_priv;
-
-	if (privkey == NULL)
-		privkey = &unused_priv;
-
-	if (index >= BIP32_INITIAL_HARDENED_CHILD)
-		status_failed(STATUS_FAIL_MASTER_IO, "Index %u too great", index);
-
-	/* Derive the BIP86 base key using the helper function */
-	struct ext_key bip86_base;
-	derive_bip86_base_key(&bip86_base);
-
-	/* Now derive the specific index: m/86'/0'/0'/0/index */
-	u32 final_path[2];
-	final_path[0] = 0;  /* change (0 for receive) */
-	final_path[1] = index;  /* address_index */
-
-	struct ext_key final_key;
-	if (bip32_key_from_parent_path(&bip86_base, final_path, 2, BIP32_FLAG_KEY_PRIVATE, &final_key) != WALLY_OK) {
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "BIP86 derivation of index %u failed", index);
-	}
-
-	/* Convert to our format */
-	memcpy(privkey->secret.data, final_key.priv_key+1, 32);
-	if (!secp256k1_ec_pubkey_create(secp256k1_ctx, &pubkey->pubkey,
-					privkey->secret.data))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "BIP86 pubkey %u create failed", index);
-}
-
 /* Handle BIP86 pubkey check request */
 static struct io_plan *handle_check_bip86_pubkey(struct io_conn *conn,
 						 struct client *c,

hsmd/hsmd_wire.csv

@@ -201,7 +201,7 @@ msgtype,hsmd_derive_bip86_key,54
 msgdata,hsmd_derive_bip86_key,index,u32,
 msgdata,hsmd_derive_bip86_key,is_change,bool,
 msgtype,hsmd_derive_bip86_key_reply,154
-msgdata,hsmd_derive_bip86_key_reply,pubkey,pubkey,
+msgdata,hsmd_derive_bip86_key_reply,bip86_base,ext_key,
 
 # Give me ECDH(node-id-secret,point)
 msgtype,hsmd_ecdh_req,1