From be4e065afbda756007faf4313b24847ee278a4b4 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 3 May 2022 14:10:21 +1000 Subject: [PATCH] change(rpc): Simplify RPC types and add documentation (#4218) * Simplify RPC types and add documentation * Derive serde traits in production code --- zebra-chain/Cargo.toml | 6 +- zebra-chain/src/transparent/address.rs | 10 +- zebra-chain/src/transparent/script.rs | 8 +- zebra-rpc/src/methods.rs | 100 ++++++++++++++---- zebra-rpc/src/methods/tests/prop.rs | 6 +- zebra-state/src/lib.rs | 2 +- zebra-state/src/service.rs | 2 +- zebra-state/src/service/finalized_state.rs | 2 +- .../service/finalized_state/disk_format.rs | 2 +- .../disk_format/transparent.rs | 7 +- 10 files changed, 97 insertions(+), 48 deletions(-) diff --git a/zebra-chain/Cargo.toml b/zebra-chain/Cargo.toml index d7cbb3b6e..f64b816f7 100644 --- a/zebra-chain/Cargo.toml +++ b/zebra-chain/Cargo.toml @@ -9,8 +9,7 @@ edition = "2021" [features] default = [] -proptest-impl = ["proptest", "proptest-derive", "zebra-test", "rand", "rand_chacha", "tokio", -"hex/serde", "serde_with"] +proptest-impl = ["proptest", "proptest-derive", "zebra-test", "rand", "rand_chacha", "tokio", "hex/serde"] bench = ["zebra-test"] [dependencies] @@ -39,7 +38,7 @@ rand_core = "0.6.3" ripemd = "0.1.1" serde = { version = "1.0.136", features = ["serde_derive", "rc"] } -serde_with = { version = "1.13.0", optional = true } +serde_with = "1.13.0" serde-big-array = "0.4.1" # Matches version used by hdwallet secp256k1 = { version = "0.21.3", features = ["serde"] } @@ -79,7 +78,6 @@ spandoc = "0.2.2" tracing = "0.1.31" hex = { version = "0.4.3", features = ["serde"] } -serde_with = "1.13.0" proptest = "0.10.1" proptest-derive = "0.3.0" diff --git a/zebra-chain/src/transparent/address.rs b/zebra-chain/src/transparent/address.rs index 5951264a2..5fac57563 100644 --- a/zebra-chain/src/transparent/address.rs +++ b/zebra-chain/src/transparent/address.rs @@ -41,14 +41,12 @@ mod magics { /// to a Bitcoin address just by removing the "t".) /// /// https://zips.z.cash/protocol/protocol.pdf#transparentaddrencoding -#[derive(Copy, Clone, Eq, PartialEq, Hash)] +#[derive( + Copy, Clone, Eq, PartialEq, Hash, serde_with::SerializeDisplay, serde_with::DeserializeFromStr, +)] #[cfg_attr( any(test, feature = "proptest-impl"), - derive( - proptest_derive::Arbitrary, - serde_with::SerializeDisplay, - serde_with::DeserializeFromStr - ) + derive(proptest_derive::Arbitrary) )] pub enum Address { /// P2SH (Pay to Script Hash) addresses diff --git a/zebra-chain/src/transparent/script.rs b/zebra-chain/src/transparent/script.rs index b6623496a..24cd9e8a4 100644 --- a/zebra-chain/src/transparent/script.rs +++ b/zebra-chain/src/transparent/script.rs @@ -9,17 +9,17 @@ use crate::serialization::{ }; /// An encoding of a Bitcoin script. -#[derive(Clone, Eq, PartialEq, Hash)] +#[derive(Clone, Eq, PartialEq, Hash, Serialize, Deserialize)] #[cfg_attr( any(test, feature = "proptest-impl"), - derive(proptest_derive::Arbitrary, Serialize, Deserialize) + derive(proptest_derive::Arbitrary) )] pub struct Script( /// # Correctness /// /// Consensus-critical serialization uses [`ZcashSerialize`]. - /// [`serde`]-based hex serialization must only be used for testing. - #[cfg_attr(any(test, feature = "proptest-impl"), serde(with = "hex"))] + /// [`serde`]-based hex serialization must only be used for RPCs and testing. + #[serde(with = "hex")] Vec, ); diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 087bbabb4..218756539 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -24,10 +24,11 @@ use zebra_chain::{ parameters::{ConsensusBranchId, Network, NetworkUpgrade}, serialization::{SerializationError, ZcashDeserialize}, transaction::{self, SerializedTransaction, Transaction, UnminedTx}, - transparent::Address, + transparent::{self, Address}, }; use zebra_network::constants::USER_AGENT; use zebra_node_services::{mempool, BoxError}; +use zebra_state::OutputIndex; use crate::queue::Queue; @@ -410,9 +411,9 @@ where let response = GetBlockChainInfo { chain, - blocks: tip_height.0, - best_block_hash: GetBestBlockHash(tip_hash), - estimated_height: estimated_height.0, + blocks: tip_height, + best_block_hash: tip_hash, + estimated_height, upgrades, consensus, }; @@ -737,20 +738,20 @@ where }; for utxo_data in utxos.utxos() { - let address = utxo_data.0.to_string(); - let txid = utxo_data.1.to_string(); - let height = utxo_data.2.height().0; - let output_index = utxo_data.2.output_index().as_usize(); - let script = utxo_data.3.lock_script.to_string(); + let address = utxo_data.0; + let txid = *utxo_data.1; + let height = utxo_data.2.height(); + let output_index = utxo_data.2.output_index(); + let script = utxo_data.3.lock_script.clone(); let satoshis = u64::from(utxo_data.3.value); let entry = GetAddressUtxos { address, txid, - height, output_index, script, satoshis, + height, }; response_utxos.push(entry); } @@ -766,7 +767,10 @@ where /// See the notes for the [`Rpc::get_info` method]. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] pub struct GetInfo { + /// The node version build number build: String, + + /// The server sub-version identifier, used as the network protocol user-agent subversion: String, } @@ -775,31 +779,46 @@ pub struct GetInfo { /// See the notes for the [`Rpc::get_blockchain_info` method]. #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] pub struct GetBlockChainInfo { + /// Current network name as defined in BIP70 (main, test, regtest) chain: String, - blocks: u32, - #[serde(rename = "bestblockhash")] - best_block_hash: GetBestBlockHash, + + /// The current number of blocks processed in the server, numeric + blocks: Height, + + /// The hash of the currently best block, in big-endian order, hex-encoded + #[serde(rename = "bestblockhash", with = "hex")] + best_block_hash: block::Hash, + + /// If syncing, the estimated height of the chain, else the current best height, numeric. + /// + /// In Zebra, this is always the height estimate, so it might be a little inaccurate. #[serde(rename = "estimatedheight")] - estimated_height: u32, + estimated_height: Height, + + /// Status of network upgrades upgrades: IndexMap, + + /// Branch IDs of the current and upcoming consensus rules consensus: TipConsensusBranch, } -/// A wrapper type with a list of strings of addresses. +/// A wrapper type with a list of transparent address strings. /// /// This is used for the input parameter of [`Rpc::get_address_balance`], /// [`Rpc::get_address_tx_ids`] and [`Rpc::get_address_utxos`]. #[derive(Clone, Debug, Eq, PartialEq, Hash, serde::Deserialize)] pub struct AddressStrings { + /// A list of transparent address strings. addresses: Vec, } impl AddressStrings { - // Creates a new `AddressStrings` given a vector. + /// Creates a new `AddressStrings` given a vector. #[cfg(test)] pub fn new(addresses: Vec) -> AddressStrings { AddressStrings { addresses } } + /// Given a list of addresses as strings: /// - check if provided list have all valid transparent addresses. /// - return valid addresses as a set of `Address`. @@ -821,6 +840,7 @@ impl AddressStrings { /// The transparent balance of a set of addresses. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, serde::Serialize)] pub struct AddressBalance { + /// The total transparent balance. balance: u64, } @@ -831,19 +851,34 @@ struct ConsensusBranchIdHex(#[serde(with = "hex")] ConsensusBranchId); /// Information about [`NetworkUpgrade`] activation. #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] struct NetworkUpgradeInfo { + /// Name of upgrade, string. + /// + /// Ignored by lightwalletd, but useful for debugging. name: NetworkUpgrade, + + /// Block height of activation, numeric. #[serde(rename = "activationheight")] activation_height: Height, + + /// Status of upgrade, string. status: NetworkUpgradeStatus, } /// The activation status of a [`NetworkUpgrade`]. #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] enum NetworkUpgradeStatus { + /// The network upgrade is currently active. + /// + /// Includes all network upgrades that have previously activated, + /// even if they are not the most recent network upgrade. #[serde(rename = "active")] Active, + + /// The network upgrade does not have an activation height. #[serde(rename = "disabled")] Disabled, + + /// The network upgrade has an activation height, but we haven't reached it yet. #[serde(rename = "pending")] Pending, } @@ -853,8 +888,11 @@ enum NetworkUpgradeStatus { /// These branch IDs are different when the next block is a network upgrade activation block. #[derive(Copy, Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] struct TipConsensusBranch { + /// Branch ID used to validate the current chain tip, big-endian, hex-encoded. #[serde(rename = "chaintip")] chain_tip: ConsensusBranchIdHex, + + /// Branch ID used to validate the next block, big-endian, hex-encoded. #[serde(rename = "nextblock")] next_block: ConsensusBranchIdHex, } @@ -903,18 +941,34 @@ pub enum GetRawTransaction { /// Response to a `getaddressutxos` RPC request. /// /// See the notes for the [`Rpc::get_address_utxos` method]. -#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize)] pub struct GetAddressUtxos { - address: String, - txid: String, - height: u32, + /// The transparent address, base58check encoded + address: transparent::Address, + + /// The output txid, in big-endian order, hex-encoded + #[serde(with = "hex")] + txid: transaction::Hash, + + /// The transparent output index, numeric #[serde(rename = "outputIndex")] - output_index: usize, - script: String, + output_index: OutputIndex, + + /// The transparent output script, hex encoded + #[serde(with = "hex")] + script: transparent::Script, + + /// The amount of zatoshis in the transparent output satoshis: u64, + + /// The block height, numeric. + /// + /// We put this field last, to match the zcashd order. + height: Height, } impl GetRawTransaction { + /// Converts `tx` and `height` into a new `GetRawTransaction` in the `verbose` format. fn from_transaction( tx: Arc, height: Option, @@ -937,7 +991,7 @@ impl GetRawTransaction { } } -/// Check if provided height range is valid +/// Check if provided height range is valid for address indexes. fn check_height_range(start: Height, end: Height, chain_height: Height) -> Result<()> { if start == Height(0) || end == Height(0) { return Err(Error::invalid_params( diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs index 03bfc023f..266836461 100644 --- a/zebra-rpc/src/methods/tests/prop.rs +++ b/zebra-rpc/src/methods/tests/prop.rs @@ -536,9 +536,9 @@ proptest! { match response { Ok(info) => { prop_assert_eq!(info.chain, network.bip70_network_name()); - prop_assert_eq!(info.blocks, block_height.0); - prop_assert_eq!(info.best_block_hash.0, block_hash); - prop_assert!(info.estimated_height < Height::MAX.0); + prop_assert_eq!(info.blocks, block_height); + prop_assert_eq!(info.best_block_hash, block_hash); + prop_assert!(info.estimated_height < Height::MAX); prop_assert_eq!( info.consensus.chain_tip.0, diff --git a/zebra-state/src/lib.rs b/zebra-state/src/lib.rs index 7f53aec89..3e8601bd9 100644 --- a/zebra-state/src/lib.rs +++ b/zebra-state/src/lib.rs @@ -35,7 +35,7 @@ pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, ReadRequest, Requ pub use response::{ReadResponse, Response}; pub use service::{ chain_tip::{ChainTipChange, LatestChainTip, TipAction}, - init, OutputLocation, TransactionLocation, + init, OutputIndex, OutputLocation, TransactionLocation, }; #[cfg(any(test, feature = "proptest-impl"))] diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index bd3b045cb..059cb6f78 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -66,7 +66,7 @@ pub mod arbitrary; #[cfg(test)] mod tests; -pub use finalized_state::{OutputLocation, TransactionLocation}; +pub use finalized_state::{OutputIndex, OutputLocation, TransactionLocation}; pub type QueuedBlock = ( PreparedBlock, diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 37ed0cfcf..3e1045f13 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -37,7 +37,7 @@ mod arbitrary; #[cfg(test)] mod tests; -pub use disk_format::{OutputLocation, TransactionLocation}; +pub use disk_format::{OutputIndex, OutputLocation, TransactionLocation}; pub(super) use zebra_db::ZebraDb; diff --git a/zebra-state/src/service/finalized_state/disk_format.rs b/zebra-state/src/service/finalized_state/disk_format.rs index 858506dbc..b7261e008 100644 --- a/zebra-state/src/service/finalized_state/disk_format.rs +++ b/zebra-state/src/service/finalized_state/disk_format.rs @@ -16,7 +16,7 @@ pub mod transparent; mod tests; pub use block::{TransactionIndex, TransactionLocation}; -pub use transparent::OutputLocation; +pub use transparent::{OutputIndex, OutputLocation}; /// Helper type for writing types to disk as raw bytes. /// Also used to convert key types to raw bytes for disk lookups. diff --git a/zebra-state/src/service/finalized_state/disk_format/transparent.rs b/zebra-state/src/service/finalized_state/disk_format/transparent.rs index d75a28038..e695c7a8e 100644 --- a/zebra-state/src/service/finalized_state/disk_format/transparent.rs +++ b/zebra-state/src/service/finalized_state/disk_format/transparent.rs @@ -7,6 +7,8 @@ use std::{cmp::max, fmt::Debug}; +use serde::{Deserialize, Serialize}; + use zebra_chain::{ amount::{self, Amount, NonNegative}, block::Height, @@ -22,8 +24,6 @@ use crate::service::finalized_state::disk_format::{ #[cfg(any(test, feature = "proptest-impl"))] use proptest_derive::Arbitrary; -#[cfg(any(test, feature = "proptest-impl"))] -use serde::{Deserialize, Serialize}; #[cfg(any(test, feature = "proptest-impl"))] mod arbitrary; @@ -46,8 +46,7 @@ pub const OUTPUT_LOCATION_DISK_BYTES: usize = // Transparent types /// A transparent output's index in its transaction. -#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] -#[cfg_attr(any(test, feature = "proptest-impl"), derive(Serialize, Deserialize))] +#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)] pub struct OutputIndex(u32); impl OutputIndex {