diff --git a/integration_test/src/lib.rs b/integration_test/src/lib.rs index ec969884..63f4da29 100644 --- a/integration_test/src/lib.rs +++ b/integration_test/src/lib.rs @@ -2,6 +2,9 @@ use std::path::PathBuf; +use bitcoin::bip32::{Fingerprint, Xpriv, Xpub}; +use bitcoin::secp256k1::{Secp256k1, XOnlyPublicKey}; +use bitcoin::Network; use node::{Conf, P2P}; use rand::distributions::Alphanumeric; use rand::Rng; @@ -148,3 +151,25 @@ pub fn three_node_network() -> (Node, Node, Node) { (node1, node2, node3) } + +/// BIP32 key set for testing. +pub struct TestKeys { + pub xprv: Xpriv, + pub xpub: Xpub, + pub fingerprint: Fingerprint, + pub x_only_public_key: XOnlyPublicKey, +} + +/// Returns deterministic test keys derived from a zero seed. +pub fn test_keys() -> TestKeys { + let secp = Secp256k1::new(); + let seed = [0u8; 32]; + let xprv = Xpriv::new_master(Network::Regtest, &seed).unwrap(); + let xpub = Xpub::from_priv(&secp, &xprv); + TestKeys { + xprv, + xpub, + fingerprint: xpub.fingerprint(), + x_only_public_key: xprv.private_key.x_only_public_key(&secp).0, + } +} diff --git a/integration_test/tests/raw_transactions.rs b/integration_test/tests/raw_transactions.rs index 90c286e1..90cdc98c 100644 --- a/integration_test/tests/raw_transactions.rs +++ b/integration_test/tests/raw_transactions.rs @@ -5,13 +5,14 @@ #![allow(non_snake_case)] // Test names intentionally use double underscore. #![allow(unused_imports)] // Because of feature gated tests. +use bitcoin::bip32::DerivationPath; use bitcoin::consensus::encode; use bitcoin::hex::FromHex as _; use bitcoin::opcodes::all::*; use bitcoin::{ absolute, consensus, hex, psbt, script, transaction, Amount, ScriptBuf, Transaction, TxOut, }; -use integration_test::{Node, NodeExt as _, Wallet}; +use integration_test::{test_keys, Node, NodeExt as _, Wallet}; use node::vtype::*; use node::{mtype, Input, Output}; // All the version specific types. @@ -127,57 +128,57 @@ fn raw_transactions__create_raw_transaction__modelled() { create_sign_send(&node); } -// Notes on testing decoding of PBST. -// -// - `bip32_derivs` field in the input list of the decoded PSBT changes shape a bunch of times. -// - In v23 a bunch of additional fields are added. -// - In v24 taproot fields are added. -// -// All this should still be handled by `into_model` because `bitcoin::Psbt` has all optional fields. +// Tests PSBT decoding across Bitcoin Core versions. +// Version-specific assertions are gated below. #[test] fn raw_transactions__decode_psbt__modelled() { let node = Node::with_wallet(Wallet::Default, &["-txindex"]); node.fund_wallet(); + // v17: utxoupdatepsbt unavailable + #[cfg(feature = "v17")] let mut psbt = create_a_psbt(&node); - // A bunch of new fields got added in v23. - // - // Add an arbitrary global xpub to see if it decodes. Before v23 this will end up in `unknown`, - // from v23 onwards in should be in its own field. - { - use std::collections::BTreeMap; - - use bitcoin::bip32::{DerivationPath, Fingerprint, Xpub}; + // v18+: utxoupdatepsbt available + #[cfg(not(feature = "v17"))] + let mut psbt = { + let psbt = create_a_psbt(&node); + let updated: UtxoUpdatePsbt = node.client.utxo_update_psbt(&psbt).expect("utxoupdatepsbt"); + updated.into_model().expect("UtxoUpdatePsbt into model").0 + }; - let mut map = BTreeMap::default(); - // Some arbitrary xpub I grabbed from rust-bitcoin. - let xpub = "xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL"; - let xpub = xpub.parse::().expect("failed to parse xpub"); - let fp = Fingerprint::from([1u8, 2, 3, 42]); - let path = - "m/84'/0'/0'/0/1".parse::().expect("failed to parse derivation path"); - map.insert(xpub, (fp, path)); + let keys = test_keys(); + let path: DerivationPath = "m/84'/0'/0'/0/1".parse().expect("valid derivation path"); + psbt.xpub.insert(keys.xpub, (keys.fingerprint, path)); - psbt.xpub = map; + // v24+: Taproot fields (e.g. `tap_internal_key`). + #[cfg(not(feature = "v23_and_below"))] + { + psbt.inputs[0].tap_internal_key = Some(keys.x_only_public_key); } let encoded = psbt.to_string(); - let json: DecodePsbt = node.client.decode_psbt(&encoded).expect("decodepsbt"); let model: Result = json.into_model(); - - #[allow(unused_variables)] let decoded = model.unwrap(); - // Before Core v23 global xpubs was not a known keypair. + // v18/v19: utxoupdatepsbt can't populate UTXO data, so fee is None + #[cfg(feature = "v19_and_below")] + assert!(decoded.fee.is_none()); + + // v20+: utxoupdatepsbt allows fee to be calculated + #[cfg(not(feature = "v19_and_below"))] + assert!(decoded.fee.expect("fee should be present").to_sat() > 0); + + // v23+: dedicated xpub field; earlier versions store in `unknown`. #[cfg(feature = "v22_and_below")] assert_eq!(decoded.psbt.unknown.len(), 1); #[cfg(not(feature = "v22_and_below"))] assert_eq!(decoded.psbt.xpub.len(), 1); - // TODO: Add a taproot field and test it with v24 + #[cfg(not(feature = "v23_and_below"))] + assert_eq!(decoded.psbt.inputs[0].tap_internal_key, Some(keys.x_only_public_key)); } #[test] diff --git a/types/src/v17/raw_transactions/error.rs b/types/src/v17/raw_transactions/error.rs index 941fd7c9..defe4514 100644 --- a/types/src/v17/raw_transactions/error.rs +++ b/types/src/v17/raw_transactions/error.rs @@ -24,6 +24,8 @@ pub enum DecodePsbtError { Inputs(PsbtInputError), /// Conversion of one of the PSBT outputs failed. Outputs(PsbtOutputError), + /// Conversion of the `fee` field failed. + Fee(ParseAmountError), } impl fmt::Display for DecodePsbtError { @@ -35,6 +37,7 @@ impl fmt::Display for DecodePsbtError { Self::Inputs(ref e) => write_err!(f, "conversion of one of the PSBT inputs failed"; e), Self::Outputs(ref e) => write_err!(f, "conversion of one of the PSBT outputs failed"; e), + Self::Fee(ref e) => write_err!(f, "conversion of the `fee` field failed"; e), } } } @@ -47,6 +50,7 @@ impl std::error::Error for DecodePsbtError { Self::Unknown(ref e) => Some(e), Self::Inputs(ref e) => Some(e), Self::Outputs(ref e) => Some(e), + Self::Fee(ref e) => Some(e), } } } diff --git a/types/src/v17/raw_transactions/into.rs b/types/src/v17/raw_transactions/into.rs index 7d9fcae1..f91f0264 100644 --- a/types/src/v17/raw_transactions/into.rs +++ b/types/src/v17/raw_transactions/into.rs @@ -124,7 +124,7 @@ impl DecodePsbt { let psbt = bitcoin::Psbt { unsigned_tx, version, xpub, proprietary, unknown, inputs, outputs }; - let fee = self.fee.map(Amount::from_sat); + let fee = self.fee.map(Amount::from_btc).transpose().map_err(E::Fee)?; Ok(model::DecodePsbt { psbt, fee }) } diff --git a/types/src/v17/raw_transactions/mod.rs b/types/src/v17/raw_transactions/mod.rs index 7950fcf5..026e7a2c 100644 --- a/types/src/v17/raw_transactions/mod.rs +++ b/types/src/v17/raw_transactions/mod.rs @@ -143,7 +143,7 @@ pub struct DecodePsbt { /// Array of transaction outputs. pub outputs: Vec, /// The transaction fee paid if all UTXOs slots in the PSBT have been filled. - pub fee: Option, + pub fee: Option, } /// An input in a partially signed Bitcoin transaction. Part of `decodepsbt`. diff --git a/types/src/v23/raw_transactions/error.rs b/types/src/v23/raw_transactions/error.rs index e9e7f3c9..6acd34c3 100644 --- a/types/src/v23/raw_transactions/error.rs +++ b/types/src/v23/raw_transactions/error.rs @@ -2,6 +2,7 @@ use core::fmt; +use bitcoin::amount::ParseAmountError; use bitcoin::{address, bip32, hex, sighash}; use super::{Bip32DerivError, PartialSignatureError, RawTransactionError, WitnessUtxoError}; @@ -22,6 +23,8 @@ pub enum DecodePsbtError { Inputs(PsbtInputError), /// Conversion of one of the PSBT outputs failed. Outputs(PsbtOutputError), + /// Conversion of the `fee` field failed. + Fee(ParseAmountError), } impl fmt::Display for DecodePsbtError { @@ -37,6 +40,7 @@ impl fmt::Display for DecodePsbtError { Self::Inputs(ref e) => write_err!(f, "conversion of one of the PSBT inputs failed"; e), Self::Outputs(ref e) => write_err!(f, "conversion of one of the PSBT outputs failed"; e), + Self::Fee(ref e) => write_err!(f, "conversion of the `fee` field failed"; e), } } } @@ -51,6 +55,7 @@ impl std::error::Error for DecodePsbtError { Self::Unknown(ref e) => Some(e), Self::Inputs(ref e) => Some(e), Self::Outputs(ref e) => Some(e), + Self::Fee(ref e) => Some(e), } } } diff --git a/types/src/v23/raw_transactions/into.rs b/types/src/v23/raw_transactions/into.rs index 7697a3b3..050c5025 100644 --- a/types/src/v23/raw_transactions/into.rs +++ b/types/src/v23/raw_transactions/into.rs @@ -67,7 +67,7 @@ impl DecodePsbt { inputs, outputs, }; - let fee = self.fee.map(Amount::from_sat); + let fee = self.fee.map(Amount::from_btc).transpose().map_err(E::Fee)?; Ok(model::DecodePsbt { psbt, fee }) } diff --git a/types/src/v23/raw_transactions/mod.rs b/types/src/v23/raw_transactions/mod.rs index 18ac7ca7..1a8273f7 100644 --- a/types/src/v23/raw_transactions/mod.rs +++ b/types/src/v23/raw_transactions/mod.rs @@ -47,7 +47,7 @@ pub struct DecodePsbt { /// Array of transaction outputs. pub outputs: Vec, /// The transaction fee paid if all UTXOs slots in the PSBT have been filled. - pub fee: Option, + pub fee: Option, } /// An item from the global xpubs list. Part of `decodepsbt`. diff --git a/types/src/v24/raw_transactions/error.rs b/types/src/v24/raw_transactions/error.rs index 490c86ba..05f92931 100644 --- a/types/src/v24/raw_transactions/error.rs +++ b/types/src/v24/raw_transactions/error.rs @@ -2,6 +2,7 @@ use core::fmt; +use bitcoin::amount::ParseAmountError; use bitcoin::taproot::{IncompleteBuilderError, TaprootBuilderError, TaprootError}; use bitcoin::{bip32, hex, secp256k1, sighash}; @@ -23,6 +24,8 @@ pub enum DecodePsbtError { Inputs(PsbtInputError), /// Conversion of one of the PSBT outputs failed. Outputs(PsbtOutputError), + /// Conversion of the `fee` field failed. + Fee(ParseAmountError), } impl fmt::Display for DecodePsbtError { @@ -38,6 +41,7 @@ impl fmt::Display for DecodePsbtError { Self::Inputs(ref e) => write_err!(f, "conversion of one of the PSBT inputs failed"; e), Self::Outputs(ref e) => write_err!(f, "conversion of one of the PSBT outputs failed"; e), + Self::Fee(ref e) => write_err!(f, "conversion of the `fee` field failed"; e), } } } @@ -52,6 +56,7 @@ impl std::error::Error for DecodePsbtError { Self::Unknown(ref e) => Some(e), Self::Inputs(ref e) => Some(e), Self::Outputs(ref e) => Some(e), + Self::Fee(ref e) => Some(e), } } } diff --git a/types/src/v24/raw_transactions/into.rs b/types/src/v24/raw_transactions/into.rs index eef03fdb..7adc1b61 100644 --- a/types/src/v24/raw_transactions/into.rs +++ b/types/src/v24/raw_transactions/into.rs @@ -72,7 +72,7 @@ impl DecodePsbt { inputs, outputs, }; - let fee = self.fee.map(Amount::from_sat); + let fee = self.fee.map(Amount::from_btc).transpose().map_err(E::Fee)?; Ok(model::DecodePsbt { psbt, fee }) } diff --git a/types/src/v24/raw_transactions/mod.rs b/types/src/v24/raw_transactions/mod.rs index 60aa463e..868346ad 100644 --- a/types/src/v24/raw_transactions/mod.rs +++ b/types/src/v24/raw_transactions/mod.rs @@ -48,7 +48,7 @@ pub struct DecodePsbt { /// Array of transaction outputs. pub outputs: Vec, /// The transaction fee paid if all UTXOs slots in the PSBT have been filled. - pub fee: Option, + pub fee: Option, } /// An item from the global xpubs list. Part of `decodepsbt`. diff --git a/types/src/v30/raw_transactions/error.rs b/types/src/v30/raw_transactions/error.rs index 490c86ba..05f92931 100644 --- a/types/src/v30/raw_transactions/error.rs +++ b/types/src/v30/raw_transactions/error.rs @@ -2,6 +2,7 @@ use core::fmt; +use bitcoin::amount::ParseAmountError; use bitcoin::taproot::{IncompleteBuilderError, TaprootBuilderError, TaprootError}; use bitcoin::{bip32, hex, secp256k1, sighash}; @@ -23,6 +24,8 @@ pub enum DecodePsbtError { Inputs(PsbtInputError), /// Conversion of one of the PSBT outputs failed. Outputs(PsbtOutputError), + /// Conversion of the `fee` field failed. + Fee(ParseAmountError), } impl fmt::Display for DecodePsbtError { @@ -38,6 +41,7 @@ impl fmt::Display for DecodePsbtError { Self::Inputs(ref e) => write_err!(f, "conversion of one of the PSBT inputs failed"; e), Self::Outputs(ref e) => write_err!(f, "conversion of one of the PSBT outputs failed"; e), + Self::Fee(ref e) => write_err!(f, "conversion of the `fee` field failed"; e), } } } @@ -52,6 +56,7 @@ impl std::error::Error for DecodePsbtError { Self::Unknown(ref e) => Some(e), Self::Inputs(ref e) => Some(e), Self::Outputs(ref e) => Some(e), + Self::Fee(ref e) => Some(e), } } } diff --git a/types/src/v30/raw_transactions/into.rs b/types/src/v30/raw_transactions/into.rs index eef03fdb..7adc1b61 100644 --- a/types/src/v30/raw_transactions/into.rs +++ b/types/src/v30/raw_transactions/into.rs @@ -72,7 +72,7 @@ impl DecodePsbt { inputs, outputs, }; - let fee = self.fee.map(Amount::from_sat); + let fee = self.fee.map(Amount::from_btc).transpose().map_err(E::Fee)?; Ok(model::DecodePsbt { psbt, fee }) } diff --git a/types/src/v30/raw_transactions/mod.rs b/types/src/v30/raw_transactions/mod.rs index 10e5e58a..da5adf36 100644 --- a/types/src/v30/raw_transactions/mod.rs +++ b/types/src/v30/raw_transactions/mod.rs @@ -48,7 +48,7 @@ pub struct DecodePsbt { /// Array of transaction outputs. pub outputs: Vec, /// The transaction fee paid if all UTXOs slots in the PSBT have been filled. - pub fee: Option, + pub fee: Option, } /// An item from the global xpubs list. Part of `decodepsbt`.