Skip to content

Commit

Permalink
Merge #985: Taproot support
Browse files Browse the repository at this point in the history
4bb2372 qa: drop specific value assertions in coin selection test (Antoine Poinsot)
7eb024a fuzz: alternate between generating wsh() and tr() descs (Antoine Poinsot)
b9a4625 ci: run functional tests both under Taproot and P2WSH (Antoine Poinsot)
687a0c2 qa: adapt hardcoded coin selection tests to Taproot (Antoine Poinsot)
ecef6bf qa: functional tests lianad using Taproot descriptors (Antoine Poinsot)
96d30db signer: taproot support in hot signer (Antoine Poinsot)
80a7dc3 signer: move p2wsh signing into a dedicated function (Antoine Poinsot)
d622258 command: also update Taproot sigs in 'updatespend' (Antoine Poinsot)
714bd3c spend: check for either p2wsh or Taproot sigs in sanity checks (Antoine Poinsot)
e05039f spend: don't populate non_witness_utxo for Taproot (Antoine Poinsot)
8596ca7 bitcoind: compare descriptors, not their string representation. (Antoine Poinsot)
602c862 bitcoind: sanity check min supported version for Taproot descriptor (Antoine Poinsot)
d3b7e4c config: unit test a valid config with a Taproot descriptor (Antoine Poinsot)
6cf8eaa config: deser_from_str isn't descriptor specific. (Antoine Poinsot)
04f4b8a descriptors: Taproot support (Antoine Poinsot)
c897d41 descriptors: encapsulate key matching logic (Antoine Poinsot)
85fdc40 descriptor: encapsulate PSBT in/out information update (Antoine Poinsot)

Pull request description:

  This introduces Taproot support in the Liana daemon / core library.

  We start by introducing support for `tr()` descriptors alongside `wsh()` descriptors in the Taproot modules. For Taproot-Liana descriptors whose primary spending path isn't a single key, we deterministically derive an unspendable internal key as per https://delvingbitcoin.org/t/unspendable-keys-in-descriptors/304/21. This is to allow signing devices to not display the internal key as a spendable key when verifying a descriptor. Currently signing device vendors signaled willingness to implement this scheme.

  We then adapt the PSBT management logic to use Taproot fields when necessary and upgrade the hot signer to provide Schnorr signatures depending on the PSBT information.

  Finally, the functional tests are adapted to be able to run the whole test suite under either P2WSH or Taproot. This is done in a somewhat hacky way as i bailed out of re-implementing a Taproot PSBT signer and finalizer in Python after implementing [TapMiniscript support in upstream python-bip380](darosior/python-bip380#23). Instead we use a small Rust program to sign PSBTs for now and we skip a test which explicitly requires an external finalizer.

ACKs for top commit:
  darosior:
    ACK 4bb2372 -- this underwent multiple rounds of review, Edouard tested it in his follow-up PR to the GUI and i wrote a couple fuzz targets exercising part of the logic introduced here

Tree-SHA512: 426032f0bcf8a27e43cf3d158da011bda648cb56534ea678d3c4a43c3f59047e4bb7f83469fd38f70ac962d1a9721e9c1f68baa60bf597bf08fa3c39fc00ebe8
  • Loading branch information
darosior committed Mar 14, 2024
2 parents b11dbfd + 4bb2372 commit b22f22f
Show file tree
Hide file tree
Showing 25 changed files with 2,055 additions and 370 deletions.
14 changes: 13 additions & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@ task:
matrix:
- USE_MIN_BITCOIN_VERSION: 'TRUE'
- USE_MIN_BITCOIN_VERSION: 'FALSE'
- USE_TAPROOT: 1
- name: 'RPC functional tests'
env:
TEST_GROUP: tests/test_rpc.py
matrix:
- USE_TAPROOT: 0
- USE_TAPROOT: 1
- name: 'Chain functional tests'
env:
TEST_GROUP: tests/test_chain.py
matrix:
- USE_TAPROOT: 0
- USE_TAPROOT: 1

cargo_registry_cache:
folders: $CARGO_HOME/registry
Expand All @@ -34,7 +41,12 @@ task:
fingerprint_script:
- rustc --version
- cat Cargo.lock
lianad_build_script: cargo build --release
tests_tools_cache:
folder: tests/tools/taproot_signer/target
fingerprint_script:
- rustc --version
- cat tests/tools/taproot_signer/Cargo.lock
lianad_build_script: cargo build --release && cd tests/tools/taproot_signer && cargo build --release

deps_script: apt update && apt install -y python3 python3-pip

Expand Down
6 changes: 4 additions & 2 deletions fuzz/fuzz_targets/descriptor_parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,12 @@ fuzz_target!(|data: &[u8]| {
desc.receive_descriptor().derive(der_index, &SECP256K1),
desc.change_descriptor().derive(der_index, &SECP256K1),
];
let mut psbt_in = Default::default();
let mut psbt_out = Default::default();
for desc in der_descs {
desc.address(Network::Bitcoin);
desc.script_pubkey();
desc.witness_script();
desc.bip32_derivations();
desc.update_psbt_in(&mut psbt_in);
desc.update_change_psbt_out(&mut psbt_out);
}
});
19 changes: 13 additions & 6 deletions fuzz/fuzz_targets/descriptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ struct Config {
pub recovery_paths: Vec<RecPathConfig>,
pub der_index: u16,
pub dummy_psbt: DummyPsbt,
pub use_taproot: bool,
}

fuzz_target!(|config: Config| {
Expand All @@ -126,7 +127,12 @@ fuzz_target!(|config: Config| {
} else {
return;
};
let policy = if let Ok(policy) = LianaPolicy::new(prim_path_info, rec_paths_info) {
let policy = if config.use_taproot {
LianaPolicy::new(prim_path_info, rec_paths_info)
} else {
LianaPolicy::new_legacy(prim_path_info, rec_paths_info)
};
let policy = if let Ok(policy) = policy {
policy
} else {
return;
Expand Down Expand Up @@ -154,11 +160,13 @@ fuzz_target!(|config: Config| {
desc.receive_descriptor().derive(der_index, &SECP256K1),
desc.change_descriptor().derive(der_index, &SECP256K1),
];
let mut psbt_in = Default::default();
let mut psbt_out = Default::default();
for desc in &der_descs {
desc.address(Network::Bitcoin);
desc.script_pubkey();
desc.witness_script();
desc.bip32_derivations();
desc.update_psbt_in(&mut psbt_in);
desc.update_change_psbt_out(&mut psbt_out);
}

// Exercise the methods gathering information from a PSBT. TODO: get more useful PSBTs.
Expand All @@ -172,12 +180,11 @@ fuzz_target!(|config: Config| {
// for outputs.
let rec_desc = &der_descs[0];
for psbt_in in psbt.inputs.iter_mut() {
psbt_in.witness_script = Some(rec_desc.witness_script());
psbt_in.bip32_derivation = rec_desc.bip32_derivations();
rec_desc.update_psbt_in(psbt_in);
}
let change_desc = &der_descs[1];
for psbt_out in psbt.outputs.iter_mut() {
psbt_out.bip32_derivation = change_desc.bip32_derivations();
change_desc.update_change_psbt_out(psbt_out);
}

// Now get the spend info again with these info.
Expand Down
33 changes: 27 additions & 6 deletions src/bitcoin/d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ const BITCOIND_RETRY_LIMIT: usize = 60;
// The minimum bitcoind version that can be used with lianad.
const MIN_BITCOIND_VERSION: u64 = 240000;

// The minimum bitcoind version that can be used with lianad and a Taproot descriptor.
const MIN_TAPROOT_BITCOIND_VERSION: u64 = 260000;

/// An error in the bitcoind interface.
#[derive(Debug)]
pub enum BitcoindError {
Expand Down Expand Up @@ -129,8 +132,8 @@ impl std::fmt::Display for BitcoindError {
BitcoindError::InvalidVersion(v) => {
write!(
f,
"Invalid bitcoind version '{}', minimum supported is '{}'.",
v, MIN_BITCOIND_VERSION
"Invalid bitcoind version '{}', minimum supported is '{}' and minimum supported if using Taproot is '{}'.",
v, MIN_BITCOIND_VERSION, MIN_TAPROOT_BITCOIND_VERSION
)
}
BitcoindError::NetworkMismatch(conf_net, bitcoind_net) => {
Expand Down Expand Up @@ -699,12 +702,16 @@ impl BitcoinD {
pub fn node_sanity_checks(
&self,
config_network: bitcoin::Network,
is_taproot: bool,
) -> Result<(), BitcoindError> {
// Check the minimum supported bitcoind version
let version = self.get_bitcoind_version();
if version < MIN_BITCOIND_VERSION {
return Err(BitcoindError::InvalidVersion(version));
}
if is_taproot && version < MIN_TAPROOT_BITCOIND_VERSION {
return Err(BitcoindError::InvalidVersion(version));
}

// Check bitcoind is running on the right network
let bitcoind_net = self.get_network_bip70();
Expand Down Expand Up @@ -747,13 +754,27 @@ impl BitcoinD {
// Check our main descriptor is imported in this wallet.
let receive_desc = main_descriptor.receive_descriptor();
let change_desc = main_descriptor.change_descriptor();
let desc_list: Vec<String> = self
let desc_list: Vec<_> = self
.list_descriptors()
.into_iter()
.map(|entry| entry.desc)
.filter_map(|entry| {
match descriptor::Descriptor::<descriptor::DescriptorPublicKey>::from_str(
&entry.desc,
) {
Ok(desc) => Some(desc),
Err(e) => {
log::error!(
"Error deserializing descriptor: {}. Descriptor: {}.",
e,
entry.desc
);
None
}
}
})
.collect();
if !desc_list.contains(&receive_desc.to_string())
|| !desc_list.contains(&change_desc.to_string())
if !desc_list.iter().any(|desc| *receive_desc == *desc)
|| !desc_list.iter().any(|desc| *change_desc == *desc)
{
return Err(BitcoindError::Wallet(
self.watchonly_wallet_path.clone(),
Expand Down
6 changes: 6 additions & 0 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,12 @@ impl DaemonControl {
psbtin
.partial_sigs
.extend(db_psbtin.partial_sigs.clone().into_iter());
psbtin
.tap_script_sigs
.extend(db_psbtin.tap_script_sigs.clone().into_iter());
if psbtin.tap_key_sig.is_none() {
psbtin.tap_key_sig = db_psbtin.tap_key_sig;
}
}
} else {
// If the transaction doesn't exist in DB already, sanity check its inputs.
Expand Down
22 changes: 21 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ where
{
let string = String::deserialize(deserializer)?;
T::from_str(&string)
.map_err(|e| de::Error::custom(format!("Error parsing descriptor '{}': '{}'", string, e)))
.map_err(|e| de::Error::custom(format!("Error parsing '{}': {}", string, e)))
}

pub fn serialize_to_string<T: std::fmt::Display, S: Serializer>(
Expand Down Expand Up @@ -319,6 +319,26 @@ mod tests {
#[cfg(unix)] // On non-UNIX there is no 'daemon' member.
assert_eq!(toml_str, serialized);

// A valid, round-tripping, config for a Taproot descriptor.
let toml_str = r#"
data_dir = '/home/wizardsardine/custom/folder/'
daemon = false
log_level = 'TRACE'
main_descriptor = 'tr([abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*,and_v(v:pk([abcdef01]xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/<0;1>/*),older(52560)))#0mt7e93c'
[bitcoin_config]
network = 'bitcoin'
poll_interval_secs = 18
[bitcoind_config]
cookie_path = '/home/user/.bitcoin/.cookie'
addr = '127.0.0.1:8332'
"#.trim_start().replace(" ", "");
let parsed = toml::from_str::<Config>(&toml_str).expect("Deserializing toml_str");
let serialized = toml::to_string_pretty(&parsed).expect("Serializing to toml");
#[cfg(unix)] // On non-UNIX there is no 'daemon' member.
assert_eq!(toml_str, serialized);

// A valid, round-tripping, config with `auth` instead of `cookie_path`
let toml_str = r#"
data_dir = '/home/wizardsardine/custom/folder/'
Expand Down
Loading

0 comments on commit b22f22f

Please sign in to comment.