Skip to content

xpubcache: allow choosing if xpub computation should be repeated #1536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/bip32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl From<bitcoin::bip32::Xpriv> for Xprv {
}
}

#[derive(Clone)]
#[derive(PartialEq, Clone)]
pub struct Xpub {
xpub: pb::XPub,
}
Expand Down
8 changes: 4 additions & 4 deletions src/rust/bitbox02-rust/src/hww/api/bitcoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ async fn xpub(
})
.await?
}
let xpub = keystore::get_xpub(keypath)
let xpub = keystore::get_xpub_twice(keypath)
.or(Err(Error::InvalidInput))?
.serialize_str(xpub_type)?;
if display {
Expand Down Expand Up @@ -163,7 +163,7 @@ pub fn derive_address_simple(
)
.or(Err(Error::InvalidInput))?;
Ok(common::Payload::from_simple(
&mut crate::xpubcache::XpubCache::new(),
&mut crate::xpubcache::XpubCache::new(crate::xpubcache::Compute::Twice),
coin_params,
simple_type,
keypath,
Expand Down Expand Up @@ -1083,7 +1083,7 @@ mod tests {
root_fingerprint: keystore::root_fingerprint().unwrap(),
keypath: KEYPATH_ACCOUNT_TESTNET.to_vec(),
xpub: Some(
crate::keystore::get_xpub(KEYPATH_ACCOUNT_TESTNET)
crate::keystore::get_xpub_once(KEYPATH_ACCOUNT_TESTNET)
.unwrap()
.into(),
),
Expand All @@ -1092,7 +1092,7 @@ mod tests {
root_fingerprint: keystore::root_fingerprint().unwrap(),
keypath: KEYPATH_ACCOUNT_MAINNET.to_vec(),
xpub: Some(
crate::keystore::get_xpub(KEYPATH_ACCOUNT_MAINNET)
crate::keystore::get_xpub_once(KEYPATH_ACCOUNT_MAINNET)
.unwrap()
.into(),
),
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hww/api/bitcoin/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ mod tests {
"sudden tenant fault inject concert weather maid people chunk youth stumble grit",
"",
);
let mut xpub_cache = Bip32XpubCache::new();
let mut xpub_cache = Bip32XpubCache::new(crate::xpubcache::Compute::Once);
let coin_params = super::super::params::get(pb::BtcCoin::Btc);
// p2wpkh
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hww/api/bitcoin/multisig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ pub fn validate(multisig: &Multisig, keypath: &[u32]) -> Result<(), Error> {
return Err(Error::InvalidInput);
}

let our_xpub = crate::keystore::get_xpub(keypath)?.serialize(None)?;
let our_xpub = crate::keystore::get_xpub_once(keypath)?.serialize(None)?;
let maybe_our_xpub =
bip32::Xpub::from(&multisig.xpubs[multisig.our_xpub_index as usize]).serialize(None)?;
if our_xpub != maybe_our_xpub {
Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ fn is_our_key(key: &pb::KeyOriginInfo, our_root_fingerprint: &[u8]) -> Result<bo
xpub: Some(xpub),
..
} if root_fingerprint.as_slice() == our_root_fingerprint => {
let our_xpub = crate::keystore::get_xpub(keypath)?.serialize(None)?;
let our_xpub = crate::keystore::get_xpub_once(keypath)?.serialize(None)?;
let maybe_our_xpub = bip32::Xpub::from(xpub).serialize(None)?;
Ok(our_xpub == maybe_our_xpub)
}
Expand Down Expand Up @@ -787,7 +787,7 @@ mod tests {

// Creates a policy for one of our own keys at keypath.
fn make_our_key(keypath: &[u32]) -> pb::KeyOriginInfo {
let our_xpub = crate::keystore::get_xpub(keypath).unwrap();
let our_xpub = crate::keystore::get_xpub_once(keypath).unwrap();
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath.to_vec(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down
18 changes: 9 additions & 9 deletions src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use super::{bip143, bip341, common, keypath};

use crate::hal::Ui;
use crate::workflow::{confirm, transaction};
use crate::xpubcache::Bip32XpubCache;
use crate::xpubcache::{Bip32XpubCache, Compute};

use alloc::string::String;
use alloc::vec::Vec;
Expand Down Expand Up @@ -681,7 +681,7 @@ async fn _process(
let validated_output_script_configs =
validate_script_configs(coin_params, &request.output_script_configs)?;

let mut xpub_cache = Bip32XpubCache::new();
let mut xpub_cache = Bip32XpubCache::new(Compute::Once);
setup_xpub_cache(&mut xpub_cache, &request.script_configs);

// For now we only allow one payment request with one output per transaction. In the future,
Expand Down Expand Up @@ -1804,7 +1804,7 @@ mod tests {
let multisig = pb::btc_script_config::Multisig {
threshold: 1,
xpubs: vec![
crate::keystore::get_xpub(keypath).unwrap().into(),
crate::keystore::get_xpub_once(keypath).unwrap().into(),
parse_xpub("xpub6ERxBysTYfQyY4USv6c6J1HNVv9hpZFN9LHVPu47Ac4rK8fLy6NnAeeAHyEsMvG4G66ay5aFZii2VM7wT3KxLKX8Q8keZPd67kRGmrD1WJj").unwrap(),
],
our_xpub_index: 0,
Expand Down Expand Up @@ -3160,7 +3160,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down Expand Up @@ -3280,7 +3280,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down Expand Up @@ -3345,7 +3345,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
],
};
Expand Down Expand Up @@ -3452,7 +3452,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down Expand Up @@ -3504,7 +3504,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down Expand Up @@ -3553,7 +3553,7 @@ mod tests {
pb::KeyOriginInfo {
root_fingerprint: crate::keystore::root_fingerprint().unwrap(),
keypath: keypath_account.to_vec(),
xpub: Some(crate::keystore::get_xpub(keypath_account).unwrap().into()),
xpub: Some(crate::keystore::get_xpub_once(keypath_account).unwrap().into()),
},
pb::KeyOriginInfo {
root_fingerprint: vec![],
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02-rust/src/hww/api/electrum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub async fn process(
{
return Err(Error::InvalidInput);
}
let xpub = keystore::get_xpub(keypath)
let xpub = keystore::get_xpub_twice(keypath)
.or(Err(Error::InvalidInput))?
.serialize_str(bip32::XPubType::Xpub)?;

Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02-rust/src/hww/api/ethereum/pubrequest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn process_address(
if !super::keypath::is_valid_keypath_address(&request.keypath) {
return Err(Error::InvalidInput);
}
let pubkey = crate::keystore::get_xpub(&request.keypath)
let pubkey = crate::keystore::get_xpub_twice(&request.keypath)
.or(Err(Error::InvalidInput))?
.pubkey_uncompressed()?;
let address = super::address::from_pubkey(&pubkey);
Expand Down Expand Up @@ -79,7 +79,7 @@ fn process_xpub(request: &pb::EthPubRequest) -> Result<Response, Error> {
if !super::keypath::is_valid_keypath_xpub(&request.keypath) {
return Err(Error::InvalidInput);
}
let xpub = keystore::get_xpub(&request.keypath)
let xpub = keystore::get_xpub_twice(&request.keypath)
.or(Err(Error::InvalidInput))?
.serialize_str(bip32::XPubType::Xpub)?;

Expand Down
47 changes: 27 additions & 20 deletions src/rust/bitbox02-rust/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ fn get_xprv(keypath: &[u32]) -> Result<bip32::Xprv, ()> {
.into())
}

fn get_xprv_twice(keypath: &[u32]) -> Result<bip32::Xprv, ()> {
let xprv = get_xprv(keypath)?;
if xprv == get_xprv(keypath)? {
Ok(xprv)
} else {
Err(())
}
}

/// Get the private key at the keypath.
pub fn secp256k1_get_private_key(keypath: &[u32]) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
let xprv = get_xprv(keypath)?;
Expand All @@ -75,19 +66,35 @@ pub fn secp256k1_get_private_key_twice(keypath: &[u32]) -> Result<zeroize::Zeroi
/// Can be used only if the keystore is unlocked. Returns the derived xpub,
/// using bip32 derivation. Derivation is done from the xprv master, so hardened
/// derivation is allowed.
pub fn get_xpub(keypath: &[u32]) -> Result<bip32::Xpub, ()> {
let xpriv = get_xprv_twice(keypath)?;
pub fn get_xpub_once(keypath: &[u32]) -> Result<bip32::Xpub, ()> {
let xpriv = get_xprv(keypath)?;
let secp = bitcoin::secp256k1::Secp256k1::new();
let xpub = bitcoin::bip32::Xpub::from_priv(&secp, &xpriv.xprv);

Ok(bip32::Xpub::from(xpub))
}

/// Can be used only if the keystore is unlocked. Returns the derived xpub,
/// using bip32 derivation. Derivation is done from the xprv master, so hardened
/// derivation is allowed.
pub fn get_xpub_twice(keypath: &[u32]) -> Result<bip32::Xpub, ()> {
let res1 = get_xpub_once(keypath)?;
let res2 = get_xpub_once(keypath)?;
if res1 != res2 {
return Err(());
}
Ok(res1)
}

/// Returns fingerprint of the root public key at m/, which are the first four bytes of its hash160
/// according to:
/// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#serialization-format
pub fn root_fingerprint() -> Result<Vec<u8>, ()> {
Ok(get_xpub(&[])?.pubkey_hash160().get(..4).ok_or(())?.to_vec())
Ok(get_xpub_twice(&[])?
.pubkey_hash160()
.get(..4)
.ok_or(())?
.to_vec())
}

fn bip85_entropy(keypath: &[u32]) -> Result<zeroize::Zeroizing<Vec<u8>>, ()> {
Expand Down Expand Up @@ -203,29 +210,29 @@ mod tests {
}

#[test]
fn test_get_xpub() {
fn test_get_xpub_twice() {
let keypath = &[44 + HARDENED, 0 + HARDENED, 0 + HARDENED];
// Also test with unhardened and non-zero elements.
let keypath_5 = &[44 + HARDENED, 1 + HARDENED, 10 + HARDENED, 1, 100];

keystore::lock();
assert!(get_xpub(keypath).is_err());
assert!(get_xpub_twice(keypath).is_err());

// 24 words
mock_unlocked_using_mnemonic(
"sleep own lobster state clean thrive tail exist cactus bitter pass soccer clinic riot dream turkey before sport action praise tunnel hood donate man",
"",
);
assert_eq!(
get_xpub(&[]).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
get_xpub_twice(&[]).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
"xpub661MyMwAqRbcEhX8d9WJh78SZrxusAzWFoykz4n5CF75uYRzixw5FZPUSoWyhaaJ1bpiPFdzdHSQqJN38PcTkyrLmxT4J2JDYfoGJQ4ioE2",
);
assert_eq!(
get_xpub(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
get_xpub_twice(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
"xpub6Cj6NNCGj2CRPHvkuEG1rbW3nrNCAnLjaoTg1P67FCGoahSsbg9WQ7YaMEEP83QDxt2kZ3hTPAPpGdyEZcfAC1C75HfR66UbjpAb39f4PnG",
);
assert_eq!(
get_xpub(keypath_5).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
get_xpub_twice(keypath_5).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
"xpub6HHn1zdtf1RjePopiTV5nxf8jY2xwbJicTQ91jV4cUJZ5EnbvXyBGDhqWt8B9JxxBt9vExi4pdWzrbrM43qSFs747VCGmSy2DPWAhg9MkUg",
);

Expand All @@ -235,7 +242,7 @@ mod tests {
"",
);
assert_eq!(
get_xpub(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
get_xpub_twice(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
"xpub6C7fKxGtTzEVxCC22U2VHx4GpaVy77DzU6KdZ1CLuHgoUGviBMWDc62uoQVxqcRa5RQbMPnffjpwxve18BG81VJhJDXnSpRe5NGKwVpXiAb",
);

Expand All @@ -245,7 +252,7 @@ mod tests {
"",
);
assert_eq!(
get_xpub(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
get_xpub_twice(keypath).unwrap().serialize_str(bip32::XPubType::Xpub).unwrap(),
"xpub6DLvpzjKpJ8k4xYrWYPmZQkUe9dkG1eRig2v6Jz4iYgo8hcpHWx87gGoCGDaB2cHFZ3ExUfe1jDiMu7Ch6gA4ULCBhvwZj29mHCPYSux3YV",
)
}
Expand Down Expand Up @@ -401,7 +408,7 @@ mod tests {
test.expected_mnemonic,
);
let keypath = &[44 + HARDENED, 0 + HARDENED, 0 + HARDENED];
let xpub = get_xpub(keypath).unwrap();
let xpub = get_xpub_once(keypath).unwrap();
assert_eq!(
xpub.serialize_str(crate::bip32::XPubType::Xpub).unwrap(),
test.expected_xpub,
Expand Down
Loading