change(rpc): Add confirmations to getrawtransaction method response (#6287)

* adds confirmation field to getrawtransaction

* Updates tests

* Adds comment about correctness

* Apply suggestion revisions from review

* fixes overflow bug and adds test for valid confirmations value

* Update test to check that confirmations isn't too high

* Update transaction request to return confirmations

* Applies suggestions from PR review

* Apply suggestions from code review

Co-authored-by: teor <teor@riseup.net>

* fixes test

* restore derives that were lost in a bad merge

---------

Co-authored-by: teor <teor@riseup.net>
This commit is contained in:
Arya 2023-03-26 19:53:44 -04:00 committed by GitHub
parent 8cf62b4a17
commit 8ba3fd921a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 237 additions and 67 deletions

View File

@ -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, io, sync::Arc};
use std::{collections::HashSet, sync::Arc};
use chrono::Utc;
use futures::{FutureExt, TryFutureExt};
@ -30,7 +30,7 @@ use zebra_chain::{
};
use zebra_network::constants::USER_AGENT;
use zebra_node_services::mempool;
use zebra_state::{HashOrHeight, OutputIndex, OutputLocation, TransactionLocation};
use zebra_state::{HashOrHeight, MinedTx, OutputIndex, OutputLocation, TransactionLocation};
use crate::{constants::MISSING_BLOCK_ERROR_CODE, queue::Queue};
@ -846,6 +846,7 @@ where
) -> BoxFuture<Result<GetRawTransaction>> {
let mut state = self.state.clone();
let mut mempool = self.mempool.clone();
let verbose = verbose != 0;
async move {
let txid = transaction::Hash::from_hex(txid_hex).map_err(|_| {
@ -878,12 +879,7 @@ where
mempool::Response::Transactions(unmined_transactions) => {
if !unmined_transactions.is_empty() {
let tx = unmined_transactions[0].transaction.clone();
return GetRawTransaction::from_transaction(tx, None, verbose != 0)
.map_err(|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
});
return Ok(GetRawTransaction::from_transaction(tx, None, 0, verbose));
}
}
_ => unreachable!("unmatched response to a transactionids request"),
@ -902,15 +898,16 @@ where
})?;
match response {
zebra_state::ReadResponse::Transaction(Some((tx, height))) => Ok(
GetRawTransaction::from_transaction(tx, Some(height), verbose != 0).map_err(
|error| Error {
code: ErrorCode::ServerError(0),
message: error.to_string(),
data: None,
},
)?,
),
zebra_state::ReadResponse::Transaction(Some(MinedTx {
tx,
height,
confirmations,
})) => Ok(GetRawTransaction::from_transaction(
tx,
Some(height),
confirmations,
verbose,
)),
zebra_state::ReadResponse::Transaction(None) => Err(Error {
code: ErrorCode::ServerError(0),
message: "Transaction not found".to_string(),
@ -1436,9 +1433,12 @@ pub enum GetRawTransaction {
/// The raw transaction, encoded as hex bytes.
#[serde(with = "hex")]
hex: SerializedTransaction,
/// The height of the block that contains the transaction, or -1 if
/// not applicable.
/// The height of the block in the best chain that contains the transaction, or -1 if
/// the transaction is in the mempool.
height: i32,
/// The confirmations of the block in the best chain that contains the transaction,
/// or 0 if the transaction is in the mempool.
confirmations: u32,
},
}
@ -1490,10 +1490,11 @@ impl GetRawTransaction {
fn from_transaction(
tx: Arc<Transaction>,
height: Option<block::Height>,
confirmations: u32,
verbose: bool,
) -> std::result::Result<Self, io::Error> {
) -> Self {
if verbose {
Ok(GetRawTransaction::Object {
GetRawTransaction::Object {
hex: tx.into(),
height: match height {
Some(height) => height
@ -1502,9 +1503,10 @@ impl GetRawTransaction {
.expect("valid block heights are limited to i32::MAX"),
None => -1,
},
})
confirmations,
}
} else {
Ok(GetRawTransaction::Raw(tx.into()))
GetRawTransaction::Raw(tx.into())
}
}
}

View File

@ -202,7 +202,7 @@ async fn test_rpc_response_data_for_network(network: Network) {
.expect("We should have a GetTreestate struct");
snapshot_rpc_z_gettreestate(tree_state, &settings);
// `getrawtransaction`
// `getrawtransaction` verbosity=0
//
// - similar to `getrawmempool` described above, a mempool request will be made to get the requested
// transaction from the mempool, response will be empty as we have this transaction in state
@ -220,7 +220,24 @@ async fn test_rpc_response_data_for_network(network: Network) {
let (response, _) = futures::join!(get_raw_transaction, mempool_req);
let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
snapshot_rpc_getrawtransaction(get_raw_transaction, &settings);
snapshot_rpc_getrawtransaction("verbosity_0", get_raw_transaction, &settings);
// `getrawtransaction` verbosity=1
let mempool_req = mempool
.expect_request_that(|request| {
matches!(request, mempool::Request::TransactionsByMinedId(_))
})
.map(|responder| {
responder.respond(mempool::Response::Transactions(vec![]));
});
// make the api call
let get_raw_transaction =
rpc.get_raw_transaction(first_block_first_transaction.hash().encode_hex(), 1u8);
let (response, _) = futures::join!(get_raw_transaction, mempool_req);
let get_raw_transaction = response.expect("We should have a GetRawTransaction struct");
snapshot_rpc_getrawtransaction("verbosity_1", get_raw_transaction, &settings);
// `getaddresstxids`
let get_address_tx_ids = rpc
@ -322,8 +339,14 @@ fn snapshot_rpc_z_gettreestate(tree_state: GetTreestate, settings: &insta::Setti
}
/// Snapshot `getrawtransaction` response, using `cargo insta` and JSON serialization.
fn snapshot_rpc_getrawtransaction(raw_transaction: GetRawTransaction, settings: &insta::Settings) {
settings.bind(|| insta::assert_json_snapshot!("get_raw_transaction", raw_transaction));
fn snapshot_rpc_getrawtransaction(
variant: &'static str,
raw_transaction: GetRawTransaction,
settings: &insta::Settings,
) {
settings.bind(|| {
insta::assert_json_snapshot!(format!("get_raw_transaction_{variant}"), raw_transaction)
});
}
/// Snapshot `getaddressbalance` response, using `cargo insta` and JSON serialization.

View File

@ -1,6 +1,5 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
assertion_line: 220
expression: raw_transaction
---
"01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000"

View File

@ -1,6 +1,5 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
assertion_line: 220
expression: raw_transaction
---
"01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000"

View File

@ -0,0 +1,9 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: raw_transaction
---
{
"hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff025100ffffffff0250c30000000000002321027a46eb513588b01b37ea24303f4b628afd12cc20df789fede0921e43cad3e875acd43000000000000017a9147d46a730d31f97b1930d3368a967c309bd4d136a8700000000",
"height": 1,
"confirmations": 10
}

View File

@ -0,0 +1,9 @@
---
source: zebra-rpc/src/methods/tests/snapshot.rs
expression: raw_transaction
---
{
"hex": "01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff03510101ffffffff0250c30000000000002321025229e1240a21004cf8338db05679fa34753706e84f6aebba086ba04317fd8f99acd43000000000000017a914ef775f1f997f122a062fff1a2d7443abd1f9c6428700000000",
"height": 1,
"confirmations": 10
}

View File

@ -8,7 +8,7 @@ use tower::buffer::Buffer;
use zebra_chain::{
amount::Amount,
block::Block,
chain_tip::NoChainTip,
chain_tip::{mock::MockChainTip, NoChainTip},
parameters::Network::*,
serialization::{ZcashDeserializeInto, ZcashSerialize},
transaction::{UnminedTx, UnminedTxId},
@ -371,9 +371,12 @@ async fn rpc_getrawtransaction() {
let mut mempool: MockService<_, _, _, BoxError> = MockService::build().for_unit_tests();
// Create a populated state service
let (_state, read_state, latest_chain_tip, _chain_tip_change) =
let (_state, read_state, _latest_chain_tip, _chain_tip_change) =
zebra_state::populated_state(blocks.clone(), Mainnet).await;
let (latest_chain_tip, latest_chain_tip_sender) = MockChainTip::new();
latest_chain_tip_sender.send_best_tip_height(Height(10));
// Init RPC
let (rpc, rpc_tx_queue_task_handle) = RpcImpl::new(
"RPC test",
@ -381,7 +384,7 @@ async fn rpc_getrawtransaction() {
false,
true,
Buffer::new(mempool.clone(), 1),
read_state,
read_state.clone(),
latest_chain_tip,
);
@ -416,29 +419,102 @@ async fn rpc_getrawtransaction() {
}
}
// Test case where transaction is _not_ in mempool.
// Skip genesis because its tx is not indexed.
for block in blocks.iter().skip(1) {
for tx in block.transactions.iter() {
let mempool_req = mempool
let make_mempool_req = |tx_hash: transaction::Hash| {
let mut mempool = mempool.clone();
async move {
mempool
.expect_request_that(|request| {
if let mempool::Request::TransactionsByMinedId(ids) = request {
ids.len() == 1 && ids.contains(&tx.hash())
ids.len() == 1 && ids.contains(&tx_hash)
} else {
false
}
})
.map(|responder| {
responder.respond(mempool::Response::Transactions(vec![]));
});
let get_tx_req = rpc.get_raw_transaction(tx.hash().encode_hex(), 0u8);
let (response, _) = futures::join!(get_tx_req, mempool_req);
.await
.respond(mempool::Response::Transactions(vec![]));
}
};
let run_state_test_case = |block_idx: usize, block: Arc<Block>, tx: Arc<Transaction>| {
let read_state = read_state.clone();
let tx_hash = tx.hash();
let get_tx_verbose_0_req = rpc.get_raw_transaction(tx_hash.encode_hex(), 0u8);
let get_tx_verbose_1_req = rpc.get_raw_transaction(tx_hash.encode_hex(), 1u8);
async move {
let (response, _) = futures::join!(get_tx_verbose_0_req, make_mempool_req(tx_hash));
let get_tx = response.expect("We should have a GetRawTransaction struct");
if let GetRawTransaction::Raw(raw_tx) = get_tx {
assert_eq!(raw_tx.as_ref(), tx.zcash_serialize_to_vec().unwrap());
} else {
unreachable!("Should return a Raw enum")
}
let (response, _) = futures::join!(get_tx_verbose_1_req, make_mempool_req(tx_hash));
let GetRawTransaction::Object { hex, height, confirmations } = response.expect("We should have a GetRawTransaction struct") else {
unreachable!("Should return a Raw enum")
};
assert_eq!(hex.as_ref(), tx.zcash_serialize_to_vec().unwrap());
assert_eq!(height, block_idx as i32);
let depth_response = read_state
.oneshot(zebra_state::ReadRequest::Depth(block.hash()))
.await
.expect("state request should succeed");
let zebra_state::ReadResponse::Depth(depth) = depth_response else {
panic!("unexpected response to Depth request");
};
let expected_confirmations = 1 + depth.expect("depth should be Some");
(confirmations, expected_confirmations)
}
};
// Test case where transaction is _not_ in mempool.
// Skip genesis because its tx is not indexed.
for (block_idx, block) in blocks.iter().enumerate().skip(1) {
for tx in block.transactions.iter() {
let (confirmations, expected_confirmations) =
run_state_test_case(block_idx, block.clone(), tx.clone()).await;
assert_eq!(confirmations, expected_confirmations);
}
}
// Test case where transaction is _not_ in mempool with a fake chain tip height of 0
// Skip genesis because its tx is not indexed.
latest_chain_tip_sender.send_best_tip_height(Height(0));
for (block_idx, block) in blocks.iter().enumerate().skip(1) {
for tx in block.transactions.iter() {
let (confirmations, expected_confirmations) =
run_state_test_case(block_idx, block.clone(), tx.clone()).await;
let is_confirmations_within_bounds = confirmations <= expected_confirmations;
assert!(
is_confirmations_within_bounds,
"confirmations should be at or below depth + 1"
);
}
}
// Test case where transaction is _not_ in mempool with a fake chain tip height of 0
// Skip genesis because its tx is not indexed.
latest_chain_tip_sender.send_best_tip_height(Height(20));
for (block_idx, block) in blocks.iter().enumerate().skip(1) {
for tx in block.transactions.iter() {
let (confirmations, expected_confirmations) =
run_state_test_case(block_idx, block.clone(), tx.clone()).await;
let is_confirmations_within_bounds = confirmations <= expected_confirmations;
assert!(
is_confirmations_within_bounds,
"confirmations should be at or below depth + 1"
);
}
}
@ -1090,7 +1166,6 @@ async fn rpc_getblocktemplate_mining_address(use_p2pkh: bool) {
amount::NonNegative,
block::{Hash, MAX_BLOCK_BYTES, ZCASH_BLOCK_VERSION},
chain_sync_status::MockSyncStatus,
chain_tip::mock::MockChainTip,
serialization::DateTime32,
work::difficulty::{CompactDifficulty, ExpandedDifficulty, U256},
};

View File

@ -30,7 +30,7 @@ use zebra_node_services::{
BoxError,
};
use zebra_state::{ReadRequest, ReadResponse};
use zebra_state::{MinedTx, ReadRequest, ReadResponse};
#[cfg(test)]
mod tests;
@ -291,8 +291,8 @@ impl Runner {
// ignore any error coming from the state
let state_response = state.clone().oneshot(request).await;
if let Ok(ReadResponse::Transaction(Some(tx))) = state_response {
response.insert(tx.0.unmined_id());
if let Ok(ReadResponse::Transaction(Some(MinedTx { tx, .. }))) = state_response {
response.insert(tx.unmined_id());
}
}

View File

@ -292,7 +292,7 @@ proptest! {
let send_task = tokio::spawn(Runner::check_state(read_state.clone(), transactions_hash_set));
let expected_request = ReadRequest::Transaction(transaction.hash());
let response = ReadResponse::Transaction(Some((Arc::new(transaction), Height(1))));
let response = ReadResponse::Transaction(Some(zebra_state::MinedTx::new(Arc::new(transaction), Height(1), 1)));
read_state
.expect_request(expected_request)

View File

@ -13,8 +13,9 @@ pub fn test_transaction_serialization() {
let expected_tx = GetRawTransaction::Object {
hex: vec![0x42].into(),
height: 1,
confirmations: 0,
};
let expected_json = r#"{"hex":"42","height":1}"#;
let expected_json = r#"{"hex":"42","height":1,"confirmations":0}"#;
let j = serde_json::to_string(&expected_tx).unwrap();
assert_eq!(j, expected_json);

View File

@ -33,7 +33,7 @@ pub use config::{check_and_delete_old_databases, Config};
pub use constants::MAX_BLOCK_REORG_HEIGHT;
pub use error::{BoxError, CloneError, CommitBlockError, ValidateContextError};
pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, ReadRequest, Request};
pub use response::{KnownBlock, ReadResponse, Response};
pub use response::{KnownBlock, MinedTx, ReadResponse, Response};
pub use service::{
chain_tip::{ChainTipChange, LatestChainTip, TipAction},
init, spawn_init,

View File

@ -90,6 +90,31 @@ pub enum KnownBlock {
Queue,
}
/// Information about a transaction in the best chain
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct MinedTx {
/// The transaction.
pub tx: Arc<Transaction>,
/// The transaction height.
pub height: block::Height,
/// The number of confirmations for this transaction
/// (1 + depth of block the transaction was found in)
pub confirmations: u32,
}
impl MinedTx {
/// Creates a new [`MinedTx`]
pub fn new(tx: Arc<Transaction>, height: block::Height, confirmations: u32) -> Self {
Self {
tx,
height,
confirmations,
}
}
}
#[derive(Clone, Debug, PartialEq, Eq)]
/// A response to a read-only
/// [`ReadStateService`](crate::service::ReadStateService)'s
@ -105,7 +130,7 @@ pub enum ReadResponse {
Block(Option<Arc<Block>>),
/// Response to [`ReadRequest::Transaction`] with the specified transaction.
Transaction(Option<(Arc<Transaction>, block::Height)>),
Transaction(Option<MinedTx>),
/// Response to [`ReadRequest::TransactionIdsForBlock`],
/// with an list of transaction hashes in block order,
@ -227,8 +252,8 @@ impl TryFrom<ReadResponse> for Response {
ReadResponse::BlockHash(hash) => Ok(Response::BlockHash(hash)),
ReadResponse::Block(block) => Ok(Response::Block(block)),
ReadResponse::Transaction(tx_and_height) => {
Ok(Response::Transaction(tx_and_height.map(|(tx, _height)| tx)))
ReadResponse::Transaction(tx_info) => {
Ok(Response::Transaction(tx_info.map(|tx_info| tx_info.tx)))
}
ReadResponse::UnspentBestChainUtxo(utxo) => Ok(Response::UnspentBestChainUtxo(utxo)),

View File

@ -1274,16 +1274,13 @@ impl Service<ReadRequest> for ReadStateService {
tokio::task::spawn_blocking(move || {
span.in_scope(move || {
let transaction_and_height = state
.non_finalized_state_receiver
.with_watch_data(|non_finalized_state| {
read::transaction(non_finalized_state.best_chain(), &state.db, hash)
});
let response =
read::mined_transaction(state.latest_best_chain(), &state.db, hash);
// The work is done in the future.
timer.finish(module_path!(), line!(), "ReadRequest::Transaction");
Ok(ReadResponse::Transaction(transaction_and_height))
Ok(ReadResponse::Transaction(response))
})
})
.map(|join_result| join_result.expect("panic in ReadRequest::Transaction"))

View File

@ -31,7 +31,8 @@ pub use address::{
utxo::{address_utxos, AddressUtxos, ADDRESS_HEIGHTS_FULL_RANGE},
};
pub use block::{
any_utxo, block, block_header, transaction, transaction_hashes_for_block, unspent_utxo, utxo,
any_utxo, block, block_header, mined_transaction, transaction_hashes_for_block, unspent_utxo,
utxo,
};
pub use find::{
best_tip, block_locator, chain_contains_hash, depth, finalized_state_contains_block_hash,

View File

@ -21,9 +21,11 @@ use zebra_chain::{
};
use crate::{
response::MinedTx,
service::{
finalized_state::ZebraDb,
non_finalized_state::{Chain, NonFinalizedState},
read::tip_height,
},
HashOrHeight,
};
@ -70,7 +72,7 @@ where
/// Returns the [`Transaction`] with [`transaction::Hash`], if it exists in the
/// non-finalized `chain` or finalized `db`.
pub fn transaction<C>(
fn transaction<C>(
chain: Option<C>,
db: &ZebraDb,
hash: transaction::Hash,
@ -93,6 +95,28 @@ where
.or_else(|| db.transaction(hash))
}
/// Returns a [`MinedTx`] for a [`Transaction`] with [`transaction::Hash`],
/// if one exists in the non-finalized `chain` or finalized `db`.
pub fn mined_transaction<C>(
chain: Option<C>,
db: &ZebraDb,
hash: transaction::Hash,
) -> Option<MinedTx>
where
C: AsRef<Chain>,
{
// # Correctness
//
// It is ok to do this lookup in two different calls. Finalized state updates
// can only add overlapping blocks, and hashes are unique.
let chain = chain.as_ref();
let (tx, height) = transaction(chain, db, hash)?;
let confirmations = 1 + tip_height(chain, db)?.0 - height.0;
Some(MinedTx::new(tx, height, confirmations))
}
/// Returns the [`transaction::Hash`]es for the block with `hash_or_height`,
/// if it exists in the non-finalized `chain` or finalized `db`.
///

View File

@ -3,7 +3,10 @@
use std::sync::Arc;
use zebra_chain::{
block::Block, parameters::Network::*, serialization::ZcashDeserializeInto, transaction,
block::{Block, Height},
parameters::Network::*,
serialization::ZcashDeserializeInto,
transaction,
};
use zebra_test::{
@ -11,7 +14,7 @@ use zebra_test::{
transcript::{ExpectedTranscriptError, Transcript},
};
use crate::{init_test_services, populated_state, ReadRequest, ReadResponse};
use crate::{init_test_services, populated_state, response::MinedTx, ReadRequest, ReadResponse};
/// Test that ReadStateService responds correctly when empty.
#[tokio::test]
@ -42,6 +45,8 @@ async fn populated_read_state_responds_correctly() -> Result<()> {
let (_state, read_state, _latest_chain_tip, _chain_tip_change) =
populated_state(blocks.clone(), Mainnet).await;
let tip_height = Height(blocks.len() as u32 - 1);
let empty_cases = Transcript::from(empty_state_test_cases());
empty_cases.check(read_state.clone()).await?;
@ -68,10 +73,11 @@ async fn populated_read_state_responds_correctly() -> Result<()> {
for transaction in &block.transactions {
let transaction_cases = vec![(
ReadRequest::Transaction(transaction.hash()),
Ok(ReadResponse::Transaction(Some((
transaction.clone(),
block.coinbase_height().unwrap(),
)))),
Ok(ReadResponse::Transaction(Some(MinedTx {
tx: transaction.clone(),
height: block.coinbase_height().unwrap(),
confirmations: 1 + tip_height.0 - block.coinbase_height().unwrap().0,
}))),
)];
let transaction_cases = Transcript::from(transaction_cases);