diff --git a/zebra-rpc/src/methods/get_block_template_rpcs.rs b/zebra-rpc/src/methods/get_block_template_rpcs.rs index 8f56d77eb30..a2dbbf57e3f 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs.rs @@ -568,6 +568,14 @@ where let next_block_height = (chain_tip_and_local_time.tip_height + 1).expect("tip is far below Height::MAX"); + tracing::debug!( + mempool_tx_hashes = ?mempool_txs + .iter() + .map(|tx| tx.transaction.id.mined_id()) + .collect::>(), + "selecting transactions for the template from the mempool" + ); + // Randomly select some mempool transactions. // // TODO: sort these transactions to match zcashd's order, to make testing easier. @@ -580,6 +588,14 @@ where ) .await; + tracing::debug!( + selected_mempool_tx_hashes = ?mempool_txs + .iter() + .map(|tx| tx.transaction.id.mined_id()) + .collect::>(), + "selected transactions for the template from the mempool" + ); + // - After this point, the template only depends on the previously fetched data. let response = GetBlockTemplate::new( diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs index 4135fb1232b..5a223795392 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/types/get_block_template.rs @@ -244,6 +244,14 @@ impl GetBlockTemplate { .map(ToString::to_string) .collect(); + tracing::debug!( + selected_txs = ?mempool_txs + .iter() + .map(|tx| (tx.transaction.id.mined_id(), tx.unpaid_actions)) + .collect::>(), + "creating template ... " + ); + GetBlockTemplate { capabilities, diff --git a/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs b/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs index 185f569c4ba..adb4af608d2 100644 --- a/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs +++ b/zebra-rpc/src/methods/get_block_template_rpcs/zip317.rs @@ -53,7 +53,7 @@ pub async fn select_mempool_transactions( fake_coinbase_transaction(network, next_block_height, miner_address, like_zcashd); // Setup the transaction lists. - let (conventional_fee_txs, low_fee_txs): (Vec<_>, Vec<_>) = mempool_txs + let (mut conventional_fee_txs, mut low_fee_txs): (Vec<_>, Vec<_>) = mempool_txs .into_iter() .partition(VerifiedUnminedTx::pays_conventional_fee); @@ -74,7 +74,7 @@ pub async fn select_mempool_transactions( while let Some(tx_weights) = conventional_fee_tx_weights { conventional_fee_tx_weights = checked_add_transaction_weighted_random( - &conventional_fee_txs, + &mut conventional_fee_txs, tx_weights, &mut selected_txs, &mut remaining_block_bytes, @@ -90,7 +90,7 @@ pub async fn select_mempool_transactions( while let Some(tx_weights) = low_fee_tx_weights { low_fee_tx_weights = checked_add_transaction_weighted_random( - &low_fee_txs, + &mut low_fee_txs, tx_weights, &mut selected_txs, &mut remaining_block_bytes, @@ -160,7 +160,7 @@ fn setup_fee_weighted_index(transactions: &[VerifiedUnminedTx]) -> Option, tx_weights: WeightedIndex, selected_txs: &mut Vec, remaining_block_bytes: &mut usize, @@ -201,20 +201,14 @@ fn checked_add_transaction_weighted_random( /// If some transactions have not yet been chosen, returns the weighted index and the transaction. /// Otherwise, just returns the transaction. fn choose_transaction_weighted_random( - candidate_txs: &[VerifiedUnminedTx], - mut weighted_index: WeightedIndex, + candidate_txs: &mut Vec, + weighted_index: WeightedIndex, ) -> (Option>, VerifiedUnminedTx) { let candidate_position = weighted_index.sample(&mut thread_rng()); - let candidate_tx = candidate_txs[candidate_position].clone(); + let candidate_tx = candidate_txs.swap_remove(candidate_position); - // Only pick each transaction once, by setting picked transaction weights to zero - if weighted_index - .update_weights(&[(candidate_position, &0.0)]) - .is_err() - { - // All weights are zero, so each transaction has either been selected or rejected - (None, candidate_tx) - } else { - (Some(weighted_index), candidate_tx) - } + // We have to regenerate this index each time we choose a transaction, due to floating-point sum inaccuracies. + // If we don't, some chosen transactions can end up with a tiny non-zero weight, leading to duplicates. + // + (setup_fee_weighted_index(candidate_txs), candidate_tx) }