Return error from TxPool level if the BlobId is known (#2067)

Return error from TxPool level if the `BlobId` is known. Currently, we only return it from the block producer level. ## Checklist - [x] New behavior is reflected in tests ### Before requesting review - [x] I have reviewed the code myself
FuelLabs · Aug 9, 2024 · 279b123 · 279b123
1 parent 005a0e6
commit 279b123
Show file tree

Hide file tree

Showing 10 changed files with 256 additions and 170 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,7 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [2061](https://github.com/FuelLabs/fuel-core/pull/2061): Allow querying filled transaction body from the status.
 
 ### Changed
--[2064](https://github.com/FuelLabs/fuel-core/pull/2064):  Allow gas price metadata values to be overridden with config
+-[2067](https://github.com/FuelLabs/fuel-core/pull/2067): Return error from TxPool level if the `BlobId` is known.
+-[2064](https://github.com/FuelLabs/fuel-core/pull/2064): Allow gas price metadata values to be overridden with config
 
 ### Fixes
 - [2060](https://github.com/FuelLabs/fuel-core/pull/2060): Use `min-gas-price` as a starting point if `start-gas-price` is zero.

diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs
@@ -34,6 +34,7 @@ use fuel_core_types::{
         relayer::message::Message,
     },
     fuel_tx::{
+        BlobId,
         ConsensusParameters,
         Transaction,
         UtxoId,
@@ -42,6 +43,7 @@ use fuel_core_types::{
         ContractId,
         Nonce,
     },
+    fuel_vm::BlobData,
     services::{
         block_importer::SharedImportResult,
         p2p::{
@@ -135,6 +137,10 @@ impl fuel_core_txpool::ports::TxPoolDb for OnChainIterableKeyValueView {
         self.storage::<ContractsRawCode>().contains_key(contract_id)
     }
 
+    fn blob_exist(&self, blob_id: &BlobId) -> StorageResult<bool> {
+        self.storage::<BlobData>().contains_key(blob_id)
+    }
+
     fn message(&self, id: &Nonce) -> StorageResult<Option<Message>> {
         self.storage::<Messages>()
             .get(id)

diff --git a/crates/services/txpool/src/containers/dependency.rs b/crates/services/txpool/src/containers/dependency.rs
@@ -6,6 +6,7 @@ use crate::{
 };
 use fuel_core_types::{
     fuel_tx::{
+        field::BlobId as BlobIdField,
         input::{
             coin::{
                 CoinPredicate,
@@ -23,7 +24,10 @@ use fuel_core_types::{
         Output,
         UtxoId,
     },
-    fuel_types::Nonce,
+    fuel_types::{
+        BlobId,
+        Nonce,
+    },
     services::txpool::ArcPoolTx,
 };
 use std::collections::{
@@ -40,6 +44,8 @@ pub struct Dependency {
     coins: HashMap<UtxoId, CoinState>,
     /// Contract-> Tx mapping.
     contracts: HashMap<ContractId, ContractState>,
+    /// Blob-> Tx mapping.
+    blobs: HashMap<BlobId, BlobState>,
     /// messageId -> tx mapping
     messages: HashMap<Nonce, MessageState>,
     /// max depth of dependency.
@@ -90,77 +96,24 @@ pub struct MessageState {
     tip: Word,
 }
 
+#[derive(Debug, Clone)]
+pub struct BlobState {
+    origin_tx_id: TxId,
+    tip: Word,
+}
+
 impl Dependency {
     pub fn new(max_depth: usize, utxo_validation: bool) -> Self {
         Self {
             coins: HashMap::new(),
             contracts: HashMap::new(),
+            blobs: HashMap::new(),
             messages: HashMap::new(),
             max_depth,
             utxo_validation,
         }
     }
 
-    /// find all dependent Transactions that are inside txpool.
-    /// Does not check db. They can be sorted by gasPrice to get order of dependency
-    pub(crate) fn find_dependent(
-        &self,
-        tx: ArcPoolTx,
-        seen: &mut HashMap<TxId, ArcPoolTx>,
-        txs: &HashMap<TxId, TxInfo>,
-    ) {
-        // for every input aggregate UtxoId and check if it is inside
-        let mut check = vec![tx.id()];
-        while let Some(parent_txhash) = check.pop() {
-            let mut is_new = false;
-            let mut parent_tx = None;
-            seen.entry(parent_txhash).or_insert_with(|| {
-                is_new = true;
-                let parent = txs.get(&parent_txhash).expect("To have tx in txpool");
-                parent_tx = Some(parent.clone());
-                parent.tx().clone()
-            });
-            // for every input check if tx_id is inside seen. if not, check coins/contract map.
-            if let Some(parent_tx) = parent_tx {
-                for input in parent_tx.inputs() {
-                    // if found and depth is not zero add it to `check`.
-                    match input {
-                        Input::CoinSigned(CoinSigned { utxo_id, .. })
-                        | Input::CoinPredicate(CoinPredicate { utxo_id, .. }) => {
-                            let state = self
-                                .coins
-                                .get(utxo_id)
-                                .expect("to find coin inside spend tx");
-                            if !state.is_in_database() {
-                                check.push(*utxo_id.tx_id());
-                            }
-                        }
-                        Input::Contract(Contract { contract_id, .. }) => {
-                            let state = self
-                                .contracts
-                                .get(contract_id)
-                                .expect("Expect to find contract in dependency");
-
-                            if !state.is_in_database() {
-                                let origin = state
-                                    .origin
-                                    .as_ref()
-                                    .expect("contract origin to be present");
-                                check.push(*origin.tx_id());
-                            }
-                        }
-                        Input::MessageCoinSigned(_)
-                        | Input::MessageCoinPredicate(_)
-                        | Input::MessageDataSigned(_)
-                        | Input::MessageDataPredicate(_) => {
-                            // Message inputs do not depend on any other fuel transactions
-                        }
-                    }
-                }
-            }
-        }
-    }
-
     fn check_if_coin_input_can_spend_output(
         output: &Output,
         input: &Input,
@@ -258,6 +211,7 @@ impl Dependency {
             usize,
             HashMap<UtxoId, CoinState>,
             HashMap<ContractId, ContractState>,
+            HashMap<BlobId, BlobState>,
             HashMap<Nonce, MessageState>,
             Vec<TxId>,
         ),
@@ -268,7 +222,34 @@ impl Dependency {
         let mut max_depth = 0;
         let mut db_coins: HashMap<UtxoId, CoinState> = HashMap::new();
         let mut db_contracts: HashMap<ContractId, ContractState> = HashMap::new();
+        let mut db_blobs: HashMap<BlobId, BlobState> = HashMap::new();
         let mut db_messages: HashMap<Nonce, MessageState> = HashMap::new();
+
+        if let PoolTransaction::Blob(checked_tx, _) = tx.as_ref() {
+            let blob_id = checked_tx.transaction().blob_id();
+            if db
+                .blob_exist(blob_id)
+                .map_err(|e| Error::Database(format!("{:?}", e)))?
+            {
+                return Err(Error::NotInsertedBlobIdAlreadyTaken(*blob_id))
+            }
+
+            if let Some(state) = self.blobs.get(blob_id) {
+                if state.tip >= tx.tip() {
+                    return Err(Error::NotInsertedCollisionBlobId(*blob_id))
+                } else {
+                    collided.push(state.origin_tx_id);
+                }
+            }
+            db_blobs.insert(
+                *blob_id,
+                BlobState {
+                    origin_tx_id: tx.id(),
+                    tip: tx.tip(),
+                },
+            );
+        }
+
         for input in tx.inputs() {
             // check if all required inputs are here.
             match input {
@@ -461,7 +442,14 @@ impl Dependency {
             // collision of other outputs is not possible.
         }
 
-        Ok((max_depth, db_coins, db_contracts, db_messages, collided))
+        Ok((
+            max_depth,
+            db_coins,
+            db_contracts,
+            db_blobs,
+            db_messages,
+            collided,
+        ))
     }
 
     /// insert tx inside dependency
@@ -475,7 +463,7 @@ impl Dependency {
     where
         DB: TxPoolDb,
     {
-        let (max_depth, db_coins, db_contracts, db_messages, collided) =
+        let (max_depth, db_coins, db_contracts, db_blobs, db_messages, collided) =
             self.check_for_collision(txs, db, tx)?;
 
         // now we are sure that transaction can be included. remove all collided transactions
@@ -519,6 +507,7 @@ impl Dependency {
         // for contracts from db that are not found in dependency, we already inserted used_by
         // and are okay to just extend current list
         self.contracts.extend(db_contracts);
+        self.blobs.extend(db_blobs);
         // insert / overwrite all applicable message id spending relations
         self.messages.extend(db_messages);
 
@@ -555,7 +544,7 @@ impl Dependency {
                     );
                 }
                 Output::Contract(_) => {
-                    // do nothing, this contract is already already found in dependencies.
+                    // do nothing, this contract is already found in dependencies.
                     // as it is tied with input and used_by is already inserted.
                 }
             };
@@ -663,6 +652,11 @@ impl Dependency {
             }
         }
 
+        if let PoolTransaction::Blob(checked_tx, _) = tx.as_ref() {
+            // remove blob state
+            self.blobs.remove(checked_tx.transaction().blob_id());
+        }
+
         removed_transactions
     }
 }

diff --git a/crates/services/txpool/src/mock_db.rs b/crates/services/txpool/src/mock_db.rs
@@ -12,11 +12,13 @@ use fuel_core_types::{
         relayer::message::Message,
     },
     fuel_tx::{
+        BlobId,
         Contract,
         ContractId,
         UtxoId,
     },
     fuel_types::Nonce,
+    fuel_vm::BlobBytes,
 };
 use std::{
     collections::{
@@ -33,6 +35,7 @@ use std::{
 pub struct Data {
     pub coins: HashMap<UtxoId, CompressedCoin>,
     pub contracts: HashMap<ContractId, Contract>,
+    pub blobs: HashMap<BlobId, BlobBytes>,
     pub messages: HashMap<Nonce, Message>,
     pub spent_messages: HashSet<Nonce>,
 }
@@ -51,6 +54,14 @@ impl MockDb {
             .insert(coin.utxo_id, coin.compress());
     }
 
+    pub fn insert_dummy_blob(&self, blob_id: BlobId) {
+        self.data
+            .lock()
+            .unwrap()
+            .blobs
+            .insert(blob_id, vec![123; 123].into());
+    }
+
     pub fn insert_message(&self, message: Message) {
         self.data
             .lock()
@@ -78,6 +89,10 @@ impl TxPoolDb for MockDb {
             .contains_key(contract_id))
     }
 
+    fn blob_exist(&self, blob_id: &BlobId) -> StorageResult<bool> {
+        Ok(self.data.lock().unwrap().blobs.contains_key(blob_id))
+    }
+
     fn message(&self, id: &Nonce) -> StorageResult<Option<Message>> {
         Ok(self.data.lock().unwrap().messages.get(id).cloned())
     }

diff --git a/crates/services/txpool/src/ports.rs b/crates/services/txpool/src/ports.rs
@@ -13,6 +13,7 @@ use fuel_core_types::{
         UtxoId,
     },
     fuel_types::{
+        BlobId,
         ContractId,
         Nonce,
     },
@@ -59,6 +60,8 @@ pub trait TxPoolDb: Send + Sync {
 
     fn contract_exist(&self, contract_id: &ContractId) -> StorageResult<bool>;
 
+    fn blob_exist(&self, blob_id: &BlobId) -> StorageResult<bool>;
+
     fn message(&self, message_id: &Nonce) -> StorageResult<Option<Message>>;
 }
 

diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs
@@ -343,10 +343,6 @@ impl<P2P, ViewProvider, GasPriceProvider, ConsensusProvider, MP>
         self.txpool.lock().find_one(&id)
     }
 
-    pub fn find_dependent(&self, ids: Vec<TxId>) -> Vec<ArcPoolTx> {
-        self.txpool.lock().find_dependent(&ids)
-    }
-
     pub fn select_transactions(&self, max_gas: u64) -> Vec<ArcPoolTx> {
         let mut guard = self.txpool.lock();
         let txs = guard.includable();

diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs
@@ -60,7 +60,6 @@ use fuel_core_types::{
     services::executor::TransactionExecutionStatus,
 };
 use std::{
-    cmp::Reverse,
     collections::HashMap,
     ops::Deref,
     sync::Arc,
@@ -173,27 +172,6 @@ impl<ViewProvider> TxPool<ViewProvider> {
         self.txs().get(hash).cloned()
     }
 
-    /// find all dependent tx and return them with requested dependencies in one list sorted by Price.
-    pub fn find_dependent(&self, hashes: &[TxId]) -> Vec<ArcPoolTx> {
-        let mut seen = HashMap::new();
-        {
-            for hash in hashes {
-                if let Some(tx) = self.txs().get(hash) {
-                    self.dependency().find_dependent(
-                        tx.tx().clone(),
-                        &mut seen,
-                        self.txs(),
-                    );
-                }
-            }
-        }
-        let mut list: Vec<_> = seen.into_values().collect();
-        // sort from high to low price
-        list.sort_by_key(|tx| Reverse(tx.tip()));
-
-        list
-    }
-
     /// The number of pending transaction in the pool.
     pub fn pending_number(&self) -> usize {
         self.by_hash.len()