Skip to content

Commit

Permalink
fix(rpc): Fix some RPC response formats to match zcashd (#4217)
Browse files Browse the repository at this point in the history
* Match `zcashd`'s version format in `getinfo`

* Remove an incorrect array wrapper in getaddressbalance

* Use a stable sort order in getrawmempool

Because we can't match zcashd's arbitrary order,
and probably shouldn't try to.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
teor2345 and mergify[bot] authored Apr 28, 2022
1 parent 83d2689 commit 7506655
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 14 deletions.
32 changes: 22 additions & 10 deletions zebra-rpc/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use tower::{buffer::Buffer, Service, ServiceExt};
use tracing::Instrument;

use zebra_chain::{
amount::{Amount, NonNegative},
block::{self, Height, SerializedBlock},
chain_tip::ChainTip,
parameters::{ConsensusBranchId, Network, NetworkUpgrade},
Expand Down Expand Up @@ -273,8 +272,15 @@ where
{
let runner = Queue::start();

let mut app_version = app_version.to_string();

// Match zcashd's version format, if the version string has anything in it
if !app_version.is_empty() && !app_version.starts_with('v') {
app_version.insert(0, 'v');
}

let rpc_impl = RpcImpl {
app_version: app_version.to_string(),
app_version,
mempool: mempool.clone(),
state: state.clone(),
latest_chain_tip: latest_chain_tip.clone(),
Expand Down Expand Up @@ -431,9 +437,9 @@ where
})?;

match response {
zebra_state::ReadResponse::AddressBalance(balance) => {
Ok(AddressBalance { balance })
}
zebra_state::ReadResponse::AddressBalance(balance) => Ok(AddressBalance {
balance: u64::from(balance),
}),
_ => unreachable!("Unexpected response from state service: {response:?}"),
}
}
Expand Down Expand Up @@ -556,10 +562,16 @@ where

match response {
mempool::Response::TransactionIds(unmined_transaction_ids) => {
Ok(unmined_transaction_ids
let mut tx_ids: Vec<String> = unmined_transaction_ids
.iter()
.map(|id| id.mined_id().encode_hex())
.collect())
.collect();

// Sort returned transaction IDs in numeric/string order.
// (zcashd's sort order appears arbitrary.)
tx_ids.sort();

Ok(tx_ids)
}
_ => unreachable!("unmatched response to a transactionids request"),
}
Expand Down Expand Up @@ -730,7 +742,7 @@ where
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 satoshis = i64::from(utxo_data.3.value);
let satoshis = u64::from(utxo_data.3.value);

let entry = GetAddressUtxos {
address,
Expand Down Expand Up @@ -809,7 +821,7 @@ impl AddressStrings {
/// The transparent balance of a set of addresses.
#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, serde::Serialize)]
pub struct AddressBalance {
balance: Amount<NonNegative>,
balance: u64,
}

/// A hex-encoded [`ConsensusBranchId`] string.
Expand Down Expand Up @@ -899,7 +911,7 @@ pub struct GetAddressUtxos {
#[serde(rename = "outputIndex")]
output_index: usize,
script: String,
satoshis: i64,
satoshis: u64,
}

impl GetRawTransaction {
Expand Down
5 changes: 3 additions & 2 deletions zebra-rpc/src/methods/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ proptest! {
);

let call_task = tokio::spawn(rpc.get_raw_mempool());
let expected_response: Vec<String> = transaction_ids
let mut expected_response: Vec<String> = transaction_ids
.iter()
.map(|id| id.mined_id().encode_hex())
.collect();
expected_response.sort();

mempool
.expect_request(mempool::Request::TransactionIds)
Expand Down Expand Up @@ -632,7 +633,7 @@ proptest! {
// Check that response contains the expected balance
let received_balance = response?;

prop_assert_eq!(received_balance, AddressBalance { balance });
prop_assert_eq!(received_balance, AddressBalance { balance: balance.into() });

// Check no further requests were made during this test
mempool.expect_no_requests().await?;
Expand Down
4 changes: 2 additions & 2 deletions zebra-rpc/src/methods/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ async fn rpc_getinfo() {
let get_info = rpc.get_info().expect("We should have a GetInfo struct");

// make sure there is a `build` field in the response,
// and that is equal to the provided string.
assert_eq!(get_info.build, "RPC test");
// and that is equal to the provided string, with an added 'v' version prefix.
assert_eq!(get_info.build, "vRPC test");

// make sure there is a `subversion` field,
// and that is equal to the Zebra user agent.
Expand Down

0 comments on commit 7506655

Please sign in to comment.