From 08c7cfb59fe8ddaaa6653e547f2db0a0d865093d Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 3 Feb 2023 11:40:23 +1000 Subject: [PATCH 1/7] clippy: remove unnecessary return statement --- zebra-rpc/src/methods/get_block_template_rpcs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs index 004eb21c510..d07ea70a605 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs @@ -825,12 +825,12 @@ where return Ok(validate_address::Response::invalid()); } - return Ok(if address.network() == network { - validate_address::Response { + if address.network() == network { + Ok(validate_address::Response { address: Some(raw_address), is_valid: true, is_script: Some(address.is_script_hash()), - } + }) } else { tracing::info!( ?network, @@ -838,8 +838,8 @@ where "invalid address in validateaddress RPC: Zebra's configured network must match address network" ); - validate_address::Response::invalid() - }); + Ok(validate_address::Response::invalid()) + } } .boxed() } From 928990d5e741c47c608082b47db80667fd88d467 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 3 Feb 2023 12:23:42 +1000 Subject: [PATCH 2/7] Add hash, height, and confirmations fields to getblock RPC --- zebra-rpc/src/methods.rs | 100 +++++++++++++++++++++--- zebra-rpc/src/methods/tests/snapshot.rs | 55 +++++++++++-- zebra-rpc/src/methods/tests/vectors.rs | 78 +++++++++++++++++- zebra-state/src/request.rs | 18 +++++ 4 files changed, 226 insertions(+), 25 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 94f8c721581..db8df3db759 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -117,7 +117,7 @@ pub trait Rpc { raw_transaction_hex: String, ) -> BoxFuture>; - /// Returns the requested block by height, as a [`GetBlock`] JSON string. + /// Returns the requested block by hash or height, as a [`GetBlock`] JSON string. /// If the block is not in Zebra's state, returns /// [error code `-8`.](https://github.com/zcash/zcash/issues/5758) /// @@ -125,7 +125,7 @@ pub trait Rpc { /// /// # Parameters /// - /// - `height`: (string, required) The height number for the block to be returned. + /// - `hash|height`: (string, required) The hash or height for the block to be returned. /// - `verbosity`: (numeric, optional, default=1) 0 for hex encoded data, 1 for a json object, /// and 2 for json object with transaction data. /// @@ -133,13 +133,16 @@ pub trait Rpc { /// /// With verbosity=1, [`lightwalletd` only reads the `tx` field of the /// result](https://github.com/zcash/lightwalletd/blob/dfac02093d85fb31fb9a8475b884dd6abca966c7/common/common.go#L152), - /// so we only return that for now. + /// and other clients only read the `hash` and `confirmations` fields, + /// so we only return a few fields for now. /// - /// `lightwalletd` only requests blocks by height, so we don't support - /// getting blocks by hash. (But we parse the height as a JSON string, not an integer). - /// `lightwalletd` also does not use verbosity=2, so we don't support it. + /// `lightwalletd` and mining clients also do not use verbosity=2, so we don't support it. #[rpc(name = "getblock")] - fn get_block(&self, height: String, verbosity: Option) -> BoxFuture>; + fn get_block( + &self, + hash_or_height: String, + verbosity: Option, + ) -> BoxFuture>; /// Returns the hash of the current best blockchain tip block, as a [`GetBlockHash`] JSON string. /// @@ -601,6 +604,20 @@ where _ => unreachable!("unmatched response to a block request"), } } else if verbosity == 1 { + // TODO: look up the hash if we only have a height, + // and look up the height if we only have a hash + let hash = hash_or_height.hash().map(GetBlockHash); + let height = hash_or_height.height(); + + // TODO: do these state queries in parallel? + + // Get transaction IDs from the transaction index + // + // # Concurrency + // + // 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. let request = zebra_state::ReadRequest::TransactionIdsForBlock(hash_or_height); let response = state .ready() @@ -611,19 +628,60 @@ where message: error.to_string(), data: None, })?; - - match response { + let tx = match response { zebra_state::ReadResponse::TransactionIdsForBlock(Some(tx_ids)) => { - let tx_ids = tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect(); - Ok(GetBlock::Object { tx: tx_ids }) + tx_ids.iter().map(|tx_id| tx_id.encode_hex()).collect() } zebra_state::ReadResponse::TransactionIdsForBlock(None) => Err(Error { code: MISSING_BLOCK_ERROR_CODE, message: "Block not found".to_string(), data: None, - }), + })?, _ => unreachable!("unmatched response to a transaction_ids_for_block request"), - } + }; + + // Get block confirmations from the block height index + // + // # Concurrency + // + // All possible responses are valid, even if a block is added to the chain, or + // the best chain changes. Clients must be able to handle chain forks, including + // different confirmation values before or after added blocks, and switching + // between -1 and multiple different confirmation values. + + // From + const NOT_IN_BEST_CHAIN_CONFIRMATIONS: i64 = -1; + + let confirmations = if let Some(hash) = hash_or_height.hash() { + let request = zebra_state::ReadRequest::Depth(hash); + let response = state + .ready() + .and_then(|service| service.call(request)) + .await + .map_err(|error| Error { + code: ErrorCode::ServerError(0), + message: error.to_string(), + data: None, + })?; + + match response { + zebra_state::ReadResponse::Depth(Some(depth)) => Some(depth.into()), + zebra_state::ReadResponse::Depth(None) => { + Some(NOT_IN_BEST_CHAIN_CONFIRMATIONS) + } + _ => unreachable!("unmatched response to a depth request"), + } + } else { + // TODO: make Depth support heights as well + None + }; + + Ok(GetBlock::Object { + hash, + confirmations, + height, + tx, + }) } else { Err(Error { code: ErrorCode::InvalidParams, @@ -1221,7 +1279,22 @@ pub enum GetBlock { Raw(#[serde(with = "hex")] SerializedBlock), /// The block object. Object { + /// The hash of the requested block. + #[serde(skip_serializing_if = "Option::is_none")] + hash: Option, + + /// The number of confirmations of this block in the best chain, + /// or -1 if it is not in the best chain. + #[serde(skip_serializing_if = "Option::is_none")] + confirmations: Option, + + /// The height of the requested block. + #[serde(skip_serializing_if = "Option::is_none")] + height: Option, + /// List of transaction IDs in block order, hex-encoded. + // + // TODO: use a typed Vec here tx: Vec, }, } @@ -1232,6 +1305,7 @@ pub enum GetBlock { /// /// Also see the notes for the [`Rpc::get_best_block_hash`] and `get_block_hash` methods. #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +#[serde(transparent)] pub struct GetBlockHash(#[serde(with = "hex")] pub block::Hash); /// Response to a `z_gettreestate` RPC request. diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index 6b1b26f523c..474d026eb57 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -103,27 +103,61 @@ async fn test_rpc_response_data_for_network(network: Network) { .expect("We should have an AddressBalance struct"); snapshot_rpc_getaddressbalance(get_address_balance, &settings); - // `getblock`, verbosity=0 + // `getblock` variants const BLOCK_HEIGHT: u32 = 1; + let block_hash = blocks[BLOCK_HEIGHT as usize].hash(); + + // `getblock`, verbosity=0, height let get_block = rpc .get_block(BLOCK_HEIGHT.to_string(), Some(0u8)) .await .expect("We should have a GetBlock struct"); - snapshot_rpc_getblock(get_block, block_data.get(&BLOCK_HEIGHT).unwrap(), &settings); + snapshot_rpc_getblock_data( + "height_verbosity_0", + get_block, + block_data.get(&BLOCK_HEIGHT).unwrap(), + &settings, + ); + + // `getblock`, verbosity=0, hash + let get_block = rpc + .get_block(block_hash.to_string(), Some(0u8)) + .await + .expect("We should have a GetBlock struct"); + snapshot_rpc_getblock_data( + "hash_verbosity_0", + get_block, + block_data.get(&BLOCK_HEIGHT).unwrap(), + &settings, + ); - // `getblock`, verbosity=1 + // `getblock`, verbosity=1, height let get_block = rpc .get_block(BLOCK_HEIGHT.to_string(), Some(1u8)) .await .expect("We should have a GetBlock struct"); - snapshot_rpc_getblock_verbose("2_args", get_block, &settings); + snapshot_rpc_getblock_verbose("height_verbosity_1", get_block, &settings); - // `getblock`, no verbosity, defaults to 1 + // `getblock`, verbosity=1, hash + let get_block = rpc + .get_block(block_hash.to_string(), Some(1u8)) + .await + .expect("We should have a GetBlock struct"); + snapshot_rpc_getblock_verbose("hash_verbosity_1", get_block, &settings); + + // `getblock`, no verbosity - defaults to 1, height let get_block = rpc .get_block(BLOCK_HEIGHT.to_string(), None) .await .expect("We should have a GetBlock struct"); - snapshot_rpc_getblock_verbose("1_arg", get_block, &settings); + snapshot_rpc_getblock_verbose("height_verbosity_default", get_block, &settings); + + // `getblock`, no verbosity - defaults to 1, hash + let get_block = rpc + .get_block(block_hash.to_string(), None) + .await + .expect("We should have a GetBlock struct"); + snapshot_rpc_getblock_verbose("hash_verbosity_default", get_block, &settings); // `getbestblockhash` let get_best_block_hash = rpc @@ -242,11 +276,16 @@ fn snapshot_rpc_getaddressbalance(address_balance: AddressBalance, settings: &in /// Check `getblock` response, using `cargo insta`, JSON serialization, and block test vectors. /// /// The snapshot file does not contain any data, but it does enforce the response format. -fn snapshot_rpc_getblock(block: GetBlock, block_data: &[u8], settings: &insta::Settings) { +fn snapshot_rpc_getblock_data( + variant: &'static str, + block: GetBlock, + block_data: &[u8], + settings: &insta::Settings, +) { let block_data = hex::encode(block_data); settings.bind(|| { - insta::assert_json_snapshot!("get_block", block, { + insta::assert_json_snapshot!(format!("get_block_data_{variant}"), block, { "." => dynamic_redaction(move |value, _path| { // assert that the block data matches, without creating a 1.5 kB snapshot file assert_eq!(value.as_str().unwrap(), block_data); diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 7317aaf0230..4f3f45277c2 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -80,7 +80,7 @@ async fn rpc_getblock() { latest_chain_tip, ); - // Make calls with verbosity=0 and check response + // Make height calls with verbosity=0 and check response for (i, block) in blocks.iter().enumerate() { let expected_result = GetBlock::Raw(block.clone().into()); @@ -99,7 +99,26 @@ async fn rpc_getblock() { assert_eq!(get_block, expected_result); } - // Make calls with verbosity=1 and check response + // Make hash calls with verbosity=0 and check response + for (i, block) in blocks.iter().enumerate() { + let expected_result = GetBlock::Raw(block.clone().into()); + + let get_block = rpc + .get_block(blocks[i].hash().to_string(), Some(0u8)) + .await + .expect("We should have a GetBlock struct"); + + assert_eq!(get_block, expected_result); + + let get_block = rpc + .get_block(block.hash().to_string(), Some(0u8)) + .await + .expect("We should have a GetBlock struct"); + + assert_eq!(get_block, expected_result); + } + + // Make height calls with verbosity=1 and check response for (i, block) in blocks.iter().enumerate() { let get_block = rpc .get_block(i.to_string(), Some(1u8)) @@ -109,15 +128,41 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { + hash: None, + confirmations: None, + height: Some(Height(i.try_into().expect("valid u32"))), tx: block .transactions .iter() .map(|tx| tx.hash().encode_hex()) - .collect() + .collect(), } ); } + // Make hash calls with verbosity=1 and check response + for (i, block) in blocks.iter().enumerate() { + let get_block = rpc + .get_block(blocks[i].hash().to_string(), Some(1u8)) + .await + .expect("We should have a GetBlock struct"); + + assert_eq!( + get_block, + GetBlock::Object { + hash: Some(GetBlockHash(block.hash())), + confirmations: Some((blocks.len() - i - 1).try_into().expect("valid i64")), + height: None, + tx: block + .transactions + .iter() + .map(|tx| tx.hash().encode_hex()) + .collect(), + } + ); + } + + // Make height calls with no verbosity (defaults to 1) and check response for (i, block) in blocks.iter().enumerate() { let get_block = rpc .get_block(i.to_string(), None) @@ -127,11 +172,36 @@ async fn rpc_getblock() { assert_eq!( get_block, GetBlock::Object { + hash: None, + confirmations: None, + height: Some(Height(i.try_into().expect("valid u32"))), + tx: block + .transactions + .iter() + .map(|tx| tx.hash().encode_hex()) + .collect(), + } + ); + } + + // Make hash calls with no verbosity (defaults to 1) and check response + for (i, block) in blocks.iter().enumerate() { + let get_block = rpc + .get_block(blocks[i].hash().to_string(), None) + .await + .expect("We should have a GetBlock struct"); + + assert_eq!( + get_block, + GetBlock::Object { + hash: Some(GetBlockHash(block.hash())), + confirmations: Some((blocks.len() - i - 1).try_into().expect("valid i64")), + height: None, tx: block .transactions .iter() .map(|tx| tx.hash().encode_hex()) - .collect() + .collect(), } ); } diff --git a/zebra-state/src/request.rs b/zebra-state/src/request.rs index 1cdfcff878b..239257e37ea 100644 --- a/zebra-state/src/request.rs +++ b/zebra-state/src/request.rs @@ -52,6 +52,24 @@ impl HashOrHeight { HashOrHeight::Height(height) => Some(height), } } + + /// Returns the hash if this is a [`HashOrHeight::Hash`]. + pub fn hash(&self) -> Option { + if let HashOrHeight::Hash(hash) = self { + Some(*hash) + } else { + None + } + } + + /// Returns the height if this is a [`HashOrHeight::Height`]. + pub fn height(&self) -> Option { + if let HashOrHeight::Height(height) = self { + Some(*height) + } else { + None + } + } } impl From for HashOrHeight { From 43ab02e275a4295eef03136d50aa6c867a069b53 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 3 Feb 2023 12:24:25 +1000 Subject: [PATCH 3/7] Remove a test that is already checked by snapshots --- zebra-rpc/src/tests/vectors.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/zebra-rpc/src/tests/vectors.rs b/zebra-rpc/src/tests/vectors.rs index 45eb4e2d67b..1eb677f91d2 100644 --- a/zebra-rpc/src/tests/vectors.rs +++ b/zebra-rpc/src/tests/vectors.rs @@ -27,12 +27,4 @@ pub fn test_block_serialization() { let j = serde_json::to_string(&expected_tx).unwrap(); assert_eq!(j, expected_json); - - let expected_tx = GetBlock::Object { - tx: vec!["42".into()], - }; - let expected_json = r#"{"tx":["42"]}"#; - let j = serde_json::to_string(&expected_tx).unwrap(); - - assert_eq!(j, expected_json); } From 142394fd3b1471f294db8c390377b86eabd2663e Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 3 Feb 2023 12:24:45 +1000 Subject: [PATCH 4/7] Document the performance requirements of the getblock RPC --- zebra-rpc/src/methods.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index db8df3db759..d061bb3a6f7 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -581,6 +581,10 @@ where })?; if verbosity == 0 { + // # Peformance + // + // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks, + // so it needs to load block data very efficiently. let request = zebra_state::ReadRequest::Block(hash_or_height); let response = state .ready() @@ -604,6 +608,14 @@ where _ => unreachable!("unmatched response to a block request"), } } else if verbosity == 1 { + // # Peformance + // + // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks, + // so it needs to load all its fields very efficiently. + // + // Currently, we get the transaction IDs from an index, which is much more + // efficient than loading all the block data and hashing all the transactions. + // TODO: look up the hash if we only have a height, // and look up the height if we only have a hash let hash = hash_or_height.hash().map(GetBlockHash); From 11e50f89701b39f54e45fb107a4bb8712cf6e034 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 3 Feb 2023 12:26:50 +1000 Subject: [PATCH 5/7] Update snapshots, use new naming scheme --- ...> get_block_data_hash_verbosity_0@mainnet_10.snap} | 0 ...> get_block_data_hash_verbosity_0@testnet_10.snap} | 0 .../get_block_data_height_verbosity_0@mainnet_10.snap | 5 +++++ .../get_block_data_height_verbosity_0@testnet_10.snap | 5 +++++ ...get_block_verbose_hash_verbosity_1@mainnet_10.snap | 11 +++++++++++ ...get_block_verbose_hash_verbosity_1@testnet_10.snap | 11 +++++++++++ ...ock_verbose_hash_verbosity_default@mainnet_10.snap | 11 +++++++++++ ...ock_verbose_hash_verbosity_default@testnet_10.snap | 11 +++++++++++ ..._block_verbose_height_verbosity_1@mainnet_10.snap} | 1 + ..._block_verbose_height_verbosity_1@testnet_10.snap} | 1 + ..._verbose_height_verbosity_default@mainnet_10.snap} | 1 + ..._verbose_height_verbosity_default@testnet_10.snap} | 1 + 12 files changed, 58 insertions(+) rename zebra-rpc/src/methods/tests/snapshots/{get_block@mainnet_10.snap => get_block_data_hash_verbosity_0@mainnet_10.snap} (100%) rename zebra-rpc/src/methods/tests/snapshots/{get_block@testnet_10.snap => get_block_data_hash_verbosity_0@testnet_10.snap} (100%) create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@mainnet_10.snap create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@testnet_10.snap create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap create mode 100644 zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap rename zebra-rpc/src/methods/tests/snapshots/{get_block_verbose_1_arg@mainnet_10.snap => get_block_verbose_height_verbosity_1@mainnet_10.snap} (91%) rename zebra-rpc/src/methods/tests/snapshots/{get_block_verbose_1_arg@testnet_10.snap => get_block_verbose_height_verbosity_1@testnet_10.snap} (91%) rename zebra-rpc/src/methods/tests/snapshots/{get_block_verbose_2_args@mainnet_10.snap => get_block_verbose_height_verbosity_default@mainnet_10.snap} (91%) rename zebra-rpc/src/methods/tests/snapshots/{get_block_verbose_2_args@testnet_10.snap => get_block_verbose_height_verbosity_default@testnet_10.snap} (91%) diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_data_hash_verbosity_0@mainnet_10.snap similarity index 100% rename from zebra-rpc/src/methods/tests/snapshots/get_block@mainnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_data_hash_verbosity_0@mainnet_10.snap diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_data_hash_verbosity_0@testnet_10.snap similarity index 100% rename from zebra-rpc/src/methods/tests/snapshots/get_block@testnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_data_hash_verbosity_0@testnet_10.snap diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@mainnet_10.snap new file mode 100644 index 00000000000..347844edddb --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@mainnet_10.snap @@ -0,0 +1,5 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +"[BlockData]" diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@testnet_10.snap new file mode 100644 index 00000000000..347844edddb --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_data_height_verbosity_0@testnet_10.snap @@ -0,0 +1,5 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +"[BlockData]" diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap new file mode 100644 index 00000000000..bc9d6b14e8a --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap @@ -0,0 +1,11 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +{ + "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", + "confirmations": 9, + "tx": [ + "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" + ] +} diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap new file mode 100644 index 00000000000..2285ae4c46f --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap @@ -0,0 +1,11 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +{ + "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", + "confirmations": 9, + "tx": [ + "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" + ] +} diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap new file mode 100644 index 00000000000..bc9d6b14e8a --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap @@ -0,0 +1,11 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +{ + "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", + "confirmations": 9, + "tx": [ + "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" + ] +} diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap new file mode 100644 index 00000000000..2285ae4c46f --- /dev/null +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap @@ -0,0 +1,11 @@ +--- +source: zebra-rpc/src/methods/tests/snapshot.rs +expression: block +--- +{ + "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", + "confirmations": 9, + "tx": [ + "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" + ] +} diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap similarity index 91% rename from zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@mainnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap index b9fc9794fa8..241484dfd5f 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@mainnet_10.snap @@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "height": 1, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap similarity index 91% rename from zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@testnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap index dbc2ce43d5e..4e586be83b1 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_1_arg@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_1@testnet_10.snap @@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "height": 1, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap similarity index 91% rename from zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@mainnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap index b9fc9794fa8..241484dfd5f 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@mainnet_10.snap @@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "height": 1, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap similarity index 91% rename from zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@testnet_10.snap rename to zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap index dbc2ce43d5e..4e586be83b1 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_2_args@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_height_verbosity_default@testnet_10.snap @@ -3,6 +3,7 @@ source: zebra-rpc/src/methods/tests/snapshot.rs expression: block --- { + "height": 1, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" ] From 42baa54a4c48d1961d61af401a24f382a5ed3487 Mon Sep 17 00:00:00 2001 From: teor Date: Mon, 6 Feb 2023 08:03:44 +1000 Subject: [PATCH 6/7] Fix off-by-one error in confirmations --- zebra-rpc/src/methods.rs | 4 +++- .../get_block_verbose_hash_verbosity_1@mainnet_10.snap | 2 +- .../get_block_verbose_hash_verbosity_1@testnet_10.snap | 2 +- .../get_block_verbose_hash_verbosity_default@mainnet_10.snap | 2 +- .../get_block_verbose_hash_verbosity_default@testnet_10.snap | 2 +- zebra-rpc/src/methods/tests/vectors.rs | 4 ++-- 6 files changed, 9 insertions(+), 7 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index d061bb3a6f7..9f736d56088 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -677,7 +677,9 @@ where })?; match response { - zebra_state::ReadResponse::Depth(Some(depth)) => Some(depth.into()), + // Confirmations are one more than the depth. + // Depth is limited by height, so it will never overflow an i64. + zebra_state::ReadResponse::Depth(Some(depth)) => Some(i64::from(depth) + 1), zebra_state::ReadResponse::Depth(None) => { Some(NOT_IN_BEST_CHAIN_CONFIRMATIONS) } diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap index bc9d6b14e8a..d4d6b540a83 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@mainnet_10.snap @@ -4,7 +4,7 @@ expression: block --- { "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", - "confirmations": 9, + "confirmations": 10, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap index 2285ae4c46f..393f918ebef 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_1@testnet_10.snap @@ -4,7 +4,7 @@ expression: block --- { "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", - "confirmations": 9, + "confirmations": 10, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap index bc9d6b14e8a..d4d6b540a83 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@mainnet_10.snap @@ -4,7 +4,7 @@ expression: block --- { "hash": "0007bc227e1c57a4a70e237cad00e7b7ce565155ab49166bc57397a26d339283", - "confirmations": 9, + "confirmations": 10, "tx": [ "851bf6fbf7a976327817c738c489d7fa657752445430922d94c983c0b9ed4609" ] diff --git a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap index 2285ae4c46f..393f918ebef 100644 --- a/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap +++ b/zebra-rpc/src/methods/tests/snapshots/get_block_verbose_hash_verbosity_default@testnet_10.snap @@ -4,7 +4,7 @@ expression: block --- { "hash": "025579869bcf52a989337342f5f57a84f3a28b968f7d6a8307902b065a668d23", - "confirmations": 9, + "confirmations": 10, "tx": [ "f37e9f691fffb635de0999491d906ee85ba40cd36dae9f6e5911a8277d7c5f75" ] diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs index 4f3f45277c2..4e08242ab5b 100644 --- a/zebra-rpc/src/methods/tests/vectors.rs +++ b/zebra-rpc/src/methods/tests/vectors.rs @@ -151,7 +151,7 @@ async fn rpc_getblock() { get_block, GetBlock::Object { hash: Some(GetBlockHash(block.hash())), - confirmations: Some((blocks.len() - i - 1).try_into().expect("valid i64")), + confirmations: Some((blocks.len() - i).try_into().expect("valid i64")), height: None, tx: block .transactions @@ -195,7 +195,7 @@ async fn rpc_getblock() { get_block, GetBlock::Object { hash: Some(GetBlockHash(block.hash())), - confirmations: Some((blocks.len() - i - 1).try_into().expect("valid i64")), + confirmations: Some((blocks.len() - i).try_into().expect("valid i64")), height: None, tx: block .transactions From d184566be597906ef33698f535ee7ae7ea63244f Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 7 Feb 2023 08:19:42 +1000 Subject: [PATCH 7/7] Fix spelling mistakes --- zebra-rpc/src/methods.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 9f736d56088..6087f68a948 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -581,7 +581,7 @@ where })?; if verbosity == 0 { - // # Peformance + // # Performance // // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks, // so it needs to load block data very efficiently. @@ -608,7 +608,7 @@ where _ => unreachable!("unmatched response to a block request"), } } else if verbosity == 1 { - // # Peformance + // # Performance // // This RPC is used in `lightwalletd`'s initial sync of 2 million blocks, // so it needs to load all its fields very efficiently.