Merge #431: fix(types): use f64 for DecodePsbt fee field

3113bf1 fix(types): use f64 for DecodePsbt fee field (deadmanoz)

Pull request description:

  As per raised in #429, addressing type discrepancies between `corepc-types` and Bitcoin Core

  Bitcoin Core returns the `fee` field in the `decodepsbt` RPC response as a floating-point (double) BTC value, not as satoshis (integer). The current implementation uses `u64` for the fee field and converts it with `Amount::from_sat()`, which causes a deserialisation failure.

  Minimal repro with a PSBT from Bitcoin Core's functional tests.

  https://github.com/bitcoin/bitcoin/blob/13891a8a685d255cb13dd5018e3d5ccc18b07c34/test/functional/data/rpc_psbt.json#L96

  ```rust
  use corepc_client::client_sync::v28::Client;
  use corepc_client::client_sync::Auth;
  use corepc_client::types::v28::DecodePsbt;

  fn main() -> Result<(), Box<dyn std::error::Error>> {
      // First valid PSBT from Bitcoin Core's test/functional/data/rpc_psbt.json (valid[0]).
      let psbt = concat!(
          "cHNidP8BAHUCAAAAASaBcTce3/KF6Tet7qSze3gADAVmy7OtZGQXE8pCFxv2AAAAAAD+////",
          "AtPf9QUAAAAAGXapFNDFmQPFusKGh2DpD9UhpGZap2UgiKwA4fUFAAAAABepFDVF5uM7gyxH",
          "BQ8k0+65PJwDlIvHh7MuEwAAAQD9pQEBAAAAAAECiaPHHqtNIOA3G7ukzGmPopXJRjr6Ljl/",
          "hTPMti+VZ+UBAAAAFxYAFL4Y0VKpsBIDna89p95PUzSe7LmF/////4b4qkOnHf8USIk6Uwpy",
          "N+9rRgi7st0tAXHmOuxqSJC0AQAAABcWABT+Pp7xp0XpdNkCxDVZQ6vLNL1TU/////8CAMLr",
          "CwAAAAAZdqkUhc/xCX/Z4Ai7NK9wnGIZeziXikiIrHL++E4sAAAAF6kUM5cluiHv1irHU6m8",
          "0GfWx6ajnQWHAkcwRAIgJxK+IuAnDzlPVoMR3HyppolwuAJf3TskAinwf4pfOiQCIAGLONfc",
          "0xTnNMkna9b7QPZzMlvEuqFEyADS8vAtsnZcASED0uFWdJQbrUqZY3LLh+GFbTZSYG2YVi/j",
          "nF6efkE/IQUCSDBFAiEA0SuFLYXc2WHS9fSrZgZU327tzHlMDDPOXMMJ/7X85Y0CIGczio4O",
          "FyXBl/saiK9Z9R5E5CVbIBZ8hoQDHAXR8lkqASECI7cr7vCWXRC+B3jv7NYfysb3mk6haTkz",
          "gHNEZPhPKrMAAAAAAAAA",
      );

      let auth = Auth::UserPass("bitcoin".to_string(), "bitcoin".to_string());
      let client = Client::new_with_auth("http://127.0.0.1:8332", auth)?;
      let decoded: DecodePsbt = client.call("decodepsbt", &[serde_json::json!(psbt)])?;
      println!("fee: {:?}", decoded.fee);
      Ok(())
  }
  ```

  ```
  Error: JsonRpc(Json(Error("invalid type: floating point `3.01e-6`, expected u64", line: 1, column: 2896)))
  ```

  This change:
  - Changes the fee field type from `u64` to `f64` in `DecodePsbt` struct (as required in v17, v23, v24, v30)
  - Updates the conversion to use `Amount::from_btc()` instead of `Amount::from_sat()`
  - Adds a `Fee(ParseAmountError)` error variant to `DecodePsbtError` for proper error handling of the fee
  - Adds an integration test to verify correct deserialisation

ACKs for top commit:
  tcharding:
    ACK 3113bf1

Tree-SHA512: de6ec1c23869df79bfb7317831531d42a1c08602af77ff1575a514f47356112f55f5327a9d6fa7923fddb5e3253768ae5013454439ff29a4fe1aa4075df1c66c

Author: tcharding

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,
+    }
+}

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::<Xpub>().expect("failed to parse xpub");
-        let fp = Fingerprint::from([1u8, 2, 3, 42]);
-        let path =
-            "m/84'/0'/0'/0/1".parse::<DerivationPath>().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
+    #[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<mtype::DecodePsbt, DecodePsbtError> = 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]

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),
         }
     }
 }

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 })
     }

types/src/v17/raw_transactions/mod.rs

@@ -143,7 +143,7 @@ pub struct DecodePsbt {
     /// Array of transaction outputs.
     pub outputs: Vec<PsbtOutput>,
     /// The transaction fee paid if all UTXOs slots in the PSBT have been filled.
-    pub fee: Option<u64>,
+    pub fee: Option<f64>,
 }
 
 /// An input in a partially signed Bitcoin transaction. Part of `decodepsbt`.

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),
         }
     }
 }

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 })
     }

types/src/v23/raw_transactions/mod.rs

@@ -47,7 +47,7 @@ pub struct DecodePsbt {
     /// Array of transaction outputs.
     pub outputs: Vec<PsbtOutput>,
     /// The transaction fee paid if all UTXOs slots in the PSBT have been filled.
-    pub fee: Option<u64>,
+    pub fee: Option<f64>,
 }
 
 /// An item from the global xpubs list. Part of `decodepsbt`.

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),
         }
     }
 }

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 })
     }