From d05122d01aa05594d0af3a1394baa6800bd06663 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 12:09:12 +0200 Subject: [PATCH 1/6] Change tip sorting to ratio between tip and max gas sorting --- Cargo.lock | 3 + Cargo.toml | 1 + crates/services/txpool/Cargo.toml | 1 + crates/services/txpool/src/containers.rs | 2 +- .../{price_sort.rs => tip_per_gas_sort.rs} | 31 +++-- crates/services/txpool/src/txpool.rs | 23 ++-- crates/services/txpool/src/txpool/tests.rs | 116 +++++++++++++++++- 7 files changed, 151 insertions(+), 26 deletions(-) rename crates/services/txpool/src/containers/{price_sort.rs => tip_per_gas_sort.rs} (51%) diff --git a/Cargo.lock b/Cargo.lock index a87d85651d8..b505169ee39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3353,6 +3353,7 @@ dependencies = [ "fuel-core-types", "itertools 0.12.1", "mockall", + "num", "parking_lot", "proptest", "rstest", @@ -5727,6 +5728,7 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" dependencies = [ + "num-bigint", "num-complex", "num-integer", "num-iter", @@ -5785,6 +5787,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 7c8e5c40a2d..cbe25428ad0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,6 +92,7 @@ derivative = { version = "2" } derive_more = { version = "0.99" } enum-iterator = "1.2" hyper = { version = "0.14.26" } +num = "0.4.3" 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..66dd30e7170 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 = { 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..4e5031e4323 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().max_gas(), info.tx().tip()), 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 = other.tip_per_gas.cmp(&self.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 bc08d775e23..3dc34199e88 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.max_gas(), tx.tip()) { 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(); From 50e4065c8c3c1f10126b65e4847b0e99edaabb7f Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 12:11:26 +0200 Subject: [PATCH 2/6] Update changelog for 1952 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9ea8261b2f..66f3c76b87a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,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` From 55145595aa9dfe1374707e83a27677e3b1aa41ca Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 15:21:40 +0200 Subject: [PATCH 3/6] Invert numer and denom in Ratio `RatioGasTipSortKey` to match the expected behavior. --- crates/services/txpool/src/containers/tip_per_gas_sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/txpool/src/containers/tip_per_gas_sort.rs b/crates/services/txpool/src/containers/tip_per_gas_sort.rs index 4e5031e4323..63ff7fa72c1 100644 --- a/crates/services/txpool/src/containers/tip_per_gas_sort.rs +++ b/crates/services/txpool/src/containers/tip_per_gas_sort.rs @@ -27,7 +27,7 @@ impl SortableKey for RatioGasTipSortKey { fn new(info: &TxInfo) -> Self { Self { - tip_per_gas: Ratio::new(info.tx().max_gas(), info.tx().tip()), + tip_per_gas: Ratio::new(info.tx().tip(), info.tx().max_gas()), tx_id: info.tx().id(), } } From 598b3293b321e7fc16e1d44812dd3521263ea3f5 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 18:07:42 +0200 Subject: [PATCH 4/6] Invert comparison sorting --- crates/services/txpool/src/containers/tip_per_gas_sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/txpool/src/containers/tip_per_gas_sort.rs b/crates/services/txpool/src/containers/tip_per_gas_sort.rs index 63ff7fa72c1..3008720e979 100644 --- a/crates/services/txpool/src/containers/tip_per_gas_sort.rs +++ b/crates/services/txpool/src/containers/tip_per_gas_sort.rs @@ -53,7 +53,7 @@ impl PartialOrd for RatioGasTipSortKey { impl Ord for RatioGasTipSortKey { fn cmp(&self, other: &Self) -> cmp::Ordering { - let cmp = other.tip_per_gas.cmp(&self.tip_per_gas); + let cmp = self.tip_per_gas.cmp(&other.tip_per_gas); if cmp == cmp::Ordering::Equal { return self.tx_id.cmp(&other.tx_id); } From 670ed43503f95b477e495a08fd025a180f6454cd Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 18:10:28 +0200 Subject: [PATCH 5/6] Fix inversion numerator/denominator in Ratio creation --- crates/services/txpool/src/txpool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 3dc34199e88..5fbd4d5dadf 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -379,7 +379,7 @@ where max_limit_hit = true; // limit is hit, check if we can push out lowest priced tx let lowest_ratio = self.by_ratio_gas_tip.lowest_value().unwrap_or_default(); - if lowest_ratio >= Ratio::new(tx.max_gas(), tx.tip()) { + if lowest_ratio >= Ratio::new(tx.tip(), tx.max_gas()) { return Err(Error::NotInsertedLimitHit) } } From 8cb3e07cf7adc095be82854c4aff8e1609a2e18b Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Tue, 11 Jun 2024 21:49:29 +0200 Subject: [PATCH 6/6] Use num_rational instead of num dependency --- Cargo.lock | 3 +-- Cargo.toml | 2 +- crates/services/txpool/Cargo.toml | 2 +- crates/services/txpool/src/containers/tip_per_gas_sort.rs | 2 +- crates/services/txpool/src/txpool.rs | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b505169ee39..7d4a6f56f3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3353,7 +3353,7 @@ dependencies = [ "fuel-core-types", "itertools 0.12.1", "mockall", - "num", + "num-rational", "parking_lot", "proptest", "rstest", @@ -5728,7 +5728,6 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" dependencies = [ - "num-bigint", "num-complex", "num-integer", "num-iter", diff --git a/Cargo.toml b/Cargo.toml index cbe25428ad0..b098586f43b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -92,7 +92,7 @@ derivative = { version = "2" } derive_more = { version = "0.99" } enum-iterator = "1.2" hyper = { version = "0.14.26" } -num = "0.4.3" +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 66dd30e7170..bdb9f5747d5 100644 --- a/crates/services/txpool/Cargo.toml +++ b/crates/services/txpool/Cargo.toml @@ -18,7 +18,7 @@ fuel-core-services = { workspace = true } fuel-core-storage = { workspace = true } fuel-core-types = { workspace = true } mockall = { workspace = true, optional = true } -num = { workspace = 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/tip_per_gas_sort.rs b/crates/services/txpool/src/containers/tip_per_gas_sort.rs index 3008720e979..412bcfbe63f 100644 --- a/crates/services/txpool/src/containers/tip_per_gas_sort.rs +++ b/crates/services/txpool/src/containers/tip_per_gas_sort.rs @@ -1,4 +1,4 @@ -use num::rational::Ratio; +use num_rational::Ratio; use crate::{ containers::sort::{ diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 5fbd4d5dadf..ee3cfd0d799 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -27,7 +27,7 @@ use fuel_core_types::{ }, tai64::Tai64, }; -use num::rational::Ratio; +use num_rational::Ratio; use crate::ports::{ GasPriceProvider,