Skip to content

Commit 7b29cf9

Browse files
committed
keystore: derive xprv once not twice when signing
We derived the xprv at keypath twice to mitigate potential bit-flips. Such a bit-flip could be devestating if it happened during computation of an xpub or address returned to the host computer, as funds sent to it could not be recovered. We plan to encrypt the seed in RAM, involving the securechip, whose use is costly (~100ms per KDF invocation). Since the xprv is needed each input to sign an input in a Bitcoin transaction, computing the xprv twice doubles the time needed to sign an input. There, computing it twice is not needed, as a potential bit flip (which is very unlikely in the first place) would simply result in invalid signatures and a rejected transaction. The keystore_get_xpub function is still called when accumulating the inputs hash of a BTC transaction (in the first round of inputs streaming in signtx.rs, using `common::Payload::from`), which is still computed twice. Here, only one round is sufficient as well, but the performance impact here is small, as the xpubs are cached using `Bip32XpubCache`. Maybe we will still improve this in another commit.
1 parent 9570b0a commit 7b29cf9

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

src/keystore.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ bool keystore_secp256k1_nonce_commit(
564564
uint8_t* signer_commitment_out)
565565
{
566566
struct ext_key xprv __attribute__((__cleanup__(keystore_zero_xkey))) = {0};
567-
if (!_get_xprv_twice(keypath, keypath_len, &xprv)) {
567+
if (!_get_xprv(keypath, keypath_len, &xprv)) {
568568
return false;
569569
}
570570
const secp256k1_context* ctx = wally_get_secp_context();
@@ -596,7 +596,7 @@ bool keystore_secp256k1_sign(
596596
return false;
597597
}
598598
struct ext_key xprv __attribute__((__cleanup__(keystore_zero_xkey))) = {0};
599-
if (!_get_xprv_twice(keypath, keypath_len, &xprv)) {
599+
if (!_get_xprv(keypath, keypath_len, &xprv)) {
600600
return false;
601601
}
602602
const secp256k1_context* ctx = wally_get_secp_context();
@@ -731,7 +731,7 @@ static bool _schnorr_bip86_keypair(
731731
return false;
732732
}
733733
struct ext_key xprv __attribute__((__cleanup__(keystore_zero_xkey))) = {0};
734-
if (!_get_xprv_twice(keypath, keypath_len, &xprv)) {
734+
if (!_get_xprv(keypath, keypath_len, &xprv)) {
735735
return false;
736736
}
737737
const uint8_t* secret_key = xprv.priv_key + 1; // first byte is 0;

0 commit comments

Comments
 (0)