From 543f0669e15a7d7da4e43d9d009fbfe9583bcec0 Mon Sep 17 00:00:00 2001 From: Conrado Gouvea Date: Sat, 14 Dec 2024 09:28:57 -0300 Subject: [PATCH] add(rpc): getblock: return transaction details with verbosity=2 (#9083) * getblock: return tx objects with verbosity=2 * fix test * use FuturesOrdered * Suggestion for "add(rpc): getblock: return transaction details with verbosity=2" (#9084) * Replaces multiple service calls (per transaction) with a single call to the state service for all of a block's transactions. * adjustments to reuse code from getrawtransaction --------- Co-authored-by: Conrado Gouvea * update snapshot --------- Co-authored-by: Arya --- zebra-rpc/src/methods.rs | 141 ++++++++++++------ ...k_verbose_hash_verbosity_2@mainnet_10.snap | 6 +- ...k_verbose_hash_verbosity_2@testnet_10.snap | 6 +- ...verbose_height_verbosity_2@mainnet_10.snap | 6 +- ...verbose_height_verbosity_2@testnet_10.snap | 6 +- zebra-rpc/src/methods/tests/vectors.rs | 24 ++- zebra-rpc/src/tests/vectors.rs | 10 +- 7 files changed, 139 insertions(+), 60 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 0d7986f033f..cfd8260d2ba 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -6,7 +6,7 @@ //! Some parts of the `zcashd` RPC documentation are outdated. //! So this implementation follows the `zcashd` server and `lightwalletd` client implementations. -use std::{collections::HashSet, fmt::Debug}; +use std::{collections::HashSet, fmt::Debug, sync::Arc}; use chrono::Utc; use futures::{stream::FuturesOrdered, FutureExt, StreamExt, TryFutureExt}; @@ -814,6 +814,12 @@ where next_block_hash, } = *block_header; + let transactions_request = match verbosity { + 1 => zebra_state::ReadRequest::TransactionIdsForBlock(hash_or_height), + 2 => zebra_state::ReadRequest::Block(hash_or_height), + _other => panic!("get_block_header_fut should be none"), + }; + // # Concurrency // // We look up by block hash so the hash, transaction IDs, and confirmations @@ -827,7 +833,7 @@ where // A block's transaction IDs are never modified, so all possible responses are // valid. Clients that query block heights must be able to handle chain forks, // including getting transaction IDs from any chain fork. - zebra_state::ReadRequest::TransactionIdsForBlock(hash_or_height), + transactions_request, // Orchard trees zebra_state::ReadRequest::OrchardTree(hash_or_height), ]; @@ -839,11 +845,27 @@ where } let tx_ids_response = futs.next().await.expect("`futs` should not be empty"); - let tx = match tx_ids_response.map_misc_error()? { + let tx: Vec<_> = match tx_ids_response.map_misc_error()? { zebra_state::ReadResponse::TransactionIdsForBlock(tx_ids) => tx_ids .ok_or_misc_error("block not found")? .iter() - .map(|tx_id| tx_id.encode_hex()) + .map(|tx_id| GetBlockTransaction::Hash(*tx_id)) + .collect(), + zebra_state::ReadResponse::Block(block) => block + .ok_or_misc_error("Block not found")? + .transactions + .iter() + .map(|tx| { + GetBlockTransaction::Object(TransactionObject::from_transaction( + tx.clone(), + Some(height), + Some( + confirmations + .try_into() + .expect("should be less than max block height, i32::MAX"), + ), + )) + }) .collect(), _ => unreachable!("unmatched response to a transaction_ids_for_block request"), }; @@ -1131,15 +1153,14 @@ where { mempool::Response::Transactions(txns) => { if let Some(tx) = txns.first() { - let hex = tx.transaction.clone().into(); - return Ok(if verbose { - GetRawTransaction::Object { - hex, - height: None, - confirmations: None, - } + GetRawTransaction::Object(TransactionObject::from_transaction( + tx.transaction.clone(), + None, + None, + )) } else { + let hex = tx.transaction.clone().into(); GetRawTransaction::Raw(hex) }); } @@ -1155,19 +1176,16 @@ where .await .map_misc_error()? { - zebra_state::ReadResponse::Transaction(Some(tx)) => { + zebra_state::ReadResponse::Transaction(Some(tx)) => Ok(if verbose { + GetRawTransaction::Object(TransactionObject::from_transaction( + tx.tx.clone(), + Some(tx.height), + Some(tx.confirmations), + )) + } else { let hex = tx.tx.into(); - - Ok(if verbose { - GetRawTransaction::Object { - hex, - height: Some(tx.height.0), - confirmations: Some(tx.confirmations), - } - } else { - GetRawTransaction::Raw(hex) - }) - } + GetRawTransaction::Raw(hex) + }), zebra_state::ReadResponse::Transaction(None) => { Err("No such mempool or main chain transaction") @@ -1779,11 +1797,9 @@ pub enum GetBlock { // `chainhistoryroot` would be here. Undocumented. TODO: decide if we want to support it // - /// List of transaction IDs in block order, hex-encoded. - // - // TODO: use a typed Vec here - // TODO: support Objects - tx: Vec, + /// List of transactions in block order, hex-encoded if verbosity=1 or + /// as objects if verbosity=2. + tx: Vec, /// The height of the requested block. #[serde(skip_serializing_if = "Option::is_none")] @@ -1811,7 +1827,7 @@ pub enum GetBlock { difficulty: Option, // `chainwork` would be here, but we don't plan on supporting it - // `anchor` would be here. Undocumented. TODO: decide if we want to support it + // `anchor` would be here. Not planned to be supported. // `chainSupply` would be here, TODO: implement // `valuePools` would be here, TODO: implement // @@ -1852,6 +1868,17 @@ impl Default for GetBlock { } } +#[derive(Clone, Debug, PartialEq, serde::Serialize)] +#[serde(untagged)] +/// The transaction list in a `getblock` call. Can be a list of transaction +/// IDs or the full transaction details depending on verbosity. +pub enum GetBlockTransaction { + /// The transaction hash, hex-encoded. + Hash(#[serde(with = "hex")] transaction::Hash), + /// The block object. + Object(TransactionObject), +} + /// Response to a `getblockheader` RPC request. /// /// See the notes for the [`Rpc::get_block_header`] method. @@ -1995,24 +2022,36 @@ pub enum GetRawTransaction { /// The raw transaction, encoded as hex bytes. Raw(#[serde(with = "hex")] SerializedTransaction), /// The transaction object. - Object { - /// The raw transaction, encoded as hex bytes. - #[serde(with = "hex")] - hex: SerializedTransaction, - /// The height of the block in the best chain that contains the tx or `None` if the tx is in - /// the mempool. - #[serde(skip_serializing_if = "Option::is_none")] - height: Option, - /// The height diff between the block containing the tx and the best chain tip + 1 or `None` - /// if the tx is in the mempool. - #[serde(skip_serializing_if = "Option::is_none")] - confirmations: Option, - }, + Object(TransactionObject), } impl Default for GetRawTransaction { fn default() -> Self { - Self::Object { + Self::Object(TransactionObject::default()) + } +} + +/// A Transaction object as returned by `getrawtransaction` and `getblock` RPC +/// requests. +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] +pub struct TransactionObject { + /// The raw transaction, encoded as hex bytes. + #[serde(with = "hex")] + pub hex: SerializedTransaction, + /// The height of the block in the best chain that contains the tx or `None` if the tx is in + /// the mempool. + #[serde(skip_serializing_if = "Option::is_none")] + pub height: Option, + /// The height diff between the block containing the tx and the best chain tip + 1 or `None` + /// if the tx is in the mempool. + #[serde(skip_serializing_if = "Option::is_none")] + pub confirmations: Option, + // TODO: many fields not yet supported +} + +impl Default for TransactionObject { + fn default() -> Self { + Self { hex: SerializedTransaction::from( [0u8; zebra_chain::transaction::MIN_TRANSPARENT_TX_SIZE as usize].to_vec(), ), @@ -2022,6 +2061,22 @@ impl Default for GetRawTransaction { } } +impl TransactionObject { + /// Converts `tx` and `height` into a new `GetRawTransaction` in the `verbose` format. + #[allow(clippy::unwrap_in_result)] + fn from_transaction( + tx: Arc, + height: Option, + confirmations: Option, + ) -> Self { + Self { + hex: tx.into(), + height: height.map(|height| height.0), + confirmations, + } + } +} + /// Response to a `getaddressutxos` RPC request. /// /// See the notes for the [`Rpc::get_address_utxos` method]. diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@mainnet_10.snap index 93010ad42d4..51729b13293 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@mainnet_10.snap @@ -10,7 +10,11 @@ expression: block "merkleroot": "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609", "finalsaplingroot": "0000000000000000000000000000000000000000000000000000000000000000", "tx": [ - "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" + { + "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000", + "height": 1, + "confirmations": 10 + } ], "time": 1477671596, "nonce": "9057977ea6d4ae867decc96359fcf2db8cdebcbfb3bd549de4f21f16cfe83475", diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@testnet_10.snap index 5bd22590f1b..51bbfc72f05 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_2@testnet_10.snap @@ -10,7 +10,11 @@ expression: block "merkleroot": "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75", "finalsaplingroot": "0000000000000000000000000000000000000000000000000000000000000000", "tx": [ - "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" + { + "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000", + "height": 1, + "confirmations": 10 + } ], "time": 1477674473, "nonce": "0000e5739438a096ca89cde16bcf6001e0c5a7ce6f7c591d26314c26c2560000", diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@mainnet_10.snap index 93010ad42d4..51729b13293 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@mainnet_10.snap @@ -10,7 +10,11 @@ expression: block "merkleroot": "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609", "finalsaplingroot": "0000000000000000000000000000000000000000000000000000000000000000", "tx": [ - "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" + { + "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000", + "height": 1, + "confirmations": 10 + } ], "time": 1477671596, "nonce": "9057977ea6d4ae867decc96359fcf2db8cdebcbfb3bd549de4f21f16cfe83475", diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@testnet_10.snap index 5bd22590f1b..51bbfc72f05 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_2@testnet_10.snap @@ -10,7 +10,11 @@ expression: block "merkleroot": "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75", "finalsaplingroot": "0000000000000000000000000000000000000000000000000000000000000000", "tx": [ - "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" + { + "hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000", + "height": 1, + "confirmations": 10 + } ], "time": 1477674473, "nonce": "0000e5739438a096ca89cde16bcf6001e0c5a7ce6f7c591d26314c26c2560000", diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 998cdc155ee..1ecde97c634 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -178,7 +178,7 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Hash(tx.hash())) .collect(), trees, size: None, @@ -221,7 +221,7 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Hash(tx.hash())) .collect(), trees, size: None, @@ -264,7 +264,11 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Object(TransactionObject { + hex: (*tx).clone().into(), + height: Some(i.try_into().expect("valid u32")), + confirmations: Some((blocks.len() - i).try_into().expect("valid i64")) + })) .collect(), trees, size: None, @@ -307,7 +311,11 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Object(TransactionObject { + hex: (*tx).clone().into(), + height: Some(i.try_into().expect("valid u32")), + confirmations: Some((blocks.len() - i).try_into().expect("valid i64")) + })) .collect(), trees, size: None, @@ -350,7 +358,7 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Hash(tx.hash())) .collect(), trees, size: None, @@ -393,7 +401,7 @@ async fn rpc_getblock() { tx: block .transactions .iter() - .map(|tx| tx.hash().encode_hex()) + .map(|tx| GetBlockTransaction::Hash(tx.hash())) .collect(), trees, size: None, @@ -774,11 +782,11 @@ async fn rpc_getrawtransaction() { let (response, _) = futures::join!(get_tx_verbose_1_req, make_mempool_req(txid)); - let GetRawTransaction::Object { + let GetRawTransaction::Object(TransactionObject { hex, height, confirmations, - } = response.expect("We should have a GetRawTransaction struct") + }) = response.expect("We should have a GetRawTransaction struct") else { unreachable!("Should return a Raw enum") }; diff --git a/zebra-rpc/src/tests/vectors.rs b/zebra-rpc/src/tests/vectors.rs index a93158b8b38..0ca221c2cf5 100644 --- a/zebra-rpc/src/tests/vectors.rs +++ b/zebra-rpc/src/tests/vectors.rs @@ -1,6 +1,6 @@ //! Fixed Zebra RPC serialization test vectors. -use crate::methods::{GetBlock, GetRawTransaction}; +use crate::methods::{GetBlock, GetRawTransaction, TransactionObject}; #[test] pub fn test_transaction_serialization() { @@ -8,22 +8,22 @@ pub fn test_transaction_serialization() { assert_eq!(serde_json::to_string(&tx).unwrap(), r#""42""#); - let tx = GetRawTransaction::Object { + let tx = GetRawTransaction::Object(TransactionObject { hex: vec![0x42].into(), height: Some(1), confirmations: Some(0), - }; + }); assert_eq!( serde_json::to_string(&tx).unwrap(), r#"{"hex":"42","height":1,"confirmations":0}"# ); - let tx = GetRawTransaction::Object { + let tx = GetRawTransaction::Object(TransactionObject { hex: vec![0x42].into(), height: None, confirmations: None, - }; + }); assert_eq!(serde_json::to_string(&tx).unwrap(), r#"{"hex":"42"}"#); }