From 4c2eb95f20fd36939998fb05ddade1394915732b Mon Sep 17 00:00:00 2001 From: 0xb10c Date: Wed, 22 Nov 2023 11:48:05 +0100 Subject: [PATCH] fix: build tx packages similar to Bitcoin Core The previous implementation build packages with a global view of the available transactions. This is different from how Bitcoin Core inserts packages into a block template. For smoother feerate distribution plots, the package building algorithm changed to only account for local packages (i.e. transactions next to each other in the block). This also simplifies the package building. --- daemon/src/model.rs | 29 ++-- daemon/src/processing.rs | 128 +++++++----------- www/templates/subpage/template_and_block.html | 1 - 3 files changed, 57 insertions(+), 101 deletions(-) diff --git a/daemon/src/model.rs b/daemon/src/model.rs index fbc5fcc..8b5361b 100644 --- a/daemon/src/model.rs +++ b/daemon/src/model.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::collections::{HashMap, HashSet}; use bitcoin::hash_types::Txid; @@ -17,6 +16,14 @@ impl TxInfo { pub fn feerate(&self) -> f32 { self.fee.to_sat() as f32 / self.tx.vsize() as f32 } + + pub fn prev_output_txids(&self) -> HashSet { + self.tx + .input + .iter() + .map(|input| input.previous_output.txid) + .collect() + } } pub struct TxPackage { @@ -38,26 +45,6 @@ impl TxPackage { } } -#[derive(PartialEq, Eq, Clone)] -pub struct TxPackageForPackageConstruction { - pub txns: RefCell>, -} - -impl TxPackageForPackageConstruction { - pub fn min_tx_pos(&self) -> i32 { - self.txns - .borrow() - .iter() - .map(|tx_info| tx_info.pos) - .min() - .unwrap_or(-1) - } - - pub fn txids(&self) -> HashSet { - self.txns.borrow().iter().map(|t| t.txid).collect() - } -} - pub struct BlockTxData { pub txids: HashSet, pub txid_to_txinfo_map: HashMap, diff --git a/daemon/src/processing.rs b/daemon/src/processing.rs index 6046324..b2d96de 100644 --- a/daemon/src/processing.rs +++ b/daemon/src/processing.rs @@ -2,9 +2,7 @@ use std::cell::RefCell; use std::collections::{HashMap, HashSet, VecDeque}; use crate::metrics; -use crate::model::{ - BlockTxData, TemplateTxData, TxInfo, TxPackage, TxPackageForPackageConstruction, -}; +use crate::model::{BlockTxData, TemplateTxData, TxInfo, TxPackage}; use miningpool_observer_shared::bitcoincore_rpc::json::{ GetBlockTemplateResult, GetBlockTxFeesResult, @@ -323,79 +321,59 @@ fn get_pool_info_or_default( } } -/// Builds a vector of transaction packages +/// Builds a vector of transaction packages. These Packages are similar to what +/// Bitcoin Core (currently) puts into blocks. pub fn build_packages(txns: &[TxInfo]) -> Vec { - let mut packages_construction: Vec = vec![]; - let mut seen_outpoints: HashSet = HashSet::new(); - - for tx_info in txns.iter() { - let outpoints_spend: HashSet = tx_info - .tx - .input - .iter() - .map(|i| i.previous_output.txid) - .collect(); - - let mut parent_packages: Vec<&TxPackageForPackageConstruction> = vec![]; - for outpoint in outpoints_spend { - if seen_outpoints.contains(&outpoint) { - '_p_finder: for package in packages_construction.iter() { - if package.txids().contains(&outpoint) { - parent_packages.push(package); - break '_p_finder; - } - } - } - } - - match parent_packages.len() { - 0 => { - packages_construction.push(TxPackageForPackageConstruction { - txns: RefCell::new(vec![tx_info.clone()]), + let txids_in_txns: HashSet = txns.iter().map(|ti| ti.tx.txid()).collect(); + let mut packages: Vec = vec![]; + + // Bitcoin Core places transactions that belong to a package next to each other. + // We consider locality when building packages. + // Bitcoin Core currently uses the best ancestor set as a package. See + // https://gist.github.com/murchandamus/5cb413fe9f26dbce57abfd344ebbfaf2#file-asb-vs-csb-png + + let mut package_txns: Vec = vec![]; + // by iterating in reverse order, we know about the children first + // This is helpful, as children reference their parents and we can directly link them + for tx_info in txns.iter().rev() { + if package_txns.is_empty() { + package_txns.push(tx_info.clone()); + } else { + let parent_txids: HashSet = + package_txns.iter().fold(HashSet::new(), |mut txids, tx| { + txids.extend(&tx.prev_output_txids()); + txids }); - } - 1 => { - parent_packages - .first() - .unwrap() // We can unwrap here as we know there is a first (as there is exactly 1) - .txns - .borrow_mut() - .push(tx_info.clone()); - } - _ => { - let min_tx_pos_package = parent_packages - .iter() - .min_by_key(|p| p.min_tx_pos()) - .unwrap(); // We can unwrap here as we know there is more than one item, which yields a min item. - - let mut new_txns: Vec = vec![tx_info.clone()]; - for p in parent_packages.iter() { - // the tnxs can be empty because we can have visited the package before - // as the parent packages vec can contain duplicate packages. This happens, - // for example, when transaction spends from both transactions in a package. - if !p.txns.borrow().is_empty() { - new_txns.append(&mut p.txns.borrow_mut()); - } - } - new_txns.sort_by_key(|tx| tx.pos); - min_tx_pos_package.txns.replace(new_txns); + if parent_txids.contains(&tx_info.tx.txid()) { + // belongs to the same package if the tx is a parent of a tx in the package + package_txns.push(tx_info.clone()); + } else if tx_info + .prev_output_txids() + .iter() + .filter(|txid| txids_in_txns.contains(*txid)) + .any(|txid| parent_txids.contains(txid)) + { + // belongs to the same package if the transaction shares a parent with a + // transaction already in the package (if this parent is in the `txns` set of + // transactions) + package_txns.push(tx_info.clone()); + } else { + packages.push(TxPackage { + txns: package_txns.clone(), + }); + package_txns.clear(); + package_txns.push(tx_info.clone()); } } - // remove empty packages (we empty packages on merge) - packages_construction.retain(|p| !p.txns.borrow().is_empty()); - - seen_outpoints.insert(tx_info.txid); } - - let packages: Vec = packages_construction - .iter() - .map(|construction_package| TxPackage { - txns: construction_package.txns.take(), - }) - .collect(); + if !package_txns.is_empty() { + packages.push(TxPackage { + txns: package_txns.clone(), + }); + } + packages.reverse(); let total_tx: usize = packages.iter().map(|p| p.txns.len()).sum(); - assert_eq!(txns.len(), total_tx); packages @@ -1240,7 +1218,7 @@ mod tests { // // 'X <- Y' means Y spends X; X is the partent of Y // A <- B <- C - let mut txns = vec![tx_a_info.clone(), tx_b_info.clone(), tx_d_info, tx_c_info]; + let mut txns = vec![tx_a_info.clone(), tx_b_info.clone(), tx_c_info, tx_d_info]; // order matters let packages = build_packages(&txns.clone()); println!("There should be exactly two packages"); @@ -1253,10 +1231,6 @@ mod tests { println!("The package weight should be the sum of the three transaction weights"); assert_eq!(package.weight(), tx_a.weight + tx_b.weight + tx_c.weight); - println!("Transactions in the package should be sorted by position (asceding)"); - assert_eq!(package.txns.first().unwrap().pos, 0); - assert_eq!(package.txns.last().unwrap().pos, 2); - // fake tx that spends from A and B let tx_e = Transaction { lock_time: LockTime::from_consensus(0), @@ -1294,7 +1268,7 @@ mod tests { tx: tx_e.clone(), }; - txns.push(tx_e_info); + txns.insert(3, tx_e_info); // order matters let packages = build_packages(&txns); @@ -1310,9 +1284,5 @@ mod tests { package.weight(), tx_a.weight + tx_b.weight + tx_c.weight + tx_e.weight().to_wu() as usize ); - - println!("Transactions in the package should be sorted by position (asceding). The new transaction should be the last."); - assert_eq!(package.txns.first().unwrap().pos, 0); - assert_eq!(package.txns.last().unwrap().pos, 3); } } diff --git a/www/templates/subpage/template_and_block.html b/www/templates/subpage/template_and_block.html index 7cceda8..639d0fc 100644 --- a/www/templates/subpage/template_and_block.html +++ b/www/templates/subpage/template_and_block.html @@ -80,7 +80,6 @@

Feerate Distribution in Template and Block

- Note: The graph might contain package ordering artifacts from transaction package construction. The packages are purposefully not sorted by feerate to show custom pool prioritizations (if present).