diff --git a/zebra-chain/src/transaction/hash.rs b/zebra-chain/src/transaction/hash.rs index 2525c1d9..5c644d0e 100644 --- a/zebra-chain/src/transaction/hash.rs +++ b/zebra-chain/src/transaction/hash.rs @@ -60,7 +60,7 @@ use super::{txid::TxIdBuilder, AuthDigest, Transaction}; /// /// [ZIP-244]: https://zips.z.cash/zip-0244 /// [Spec: Transaction Identifiers]: https://zips.z.cash/protocol/protocol.pdf#txnidentifiers -#[derive(Copy, Clone, Eq, PartialEq, Serialize, Deserialize, Hash)] +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, Hash)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub struct Hash(pub [u8; 32]); diff --git a/zebra-rpc/src/methods.rs b/zebra-rpc/src/methods.rs index 43b53a8e..4331383c 100644 --- a/zebra-rpc/src/methods.rs +++ b/zebra-rpc/src/methods.rs @@ -647,9 +647,24 @@ where // TODO: use a generic error constructor (#5548) fn get_raw_mempool(&self) -> BoxFuture>> { + #[cfg(feature = "getblocktemplate-rpcs")] + use zebra_chain::block::MAX_BLOCK_BYTES; + + #[cfg(feature = "getblocktemplate-rpcs")] + /// Determines whether the output of this RPC is sorted like zcashd + const SHOULD_USE_ZCASHD_ORDER: bool = true; + let mut mempool = self.mempool.clone(); async move { + #[cfg(feature = "getblocktemplate-rpcs")] + let request = if SHOULD_USE_ZCASHD_ORDER { + mempool::Request::FullTransactions + } else { + mempool::Request::TransactionIds + }; + + #[cfg(not(feature = "getblocktemplate-rpcs"))] let request = mempool::Request::TransactionIds; // `zcashd` doesn't check if it is synced to the tip here, so we don't either. @@ -664,6 +679,28 @@ where })?; match response { + #[cfg(feature = "getblocktemplate-rpcs")] + mempool::Response::FullTransactions(mut transactions) => { + // Sort transactions in descending order by fee/size, using hash in serialized byte order as a tie-breaker + transactions.sort_by_cached_key(|tx| { + // zcashd uses modified fee here but Zebra doesn't currently + // support prioritizing transactions + std::cmp::Reverse(( + i64::from(tx.miner_fee) as u128 * MAX_BLOCK_BYTES as u128 + / tx.transaction.size as u128, + // transaction hashes are compared in their serialized byte-order. + tx.transaction.id.mined_id(), + )) + }); + + let tx_ids: Vec = transactions + .iter() + .map(|unmined_tx| unmined_tx.transaction.id.mined_id().encode_hex()) + .collect(); + + Ok(tx_ids) + } + mempool::Response::TransactionIds(unmined_transaction_ids) => { let mut tx_ids: Vec = unmined_transaction_ids .iter() @@ -671,11 +708,11 @@ where .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"), } } diff --git a/zebra-rpc/src/methods/tests/prop.rs b/zebra-rpc/src/methods/tests/prop.rs index 01692c34..822a7820 100644 --- a/zebra-rpc/src/methods/tests/prop.rs +++ b/zebra-rpc/src/methods/tests/prop.rs @@ -18,7 +18,7 @@ use zebra_chain::{ NetworkUpgrade, }, serialization::{ZcashDeserialize, ZcashSerialize}, - transaction::{self, Transaction, UnminedTx, UnminedTxId}, + transaction::{self, Transaction, UnminedTx, VerifiedUnminedTx}, transparent, }; use zebra_node_services::mempool; @@ -313,7 +313,7 @@ proptest! { /// Make the mock mempool service return a list of transaction IDs, and check that the RPC call /// returns those IDs as hexadecimal strings. #[test] - fn mempool_transactions_are_sent_to_caller(transaction_ids in any::>()) { + fn mempool_transactions_are_sent_to_caller(transactions in any::>()) { let (runtime, _init_guard) = zebra_test::init_async(); let _guard = runtime.enter(); @@ -334,16 +334,56 @@ proptest! { ); let call_task = tokio::spawn(rpc.get_raw_mempool()); - let mut expected_response: Vec = transaction_ids - .iter() - .map(|id| id.mined_id().encode_hex()) - .collect(); - expected_response.sort(); - mempool - .expect_request(mempool::Request::TransactionIds) - .await? - .respond(mempool::Response::TransactionIds(transaction_ids)); + + #[cfg(not(feature = "getblocktemplate-rpcs"))] + let expected_response = { + let transaction_ids: HashSet<_> = transactions + .iter() + .map(|tx| tx.transaction.id) + .collect(); + + let mut expected_response: Vec = transaction_ids + .iter() + .map(|id| id.mined_id().encode_hex()) + .collect(); + expected_response.sort(); + + mempool + .expect_request(mempool::Request::TransactionIds) + .await? + .respond(mempool::Response::TransactionIds(transaction_ids)); + + expected_response + }; + + // Note: this depends on `SHOULD_USE_ZCASHD_ORDER` being true. + #[cfg(feature = "getblocktemplate-rpcs")] + let expected_response = { + let mut expected_response = transactions.clone(); + expected_response.sort_by_cached_key(|tx| { + // zcashd uses modified fee here but Zebra doesn't currently + // support prioritizing transactions + std::cmp::Reverse(( + i64::from(tx.miner_fee) as u128 * zebra_chain::block::MAX_BLOCK_BYTES as u128 + / tx.transaction.size as u128, + // transaction hashes are compared in their serialized byte-order. + tx.transaction.id.mined_id(), + )) + }); + + let expected_response = expected_response + .iter() + .map(|tx| tx.transaction.id.mined_id().encode_hex()) + .collect(); + + mempool + .expect_request(mempool::Request::FullTransactions) + .await? + .respond(mempool::Response::FullTransactions(transactions)); + + expected_response + }; mempool.expect_no_requests().await?; state.expect_no_requests().await?; diff --git a/zebra-rpc/src/methods/tests/snapshot.rs b/zebra-rpc/src/methods/tests/snapshot.rs index ca333a90..f25593e4 100644 --- a/zebra-rpc/src/methods/tests/snapshot.rs +++ b/zebra-rpc/src/methods/tests/snapshot.rs @@ -129,6 +129,15 @@ async fn test_rpc_response_data_for_network(network: Network) { // - a request to get all mempool transactions will be made by `getrawmempool` behind the scenes. // - as we have the mempool mocked we need to expect a request and wait for a response, // which will be an empty mempool in this case. + // Note: this depends on `SHOULD_USE_ZCASHD_ORDER` being true. + #[cfg(feature = "getblocktemplate-rpcs")] + let mempool_req = mempool + .expect_request_that(|request| matches!(request, mempool::Request::FullTransactions)) + .map(|responder| { + responder.respond(mempool::Response::FullTransactions(vec![])); + }); + + #[cfg(not(feature = "getblocktemplate-rpcs"))] let mempool_req = mempool .expect_request_that(|request| matches!(request, mempool::Request::TransactionIds)) .map(|responder| {