Merge #534: Update GetBlockStats fields to be optional

94f6a4f Update the docs for GetBlockStats (Abeeujah)
ec827c2 Update GetBlockStats fields to be optional (Abeeujah)

Pull request description:

  This PR aims to update the `GetBlockStats` model/response to match the core's spec for [Docs fix](https://bitcoincore.org/en/doc/23.0.0/rpc/blockchain/getblockstats/) the rpc spec for getblockstats specifies that it takes in two arguments

  1. hash_or_height    (string or numeric, required) The block hash or height of the target block
  2. stats             (json array, optional, default=all values)

  If the stats arg is provided, only the value for the stats is returned, if not provided it defaults to all, and all the fields values are returned; indicating that all the fields for `GetBlockStats` is optional.

  To better model and represent this, all the fields in v17, v25, and `model::GetBlockStats` are now wrapped in `Option`.

  - The docs has been updated to include the optional stats argument
  - Tests has also been updated to accept the stats argument, and confirm it behaves as specified for all versions of core.

ACKs for top commit:
  jamillambert:
    ACK 94f6a4f
  tcharding:
    ACK 94f6a4f

Tree-SHA512: 8154b831897f85ffdceedb02a28dd00f7980d08d3daf1fcd7d793b46d266aa3c2e962a8f3b072a5f14339cc3b94ccd5d4af05fa1ceee3d3753baebb54ca05c56

Author: tcharding

client/src/client_sync/v17/blockchain.rs

@@ -112,12 +112,20 @@ macro_rules! impl_client_v17__get_block_header {
 macro_rules! impl_client_v17__get_block_stats {
     () => {
         impl Client {
-            pub fn get_block_stats_by_height(&self, height: u32) -> Result<GetBlockStats> {
-                self.call("getblockstats", &[into_json(height)?])
+            pub fn get_block_stats_by_height(
+                &self,
+                height: u32,
+                stats: Option<&[&str]>,
+            ) -> Result<GetBlockStats> {
+                self.call("getblockstats", &[into_json(height)?, into_json(stats)?])
             }
 
-            pub fn get_block_stats_by_block_hash(&self, hash: &BlockHash) -> Result<GetBlockStats> {
-                self.call("getblockstats", &[into_json(hash)?])
+            pub fn get_block_stats_by_block_hash(
+                &self,
+                hash: &BlockHash,
+                stats: Option<&[&str]>,
+            ) -> Result<GetBlockStats> {
+                self.call("getblockstats", &[into_json(hash)?, into_json(stats)?])
             }
         }
     };

integration_test/tests/blockchain.rs

@@ -197,45 +197,53 @@ fn blockchain__get_block_header__modelled() {
 #[test]
 fn blockchain__get_block_stats__modelled() {
     // Version 17 and 18 cannot call `getblockstats` if `-txindex` is not enabled.
-    #[cfg(not(feature = "v18_and_below"))]
-    getblockstats();
+    // newer versions do not.
+    let node = if cfg!(feature = "v18_and_below") {
+        Node::with_wallet(Wallet::Default, &["-txindex"])
+    } else {
+        Node::with_wallet(Wallet::Default, &[])
+    };
+    node.fund_wallet();
 
-    // All versions including 17 and 18 can `getblockstats` if `-txindex` is enabled.
-    getblockstats_txindex();
+    get_block_stats_by_height(&node);
+    get_block_stats_by_block_hash(&node);
+    get_block_stats_with_stats(&node);
 }
 
-#[cfg(not(feature = "v18_and_below"))]
-fn getblockstats() {
-    let node = Node::with_wallet(Wallet::Default, &[]);
-    node.fund_wallet();
+fn get_block_stats_by_height(node: &Node) {
+    let json: GetBlockStats =
+        node.client.get_block_stats_by_height(101, None).expect("getblockstats");
 
-    let json: GetBlockStats = node.client.get_block_stats_by_height(1).expect("getblockstats");
     let model: Result<mtype::GetBlockStats, GetBlockStatsError> = json.into_model();
-    model.unwrap();
+    let model = model.unwrap();
 
-    // No need for explicit types, used explicitly in test below.
+    assert_eq!(model.height, Some(101));
+    assert!(model.block_hash.is_some());
+}
+
+fn get_block_stats_by_block_hash(node: &Node) {
     let block_hash = node.client.best_block_hash().expect("best_block_hash failed");
     let json: GetBlockStats =
-        node.client.get_block_stats_by_block_hash(&block_hash).expect("getblockstats");
-    let model: Result<mtype::GetBlockStats, GetBlockStatsError> = json.into_model();
-    model.unwrap();
+        node.client.get_block_stats_by_block_hash(&block_hash, None).expect("getblockstats");
+
+    let model = json.into_model().unwrap(); // Explicit error type already used above.
+
+    assert_eq!(model.block_hash, Some(block_hash));
+    assert!(model.height.is_some());
 }
 
-fn getblockstats_txindex() {
-    let node = Node::with_wallet(Wallet::Default, &["-txindex"]);
-    node.fund_wallet();
+fn get_block_stats_with_stats(node: &Node) {
+    let json: GetBlockStats = node
+        .client
+        .get_block_stats_by_height(101, Some(&["minfeerate", "avgfeerate"]))
+        .expect("getblockstats");
 
-    // Get block stats by height.
-    let json: GetBlockStats = node.client.get_block_stats_by_height(101).expect("getblockstats");
-    let model: Result<mtype::GetBlockStats, GetBlockStatsError> = json.into_model();
-    model.unwrap();
+    let model = json.into_model().unwrap(); // Explicit error type already used above.
 
-    // Get block stats by block hash.
-    let block_hash = node.client.best_block_hash().expect("best_block_hash failed");
-    let json: GetBlockStats =
-        node.client.get_block_stats_by_block_hash(&block_hash).expect("getblockstats");
-    let model: Result<mtype::GetBlockStats, GetBlockStatsError> = json.into_model();
-    model.unwrap();
+    assert!(model.minimum_fee_rate.is_some());
+    assert!(model.average_fee_rate.is_some());
+    assert!(model.block_hash.is_none());
+    assert!(model.height.is_none());
 }
 
 #[test]

types/src/model/blockchain.rs

@@ -384,66 +384,68 @@ pub struct GetBlockHeaderVerbose {
 }
 
 /// Models the result of JSON-RPC method `getblockstats`.
+///
+/// All fields are optional because the caller can select which stats to compute.
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
 pub struct GetBlockStats {
     /// Average fee in the block.
-    pub average_fee: Amount,
+    pub average_fee: Option<Amount>,
     /// Average feerate.
     pub average_fee_rate: Option<FeeRate>,
     /// Average transaction size.
-    pub average_tx_size: u32,
+    pub average_tx_size: Option<u32>,
     /// The block hash (to check for potential reorgs).
-    pub block_hash: BlockHash,
+    pub block_hash: Option<BlockHash>,
     /// Feerates at the 10th, 25th, 50th, 75th, and 90th percentile weight unit (in satoshis per virtual byte).
-    pub fee_rate_percentiles: Vec<Option<FeeRate>>,
+    pub fee_rate_percentiles: Option<Vec<Option<FeeRate>>>,
     /// The height of the block.
-    pub height: u32,
+    pub height: Option<u32>,
     /// The number of inputs (excluding coinbase).
-    pub inputs: u32,
+    pub inputs: Option<u32>,
     /// Maximum fee in the block.
-    pub max_fee: Amount,
+    pub max_fee: Option<Amount>,
     /// Maximum feerate (in satoshis per virtual byte).
     pub max_fee_rate: Option<FeeRate>,
     /// Maximum transaction size.
-    pub max_tx_size: u32,
+    pub max_tx_size: Option<u32>,
     /// Truncated median fee in the block.
-    pub median_fee: Amount,
+    pub median_fee: Option<Amount>,
     /// The block median time past.
-    pub median_time: u32,
+    pub median_time: Option<u32>,
     /// Truncated median transaction size
-    pub median_tx_size: u32,
+    pub median_tx_size: Option<u32>,
     /// Minimum fee in the block.
-    pub minimum_fee: Amount,
+    pub minimum_fee: Option<Amount>,
     /// Minimum feerate (in satoshis per virtual byte).
     pub minimum_fee_rate: Option<FeeRate>,
     /// Minimum transaction size.
-    pub minimum_tx_size: u32,
+    pub minimum_tx_size: Option<u32>,
     /// The number of outputs.
-    pub outputs: u32,
+    pub outputs: Option<u32>,
     /// The block subsidy.
-    pub subsidy: Amount,
+    pub subsidy: Option<Amount>,
     /// Total size of all segwit transactions.
-    pub segwit_total_size: u32,
+    pub segwit_total_size: Option<u32>,
     /// Total weight of all segwit transactions divided by segwit scale factor (4).
     pub segwit_total_weight: Option<Weight>,
     /// The number of segwit transactions.
-    pub segwit_txs: u32,
+    pub segwit_txs: Option<u32>,
     /// The block time.
-    pub time: u32,
+    pub time: Option<u32>,
     /// Total amount in all outputs (excluding coinbase and thus reward [ie subsidy + totalfee]).
-    pub total_out: Amount,
+    pub total_out: Option<Amount>,
     /// Total size of all non-coinbase transactions.
-    pub total_size: u32,
+    pub total_size: Option<u32>,
     /// Total weight of all non-coinbase transactions divided by segwit scale factor (4).
     pub total_weight: Option<Weight>,
     /// The fee total.
-    pub total_fee: Amount,
+    pub total_fee: Option<Amount>,
     /// The number of transactions (excluding coinbase).
-    pub txs: u32,
+    pub txs: Option<u32>,
     /// The increase/decrease in the number of unspent outputs.
-    pub utxo_increase: i32,
+    pub utxo_increase: Option<i32>,
     /// The increase/decrease in size for the utxo index (not discounting op_return and similar).
-    pub utxo_size_increase: i32,
+    pub utxo_size_increase: Option<i32>,
     /// The increase/decrease in the number of unspent outputs, not counting unspendables.
     /// v25 and later only.
     pub utxo_increase_actual: Option<i32>,

types/src/v17/blockchain/into.rs

@@ -222,48 +222,59 @@ impl GetBlockStats {
         use GetBlockStatsError as E;
 
         // `FeeRate::sat_per_vb` returns an option if value overflows.
-        let average_fee_rate = FeeRate::from_sat_per_vb(self.average_fee_rate);
-        let block_hash = self.block_hash.parse::<BlockHash>().map_err(E::BlockHash)?;
+        let average_fee_rate = self.average_fee_rate.and_then(FeeRate::from_sat_per_vb);
+        let block_hash =
+            self.block_hash.map(|h| h.parse::<BlockHash>()).transpose().map_err(E::BlockHash)?;
         let fee_rate_percentiles = self
             .fee_rate_percentiles
-            .iter()
-            .map(|vb| FeeRate::from_sat_per_vb(*vb))
-            .collect::<Vec<Option<FeeRate>>>();
-        let max_fee_rate = FeeRate::from_sat_per_vb(self.max_fee_rate);
-        let minimum_fee_rate = FeeRate::from_sat_per_vb(self.minimum_fee_rate);
+            .map(|arr| arr.iter().map(|vb| FeeRate::from_sat_per_vb(*vb)).collect());
+        let max_fee_rate = self.max_fee_rate.and_then(FeeRate::from_sat_per_vb);
+        let minimum_fee_rate = self.minimum_fee_rate.and_then(FeeRate::from_sat_per_vb);
 
         // FIXME: Double check that these values are virtual bytes and not weight units.
-        let segwit_total_weight = Weight::from_vb(self.segwit_total_weight);
-        let total_weight = Weight::from_vb(self.total_weight);
+        let segwit_total_weight = self.segwit_total_weight.and_then(Weight::from_vb);
+        let total_weight = self.total_weight.and_then(Weight::from_vb);
 
         Ok(model::GetBlockStats {
-            average_fee: Amount::from_sat(self.average_fee),
+            average_fee: self.average_fee.map(Amount::from_sat),
             average_fee_rate,
-            average_tx_size: crate::to_u32(self.average_tx_size, "average_tx_size")?,
+            average_tx_size: self
+                .average_tx_size
+                .map(|v| crate::to_u32(v, "average_tx_size"))
+                .transpose()?,
             block_hash,
             fee_rate_percentiles,
-            height: crate::to_u32(self.height, "height")?,
-            inputs: crate::to_u32(self.inputs, "inputs")?,
-            max_fee: Amount::from_sat(self.max_fee),
+            height: self.height.map(|v| crate::to_u32(v, "height")).transpose()?,
+            inputs: self.inputs.map(|v| crate::to_u32(v, "inputs")).transpose()?,
+            max_fee: self.max_fee.map(Amount::from_sat),
             max_fee_rate,
-            max_tx_size: crate::to_u32(self.max_tx_size, "max_tx_size")?,
-            median_fee: Amount::from_sat(self.median_fee),
-            median_time: crate::to_u32(self.median_time, "median_time")?,
-            median_tx_size: crate::to_u32(self.median_tx_size, "median_tx_size")?,
-            minimum_fee: Amount::from_sat(self.minimum_fee),
+            max_tx_size: self.max_tx_size.map(|v| crate::to_u32(v, "max_tx_size")).transpose()?,
+            median_fee: self.median_fee.map(Amount::from_sat),
+            median_time: self.median_time.map(|v| crate::to_u32(v, "median_time")).transpose()?,
+            median_tx_size: self
+                .median_tx_size
+                .map(|v| crate::to_u32(v, "median_tx_size"))
+                .transpose()?,
+            minimum_fee: self.minimum_fee.map(Amount::from_sat),
             minimum_fee_rate,
-            minimum_tx_size: crate::to_u32(self.minimum_tx_size, "minimum_tx_size")?,
-            outputs: crate::to_u32(self.outputs, "outputs")?,
-            subsidy: Amount::from_sat(self.subsidy),
-            segwit_total_size: crate::to_u32(self.segwit_total_size, "segwit_total_size")?,
+            minimum_tx_size: self
+                .minimum_tx_size
+                .map(|v| crate::to_u32(v, "minimum_tx_size"))
+                .transpose()?,
+            outputs: self.outputs.map(|v| crate::to_u32(v, "outputs")).transpose()?,
+            subsidy: self.subsidy.map(Amount::from_sat),
+            segwit_total_size: self
+                .segwit_total_size
+                .map(|v| crate::to_u32(v, "segwit_total_size"))
+                .transpose()?,
             segwit_total_weight,
-            segwit_txs: crate::to_u32(self.segwit_txs, "segwit_txs")?,
-            time: crate::to_u32(self.time, "time")?,
-            total_out: Amount::from_sat(self.total_out),
-            total_size: crate::to_u32(self.total_size, "total_size")?,
+            segwit_txs: self.segwit_txs.map(|v| crate::to_u32(v, "segwit_txs")).transpose()?,
+            time: self.time.map(|v| crate::to_u32(v, "time")).transpose()?,
+            total_out: self.total_out.map(Amount::from_sat),
+            total_size: self.total_size.map(|v| crate::to_u32(v, "total_size")).transpose()?,
             total_weight,
-            total_fee: Amount::from_sat(self.total_fee),
-            txs: crate::to_u32(self.txs, "txs")?,
+            total_fee: self.total_fee.map(Amount::from_sat),
+            txs: self.txs.map(|v| crate::to_u32(v, "txs")).transpose()?,
             utxo_increase: self.utxo_increase,
             utxo_size_increase: self.utxo_size_increase,
             utxo_increase_actual: None,      // v25 and later only.