Merge #323: Fix getaddressinfo

6cc26f0 Run the formatter (Jamil Lambert, PhD)
b17157e Fix getaddressinfo (Jamil Lambert, PhD)

Pull request description:

  The test was marked as TODO and the fields in the structs do not match what is returned by the RPC.

  In v20 the return field `labels` changes from a vector of json objects that was modelled as `GetAddressInfoLabel` to a `Vec<String>`. To allow for this make the modelled type a `Vec<String>`, this means for v17-v19 the `name` field is modelled and the  `purpose` field of `GetAddressInfoLabel` is not.

  - Fix all of the fields in v17.
  - Redefine the types for changes in v18, v29, v22 and v28.
  - Update the model for to allow for the changes in return types.
  - Update the test using different `Address` types to check the inner structs contain expected values.

ACKs for top commit:
  tcharding:
    ACK 6cc26f0

Tree-SHA512: 52f371b06097e5b832edc1a2c0558329081b11b71cb9319fa72b0b157312ceda33b918d5fb0fb657e19225e1c705e09c84376d1c580d656f41ad7f8f4b6d4dd9

Author: tcharding

integration_test/tests/wallet.rs

@@ -6,7 +6,7 @@
 #![allow(unused_imports)] // Some imports are only used in specific versions.
 
 use bitcoin::address::{Address, KnownHrp, NetworkChecked};
-use bitcoin::{secp256k1, Amount, CompressedPublicKey, FeeRate, PrivateKey, PublicKey};
+use bitcoin::{secp256k1, Amount, CompressedPublicKey, FeeRate, Network, PrivateKey, PublicKey};
 use integration_test::{Node, NodeExt as _, Wallet};
 use node::{mtype, AddressType, ImportMultiRequest, ImportMultiScriptPubKey, ImportMultiTimestamp};
 
@@ -192,15 +192,34 @@ fn wallet__get_addresses_by_label__modelled() {
 }
 
 #[test]
-#[cfg(feature = "TODO")]        // FIXME: The types are broken.
-// TODO: Consider testing a few different address types.
 fn wallet__get_address_info__modelled() {
     let node = Node::with_wallet(Wallet::Default, &[]);
-    let address = node.client.new_address().expect("failed to create new address");
 
-    let json: GetAddressInfo = node.client.get_address_info(&address).expect("getaddressinfo");
+    // Test an address with a label.
+    let label_name = "test-label";
+    let addr = node.client.new_address_with_label(label_name).unwrap().assume_checked();
+    let json: GetAddressInfo = node.client.get_address_info(&addr).expect("getaddressinfo legacy");
     let model: Result<mtype::GetAddressInfo, GetAddressInfoError> = json.into_model();
-    model.unwrap();
+    let model = model.unwrap();
+    assert_eq!(model.address.assume_checked(), addr);
+    assert_eq!(model.labels[0], label_name);
+
+    // Test a SegWit address with embedded information.
+    let addr_p2sh = node.client.new_address_with_type(AddressType::P2shSegwit).unwrap();
+    let json: GetAddressInfo = node.client.get_address_info(&addr_p2sh).expect("getaddressinfo p2sh-segwit");
+    let model: Result<mtype::GetAddressInfo, GetAddressInfoError> = json.into_model();
+    let model = model.unwrap();
+    let embedded = model.embedded.unwrap();
+    assert_eq!(model.address.assume_checked(), addr_p2sh);
+    assert_eq!(model.script.unwrap(), mtype::ScriptType::WitnessV0KeyHash);
+    assert!(embedded.address.is_valid_for_network(Network::Regtest));
+
+    // Test a Bech32 address.
+    let addr_bech32 = node.client.new_address_with_type(AddressType::Bech32).unwrap();
+    let json: GetAddressInfo = node.client.get_address_info(&addr_bech32).expect("getaddressinfo bech32");
+    let model: Result<mtype::GetAddressInfo, GetAddressInfoError> = json.into_model();
+    let model = model.unwrap();
+    assert_eq!(model.address.assume_checked(), addr_bech32);
 }
 
 #[test]

types/src/model/mod.rs

@@ -51,17 +51,17 @@ pub use self::{
         SignMessageWithPrivKey, ValidateAddress,
     },
     wallet::{
-        AddMultisigAddress, AddressInformation, AddressLabel, AddressPurpose, Bip125Replaceable,
-        BumpFee, CreateWallet, DumpPrivKey, GetAddressInfo, GetAddressInfoEmbedded,
-        GetAddressesByLabel, GetBalance, GetBalances, GetBalancesMine, GetBalancesWatchOnly,
-        GetNewAddress, GetRawChangeAddress, GetReceivedByAddress, GetReceivedByLabel,
-        GetTransaction, GetTransactionDetail, GetUnconfirmedBalance, GetWalletInfo,
-        LastProcessedBlock, ListAddressGroupings, ListAddressGroupingsItem, ListLabels,
-        ListLockUnspent, ListLockUnspentItem, ListReceivedByAddress, ListReceivedByAddressItem,
-        ListReceivedByLabel, ListReceivedByLabelItem, ListSinceBlock, ListSinceBlockTransaction,
-        ListTransactions, ListTransactionsItem, ListUnspent, ListUnspentItem, ListWallets,
-        LoadWallet, PsbtBumpFee, RescanBlockchain, ScriptType, Send, SendAll, SendMany,
-        SendToAddress, SignMessage, SimulateRawTransaction, TransactionCategory, UnloadWallet,
-        WalletCreateFundedPsbt, WalletDisplayAddress, WalletProcessPsbt,
+        AddMultisigAddress, AddressInformation, AddressPurpose, Bip125Replaceable, BumpFee,
+        CreateWallet, DumpPrivKey, GetAddressInfo, GetAddressInfoEmbedded, GetAddressesByLabel,
+        GetBalance, GetBalances, GetBalancesMine, GetBalancesWatchOnly, GetNewAddress,
+        GetRawChangeAddress, GetReceivedByAddress, GetReceivedByLabel, GetTransaction,
+        GetTransactionDetail, GetUnconfirmedBalance, GetWalletInfo, LastProcessedBlock,
+        ListAddressGroupings, ListAddressGroupingsItem, ListLabels, ListLockUnspent,
+        ListLockUnspentItem, ListReceivedByAddress, ListReceivedByAddressItem, ListReceivedByLabel,
+        ListReceivedByLabelItem, ListSinceBlock, ListSinceBlockTransaction, ListTransactions,
+        ListTransactionsItem, ListUnspent, ListUnspentItem, ListWallets, LoadWallet, PsbtBumpFee,
+        RescanBlockchain, ScriptType, Send, SendAll, SendMany, SendToAddress, SignMessage,
+        SimulateRawTransaction, TransactionCategory, UnloadWallet, WalletCreateFundedPsbt,
+        WalletDisplayAddress, WalletProcessPsbt,
     },
 };

types/src/model/wallet.rs

@@ -117,8 +117,16 @@ pub struct GetAddressInfo {
     pub is_mine: bool,
     /// If the address is watchonly.
     pub is_watch_only: bool,
+    /// Whether we know how to spend coins sent to this address, ignoring the possible lack of private keys.
+    pub solvable: Option<bool>,
+    /// A descriptor for spending coins sent to this address (only when solvable).
+    pub descriptor: Option<String>,
+    /// The descriptor used to derive this address if this is a descriptor wallet
+    pub parent_descriptor: Option<String>,
     /// If the key is a script.
-    pub is_script: bool,
+    pub is_script: Option<bool>,
+    /// If the address was used for change output.
+    pub is_change: Option<bool>,
     /// If the address is a witness address.
     pub is_witness: bool,
     /// The version number of the witness program.
@@ -140,18 +148,20 @@ pub struct GetAddressInfo {
     pub pubkey: Option<PublicKey>,
     /// Information about the address embedded in P2SH or P2WSH, if relevant and known.
     pub embedded: Option<GetAddressInfoEmbedded>,
-    /// If the address is compressed.
-    pub is_compressed: bool,
+    /// If the pubkey is compressed.
+    pub is_compressed: Option<bool>,
     /// The label associated with the address, "" is the default account.
-    pub label: String,
+    pub label: Option<String>,
     /// The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT).
     pub timestamp: Option<u32>,
     /// The HD keypath if the key is HD and available.
     pub hd_key_path: Option<bip32::DerivationPath>,
     /// The Hash160 of the HD seed.
     pub hd_seed_id: Option<hash160::Hash>,
+    /// The fingerprint of the master key.
+    pub hd_master_fingerprint: Option<bip32::Fingerprint>,
     /// Labels associated with the address.
-    pub labels: Vec<AddressLabel>,
+    pub labels: Vec<String>,
 }
 
 /// An address script type.
@@ -177,16 +187,6 @@ pub enum ScriptType {
     WitnessUnknown,
 }
 
-/// An address label.
-#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
-#[serde(deny_unknown_fields)]
-pub struct AddressLabel {
-    /// The address label.
-    pub name: String,
-    /// Purpose of address (send or receive).
-    pub purpose: AddressPurpose,
-}
-
 /// The `embedded` field of `GetAddressInfo`.
 ///
 /// It includes all getaddressinfo output fields for the embedded address, excluding metadata
@@ -200,8 +200,16 @@ pub struct GetAddressInfoEmbedded {
     pub address: Address<NetworkUnchecked>,
     /// The hex encoded scriptPubKey generated by the address.
     pub script_pubkey: ScriptBuf,
+    /// Whether we know how to spend coins sent to this address, ignoring the possible lack of private keys.
+    pub solvable: Option<bool>,
+    /// A descriptor for spending coins sent to this address (only when solvable).
+    pub descriptor: Option<String>,
+    /// The descriptor used to derive this address if this is a descriptor wallet
+    pub parent_descriptor: Option<String>,
     /// If the key is a script.
-    pub is_script: bool,
+    pub is_script: Option<bool>,
+    /// If the address was used for change output.
+    pub is_change: Option<bool>,
     /// If the address is a witness address.
     pub is_witness: bool,
     /// The version number of the witness program.
@@ -215,17 +223,17 @@ pub struct GetAddressInfoEmbedded {
     /// The redeemscript for the p2sh address.
     pub hex: Option<ScriptBuf>,
     /// Array of pubkeys associated with the known redeemscript (only if "script" is "multisig").
-    pub pubkeys: Vec<PublicKey>,
+    pub pubkeys: Option<Vec<PublicKey>>,
     /// Number of signatures required to spend multisig output (only if "script" is "multisig").
     pub sigs_required: Option<u32>,
     /// The hex value of the raw public key, for single-key addresses (possibly embedded in P2SH or P2WSH).
     pub pubkey: Option<PublicKey>,
     /// If the address is compressed.
-    pub is_compressed: bool,
+    pub is_compressed: Option<bool>,
     /// The label associated with the address, "" is the default account.
-    pub label: String,
+    pub label: Option<String>,
     /// Labels associated with the address.
-    pub labels: Vec<AddressLabel>,
+    pub labels: Option<Vec<String>>,
 }
 
 /// Models the result of JSON-RPC method `getbalance`.

types/src/v17/mod.rs

@@ -163,7 +163,7 @@
 //! | getaccountaddress                  | returns nothing |                                        |
 //! | getaddressbyaccount                | returns nothing |                                        |
 //! | getaddressesbylabel                | version + model |                                        |
-//! | getaddressinfo                     | version + model | UNTESTED                               |
+//! | getaddressinfo                     | version + model |                                        |
 //! | getbalance                         | version + model |                                        |
 //! | getnewaddress                      | version + model |                                        |
 //! | getrawchangeaddress                | version + model |                                        |
@@ -283,9 +283,9 @@ pub use self::{
         ListReceivedByAddressError, ListReceivedByAddressItem, ListSinceBlock, ListSinceBlockError,
         ListSinceBlockTransaction, ListSinceBlockTransactionError, ListTransactions,
         ListTransactionsItem, ListTransactionsItemError, ListUnspent, ListUnspentItem,
-        ListUnspentItemError, ListWallets, LoadWallet, LockUnspent, RescanBlockchain, SendMany,
-        SendToAddress, SetTxFee, SignMessage, TransactionCategory, WalletCreateFundedPsbt,
-        WalletCreateFundedPsbtError, WalletProcessPsbt,
+        ListUnspentItemError, ListWallets, LoadWallet, LockUnspent, RescanBlockchain, ScriptType,
+        SendMany, SendToAddress, SetTxFee, SignMessage, TransactionCategory,
+        WalletCreateFundedPsbt, WalletCreateFundedPsbtError, WalletProcessPsbt,
     },
     zmq::GetZmqNotifications,
 };

types/src/v17/wallet/into.rs

@@ -169,14 +169,18 @@ impl GetAddressInfo {
             .map_err(E::HdKeyPath)?;
         let hd_seed_id =
             self.hd_seed_id.map(|s| s.parse::<hash160::Hash>()).transpose().map_err(E::HdSeedId)?;
-        let labels = self.labels.into_iter().map(|label| label.into_model()).collect();
+        let labels = self.labels.into_iter().map(|label| label.name).collect();
 
         Ok(model::GetAddressInfo {
             address,
             script_pubkey,
             is_mine: self.is_mine,
             is_watch_only: self.is_watch_only,
-            is_script: self.is_script,
+            solvable: None,          // v18 and above only.
+            descriptor: None,        // v18 and above only.
+            parent_descriptor: None, // v22 and above only.
+            is_script: Some(self.is_script),
+            is_change: None, // v18 and above only.
             is_witness: self.is_witness,
             witness_version,
             witness_program,
@@ -187,10 +191,11 @@ impl GetAddressInfo {
             pubkey,
             embedded,
             is_compressed: self.is_compressed,
-            label: self.label,
+            label: Some(self.label),
             timestamp: self.timestamp,
             hd_key_path,
             hd_seed_id,
+            hd_master_fingerprint: None, // v18 and above only.
             labels,
         })
     }
@@ -243,21 +248,20 @@ impl GetAddressInfoEmbedded {
         let script = self.script.map(|s| s.into_model());
         let redeem_script =
             self.hex.map(|hex| ScriptBuf::from_hex(&hex).map_err(E::Hex)).transpose()?;
-        let pubkeys = self
-            .pubkeys
-            .iter()
-            .map(|s| s.parse::<PublicKey>())
-            .collect::<Result<Vec<_>, _>>()
-            .map_err(E::Pubkeys)?;
+        let pubkeys = None;
         let sigs_required =
             self.sigs_required.map(|s| crate::to_u32(s, "sigs_required")).transpose()?;
         let pubkey = self.pubkey.map(|s| s.parse::<PublicKey>()).transpose().map_err(E::Pubkey)?;
-        let labels = self.labels.into_iter().map(|label| label.into_model()).collect();
+        let labels = self.labels.map(|labels| labels.into_iter().map(|label| label.name).collect());
 
         Ok(model::GetAddressInfoEmbedded {
             address,
             script_pubkey,
+            solvable: None,          // v18 and above only.
+            descriptor: None,        // v18 and above only.
+            parent_descriptor: None, // v22 and above only.
             is_script: self.is_script,
+            is_change: None, // v18 and above only.
             is_witness: self.is_witness,
             witness_version,
             witness_program,
@@ -273,13 +277,6 @@ impl GetAddressInfoEmbedded {
     }
 }
 
-impl GetAddressInfoLabel {
-    /// Converts version specific type to a version nonspecific, more strongly typed type.
-    pub fn into_model(self) -> model::AddressLabel {
-        model::AddressLabel { name: self.name, purpose: self.purpose.into_model() }
-    }
-}
-
 impl GetBalance {
     /// Converts version specific type to a version nonspecific, more strongly typed type.
     pub fn into_model(self) -> Result<model::GetBalance, ParseAmountError> {

types/src/v17/wallet/mod.rs

@@ -273,11 +273,9 @@ pub struct GetAddressInfo {
     pub embedded: Option<GetAddressInfoEmbedded>,
     /// If the address is compressed.
     #[serde(rename = "iscompressed")]
-    pub is_compressed: bool,
+    pub is_compressed: Option<bool>,
     /// The label associated with the address, "" is the default account.
     pub label: String,
-    /// DEPRECATED. The account associated with the address, "" is the default account.
-    pub account: String,
     /// The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT).
     pub timestamp: Option<u32>,
     /// The HD keypath if the key is HD and available.
@@ -342,7 +340,7 @@ pub struct GetAddressInfoEmbedded {
     pub script_pubkey: String,
     /// If the key is a script.
     #[serde(rename = "isscript")]
-    pub is_script: bool,
+    pub is_script: Option<bool>,
     /// If the address is a witness address.
     #[serde(rename = "iswitness")]
     pub is_witness: bool,
@@ -356,20 +354,18 @@ pub struct GetAddressInfoEmbedded {
     pub script: Option<ScriptType>,
     /// The redeemscript for the p2sh address.
     pub hex: Option<String>,
-    /// Array of pubkeys associated with the known redeemscript (only if "script" is "multisig").
-    pub pubkeys: Vec<String>,
     /// Number of signatures required to spend multisig output (only if "script" is "multisig").
     #[serde(rename = "sigsrequired")]
     pub sigs_required: Option<i64>,
     /// The hex value of the raw public key, for single-key addresses (possibly embedded in P2SH or P2WSH).
     pub pubkey: Option<String>,
     /// If the address is compressed.
     #[serde(rename = "iscompressed")]
-    pub is_compressed: bool,
+    pub is_compressed: Option<bool>,
     /// The label associated with the address, "" is the default account.
-    pub label: String,
+    pub label: Option<String>,
     /// Array of labels associated with the address.
-    pub labels: Vec<GetAddressInfoLabel>,
+    pub labels: Option<Vec<GetAddressInfoLabel>>,
 }
 
 /// The `label` field of `GetAddressInfo` (and `GetAddressInfoEmbedded`).