From 7506655774f53bd8fc3b76f28efdda7a3255456a Mon Sep 17 00:00:00 2001
From: teor <teor@riseup.net>
Date: Thu, 28 Apr 2022 20:19:02 +1000
Subject: [PATCH] fix(rpc): Fix some RPC response formats to match `zcashd`
 (#4217)

* 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>
---
 zebra-rpc/src/methods.rs               | 32 ++++++++++++++++++--------
 zebra-rpc/src/methods/tests/prop.rs    |  5 ++--
 zebra-rpc/src/methods/tests/vectors.rs |  4 ++--
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs
index 83387ddfb9a..087bbabb497 100644
--- a/zebra-rpc/src/methods.rs
+++ b/zebra-rpc/src/methods.rs
@@ -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},
@@ -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(),
@@ -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:?}"),
             }
         }
@@ -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"),
             }
@@ -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,
@@ -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.
@@ -899,7 +911,7 @@ pub struct GetAddressUtxos {
     #[serde(rename = "outputIndex")]
     output_index: usize,
     script: String,
-    satoshis: i64,
+    satoshis: u64,
 }
 
 impl GetRawTransaction {
diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs
index 0146d8b4c3b..03bfc023fdd 100644
--- a/zebra-rpc/src/methods/tests/prop.rs
+++ b/zebra-rpc/src/methods/tests/prop.rs
@@ -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)
@@ -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?;
diff --git a/zebra-rpc/src/methods/tests/vectors.rs b/zebra-rpc/src/methods/tests/vectors.rs
index 65f363b1751..aafa8640ff8 100644
--- a/zebra-rpc/src/methods/tests/vectors.rs
+++ b/zebra-rpc/src/methods/tests/vectors.rs
@@ -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.