types: Clean up the ScriptPubkey type

The `ScriptPubkey` type is a type that is returned by Core in various
places. It seems like the Core devs put a fair bit of effort into
it (probably because of this) and when the type changed in v22 it was
changed in a way that was completely backwards compatible.

I failed to fully understand this when first adding the type.

With this patch applied there is now only one `ScriptPubkey` type (in
`libr.s`) and also we add a model type.

The `ScriptPubkey` type is unusual in that its fields can be used to
create `TxOut` if tx value is known - which for example in `GetTxOut` it
is.

So here we also remove the v22 `GetTxOut` type since it only existed
because of the perceived difference in `ScriptPubkey` which no longer
exists.

Fix: rust-bitcoin#116

Author: tcharding

types/src/lib.rs

@@ -35,6 +35,7 @@ pub mod model;
 
 use core::fmt;
 
+use bitcoin::address::{self, Address, NetworkUnchecked};
 use bitcoin::amount::ParseAmountError;
 use bitcoin::hex::{self, FromHex as _};
 use bitcoin::{Amount, FeeRate, ScriptBuf, Witness};
@@ -172,25 +173,43 @@ pub fn compact_size_decode(slice: &mut &[u8]) -> u64 {
 }
 
 /// Data returned by Core for a script pubkey.
+///
+/// This is used by methods in the blockchain section and in the raw transaction section (i.e raw
+/// transaction and psbt methods). The shape changed in Core v22 but the new shape is fully
+/// backwards compatible so we only provide it not a v17 specific type. The `mtype::ScriptPubkey`
+/// mirrors this design (but with concrete `rust-bitcoin` types).
 #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub struct ScriptPubkey {
     /// Script assembly.
     pub asm: String,
     /// Script hex.
     pub hex: String,
+    /// Number of required signatures - deprecated in Core v22.
+    ///
+    /// Only returned before in versions prior to 22 or for version 22 onwards if
+    /// config option `-deprecatedrpc=addresses` is passed.
     #[serde(rename = "reqSigs")]
     pub req_sigs: Option<i64>,
     /// The type, eg pubkeyhash.
     #[serde(rename = "type")]
     pub type_: String,
-    /// Array of bitcoin address.
+    /// Bitcoin address (only if a well-defined address exists).
+    pub address: Option<String>,
+    /// Array of bitcoin addresses - deprecated in Core v22.
+    ///
+    /// Only returned before in versions prior to 22 or for version 22 onwards if
+    /// config option `-deprecatedrpc=addresses` is passed.
     pub addresses: Option<Vec<String>>,
 }
 
 impl ScriptPubkey {
     fn script_buf(&self) -> Result<ScriptBuf, hex::HexToBytesError> {
         ScriptBuf::from_hex(&self.hex)
     }
+
+    fn address(&self) -> Option<Result<Address<NetworkUnchecked>, address::ParseError>> {
+        self.address.as_ref().map(|addr| addr.parse::<Address<_>>())
+    }
 }
 
 /// Data returned by Core for a script signature.

types/src/model/blockchain.rs

@@ -455,7 +455,9 @@ pub struct GetTxOut {
     /// The returned `TxOut` (strongly typed).
     pub tx_out: TxOut,
     /// Address that `tx_out` spends to.
-    pub addresses: Vec<Address<NetworkUnchecked>>,
+    ///
+    /// Only if a well-defined address exists.
+    pub address: Option<Address<NetworkUnchecked>>,
     /// Coinbase or not.
     pub coinbase: bool,
 }

types/src/v17/blockchain/error.rs

@@ -473,6 +473,9 @@ impl From<ParseAmountError> for GetMempoolInfoError {
 }
 
 /// Error when converting a `GetTxOut` type into the model type.
+///
+/// Note that variants in this type are not named in the usual fashion. The `ScriptBuf` and
+/// `Address` variants are named after the functions on [`crate::ScriptPubkey`].
 #[derive(Debug)]
 pub enum GetTxOutError {
     /// Conversion of numeric type to expected type failed.
@@ -481,10 +484,10 @@ pub enum GetTxOutError {
     BestBlock(hex::HexToArrayError),
     /// Conversion of the transaction `value` field failed.
     Value(amount::ParseAmountError),
-    /// Conversion of the transaction `script_pubkey` field failed.
-    ScriptPubkey(hex::HexToBytesError),
-    /// Conversion of the transaction `addresses` field failed.
-    Addresses(address::ParseError),
+    /// Conversion of the `ScriptPubkey` hex to a `ScriptBuf` failed.
+    ScriptBuf(hex::HexToBytesError),
+    /// Conversion of the `ScriptPubkey` `address` field failed.
+    Address(address::ParseError),
 }
 
 impl fmt::Display for GetTxOutError {
@@ -495,9 +498,10 @@ impl fmt::Display for GetTxOutError {
             Numeric(ref e) => write_err!(f, "numeric"; e),
             BestBlock(ref e) => write_err!(f, "conversion of the `beast_block` field failed"; e),
             Value(ref e) => write_err!(f, "conversion of the `value` field failed"; e),
-            ScriptPubkey(ref e) =>
-                write_err!(f, "conversion of the `script_pubkey` field failed"; e),
-            Addresses(ref e) => write_err!(f, "conversion of the `addresses` field failed"; e),
+            ScriptBuf(ref e) =>
+                write_err!(f, "conversion of the `ScriptPubkey` hex to a `ScriptBuf` failed"; e),
+            Address(ref e) =>
+                write_err!(f, "conversion of the `ScriptPubkey` `address` field failed"; e),
         }
     }
 }
@@ -511,8 +515,8 @@ impl std::error::Error for GetTxOutError {
             Numeric(ref e) => Some(e),
             BestBlock(ref e) => Some(e),
             Value(ref e) => Some(e),
-            ScriptPubkey(ref e) => Some(e),
-            Addresses(ref e) => Some(e),
+            ScriptBuf(ref e) => Some(e),
+            Address(ref e) => Some(e),
         }
     }
 }

types/src/v17/blockchain/into.rs

@@ -490,23 +490,16 @@ impl GetTxOut {
         let best_block = self.best_block.parse::<BlockHash>().map_err(E::BestBlock)?;
         let tx_out = TxOut {
             value: Amount::from_btc(self.value).map_err(E::Value)?,
-            script_pubkey: ScriptBuf::from_hex(&self.script_pubkey.hex).map_err(E::ScriptPubkey)?,
+            script_pubkey: self.script_pubkey.script_buf().map_err(E::ScriptBuf)?,
         };
 
-        let addresses = match self.script_pubkey.addresses {
-            Some(addresses) => addresses
-                .into_iter()
-                .map(|address| address.parse::<Address<_>>())
-                .collect::<Result<Vec<_>, _>>()
-                .map_err(E::Addresses)?,
-            None => vec![],
-        };
+        let address = self.script_pubkey.address().transpose().map_err(E::Address)?;
 
         Ok(model::GetTxOut {
             best_block,
             confirmations: self.confirmations,
             tx_out,
-            addresses,
+            address,
             coinbase: self.coinbase,
         })
     }

types/src/v17/blockchain/mod.rs

@@ -10,7 +10,7 @@ mod into;
 use alloc::collections::BTreeMap;
 
 use bitcoin::hex::FromHex;
-use bitcoin::{Address, Amount, FeeRate, Network, ScriptBuf, TxMerkleNode, TxOut, Wtxid};
+use bitcoin::{Amount, FeeRate, Network, TxMerkleNode, TxOut, Wtxid};
 use serde::{Deserialize, Serialize};
 
 // TODO: Remove wildcard, use explicit types.

types/src/v22/blockchain.rs

@@ -243,14 +243,10 @@
 //! </details>
 
 // JSON-RPC types by API section.
-mod blockchain;
 mod control;
 
 #[doc(inline)]
-pub use self::{
-    blockchain::{GetTxOut, GetTxOutError, ScriptPubkey},
-    control::Logging,
-};
+pub use self::control::Logging;
 #[doc(inline)]
 pub use crate::{
     v17::{
@@ -271,18 +267,19 @@ pub use crate::{
         GetPeerInfo, GetRawChangeAddress, GetRawMempool, GetRawMempoolVerbose, GetRawTransaction,
         GetRawTransactionVerbose, GetRawTransactionVerboseError, GetReceivedByAddress,
         GetTransaction, GetTransactionDetail, GetTransactionDetailError, GetTransactionError,
-        GetTxOutSetInfo, GetTxOutSetInfoError, GetUnconfirmedBalance, GetWalletInfo,
-        GetWalletInfoError, GetZmqNotifications, ListAddressGroupings, ListAddressGroupingsError,
-        ListAddressGroupingsItem, ListBanned, ListLabels, ListLockUnspent, ListLockUnspentItem,
-        ListLockUnspentItemError, ListReceivedByAddress, ListReceivedByAddressError,
-        ListReceivedByAddressItem, ListSinceBlock, ListSinceBlockError, ListSinceBlockTransaction,
-        ListSinceBlockTransactionError, ListTransactions, ListTransactionsItem,
-        ListTransactionsItemError, ListUnspent, ListUnspentItem, ListUnspentItemError, ListWallets,
-        LoadWallet, Locked, PeerInfo, PruneBlockchain, RawTransactionError, RawTransactionInput,
-        RawTransactionOutput, RescanBlockchain, SendMany, SendRawTransaction, SendToAddress,
-        SignMessage, SignRawTransaction, SignRawTransactionError, SoftforkReject,
-        TestMempoolAccept, TransactionCategory, UploadTarget, VerifyChain, VerifyTxOutProof,
-        WalletCreateFundedPsbt, WalletCreateFundedPsbtError, WalletProcessPsbt, WitnessUtxo,
+        GetTxOut, GetTxOutError, GetTxOutSetInfo, GetTxOutSetInfoError, GetUnconfirmedBalance,
+        GetWalletInfo, GetWalletInfoError, GetZmqNotifications, ListAddressGroupings,
+        ListAddressGroupingsError, ListAddressGroupingsItem, ListBanned, ListLabels,
+        ListLockUnspent, ListLockUnspentItem, ListLockUnspentItemError, ListReceivedByAddress,
+        ListReceivedByAddressError, ListReceivedByAddressItem, ListSinceBlock, ListSinceBlockError,
+        ListSinceBlockTransaction, ListSinceBlockTransactionError, ListTransactions,
+        ListTransactionsItem, ListTransactionsItemError, ListUnspent, ListUnspentItem,
+        ListUnspentItemError, ListWallets, LoadWallet, Locked, PeerInfo, PruneBlockchain,
+        RawTransactionError, RawTransactionInput, RawTransactionOutput, RescanBlockchain, SendMany,
+        SendRawTransaction, SendToAddress, SignMessage, SignRawTransaction,
+        SignRawTransactionError, SoftforkReject, TestMempoolAccept, TransactionCategory,
+        UploadTarget, VerifyChain, VerifyTxOutProof, WalletCreateFundedPsbt,
+        WalletCreateFundedPsbtError, WalletProcessPsbt, WitnessUtxo,
     },
     v18::{
         ActiveCommand, AnalyzePsbt, AnalyzePsbtError, AnalyzePsbtInput, AnalyzePsbtInputMissing,
@@ -298,4 +295,5 @@ pub use crate::{
         SoftforkType,
     },
     v21::UnloadWallet,
+    ScriptPubkey,
 };

types/src/v22/mod.rs

@@ -266,18 +266,19 @@ pub use crate::{
         GetPeerInfo, GetRawChangeAddress, GetRawMempool, GetRawMempoolVerbose, GetRawTransaction,
         GetRawTransactionVerbose, GetRawTransactionVerboseError, GetReceivedByAddress,
         GetTransaction, GetTransactionDetail, GetTransactionDetailError, GetTransactionError,
-        GetTxOutSetInfo, GetTxOutSetInfoError, GetUnconfirmedBalance, GetWalletInfo,
-        GetWalletInfoError, GetZmqNotifications, ListAddressGroupings, ListAddressGroupingsError,
-        ListAddressGroupingsItem, ListBanned, ListLabels, ListLockUnspent, ListLockUnspentItem,
-        ListLockUnspentItemError, ListReceivedByAddress, ListReceivedByAddressError,
-        ListReceivedByAddressItem, ListSinceBlock, ListSinceBlockError, ListSinceBlockTransaction,
-        ListSinceBlockTransactionError, ListTransactions, ListTransactionsItem,
-        ListTransactionsItemError, ListUnspent, ListUnspentItem, ListUnspentItemError, ListWallets,
-        LoadWallet, Locked, PeerInfo, PruneBlockchain, RawTransactionError, RawTransactionInput,
-        RawTransactionOutput, RescanBlockchain, SendMany, SendRawTransaction, SendToAddress,
-        SignMessage, SignRawTransaction, SignRawTransactionError, SoftforkReject,
-        TestMempoolAccept, TransactionCategory, UploadTarget, VerifyChain, VerifyTxOutProof,
-        WalletCreateFundedPsbt, WalletCreateFundedPsbtError, WalletProcessPsbt, WitnessUtxo,
+        GetTxOut, GetTxOutError, GetTxOutSetInfo, GetTxOutSetInfoError, GetUnconfirmedBalance,
+        GetWalletInfo, GetWalletInfoError, GetZmqNotifications, ListAddressGroupings,
+        ListAddressGroupingsError, ListAddressGroupingsItem, ListBanned, ListLabels,
+        ListLockUnspent, ListLockUnspentItem, ListLockUnspentItemError, ListReceivedByAddress,
+        ListReceivedByAddressError, ListReceivedByAddressItem, ListSinceBlock, ListSinceBlockError,
+        ListSinceBlockTransaction, ListSinceBlockTransactionError, ListTransactions,
+        ListTransactionsItem, ListTransactionsItemError, ListUnspent, ListUnspentItem,
+        ListUnspentItemError, ListWallets, LoadWallet, Locked, PeerInfo, PruneBlockchain,
+        RawTransactionError, RawTransactionInput, RawTransactionOutput, RescanBlockchain, SendMany,
+        SendRawTransaction, SendToAddress, SignMessage, SignRawTransaction,
+        SignRawTransactionError, SoftforkReject, TestMempoolAccept, TransactionCategory,
+        UploadTarget, VerifyChain, VerifyTxOutProof, WalletCreateFundedPsbt,
+        WalletCreateFundedPsbtError, WalletProcessPsbt, WitnessUtxo,
     },
     v18::{
         ActiveCommand, AnalyzePsbt, AnalyzePsbtError, AnalyzePsbtInput, AnalyzePsbtInputMissing,
@@ -293,5 +294,5 @@ pub use crate::{
         SoftforkType,
     },
     v21::UnloadWallet,
-    v22::{GetTxOut, GetTxOutError, Logging, ScriptPubkey},
+    v22::{Logging, ScriptPubkey},
 };