diff --git a/CHANGELOG.md b/CHANGELOG.md index af4422adee6..1fe231c44c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Changed - [#1942](https://github.com/FuelLabs/fuel-core/pull/1942): Sequential relayer's commits. +- [#1952](https://github.com/FuelLabs/fuel-core/pull/1952): Change tip sorting to ratio between tip and max gas sorting in txpool ### Fixed - [#1950](https://github.com/FuelLabs/fuel-core/pull/1950): Fix cursor `BlockHeight` encoding in `SortedTXCursor` diff --git a/Cargo.lock b/Cargo.lock index 733fa30222a..368f95ac194 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3368,6 +3368,7 @@ dependencies = [ "fuel-core-types", "itertools 0.12.1", "mockall", + "num-rational", "parking_lot", "proptest", "rstest", @@ -5800,6 +5801,7 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" dependencies = [ + "num-bigint", "num-integer", "num-traits", ] diff --git a/Cargo.toml b/Cargo.toml index 1678447f772..00c464bf3e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ derivative = { version = "2" } derive_more = { version = "0.99" } enum-iterator = "1.2" hyper = { version = "0.14.26" } +num-rational = "0.4.2" primitive-types = { version = "0.12", default-features = false } rand = "0.8" parking_lot = "0.12" diff --git a/crates/services/txpool/Cargo.toml b/crates/services/txpool/Cargo.toml index 17d6f59d705..bdb9f5747d5 100644 --- a/crates/services/txpool/Cargo.toml +++ b/crates/services/txpool/Cargo.toml @@ -18,6 +18,7 @@ fuel-core-services = { workspace = true } fuel-core-storage = { workspace = true } fuel-core-types = { workspace = true } mockall = { workspace = true, optional = true } +num-rational = { workspace = true } parking_lot = { workspace = true } tokio = { workspace = true, default-features = false, features = ["sync"] } tokio-rayon = { workspace = true } diff --git a/crates/services/txpool/src/containers.rs b/crates/services/txpool/src/containers.rs index 6f7a4345e6d..93d4f1cac9d 100644 --- a/crates/services/txpool/src/containers.rs +++ b/crates/services/txpool/src/containers.rs @@ -1,4 +1,4 @@ pub mod dependency; -pub mod price_sort; pub mod sort; pub mod time_sort; +pub mod tip_per_gas_sort; diff --git a/crates/services/txpool/src/containers/price_sort.rs b/crates/services/txpool/src/containers/tip_per_gas_sort.rs similarity index 51% rename from crates/services/txpool/src/containers/price_sort.rs rename to crates/services/txpool/src/containers/tip_per_gas_sort.rs index d6c4706a661..412bcfbe63f 100644 --- a/crates/services/txpool/src/containers/price_sort.rs +++ b/crates/services/txpool/src/containers/tip_per_gas_sort.rs @@ -1,3 +1,5 @@ +use num_rational::Ratio; + use crate::{ containers::sort::{ Sort, @@ -9,48 +11,51 @@ use crate::{ use std::cmp; /// all transactions sorted by min/max price -pub type TipSort = Sort; +pub type RatioGasTipSort = Sort; + +/// A ratio between gas and tip +pub type RatioGasTip = Ratio; #[derive(Clone, Debug)] -pub struct TipSortKey { - tip: Word, +pub struct RatioGasTipSortKey { + tip_per_gas: RatioGasTip, tx_id: TxId, } -impl SortableKey for TipSortKey { - type Value = GasPrice; +impl SortableKey for RatioGasTipSortKey { + type Value = RatioGasTip; fn new(info: &TxInfo) -> Self { Self { - tip: info.tx().tip(), + tip_per_gas: Ratio::new(info.tx().tip(), info.tx().max_gas()), tx_id: info.tx().id(), } } fn value(&self) -> &Self::Value { - &self.tip + &self.tip_per_gas } } -impl PartialEq for TipSortKey { +impl PartialEq for RatioGasTipSortKey { fn eq(&self, other: &Self) -> bool { self.tx_id == other.tx_id } } -impl Eq for TipSortKey {} +impl Eq for RatioGasTipSortKey {} -impl PartialOrd for TipSortKey { +impl PartialOrd for RatioGasTipSortKey { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } -impl Ord for TipSortKey { +impl Ord for RatioGasTipSortKey { fn cmp(&self, other: &Self) -> cmp::Ordering { - let cmp = self.tip.cmp(&other.tip); + let cmp = self.tip_per_gas.cmp(&other.tip_per_gas); if cmp == cmp::Ordering::Equal { - return self.tx_id.cmp(&other.tx_id) + return self.tx_id.cmp(&other.tx_id); } cmp } diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index c8dbba6dddb..736a6b17bcb 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -1,8 +1,8 @@ use crate::{ containers::{ dependency::Dependency, - price_sort::TipSort, time_sort::TimeSort, + tip_per_gas_sort::RatioGasTipSort, }, ports::TxPoolDb, service::TxStatusChange, @@ -27,6 +27,7 @@ use fuel_core_types::{ }, tai64::Tai64, }; +use num_rational::Ratio; use crate::ports::{ GasPriceProvider, @@ -72,7 +73,7 @@ mod tests; #[derive(Debug, Clone)] pub struct TxPool { by_hash: HashMap, - by_tip: TipSort, + by_ratio_gas_tip: RatioGasTipSort, by_time: TimeSort, by_dependency: Dependency, config: Config, @@ -85,7 +86,7 @@ impl TxPool { Self { by_hash: HashMap::new(), - by_tip: TipSort::default(), + by_ratio_gas_tip: RatioGasTipSort::default(), by_time: TimeSort::default(), by_dependency: Dependency::new(max_depth, config.utxo_validation), config, @@ -113,7 +114,11 @@ impl TxPool { /// Return all sorted transactions that are includable in next block. pub fn sorted_includable(&self) -> impl Iterator + '_ { - self.by_tip.sort.iter().rev().map(|(_, tx)| tx.clone()) + self.by_ratio_gas_tip + .sort + .iter() + .rev() + .map(|(_, tx)| tx.clone()) } pub fn remove_inner(&mut self, tx: &ArcPoolTx) -> Vec { @@ -139,7 +144,7 @@ impl TxPool { let info = self.by_hash.remove(tx_id); if let Some(info) = &info { self.by_time.remove(info); - self.by_tip.remove(info); + self.by_ratio_gas_tip.remove(info); } info @@ -373,8 +378,8 @@ where if self.by_hash.len() >= self.config.max_tx { max_limit_hit = true; // limit is hit, check if we can push out lowest priced tx - let lowest_tip = self.by_tip.lowest_value().unwrap_or_default(); - if lowest_tip >= tx.tip() { + let lowest_ratio = self.by_ratio_gas_tip.lowest_value().unwrap_or_default(); + if lowest_ratio >= Ratio::new(tx.tip(), tx.max_gas()) { return Err(Error::NotInsertedLimitHit) } } @@ -387,7 +392,7 @@ where let rem = self.by_dependency.insert(&self.by_hash, view, &tx)?; let info = TxInfo::new(tx.clone()); let submitted_time = info.submitted_time(); - self.by_tip.insert(&info); + self.by_ratio_gas_tip.insert(&info); self.by_time.insert(&info); self.by_hash.insert(tx.id(), info); @@ -395,7 +400,7 @@ where let removed = if rem.is_empty() { if max_limit_hit { // remove last tx from sort - let rem_tx = self.by_tip.lowest_tx().unwrap(); // safe to unwrap limit is hit + let rem_tx = self.by_ratio_gas_tip.lowest_tx().unwrap(); // safe to unwrap limit is hit self.remove_inner(&rem_tx); vec![rem_tx] } else { diff --git a/crates/services/txpool/src/txpool/tests.rs b/crates/services/txpool/src/txpool/tests.rs index b6057e881d2..da7b736f6af 100644 --- a/crates/services/txpool/src/txpool/tests.rs +++ b/crates/services/txpool/src/txpool/tests.rs @@ -54,7 +54,7 @@ use std::{ use super::check_single_tx; -const GAS_LIMIT: Word = 1000; +const GAS_LIMIT: Word = 100000; async fn check_unwrap_tx(tx: Transaction, config: &Config) -> Checked { let gas_price = 0; @@ -791,7 +791,7 @@ async fn tx_depth_hit() { } #[tokio::test] -async fn sorted_out_tx1_2_4() { +async fn sorted_out_tx1_2_3() { let mut context = TextContext::default(); let (_, gas_coin) = context.setup_coin(); @@ -835,7 +835,7 @@ async fn sorted_out_tx1_2_4() { .expect("Tx2 should be Ok, got Err"); txpool .insert_single(tx3) - .expect("Tx4 should be Ok, got Err"); + .expect("Tx3 should be Ok, got Err"); let txs = txpool.sorted_includable().collect::>(); @@ -845,6 +845,116 @@ async fn sorted_out_tx1_2_4() { assert_eq!(txs[2].id(), tx2_id, "Third should be tx2"); } +#[tokio::test] +async fn sorted_out_tx_same_tips() { + let mut context = TextContext::default(); + + let (_, gas_coin) = context.setup_coin(); + let tx1 = TransactionBuilder::script(vec![], vec![]) + .tip(10) + .max_fee_limit(10) + .script_gas_limit(GAS_LIMIT) + .add_input(gas_coin) + .finalize_as_transaction(); + + let (_, gas_coin) = context.setup_coin(); + let tx2 = TransactionBuilder::script(vec![], vec![]) + .tip(10) + .max_fee_limit(10) + .script_gas_limit(GAS_LIMIT / 2) + .add_input(gas_coin) + .finalize_as_transaction(); + + let (_, gas_coin) = context.setup_coin(); + let tx3 = TransactionBuilder::script(vec![], vec![]) + .tip(10) + .max_fee_limit(10) + .script_gas_limit(GAS_LIMIT / 4) + .add_input(gas_coin) + .finalize_as_transaction(); + + let tx1_id = tx1.id(&ChainId::default()); + let tx2_id = tx2.id(&ChainId::default()); + let tx3_id = tx3.id(&ChainId::default()); + + let mut txpool = context.build(); + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config).await; + + txpool + .insert_single(tx1) + .expect("Tx1 should be Ok, got Err"); + txpool + .insert_single(tx2) + .expect("Tx2 should be Ok, got Err"); + txpool + .insert_single(tx3) + .expect("Tx4 should be Ok, got Err"); + + let txs = txpool.sorted_includable().collect::>(); + + assert_eq!(txs.len(), 3, "Should have 3 txs"); + assert_eq!(txs[0].id(), tx3_id, "First should be tx3"); + assert_eq!(txs[1].id(), tx2_id, "Second should be tx2"); + assert_eq!(txs[2].id(), tx1_id, "Third should be tx1"); +} + +#[tokio::test] +async fn sorted_out_tx_profitable_ratios() { + let mut context = TextContext::default(); + + let (_, gas_coin) = context.setup_coin(); + let tx1 = TransactionBuilder::script(vec![], vec![]) + .tip(4) + .max_fee_limit(4) + .script_gas_limit(GAS_LIMIT) + .add_input(gas_coin) + .finalize_as_transaction(); + + let (_, gas_coin) = context.setup_coin(); + let tx2 = TransactionBuilder::script(vec![], vec![]) + .tip(2) + .max_fee_limit(2) + .script_gas_limit(GAS_LIMIT / 10) + .add_input(gas_coin) + .finalize_as_transaction(); + + let (_, gas_coin) = context.setup_coin(); + let tx3 = TransactionBuilder::script(vec![], vec![]) + .tip(1) + .max_fee_limit(1) + .script_gas_limit(GAS_LIMIT / 100) + .add_input(gas_coin) + .finalize_as_transaction(); + + let tx1_id = tx1.id(&ChainId::default()); + let tx2_id = tx2.id(&ChainId::default()); + let tx3_id = tx3.id(&ChainId::default()); + + let mut txpool = context.build(); + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config).await; + + txpool + .insert_single(tx1) + .expect("Tx1 should be Ok, got Err"); + txpool + .insert_single(tx2) + .expect("Tx2 should be Ok, got Err"); + txpool + .insert_single(tx3) + .expect("Tx4 should be Ok, got Err"); + + let txs = txpool.sorted_includable().collect::>(); + + assert_eq!(txs.len(), 3, "Should have 3 txs"); + assert_eq!(txs[0].id(), tx3_id, "First should be tx3"); + assert_eq!(txs[1].id(), tx2_id, "Second should be tx2"); + assert_eq!(txs[2].id(), tx1_id, "Third should be tx1"); +} + #[tokio::test] async fn find_dependent_tx1_tx2() { let mut context = TextContext::default();