diff --git a/integration_test/tests/raw_transactions.rs b/integration_test/tests/raw_transactions.rs index 90c286e1..bfc81321 100644 --- a/integration_test/tests/raw_transactions.rs +++ b/integration_test/tests/raw_transactions.rs @@ -127,18 +127,37 @@ fn raw_transactions__create_raw_transaction__modelled() { create_sign_send(&node); } -// Notes on testing decoding of PBST. +// Notes on testing decoding of PSBT. // -// - `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. +// Version-specific behavior: +// - v17: Base PSBT support via `createpsbt`/`decodepsbt`. No `utxoupdatepsbt`. +// - v18-v19: `utxoupdatepsbt` added but default address type is p2sh-segwit; `utxoupdatepsbt` +// can't detect p2sh-segwit as segwit without descriptors, so UTXO data isn't populated. +// - v20+: Default address type changed to bech32 (PR #16884). Native segwit is directly +// detected, so UTXO data is populated and fee calculation works. +// - v23: Global xpubs field added (previously ended up in `unknown`). +// - v24: Taproot fields added (`tap_internal_key`, etc). +// - v30: MuSig2 fields added. TODO: Test MuSig2. // -// All this should still be handled by `into_model` because `bitcoin::Psbt` has all optional fields. +// The `bip32_derivs` field in inputs changes shape across versions. +// All version differences should be handled by `into_model` because `bitcoin::Psbt` has optional fields. #[test] fn raw_transactions__decode_psbt__modelled() { let node = Node::with_wallet(Wallet::Default, &["-txindex"]); node.fund_wallet(); + // utxoupdatepsbt (v18+) + // Looks up UTXOs from chain and populates `witness_utxo` field + // so that fee can be calculated (fee = sum(inputs) - sum(outputs)). + #[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 + }; + + // v17: No utxoupdatepsbt available, so input values are unknown. + #[cfg(feature = "v17")] let mut psbt = create_a_psbt(&node); // A bunch of new fields got added in v23. @@ -158,18 +177,36 @@ fn raw_transactions__decode_psbt__modelled() { let path = "m/84'/0'/0'/0/1".parse::().expect("failed to parse derivation path"); map.insert(xpub, (fp, path)); - psbt.xpub = map; } - let encoded = psbt.to_string(); + // Add an arbitrary tap_internal_key for v24+. + #[cfg(not(feature = "v23_and_below"))] + let tap_key = { + use bitcoin::secp256k1::XOnlyPublicKey; + let tap_key_bytes = <[u8; 32]>::from_hex( + "2afc20bf379bc96a2f4e9e63ffceb8652b2b6a097f63fbee6ecec2a49a48010e", + ) + .unwrap(); + let key = XOnlyPublicKey::from_slice(&tap_key_bytes).unwrap(); + psbt.inputs[0].tap_internal_key = Some(key); + 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(); + // Fee requires UTXO data. v18/v19 default to p2sh-segwit addresses which utxoupdatepsbt + // can't detect without descriptors. v20+ defaults to bech32 (PR #16884) which works. + #[cfg(feature = "v19_and_below")] + assert!(decoded.fee.is_none()); + + #[cfg(not(feature = "v19_and_below"))] + assert!(decoded.fee.expect("fee should be present").to_sat() > 0); + // Before Core v23 global xpubs was not a known keypair. #[cfg(feature = "v22_and_below")] assert_eq!(decoded.psbt.unknown.len(), 1); @@ -177,7 +214,9 @@ fn raw_transactions__decode_psbt__modelled() { #[cfg(not(feature = "v22_and_below"))] assert_eq!(decoded.psbt.xpub.len(), 1); - // TODO: Add a taproot field and test it with v24 + // v24+ Taproot fields (e.g. `tap_internal_key`). + #[cfg(not(feature = "v23_and_below"))] + assert_eq!(decoded.psbt.inputs[0].tap_internal_key, Some(tap_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`.