Change tip sorting to ratio between tip and max gas sorting in txpool (…

…#1952)

Closes #1909

Change tip sorting to ratio between tip and max gas sorting in txpool

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests
- [x] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [x] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?

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`

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"

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 }

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;

crates/services/txpool/src/containers/price_sort.rs → 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<TipSortKey>;
+pub type RatioGasTipSort = Sort<RatioGasTipSortKey>;
+
+/// A ratio between gas and tip
+pub type RatioGasTip = Ratio<Word>;
 
 #[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<cmp::Ordering> {
         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
     }

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<ViewProvider> {
     by_hash: HashMap<TxId, TxInfo>,
-    by_tip: TipSort,
+    by_ratio_gas_tip: RatioGasTipSort,
     by_time: TimeSort,
     by_dependency: Dependency,
     config: Config,
@@ -85,7 +86,7 @@ impl<ViewProvider> TxPool<ViewProvider> {
 
         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<ViewProvider> TxPool<ViewProvider> {
 
     /// Return all sorted transactions that are includable in next block.
     pub fn sorted_includable(&self) -> impl Iterator<Item = ArcPoolTx> + '_ {
-        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<ArcPoolTx> {
@@ -139,7 +144,7 @@ impl<ViewProvider> TxPool<ViewProvider> {
         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,15 +392,15 @@ 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);
 
         // if some transaction were removed so we don't need to check limit
         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 {

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<Transaction> {
     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::<Vec<_>>();
 
@@ -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::<Vec<_>>();
+
+    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::<Vec<_>>();
+
+    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();