refactor: move TxPoolError from fuel-core-types to fuel-core-txpool (#…

…2088) ## Linked Issues/PRs - Closes #2082 ## Description This PR does three things: 1. The `Error` type defined in `crates/types/src/services/txpool.rs` is moved to `crates/services/txpool/src/error.rs`. 2. This error type now also implements `From<WasmValidityError>`. 3. Refactor to use `derive_more` instead of `thiserror` for the error type. ## 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 --------- Co-authored-by: Aaryamann Challani <[email protected]> Co-authored-by: Green Baneling <[email protected]>
FuelLabs · Aug 16, 2024 · 6effb64 · 6effb64
1 parent a2d8d2d
commit 6effb64
Show file tree

Hide file tree

Showing 12 changed files with 195 additions and 163 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,13 +13,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - [2076](https://github.com/FuelLabs/fuel-core/pull/2076): Replace usages of `iter_all` with `iter_all_keys` where necessary.
 
 #### Breaking
-
+- [2080](https://github.com/FuelLabs/fuel-core/pull/2080): Reject Upgrade txs with invalid wasm on txpool level.
+- [2082](https://github.com/FuelLabs/fuel-core/pull/2088): Move `TxPoolError` from `fuel-core-types` to `fuel-core-txpool`.
 - [2086](https://github.com/FuelLabs/fuel-core/pull/2086): Added support for PoA key rotation.
 - [2086](https://github.com/FuelLabs/fuel-core/pull/2086): Support overriding of the non consensus parameters in the chan config.
 
-### Breaking
--[2080](https://github.com/FuelLabs/fuel-core/pull/2080): Reject Upgrade txs with invalid wasm on txpool level.
-
 ## [Version 0.32.1]
 
 ### Added

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs b/crates/fuel-core/src/service/adapters/fuel_gas_price_provider.rs
@@ -4,14 +4,12 @@ use fuel_core_gas_price_service::{
     SharedGasPriceAlgo,
 };
 use fuel_core_producer::block_producer::gas_price::GasPriceProvider as ProducerGasPriceProvider;
-use fuel_core_txpool::ports::GasPriceProvider as TxPoolGasPriceProvider;
-use fuel_core_types::{
-    fuel_types::BlockHeight,
-    services::txpool::{
-        Error as TxPoolError,
-        Result as TxPoolResult,
-    },
+use fuel_core_txpool::{
+    ports::GasPriceProvider as TxPoolGasPriceProvider,
+    Error as TxPoolError,
+    Result as TxPoolResult,
 };
+use fuel_core_types::fuel_types::BlockHeight;
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 

diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs
@@ -21,11 +21,14 @@ use fuel_core_storage::{
     Result as StorageResult,
     StorageAsRef,
 };
-use fuel_core_txpool::ports::{
-    BlockImporter,
-    ConsensusParametersProvider as ConsensusParametersProviderTrait,
-    GasPriceProvider,
-    MemoryPool,
+use fuel_core_txpool::{
+    ports::{
+        BlockImporter,
+        ConsensusParametersProvider as ConsensusParametersProviderTrait,
+        GasPriceProvider,
+        MemoryPool,
+    },
+    Result as TxPoolResult,
 };
 use fuel_core_types::{
     blockchain::header::ConsensusParametersVersion,
@@ -51,7 +54,6 @@ use fuel_core_types::{
             GossipsubMessageInfo,
             TransactionGossipData,
         },
-        txpool::Result as TxPoolResult,
     },
 };
 use std::sync::Arc;

diff --git a/crates/services/txpool/Cargo.toml b/crates/services/txpool/Cargo.toml
@@ -13,6 +13,7 @@ description = "Transaction pool"
 [dependencies]
 anyhow = { workspace = true }
 async-trait = { workspace = true }
+derive_more = { workspace = true }
 fuel-core-metrics = { workspace = true }
 fuel-core-services = { workspace = true }
 fuel-core-storage = { workspace = true }

diff --git a/crates/services/txpool/src/error.rs b/crates/services/txpool/src/error.rs
@@ -0,0 +1,162 @@
+use fuel_core_types::{
+    fuel_tx::{
+        Address,
+        BlobId,
+        ContractId,
+        TxId,
+        UtxoId,
+    },
+    fuel_types::Nonce,
+    fuel_vm::checked_transaction::CheckError,
+};
+
+use crate::ports::WasmValidityError;
+
+pub type Result<T> = core::result::Result<T, Error>;
+
+#[allow(missing_docs)]
+#[derive(Debug, Clone, derive_more::Display)]
+#[non_exhaustive]
+pub enum Error {
+    #[display(fmt = "Gas price not found for block height {_0}")]
+    GasPriceNotFound(String),
+    #[display(fmt = "Failed to calculate max block bytes: {_0}")]
+    MaxBlockBytes(String),
+    #[display(fmt = "TxPool requires that the transaction contains metadata")]
+    NoMetadata,
+    #[display(fmt = "TxPool doesn't support this type of transaction.")]
+    NotSupportedTransactionType,
+    #[display(fmt = "Transaction is not inserted. Hash is already known")]
+    NotInsertedTxKnown,
+    #[display(
+        fmt = "Transaction is not inserted. Pool limit is hit, try to increase gas_price"
+    )]
+    NotInsertedLimitHit,
+    #[display(
+        fmt = "Transaction is not inserted. Attempting to upgrade WASM bytecode when WASM support is not enabled."
+    )]
+    NotInsertedWasmNotEnabled,
+    #[display(
+        fmt = "Transaction is not inserted. WASM bytecode matching the given root was not found."
+    )]
+    NotInsertedWasmNotFound,
+    #[display(
+        fmt = "Transaction is not inserted. WASM bytecode contents are not valid."
+    )]
+    NotInsertedInvalidWasm,
+    #[display(fmt = "Transaction is not inserted. The gas price is too low.")]
+    NotInsertedGasPriceTooLow,
+    #[display(
+        fmt = "Transaction is not inserted. Higher priced tx {_0:#x} has already spent this UTXO output: {_1:#x}"
+    )]
+    NotInsertedCollision(TxId, UtxoId),
+    #[display(
+        fmt = "Transaction is not inserted. Higher priced tx has created contract with ContractId {_0:#x}"
+    )]
+    NotInsertedCollisionContractId(ContractId),
+    #[display(
+        fmt = "Transaction is not inserted. Higher priced tx has uploaded the blob with BlobId {_0:#x}"
+    )]
+    NotInsertedCollisionBlobId(BlobId),
+    #[display(
+        fmt = "Transaction is not inserted. A higher priced tx {_0:#x} is already spending this message: {_1:#x}"
+    )]
+    NotInsertedCollisionMessageId(TxId, Nonce),
+    #[display(fmt = "Transaction is not inserted. UTXO input does not exist: {_0:#x}")]
+    NotInsertedInputContractDoesNotExist(ContractId),
+    #[display(fmt = "Transaction is not inserted. ContractId is already taken {_0:#x}")]
+    NotInsertedContractIdAlreadyTaken(ContractId),
+    #[display(fmt = "Transaction is not inserted. BlobId is already taken {_0:#x}")]
+    NotInsertedBlobIdAlreadyTaken(BlobId),
+    #[display(fmt = "Transaction is not inserted. UTXO does not exist: {_0:#x}")]
+    NotInsertedInputUtxoIdNotDoesNotExist(UtxoId),
+    #[display(fmt = "Transaction is not inserted. UTXO is spent: {_0:#x}")]
+    NotInsertedInputUtxoIdSpent(UtxoId),
+    #[display(
+        fmt = "Transaction is not inserted. Message id {_0:#x} does not match any received message from the DA layer."
+    )]
+    NotInsertedInputMessageUnknown(Nonce),
+    #[display(
+        fmt = "Transaction is not inserted. UTXO requires Contract input {_0:#x} that is priced lower"
+    )]
+    NotInsertedContractPricedLower(ContractId),
+    #[display(
+        fmt = "Transaction is not inserted. Input coin does not match the values from database"
+    )]
+    NotInsertedIoCoinMismatch,
+    #[display(
+        fmt = "Transaction is not inserted. Input output mismatch. Coin owner is different from expected input"
+    )]
+    NotInsertedIoWrongOwner,
+    #[display(
+        fmt = "Transaction is not inserted. Input output mismatch. Coin output does not match expected input"
+    )]
+    NotInsertedIoWrongAmount,
+    #[display(
+        fmt = "Transaction is not inserted. Input output mismatch. Coin output asset_id does not match expected inputs"
+    )]
+    NotInsertedIoWrongAssetId,
+    #[display(
+        fmt = "Transaction is not inserted. Input message does not match the values from database"
+    )]
+    NotInsertedIoMessageMismatch,
+    #[display(
+        fmt = "Transaction is not inserted. Input output mismatch. Expected coin but output is contract"
+    )]
+    NotInsertedIoContractOutput,
+    #[display(
+        fmt = "Transaction is not inserted. Maximum depth of dependent transaction chain reached"
+    )]
+    NotInsertedMaxDepth,
+    // small todo for now it can pass but in future we should include better messages
+    #[display(fmt = "Transaction removed.")]
+    Removed,
+    #[display(
+        fmt = "Transaction expired because it exceeded the configured time to live `tx-pool-ttl`."
+    )]
+    TTLReason,
+    #[display(fmt = "Transaction squeezed out because {_0}")]
+    SqueezedOut(String),
+    #[display(fmt = "Invalid transaction data: {_0:?}")]
+    ConsensusValidity(CheckError),
+    #[display(fmt = "Mint transactions are disallowed from the txpool")]
+    MintIsDisallowed,
+    #[display(fmt = "The UTXO `{_0}` is blacklisted")]
+    BlacklistedUTXO(UtxoId),
+    #[display(fmt = "The owner `{_0}` is blacklisted")]
+    BlacklistedOwner(Address),
+    #[display(fmt = "The contract `{_0}` is blacklisted")]
+    BlacklistedContract(ContractId),
+    #[display(fmt = "The message `{_0}` is blacklisted")]
+    BlacklistedMessage(Nonce),
+    #[display(fmt = "Database error: {_0}")]
+    Database(String),
+    // TODO: We need it for now until channels are removed from TxPool.
+    #[display(fmt = "Got some unexpected error: {_0}")]
+    Other(String),
+}
+
+impl std::error::Error for Error {}
+
+#[cfg(feature = "test-helpers")]
+impl PartialEq for Error {
+    fn eq(&self, other: &Self) -> bool {
+        self.to_string().eq(&other.to_string())
+    }
+}
+
+impl From<CheckError> for Error {
+    fn from(e: CheckError) -> Self {
+        Error::ConsensusValidity(e)
+    }
+}
+
+impl From<WasmValidityError> for Error {
+    fn from(err: WasmValidityError) -> Self {
+        match err {
+            WasmValidityError::NotEnabled => Error::NotInsertedWasmNotEnabled,
+            WasmValidityError::NotFound => Error::NotInsertedWasmNotFound,
+            WasmValidityError::NotValid => Error::NotInsertedInvalidWasm,
+        }
+    }
+}
diff --git a/crates/services/txpool/src/lib.rs b/crates/services/txpool/src/lib.rs
@@ -17,6 +17,7 @@ use std::{
 
 pub mod config;
 mod containers;
+pub mod error;
 pub mod ports;
 pub mod service;
 mod transaction_selector;
@@ -29,7 +30,10 @@ pub mod mock_db;
 pub use mock_db::MockDb;
 
 pub use config::Config;
-pub use fuel_core_types::services::txpool::Error;
+pub use error::{
+    Error,
+    Result,
+};
 pub use service::{
     new_service,
     Service,

diff --git a/crates/services/txpool/src/ports.rs b/crates/services/txpool/src/ports.rs
@@ -1,4 +1,7 @@
-use crate::types::GasPrice;
+use crate::{
+    types::GasPrice,
+    Result as TxPoolResult,
+};
 use fuel_core_services::stream::BoxStream;
 use fuel_core_storage::Result as StorageResult;
 use fuel_core_types::{
@@ -26,7 +29,6 @@ use fuel_core_types::{
             GossipsubMessageInfo,
             NetworkData,
         },
-        txpool::Result as TxPoolResult,
     },
 };
 use std::{

diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs
@@ -39,7 +39,6 @@ use fuel_core_types::{
         },
         txpool::{
             ArcPoolTx,
-            Error,
             InsertionResult,
             TransactionStatus,
         },
@@ -247,7 +246,7 @@ where
             _ = self.ttl_timer.tick() => {
                 let removed = self.tx_pool_shared_state.txpool.lock().prune_old_txs();
                 for tx in removed {
-                    self.tx_pool_shared_state.tx_status_sender.send_squeezed_out(tx.id(), Error::TTLReason);
+                    self.tx_pool_shared_state.tx_status_sender.send_squeezed_out(tx.id(), TxPoolError::TTLReason);
                 }
 
                 should_continue = true
@@ -311,7 +310,7 @@ where
                                 },
                                 // Use similar p2p punishment rules as bitcoin
                                 // https://github.com/bitcoin/bitcoin/blob/6ff0aa089c01ff3e610ecb47814ed739d685a14c/src/net_processing.cpp#L1856
-                                Some(Err(Error::ConsensusValidity(_))) | Some(Err(Error::MintIsDisallowed)) => {
+                                Some(Err(TxPoolError::ConsensusValidity(_))) | Some(Err(TxPoolError::MintIsDisallowed)) => {
                                     GossipsubMessageAcceptance::Reject
                                 },
                                 _ => GossipsubMessageAcceptance::Ignore
@@ -430,7 +429,7 @@ where
     pub async fn insert(
         &self,
         txs: Vec<Arc<Transaction>>,
-    ) -> Vec<Result<InsertionResult, Error>> {
+    ) -> Vec<Result<InsertionResult, TxPoolError>> {
         // verify txs
         let current_height = *self.current_height.lock();
         let (version, params) = self
@@ -489,7 +488,7 @@ where
             .into_iter()
             .map(|check_result| match check_result {
                 None => insertion.next().unwrap_or_else(|| {
-                    Err(Error::Other(
+                    Err(TxPoolError::Other(
                         anyhow::anyhow!("Insertion result is missing").to_string(),
                     ))
                 }),

diff --git a/crates/services/txpool/src/service/test_helpers.rs b/crates/services/txpool/src/service/test_helpers.rs
@@ -8,6 +8,7 @@ use crate::{
     test_helpers::MockWasmChecker,
     types::GasPrice,
     MockDb,
+    Result as TxPoolResult,
 };
 use fuel_core_services::{
     stream::BoxStream,
@@ -31,7 +32,6 @@ use fuel_core_types::{
     services::{
         block_importer::ImportResult,
         p2p::GossipsubMessageAcceptance,
-        txpool::Result as TxPoolResult,
     },
 };
 use std::cell::RefCell;

diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs
@@ -7,7 +7,6 @@ use crate::{
     ports::{
         TxPoolDb,
         WasmChecker as WasmCheckerConstraint,
-        WasmValidityError,
     },
     service::TxStatusChange,
     types::*,
@@ -374,14 +373,7 @@ where
             if let UpgradePurpose::StateTransition { root } =
                 upgrade.transaction().upgrade_purpose()
             {
-                if let Err(err) = self.wasm_checker.validate_uploaded_wasm(root) {
-                    // TODO: should be a From<_> impl, see https://github.com/FuelLabs/fuel-core/issues/2082
-                    return Err(match err {
-                        WasmValidityError::NotEnabled => Error::NotInsertedWasmNotEnabled,
-                        WasmValidityError::NotFound => Error::NotInsertedWasmNotFound,
-                        WasmValidityError::NotValid => Error::NotInsertedInvalidWasm,
-                    });
-                }
+                self.wasm_checker.validate_uploaded_wasm(root)?;
             }
         }