From f49484a27f40928cba8947f6a812da745caa0300 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 5 Mar 2024 13:57:26 -0800 Subject: [PATCH 01/19] Add `StaticGasPrice` impl for `ProducerGasPrice` --- .../src/service/adapters/producer.rs | 24 +++++++++++++- crates/fuel-core/src/service/sub_services.rs | 5 +++ .../services/producer/src/block_producer.rs | 32 ++++++++++++++----- .../producer/src/block_producer/gas_price.rs | 29 +++++++++++++++++ 4 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 crates/services/producer/src/block_producer/gas_price.rs diff --git a/crates/fuel-core/src/service/adapters/producer.rs b/crates/fuel-core/src/service/adapters/producer.rs index 573bc66879..6c31efd75c 100644 --- a/crates/fuel-core/src/service/adapters/producer.rs +++ b/crates/fuel-core/src/service/adapters/producer.rs @@ -12,7 +12,13 @@ use crate::{ }, }; use fuel_core_executor::executor::OnceTransactionsSource; -use fuel_core_producer::ports::TxPool; +use fuel_core_producer::{ + block_producer::gas_price::{ + GasPriceParams, + ProducerGasPrice, + }, + ports::TxPool, +}; use fuel_core_storage::{ not_found, tables::FuelBlocks, @@ -140,3 +146,19 @@ impl fuel_core_producer::ports::BlockProducerDatabase for Database { self.storage::().root(height).map(Into::into) } } + +pub struct StaticGasPrice { + pub gas_price: u64, +} + +impl StaticGasPrice { + pub fn new(gas_price: u64) -> Self { + Self { gas_price } + } +} + +impl ProducerGasPrice for StaticGasPrice { + fn gas_price(&self, _params: GasPriceParams) -> u64 { + self.gas_price + } +} diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 3840b04815..78341aff73 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -28,6 +28,7 @@ use tokio::sync::Mutex; #[cfg(feature = "relayer")] use crate::relayer::Config as RelayerConfig; +use crate::service::adapters::producer::StaticGasPrice; #[cfg(feature = "relayer")] use fuel_core_types::blockchain::primitives::DaBlockHeight; @@ -40,6 +41,7 @@ pub type BlockProducerService = fuel_core_producer::block_producer::Producer< Database, TxPoolAdapter, ExecutorAdapter, + StaticGasPrice, >; pub type GraphQL = fuel_core_graphql_api::api_service::Service; @@ -141,6 +143,8 @@ pub fn init_sub_services( ); let tx_pool_adapter = TxPoolAdapter::new(txpool.shared.clone()); + let gas_price_provider = StaticGasPrice::new(config.block_producer.gas_price); + let block_producer = fuel_core_producer::Producer { config: config.block_producer.clone(), view_provider: database.on_chain().clone(), @@ -148,6 +152,7 @@ pub fn init_sub_services( executor: Arc::new(executor), relayer: Box::new(relayer_adapter.clone()), lock: Mutex::new(()), + gas_price_provider, }; let producer_adapter = BlockProducerAdapter::new(block_producer); diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index 7326b647d2..8b382ef765 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -1,4 +1,5 @@ use crate::{ + block_producer::gas_price::ProducerGasPrice, ports, ports::BlockProducerDatabase, Config, @@ -42,6 +43,8 @@ use tracing::debug; #[cfg(test)] mod tests; +pub mod gas_price; + #[derive(Debug, derive_more::Display)] pub enum Error { #[display(fmt = "Genesis block is absent")] @@ -70,7 +73,7 @@ impl From for anyhow::Error { } } -pub struct Producer { +pub struct Producer { pub config: Config, pub view_provider: ViewProvider, pub txpool: TxPool, @@ -79,12 +82,15 @@ pub struct Producer { // use a tokio lock since we want callers to yield until the previous block // execution has completed (which may take a while). pub lock: Mutex<()>, + pub gas_price_provider: GasPriceProvider, } -impl Producer +impl + Producer where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, + GasPriceProvider: ProducerGasPrice, { /// Produces and execute block for the specified height. async fn produce_and_execute( @@ -112,11 +118,13 @@ where let header = self.new_header(height, block_time).await?; + let gas_price = self.gas_price_provider.gas_price(height.into()); + let component = Components { header_to_produce: header, transactions_source: source, // TODO: Provide gas price https://github.com/FuelLabs/fuel-core/issues/1642 - gas_price: self.config.gas_price, + gas_price, gas_limit: max_gas, }; @@ -134,12 +142,14 @@ where } } -impl Producer +impl + Producer where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, TxPool: ports::TxPool + 'static, Executor: ports::Executor + 'static, + GasPriceProvider: ProducerGasPrice, { /// Produces and execute block for the specified height with transactions from the `TxPool`. pub async fn produce_and_execute_block_txpool( @@ -158,11 +168,13 @@ where } } -impl Producer +impl + Producer where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, Executor: ports::Executor> + 'static, + GasPriceProvider: ProducerGasPrice, { /// Produces and execute block for the specified height with `transactions`. pub async fn produce_and_execute_block_transactions( @@ -177,11 +189,13 @@ where } } -impl Producer +impl + Producer where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, Executor: ports::DryRunner + 'static, + GasPriceProvider: ProducerGasPrice, { // TODO: Support custom `block_time` for `dry_run`. /// Simulates multiple transactions without altering any state. Does not acquire the production lock. @@ -201,6 +215,8 @@ where .expect("It is impossible to overflow the current block height") }); + let gas_price = self.gas_price_provider.gas_price(height.into()); + // The dry run execution should use the state of the blockchain based on the // last available block, not on the upcoming one. It means that we need to // use the same configuration as the last block -> the same DA height. @@ -211,7 +227,7 @@ where header_to_produce: header, transactions_source: transactions.clone(), // TODO: Provide gas price https://github.com/FuelLabs/fuel-core/issues/1642 - gas_price: self.config.gas_price, + gas_price, gas_limit: u64::MAX, }; @@ -239,7 +255,7 @@ where } } -impl Producer +impl Producer where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs new file mode 100644 index 0000000000..a7c162828b --- /dev/null +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -0,0 +1,29 @@ +use fuel_core_types::fuel_types::BlockHeight; + +/// The parameters required to retrieve the gas price for a block +pub struct GasPriceParams { + block_height: BlockHeight, +} + +impl GasPriceParams { + /// Create a new `GasPriceParams` instance + pub fn new(block_height: BlockHeight) -> Self { + Self { block_height } + } + + pub fn block_height(&self) -> BlockHeight { + self.block_height + } +} + +impl From for GasPriceParams { + fn from(block_height: BlockHeight) -> Self { + Self { block_height } + } +} + +/// Interface for retrieving the gas price for a block +pub trait ProducerGasPrice { + /// The gas price for all transactions in the block. + fn gas_price(&self, params: GasPriceParams) -> u64; +} From e0ae5d155cd801498f3d5e93ce9727a9ba654b44 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 13 Mar 2024 14:22:34 -0700 Subject: [PATCH 02/19] Use static gas price adapter in test harness --- .../src/service/adapters/producer.rs | 24 +------------------ crates/fuel-core/src/service/sub_services.rs | 2 +- .../producer/src/block_producer/gas_price.rs | 16 +++++++++++++ .../producer/src/block_producer/tests.rs | 9 +++++-- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/producer.rs b/crates/fuel-core/src/service/adapters/producer.rs index 6c31efd75c..573bc66879 100644 --- a/crates/fuel-core/src/service/adapters/producer.rs +++ b/crates/fuel-core/src/service/adapters/producer.rs @@ -12,13 +12,7 @@ use crate::{ }, }; use fuel_core_executor::executor::OnceTransactionsSource; -use fuel_core_producer::{ - block_producer::gas_price::{ - GasPriceParams, - ProducerGasPrice, - }, - ports::TxPool, -}; +use fuel_core_producer::ports::TxPool; use fuel_core_storage::{ not_found, tables::FuelBlocks, @@ -146,19 +140,3 @@ impl fuel_core_producer::ports::BlockProducerDatabase for Database { self.storage::().root(height).map(Into::into) } } - -pub struct StaticGasPrice { - pub gas_price: u64, -} - -impl StaticGasPrice { - pub fn new(gas_price: u64) -> Self { - Self { gas_price } - } -} - -impl ProducerGasPrice for StaticGasPrice { - fn gas_price(&self, _params: GasPriceParams) -> u64 { - self.gas_price - } -} diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 78341aff73..8b3ba689ce 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -23,12 +23,12 @@ use crate::{ }, }; use fuel_core_poa::Trigger; +use fuel_core_producer::block_producer::gas_price::StaticGasPrice; use std::sync::Arc; use tokio::sync::Mutex; #[cfg(feature = "relayer")] use crate::relayer::Config as RelayerConfig; -use crate::service::adapters::producer::StaticGasPrice; #[cfg(feature = "relayer")] use fuel_core_types::blockchain::primitives::DaBlockHeight; diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index a7c162828b..87b7894f8f 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -27,3 +27,19 @@ pub trait ProducerGasPrice { /// The gas price for all transactions in the block. fn gas_price(&self, params: GasPriceParams) -> u64; } + +pub struct StaticGasPrice { + pub gas_price: u64, +} + +impl StaticGasPrice { + pub fn new(gas_price: u64) -> Self { + Self { gas_price } + } +} + +impl ProducerGasPrice for StaticGasPrice { + fn gas_price(&self, _params: GasPriceParams) -> u64 { + self.gas_price + } +} diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index 8d68db2eea..b7d84d94b9 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -1,5 +1,8 @@ use crate::{ - block_producer::Error, + block_producer::{ + gas_price::StaticGasPrice, + Error, + }, mocks::{ FailingMockExecutor, MockDb, @@ -251,7 +254,8 @@ impl TestContext { } } - pub fn producer(self) -> Producer { + pub fn producer(self) -> Producer { + let gas_price = self.config.gas_price; Producer { config: self.config, view_provider: self.db, @@ -259,6 +263,7 @@ impl TestContext { executor: self.executor, relayer: Box::new(self.relayer), lock: Default::default(), + gas_price_provider: StaticGasPrice::new(gas_price), } } } From e6f9253accf82c261690f5d6ce4157d280776035 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 13 Mar 2024 14:33:53 -0700 Subject: [PATCH 03/19] MAke `gas_price` fallible --- Cargo.lock | 1 + crates/services/producer/Cargo.toml | 1 + crates/services/producer/src/block_producer.rs | 4 ++-- .../producer/src/block_producer/gas_price.rs | 13 ++++++++++--- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6370d40657..31e66a45dd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3075,6 +3075,7 @@ dependencies = [ "fuel-core-trace", "fuel-core-types", "rand", + "thiserror", "tokio", "tokio-rayon", "tracing", diff --git a/crates/services/producer/Cargo.toml b/crates/services/producer/Cargo.toml index fee135e14b..fbe5da2fcd 100644 --- a/crates/services/producer/Cargo.toml +++ b/crates/services/producer/Cargo.toml @@ -18,6 +18,7 @@ fuel-core-types = { workspace = true } tokio = { workspace = true, features = ["full"] } tokio-rayon = { workspace = true } tracing = { workspace = true } +thiserror = "1.0.57" [dev-dependencies] fuel-core-producer = { path = "", features = ["test-helpers"] } diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index 8b382ef765..286d539ac9 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -118,7 +118,7 @@ where let header = self.new_header(height, block_time).await?; - let gas_price = self.gas_price_provider.gas_price(height.into()); + let gas_price = self.gas_price_provider.gas_price(height.into())?; let component = Components { header_to_produce: header, @@ -215,7 +215,7 @@ where .expect("It is impossible to overflow the current block height") }); - let gas_price = self.gas_price_provider.gas_price(height.into()); + let gas_price = self.gas_price_provider.gas_price(height.into())?; // The dry run execution should use the state of the blockchain based on the // last available block, not on the upcoming one. It means that we need to diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index 87b7894f8f..b0666a3185 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -1,4 +1,5 @@ use fuel_core_types::fuel_types::BlockHeight; +use thiserror::Error; /// The parameters required to retrieve the gas price for a block pub struct GasPriceParams { @@ -22,10 +23,16 @@ impl From for GasPriceParams { } } +#[derive(Error, Debug)] +pub enum GasPriceError { + #[error("Failed to retrieve gas price for block: {0}")] + GasPriceRetrievalFailed(anyhow::Error), +} + /// Interface for retrieving the gas price for a block pub trait ProducerGasPrice { /// The gas price for all transactions in the block. - fn gas_price(&self, params: GasPriceParams) -> u64; + fn gas_price(&self, params: GasPriceParams) -> Result; } pub struct StaticGasPrice { @@ -39,7 +46,7 @@ impl StaticGasPrice { } impl ProducerGasPrice for StaticGasPrice { - fn gas_price(&self, _params: GasPriceParams) -> u64 { - self.gas_price + fn gas_price(&self, _params: GasPriceParams) -> Result { + Ok(self.gas_price) } } From 7beee61fc06f8780f47ddc6f6fc741e91eb6a576 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 13 Mar 2024 16:49:21 -0700 Subject: [PATCH 04/19] Remove TODO, update CHANGELOG --- CHANGELOG.md | 1 + crates/services/producer/src/block_producer.rs | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd9152bd03..67b67b37b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Description of the upcoming release here. ### Added +- [#1752](https://github.com/FuelLabs/fuel-core/pull/1752): Add `ProducerGasPrice` trait that the `Producer` depends on to get the gas price for the block. - [#1747](https://github.com/FuelLabs/fuel-core/pull/1747): The DA block height is now included in the genesis state. - [#1740](https://github.com/FuelLabs/fuel-core/pull/1740): Remove optional fields from genesis configs - [#1737](https://github.com/FuelLabs/fuel-core/pull/1737): Remove temporary tables for calculating roots during genesis. diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index 286d539ac9..71a5efdee6 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -123,7 +123,6 @@ where let component = Components { header_to_produce: header, transactions_source: source, - // TODO: Provide gas price https://github.com/FuelLabs/fuel-core/issues/1642 gas_price, gas_limit: max_gas, }; @@ -226,7 +225,6 @@ where let component = Components { header_to_produce: header, transactions_source: transactions.clone(), - // TODO: Provide gas price https://github.com/FuelLabs/fuel-core/issues/1642 gas_price, gas_limit: u64::MAX, }; From 55ac2ee0bf24812f35c07c19dc529b596e5490d3 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 13 Mar 2024 16:50:15 -0700 Subject: [PATCH 05/19] Sort `Cargo.toml` --- crates/services/producer/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/producer/Cargo.toml b/crates/services/producer/Cargo.toml index fbe5da2fcd..3c0a7bcc41 100644 --- a/crates/services/producer/Cargo.toml +++ b/crates/services/producer/Cargo.toml @@ -15,10 +15,10 @@ async-trait = { workspace = true } derive_more = { workspace = true } fuel-core-storage = { workspace = true } fuel-core-types = { workspace = true } +thiserror = "1.0.57" tokio = { workspace = true, features = ["full"] } tokio-rayon = { workspace = true } tracing = { workspace = true } -thiserror = "1.0.57" [dev-dependencies] fuel-core-producer = { path = "", features = ["test-helpers"] } From 7eadf7a0b0e4de24295e5cf8a689b735c7b589b0 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 14 Mar 2024 12:14:10 -0700 Subject: [PATCH 06/19] Add unimplemented tests --- .../producer/src/block_producer/tests.rs | 50 ++++++++++++++++++- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index b7d84d94b9..a7a7901f9d 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -1,6 +1,10 @@ +#![allow(non_snake_case)] use crate::{ block_producer::{ - gas_price::StaticGasPrice, + gas_price::{ + ProducerGasPrice, + StaticGasPrice, + }, Error, }, mocks::{ @@ -206,6 +210,37 @@ async fn production_fails_on_execution_error() { ); } +// TODO: Add test that checks the gas price on the mint tx after `Executor` refactor +// https://github.com/FuelLabs/fuel-core/issues/1751 +#[ignore] +#[tokio::test] +async fn produce_and_execute_block_txpool__includes_gas_price_provided_on_mint_tx() { + todo!() + // // given + // let gas_price = 1_000; + // let gas_price_provider = StaticGasPrice::new(gas_price); + // let ctx = TestContext::default(); + // let producer = ctx.producer_with_gas_price_provider(gas_price_provider); + // + // // when + // let changes = producer + // .produce_and_execute_block_txpool(1u32.into(), Tai64::now(), 1_000_000_000) + // .await + // .unwrap(); + // + // // then + // let mint_tx = changes.get_mint_tx(); + // assert_eq!(mint_tx.gas_price, gas_price); +} + +// TODO: Add test that checks the failure case for gas_price after `Executor` refactor +// https://github.com/FuelLabs/fuel-core/issues/1751 +#[ignore] +#[tokio::test] +async fn produce_and_execute_block_txpool__block_production_fails() { + todo!() +} + struct TestContext { config: Config, db: MockDb, @@ -256,6 +291,17 @@ impl TestContext { pub fn producer(self) -> Producer { let gas_price = self.config.gas_price; + let static_gas_price = StaticGasPrice::new(gas_price); + self.producer_with_gas_price_provider(static_gas_price) + } + + pub fn producer_with_gas_price_provider( + self, + gas_price_provider: GasPriceProvider, + ) -> Producer + where + GasPriceProvider: ProducerGasPrice, + { Producer { config: self.config, view_provider: self.db, @@ -263,7 +309,7 @@ impl TestContext { executor: self.executor, relayer: Box::new(self.relayer), lock: Default::default(), - gas_price_provider: StaticGasPrice::new(gas_price), + gas_price_provider, } } } From bd6931601eba37c568614421ba433611402782c9 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 14 Mar 2024 12:23:09 -0700 Subject: [PATCH 07/19] Refactor gas_price for configs --- bin/fuel-core/src/cli/run.rs | 2 +- crates/fuel-core/src/service/config.rs | 9 +++++---- crates/fuel-core/src/service/sub_services.rs | 2 +- crates/services/producer/src/config.rs | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bin/fuel-core/src/cli/run.rs b/bin/fuel-core/src/cli/run.rs index 0b846b5f59..ce0f1d5742 100644 --- a/bin/fuel-core/src/cli/run.rs +++ b/bin/fuel-core/src/cli/run.rs @@ -354,9 +354,9 @@ impl Command { block_producer: ProducerConfig { utxo_validation, coinbase_recipient, - gas_price: min_gas_price, metrics, }, + static_gas_price: min_gas_price, block_importer, #[cfg(feature = "relayer")] relayer: relayer_cfg, diff --git a/crates/fuel-core/src/service/config.rs b/crates/fuel-core/src/service/config.rs index b51b26553b..f2f6101873 100644 --- a/crates/fuel-core/src/service/config.rs +++ b/crates/fuel-core/src/service/config.rs @@ -55,6 +55,7 @@ pub struct Config { pub vm: VMConfig, pub txpool: fuel_core_txpool::Config, pub block_producer: fuel_core_producer::Config, + pub static_gas_price: u64, pub block_importer: fuel_core_importer::Config, #[cfg(feature = "relayer")] pub relayer: Option, @@ -111,9 +112,9 @@ impl Config { ..fuel_core_txpool::Config::default() }, block_producer: fuel_core_producer::Config { - gas_price: min_gas_price, ..Default::default() }, + static_gas_price: min_gas_price, block_importer, #[cfg(feature = "relayer")] relayer: None, @@ -144,14 +145,14 @@ impl Config { self.txpool.chain_config = self.chain_config.clone(); } - if self.txpool.min_gas_price != self.block_producer.gas_price { + if self.txpool.min_gas_price != self.static_gas_price { tracing::warn!( "The `min_gas_price` of `TxPool` was inconsistent with `BlockProducer`" ); let min_gas_price = - core::cmp::max(self.txpool.min_gas_price, self.block_producer.gas_price); + core::cmp::max(self.txpool.min_gas_price, self.static_gas_price); self.txpool.min_gas_price = min_gas_price; - self.block_producer.gas_price = min_gas_price; + self.static_gas_price = min_gas_price; } if self.txpool.utxo_validation != self.utxo_validation { tracing::warn!("The `utxo_validation` of `TxPool` was inconsistent"); diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 8b3ba689ce..874aed1437 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -143,7 +143,7 @@ pub fn init_sub_services( ); let tx_pool_adapter = TxPoolAdapter::new(txpool.shared.clone()); - let gas_price_provider = StaticGasPrice::new(config.block_producer.gas_price); + let gas_price_provider = StaticGasPrice::new(config.static_gas_price); let block_producer = fuel_core_producer::Producer { config: config.block_producer.clone(), diff --git a/crates/services/producer/src/config.rs b/crates/services/producer/src/config.rs index 4345b84b23..71efd451a1 100644 --- a/crates/services/producer/src/config.rs +++ b/crates/services/producer/src/config.rs @@ -4,6 +4,5 @@ use fuel_core_types::fuel_types::ContractId; pub struct Config { pub utxo_validation: bool, pub coinbase_recipient: Option, - pub gas_price: u64, pub metrics: bool, } From d4eb224dec329341765acc1bad278b0211aa33e0 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 14 Mar 2024 14:02:22 -0700 Subject: [PATCH 08/19] Use static gas price impl in tx pool code --- Cargo.lock | 1 - benches/benches/block_target_gas.rs | 2 +- benches/benches/transaction_throughput.rs | 2 +- bin/fuel-core/src/cli/run.rs | 1 - crates/fuel-core/src/p2p_test_helpers.rs | 2 +- crates/fuel-core/src/service.rs | 4 +- crates/fuel-core/src/service/adapters.rs | 40 ++++++++-- crates/fuel-core/src/service/config.rs | 10 --- crates/fuel-core/src/service/query.rs | 4 +- crates/fuel-core/src/service/sub_services.rs | 12 +-- crates/services/producer/Cargo.toml | 1 - .../services/producer/src/block_producer.rs | 10 ++- .../producer/src/block_producer/gas_price.rs | 25 +++--- crates/services/txpool/src/config.rs | 6 +- crates/services/txpool/src/service.rs | 76 ++++++++++++------- crates/services/txpool/src/txpool.rs | 37 ++++++--- crates/types/src/services/txpool.rs | 2 + tests/test-helpers/src/builder.rs | 1 - tests/tests/tx/txpool.rs | 2 +- 19 files changed, 148 insertions(+), 90 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31e66a45dd..6370d40657 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3075,7 +3075,6 @@ dependencies = [ "fuel-core-trace", "fuel-core-types", "rand", - "thiserror", "tokio", "tokio-rayon", "tracing", diff --git a/benches/benches/block_target_gas.rs b/benches/benches/block_target_gas.rs index 362a622004..cb1bc1617b 100644 --- a/benches/benches/block_target_gas.rs +++ b/benches/benches/block_target_gas.rs @@ -419,7 +419,7 @@ fn run_with_service_with_extra_inputs( let mut sub = shared.block_importer.block_importer.subscribe(); shared - .txpool + .txpool_shared_state .insert(vec![std::sync::Arc::new(tx)]) .await .into_iter() diff --git a/benches/benches/transaction_throughput.rs b/benches/benches/transaction_throughput.rs index c495f1b134..e61130a2f4 100644 --- a/benches/benches/transaction_throughput.rs +++ b/benches/benches/transaction_throughput.rs @@ -105,7 +105,7 @@ where test_builder.finalize().await; // insert all transactions - srv.shared.txpool.insert(transactions).await; + srv.shared.txpool_shared_state.insert(transactions).await; let _ = client.produce_blocks(1, None).await; // sanity check block to ensure the transactions were actually processed diff --git a/bin/fuel-core/src/cli/run.rs b/bin/fuel-core/src/cli/run.rs index ce0f1d5742..342263cd37 100644 --- a/bin/fuel-core/src/cli/run.rs +++ b/bin/fuel-core/src/cli/run.rs @@ -344,7 +344,6 @@ impl Command { tx_max_number, tx_max_depth, chain_conf, - min_gas_price, utxo_validation, metrics, tx_pool_ttl.into(), diff --git a/crates/fuel-core/src/p2p_test_helpers.rs b/crates/fuel-core/src/p2p_test_helpers.rs index 318b716ef8..bbb7746e97 100644 --- a/crates/fuel-core/src/p2p_test_helpers.rs +++ b/crates/fuel-core/src/p2p_test_helpers.rs @@ -472,7 +472,7 @@ impl Node { let tx_result = self .node .shared - .txpool + .txpool_shared_state .insert(vec![Arc::new(tx.clone())]) .await .pop() diff --git a/crates/fuel-core/src/service.rs b/crates/fuel-core/src/service.rs index b976ec96de..4bb003ecad 100644 --- a/crates/fuel-core/src/service.rs +++ b/crates/fuel-core/src/service.rs @@ -24,6 +24,7 @@ use fuel_core_storage::{ }; use std::net::SocketAddr; +use crate::service::adapters::StaticGasPrice; pub use config::{ Config, DbType, @@ -44,7 +45,8 @@ pub struct SharedState { /// The PoA adaptor around the shared state of the consensus module. pub poa_adapter: PoAAdapter, /// The transaction pool shared state. - pub txpool: fuel_core_txpool::service::SharedState, + pub txpool_shared_state: + fuel_core_txpool::service::SharedState, /// The P2P network shared state. #[cfg(feature = "p2p")] pub network: Option, diff --git a/crates/fuel-core/src/service/adapters.rs b/crates/fuel-core/src/service/adapters.rs index a688439c71..7a3aa06f86 100644 --- a/crates/fuel-core/src/service/adapters.rs +++ b/crates/fuel-core/src/service/adapters.rs @@ -10,8 +10,15 @@ use fuel_core_consensus_module::{ RelayerConsensusConfig, }; use fuel_core_executor::executor::Executor; +use fuel_core_producer::block_producer::gas_price::{ + GasPriceParams, + ProducerGasPrice, +}; use fuel_core_services::stream::BoxStream; -use fuel_core_txpool::service::SharedState as TxPoolSharedState; +use fuel_core_txpool::{ + service::SharedState as TxPoolSharedState, + txpool::TxPoolGasPrice, +}; #[cfg(feature = "p2p")] use fuel_core_types::services::p2p::peer_reputation::AppScore; use fuel_core_types::{ @@ -33,6 +40,29 @@ pub mod relayer; pub mod sync; pub mod txpool; +#[derive(Debug, Clone)] +pub struct StaticGasPrice { + pub gas_price: u64, +} + +impl StaticGasPrice { + pub fn new(gas_price: u64) -> Self { + Self { gas_price } + } +} + +impl TxPoolGasPrice for StaticGasPrice { + fn gas_price(&self, _block_height: BlockHeight) -> Option { + Some(self.gas_price) + } +} + +impl ProducerGasPrice for StaticGasPrice { + fn gas_price(&self, _block_height: GasPriceParams) -> Option { + Some(self.gas_price) + } +} + #[derive(Clone)] pub struct PoAAdapter { shared_state: Option, @@ -40,24 +70,24 @@ pub struct PoAAdapter { #[derive(Clone)] pub struct TxPoolAdapter { - service: TxPoolSharedState, + service: TxPoolSharedState, } impl TxPoolAdapter { - pub fn new(service: TxPoolSharedState) -> Self { + pub fn new(service: TxPoolSharedState) -> Self { Self { service } } } #[derive(Clone)] pub struct TransactionsSource { - txpool: TxPoolSharedState, + txpool: TxPoolSharedState, _block_height: BlockHeight, } impl TransactionsSource { pub fn new( - txpool: TxPoolSharedState, + txpool: TxPoolSharedState, block_height: BlockHeight, ) -> Self { Self { diff --git a/crates/fuel-core/src/service/config.rs b/crates/fuel-core/src/service/config.rs index f2f6101873..f81713bc58 100644 --- a/crates/fuel-core/src/service/config.rs +++ b/crates/fuel-core/src/service/config.rs @@ -106,7 +106,6 @@ impl Config { utxo_validation, txpool: fuel_core_txpool::Config { chain_config, - min_gas_price, utxo_validation, transaction_ttl: Duration::from_secs(60 * 100000000), ..fuel_core_txpool::Config::default() @@ -145,15 +144,6 @@ impl Config { self.txpool.chain_config = self.chain_config.clone(); } - if self.txpool.min_gas_price != self.static_gas_price { - tracing::warn!( - "The `min_gas_price` of `TxPool` was inconsistent with `BlockProducer`" - ); - let min_gas_price = - core::cmp::max(self.txpool.min_gas_price, self.static_gas_price); - self.txpool.min_gas_price = min_gas_price; - self.static_gas_price = min_gas_price; - } if self.txpool.utxo_validation != self.utxo_validation { tracing::warn!("The `utxo_validation` of `TxPool` was inconsistent"); self.txpool.utxo_validation = self.utxo_validation; diff --git a/crates/fuel-core/src/service/query.rs b/crates/fuel-core/src/service/query.rs index 2144a20cf3..15b915daff 100644 --- a/crates/fuel-core/src/service/query.rs +++ b/crates/fuel-core/src/service/query.rs @@ -26,7 +26,7 @@ impl FuelService { pub async fn submit(&self, tx: Transaction) -> anyhow::Result { let results: Vec<_> = self .shared - .txpool + .txpool_shared_state .insert(vec![Arc::new(tx)]) .await .into_iter() @@ -80,7 +80,7 @@ impl FuelService { &self, id: Bytes32, ) -> anyhow::Result>> { - let txpool = self.shared.txpool.clone(); + let txpool = self.shared.txpool_shared_state.clone(); let db = self.shared.database.off_chain().clone(); let rx = txpool.tx_update_subscribe(id)?; Ok(transaction_status_change( diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 874aed1437..754223afc9 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -23,12 +23,12 @@ use crate::{ }, }; use fuel_core_poa::Trigger; -use fuel_core_producer::block_producer::gas_price::StaticGasPrice; use std::sync::Arc; use tokio::sync::Mutex; #[cfg(feature = "relayer")] use crate::relayer::Config as RelayerConfig; +use crate::service::StaticGasPrice; #[cfg(feature = "relayer")] use fuel_core_types::blockchain::primitives::DaBlockHeight; @@ -36,7 +36,7 @@ pub type PoAService = fuel_core_poa::Service; #[cfg(feature = "p2p")] pub type P2PService = fuel_core_p2p::service::Service; -pub type TxPoolService = fuel_core_txpool::Service; +pub type TxPoolService = fuel_core_txpool::Service; pub type BlockProducerService = fuel_core_producer::block_producer::Producer< Database, TxPoolAdapter, @@ -134,17 +134,17 @@ pub fn init_sub_services( #[cfg(not(feature = "p2p"))] let p2p_adapter = P2PAdapter::new(); + let gas_price_provider = StaticGasPrice::new(config.static_gas_price); let txpool = fuel_core_txpool::new_service( config.txpool.clone(), database.on_chain().clone(), importer_adapter.clone(), p2p_adapter.clone(), last_height, + gas_price_provider.clone(), ); let tx_pool_adapter = TxPoolAdapter::new(txpool.shared.clone()); - let gas_price_provider = StaticGasPrice::new(config.static_gas_price); - let block_producer = fuel_core_producer::Producer { config: config.block_producer.clone(), view_provider: database.on_chain().clone(), @@ -208,7 +208,7 @@ pub fn init_sub_services( utxo_validation: config.utxo_validation, debug: config.debug, vm_backtrace: config.vm.backtrace, - min_gas_price: config.txpool.min_gas_price, + min_gas_price: config.static_gas_price, max_tx: config.txpool.max_tx, max_depth: config.txpool.max_depth, chain_name: config.chain_config.chain_name.clone(), @@ -231,7 +231,7 @@ pub fn init_sub_services( let shared = SharedState { poa_adapter, - txpool: txpool.shared.clone(), + txpool_shared_state: txpool.shared.clone(), #[cfg(feature = "p2p")] network: network.as_ref().map(|n| n.shared.clone()), #[cfg(feature = "relayer")] diff --git a/crates/services/producer/Cargo.toml b/crates/services/producer/Cargo.toml index 3c0a7bcc41..fee135e14b 100644 --- a/crates/services/producer/Cargo.toml +++ b/crates/services/producer/Cargo.toml @@ -15,7 +15,6 @@ async-trait = { workspace = true } derive_more = { workspace = true } fuel-core-storage = { workspace = true } fuel-core-types = { workspace = true } -thiserror = "1.0.57" tokio = { workspace = true, features = ["full"] } tokio-rayon = { workspace = true } tracing = { workspace = true } diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index 71a5efdee6..be56e675fa 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -118,7 +118,10 @@ where let header = self.new_header(height, block_time).await?; - let gas_price = self.gas_price_provider.gas_price(height.into())?; + let gas_price = self + .gas_price_provider + .gas_price(height.into()) + .ok_or(anyhow!("No gas price found for block {height:?}"))?; let component = Components { header_to_produce: header, @@ -214,7 +217,10 @@ where .expect("It is impossible to overflow the current block height") }); - let gas_price = self.gas_price_provider.gas_price(height.into())?; + let gas_price = self + .gas_price_provider + .gas_price(height.into()) + .ok_or(anyhow!("No gas price found for height {height:?}"))?; // The dry run execution should use the state of the blockchain based on the // last available block, not on the upcoming one. It means that we need to diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index b0666a3185..22c9a2b94b 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -1,5 +1,4 @@ use fuel_core_types::fuel_types::BlockHeight; -use thiserror::Error; /// The parameters required to retrieve the gas price for a block pub struct GasPriceParams { @@ -23,30 +22,26 @@ impl From for GasPriceParams { } } -#[derive(Error, Debug)] -pub enum GasPriceError { - #[error("Failed to retrieve gas price for block: {0}")] - GasPriceRetrievalFailed(anyhow::Error), -} - /// Interface for retrieving the gas price for a block pub trait ProducerGasPrice { /// The gas price for all transactions in the block. - fn gas_price(&self, params: GasPriceParams) -> Result; + fn gas_price(&self, params: GasPriceParams) -> Option; } -pub struct StaticGasPrice { - pub gas_price: u64, +pub struct MockGasPrice { + pub gas_price: Option, } -impl StaticGasPrice { +impl MockGasPrice { pub fn new(gas_price: u64) -> Self { - Self { gas_price } + Self { + gas_price: Some(gas_price), + } } } -impl ProducerGasPrice for StaticGasPrice { - fn gas_price(&self, _params: GasPriceParams) -> Result { - Ok(self.gas_price) +impl ProducerGasPrice for MockGasPrice { + fn gas_price(&self, _params: GasPriceParams) -> Option { + self.gas_price.clone() } } diff --git a/crates/services/txpool/src/config.rs b/crates/services/txpool/src/config.rs index 6c6a46d052..954d900362 100644 --- a/crates/services/txpool/src/config.rs +++ b/crates/services/txpool/src/config.rs @@ -63,7 +63,7 @@ pub struct Config { /// max depth of connected UTXO excluding contracts pub max_depth: usize, /// The minimum allowed gas price - pub min_gas_price: u64, + // pub min_gas_price: u64, /// Flag to disable utxo existence and signature checks pub utxo_validation: bool, /// chain config @@ -82,7 +82,6 @@ impl Default for Config { fn default() -> Self { let max_tx = 4064; let max_depth = 10; - let min_gas_price = 0; let utxo_validation = true; let metrics = false; // 5 minute TTL @@ -92,7 +91,6 @@ impl Default for Config { max_tx, max_depth, ChainConfig::default(), - min_gas_price, utxo_validation, metrics, transaction_ttl, @@ -108,7 +106,6 @@ impl Config { max_tx: usize, max_depth: usize, chain_config: ChainConfig, - min_gas_price: u64, utxo_validation: bool, metrics: bool, transaction_ttl: Duration, @@ -120,7 +117,6 @@ impl Config { Self { max_tx, max_depth, - min_gas_price, utxo_validation, chain_config, metrics, diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs index 9758cfaee1..8e92295f7e 100644 --- a/crates/services/txpool/src/service.rs +++ b/crates/services/txpool/src/service.rs @@ -50,6 +50,7 @@ use fuel_core_types::{ tai64::Tai64, }; +use crate::txpool::TxPoolGasPrice; use anyhow::anyhow; use fuel_core_storage::transactional::AtomicView; use fuel_core_types::services::block_importer::SharedImportResult; @@ -72,7 +73,7 @@ use self::update_sender::{ mod update_sender; -pub type Service = ServiceRunner>; +pub type Service = ServiceRunner>; #[derive(Clone)] pub struct TxStatusChange { @@ -120,16 +121,21 @@ impl TxStatusChange { } } -pub struct SharedState { +pub struct SharedState { tx_status_sender: TxStatusChange, txpool: Arc>>, p2p: Arc, consensus_params: ConsensusParameters, current_height: Arc>, config: Config, + gas_price_provider: GasPriceProvider, } -impl Clone for SharedState { +impl Clone + for SharedState +where + GasPriceProvider: Clone, +{ fn clone(&self) -> Self { Self { tx_status_sender: self.tx_status_sender.clone(), @@ -138,32 +144,35 @@ impl Clone for SharedState { consensus_params: self.consensus_params.clone(), current_height: self.current_height.clone(), config: self.config.clone(), + gas_price_provider: self.gas_price_provider.clone(), } } } -pub struct Task { +pub struct Task { gossiped_tx_stream: BoxStream, committed_block_stream: BoxStream, - shared: SharedState, + tx_pool_shared_state: SharedState, ttl_timer: tokio::time::Interval, } #[async_trait::async_trait] -impl RunnableService for Task +impl RunnableService + for Task where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, + GasPriceProvider: TxPoolGasPrice + Send + Sync + Clone, { const NAME: &'static str = "TxPool"; - type SharedData = SharedState; - type Task = Task; + type SharedData = SharedState; + type Task = Task; type TaskParams = (); fn shared_data(&self) -> Self::SharedData { - self.shared.clone() + self.tx_pool_shared_state.clone() } async fn into_task( @@ -177,11 +186,13 @@ where } #[async_trait::async_trait] -impl RunnableTask for Task +impl RunnableTask + for Task where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, + GasPriceProvider: TxPoolGasPrice + Send + Sync, { async fn run(&mut self, watcher: &mut StateWatcher) -> anyhow::Result { let should_continue; @@ -194,9 +205,9 @@ where } _ = self.ttl_timer.tick() => { - let removed = self.shared.txpool.lock().prune_old_txs(); + let removed = self.tx_pool_shared_state.txpool.lock().prune_old_txs(); for tx in removed { - self.shared.tx_status_sender.send_squeezed_out(tx.id(), Error::TTLReason); + self.tx_pool_shared_state.tx_status_sender.send_squeezed_out(tx.id(), Error::TTLReason); } should_continue = true @@ -209,11 +220,11 @@ where .entity.header().height(); { - let mut lock = self.shared.txpool.lock(); + let mut lock = self.tx_pool_shared_state.txpool.lock(); lock.block_update( &result.tx_status, ); - *self.shared.current_height.lock() = new_height; + *self.tx_pool_shared_state.current_height.lock() = new_height; } should_continue = true; } else { @@ -223,11 +234,11 @@ where new_transaction = self.gossiped_tx_stream.next() => { if let Some(GossipData { data: Some(tx), message_id, peer_id }) = new_transaction { - let id = tx.id(&self.shared.consensus_params.chain_id); - let current_height = *self.shared.current_height.lock(); + let id = tx.id(&self.tx_pool_shared_state.consensus_params.chain_id); + let current_height = *self.tx_pool_shared_state.current_height.lock(); // verify tx - let checked_tx = check_single_tx(tx, current_height, &self.shared.config).await; + let checked_tx = check_single_tx(tx, current_height, &self.tx_pool_shared_state.config, &self.tx_pool_shared_state.gas_price_provider).await; let acceptance = match checked_tx { Ok(tx) => { @@ -236,8 +247,8 @@ where // insert tx let mut result = tracing::info_span!("Received tx via gossip", %id) .in_scope(|| { - self.shared.txpool.lock().insert( - &self.shared.tx_status_sender, + self.tx_pool_shared_state.txpool.lock().insert( + &self.tx_pool_shared_state.tx_status_sender, txs ) }); @@ -265,7 +276,7 @@ where peer_id, }; - let _ = self.shared.p2p.notify_gossip_transaction_validity(message_info, acceptance); + let _ = self.tx_pool_shared_state.p2p.notify_gossip_transaction_validity(message_info, acceptance); should_continue = true; } else { @@ -289,7 +300,9 @@ where // Instead, `fuel-core` can create a `DatabaseWithTxPool` that aggregates `TxPool` and // storage `Database` together. GraphQL will retrieve data from this `DatabaseWithTxPool` via // `StorageInspect` trait. -impl SharedState { +impl + SharedState +{ pub fn pending_number(&self) -> usize { self.txpool.lock().pending_number() } @@ -354,11 +367,13 @@ impl SharedState { } } -impl SharedState +impl + SharedState where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, + GasPriceProvider: TxPoolGasPrice, { #[tracing::instrument(name = "insert_submitted_txn", skip_all)] pub async fn insert( @@ -368,7 +383,13 @@ where // verify txs let current_height = *self.current_height.lock(); - let checked_txs = check_transactions(&txs, current_height, &self.config).await; + let checked_txs = check_transactions( + &txs, + current_height, + &self.config, + &self.gas_price_provider, + ) + .await; let mut valid_txs = vec![]; @@ -444,18 +465,20 @@ pub enum TxStatusMessage { FailedStatus, } -pub fn new_service( +pub fn new_service( config: Config, provider: ViewProvider, importer: Importer, p2p: P2P, current_height: BlockHeight, -) -> Service + gas_price_provider: GasPriceProvider, +) -> Service where Importer: BlockImporter, P2P: PeerToPeer + 'static, ViewProvider: AtomicView, ViewProvider::View: TxPoolDb, + GasPriceProvider: TxPoolGasPrice + Send + Sync + Clone, { let p2p = Arc::new(p2p); let gossiped_tx_stream = p2p.gossiped_transaction_events(); @@ -468,7 +491,7 @@ where let task = Task { gossiped_tx_stream, committed_block_stream, - shared: SharedState { + tx_pool_shared_state: SharedState { tx_status_sender: TxStatusChange::new( number_of_active_subscription, // The connection should be closed automatically after the `SqueezedOut` event. @@ -482,6 +505,7 @@ where consensus_params, current_height: Arc::new(ParkingMutex::new(current_height)), config, + gas_price_provider, }, ttl_timer, }; diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 2106f64abf..7ff5312a48 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -76,6 +76,10 @@ pub struct TxPool { database: ViewProvider, } +pub trait TxPoolGasPrice { + fn gas_price(&self, block_height: BlockHeight) -> Option; +} + impl TxPool { pub fn new(config: Config, database: ViewProvider) -> Self { let max_depth = config.max_depth; @@ -463,25 +467,34 @@ where } } -pub async fn check_transactions( +pub async fn check_transactions( txs: &[Arc], current_height: BlockHeight, config: &Config, + gas_price_provider: &GasPriceProvider, ) -> Vec, Error>> { let mut checked_txs = Vec::with_capacity(txs.len()); for tx in txs.iter() { - checked_txs - .push(check_single_tx(tx.deref().clone(), current_height, config).await); + checked_txs.push( + check_single_tx( + tx.deref().clone(), + current_height, + config, + gas_price_provider, + ) + .await, + ); } checked_txs } -pub async fn check_single_tx( +pub async fn check_single_tx( tx: Transaction, current_height: BlockHeight, config: &Config, + gas_price_provider: &GasPrice, ) -> Result, Error> { if tx.is_mint() { return Err(Error::NotSupportedTransactionType) @@ -507,7 +520,11 @@ pub async fn check_single_tx( tx.into_checked_basic(current_height, &config.chain_config.consensus_parameters)? }; - let tx = verify_tx_min_gas_price(tx, config)?; + let gas_price = gas_price_provider + .gas_price(current_height) + .ok_or(Error::GasPriceNotFound(current_height))?; + + let tx = verify_tx_min_gas_price(tx, config, gas_price)?; Ok(tx) } @@ -515,20 +532,20 @@ pub async fn check_single_tx( fn verify_tx_min_gas_price( tx: Checked, config: &Config, + gas_price: GasPrice, ) -> Result, Error> { let tx: CheckedTransaction = tx.into(); - let min_gas_price = config.min_gas_price; let gas_costs = &config.chain_config.consensus_parameters.gas_costs; let fee_parameters = &config.chain_config.consensus_parameters.fee_params; let read = match tx { CheckedTransaction::Script(script) => { - let read = script.into_ready(min_gas_price, gas_costs, fee_parameters)?; - let (_, checked) = read.decompose(); + let ready = script.into_ready(gas_price, gas_costs, fee_parameters)?; + let (_, checked) = ready.decompose(); CheckedTransaction::Script(checked) } CheckedTransaction::Create(create) => { - let read = create.into_ready(min_gas_price, gas_costs, fee_parameters)?; - let (_, checked) = read.decompose(); + let ready = create.into_ready(gas_price, gas_costs, fee_parameters)?; + let (_, checked) = ready.decompose(); CheckedTransaction::Create(checked) } CheckedTransaction::Mint(_) => return Err(Error::MintIsDisallowed), diff --git a/crates/types/src/services/txpool.rs b/crates/types/src/services/txpool.rs index c4b39b53e7..573a44967d 100644 --- a/crates/types/src/services/txpool.rs +++ b/crates/types/src/services/txpool.rs @@ -241,6 +241,8 @@ pub fn from_executor_to_status( #[derive(thiserror::Error, Debug, Clone)] #[non_exhaustive] pub enum Error { + #[error("Gas price not found for block height {0}")] + GasPriceNotFound(BlockHeight), #[error("TxPool required that transaction contains metadata")] NoMetadata, #[error("TxPool doesn't support this type of transaction.")] diff --git a/tests/test-helpers/src/builder.rs b/tests/test-helpers/src/builder.rs index 6c2aa08d2e..0972e09fa0 100644 --- a/tests/test-helpers/src/builder.rs +++ b/tests/test-helpers/src/builder.rs @@ -207,7 +207,6 @@ impl TestSetupBuilder { utxo_validation: self.utxo_validation, txpool: fuel_core_txpool::Config { chain_config: chain_conf.clone(), - min_gas_price: self.min_gas_price, ..fuel_core_txpool::Config::default() }, chain_config: chain_conf, diff --git a/tests/tests/tx/txpool.rs b/tests/tests/tx/txpool.rs index f14c22b52a..b4856dabb9 100644 --- a/tests/tests/tx/txpool.rs +++ b/tests/tests/tx/txpool.rs @@ -63,7 +63,7 @@ async fn txs_max_script_gas_limit() { .into_iter() .map(|script| Arc::new(fuel_tx::Transaction::from(script))) .collect::>(); - srv.shared.txpool.insert(txs).await; + srv.shared.txpool_shared_state.insert(txs).await; tokio::time::sleep(Duration::from_secs(1)).await; From 51e03b5b7fcf8a969ab65fb709fd528308897f2f Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 14 Mar 2024 17:31:41 -0700 Subject: [PATCH 09/19] Add GraphQL stuff, fix tests --- .../tests/integration_tests.rs | 2 +- crates/client/assets/schema.sdl | 1 - crates/client/src/client/schema/node_info.rs | 1 - ...fo__tests__node_info_query_gql_output.snap | 1 - crates/client/src/client/types/node_info.rs | 2 - crates/fuel-core/src/graphql_api.rs | 1 - .../fuel-core/src/graphql_api/api_service.rs | 5 + crates/fuel-core/src/graphql_api/ports.rs | 6 + crates/fuel-core/src/schema/gas_price.rs | 40 +++-- crates/fuel-core/src/schema/node_info.rs | 6 - crates/fuel-core/src/service/adapters.rs | 12 ++ crates/fuel-core/src/service/sub_services.rs | 4 +- .../producer/src/block_producer/gas_price.rs | 12 +- .../producer/src/block_producer/tests.rs | 13 +- crates/services/txpool/src/config.rs | 2 - .../txpool/src/service/test_helpers.rs | 8 +- crates/services/txpool/src/txpool.rs | 23 +++ crates/services/txpool/src/txpool/tests.rs | 170 +++++++++++------- tests/test-helpers/src/builder.rs | 1 + tests/tests/gas_price.rs | 21 ++- tests/tests/node_info.rs | 2 - 21 files changed, 224 insertions(+), 109 deletions(-) diff --git a/bin/e2e-test-client/tests/integration_tests.rs b/bin/e2e-test-client/tests/integration_tests.rs index a8b2ed4f8b..1221bf4bcd 100644 --- a/bin/e2e-test-client/tests/integration_tests.rs +++ b/bin/e2e-test-client/tests/integration_tests.rs @@ -117,7 +117,7 @@ fn dev_config() -> Config { ); config.chain_config = chain_config; - config.block_producer.gas_price = 1; + config.static_gas_price = 1; config.state_reader = StateReader::in_memory(state_config); config.block_producer.coinbase_recipient = Some( diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index a5b2f4bc9c..151b7e935f 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -667,7 +667,6 @@ type Mutation { type NodeInfo { utxoValidation: Boolean! vmBacktrace: Boolean! - minGasPrice: U64! maxTx: U64! maxDepth: U64! nodeVersion: String! diff --git a/crates/client/src/client/schema/node_info.rs b/crates/client/src/client/schema/node_info.rs index 018551e11a..30037a6e91 100644 --- a/crates/client/src/client/schema/node_info.rs +++ b/crates/client/src/client/schema/node_info.rs @@ -20,7 +20,6 @@ use std::{ pub struct NodeInfo { pub utxo_validation: bool, pub vm_backtrace: bool, - pub min_gas_price: U64, pub max_tx: U64, pub max_depth: U64, pub node_version: String, diff --git a/crates/client/src/client/schema/snapshots/fuel_core_client__client__schema__node_info__tests__node_info_query_gql_output.snap b/crates/client/src/client/schema/snapshots/fuel_core_client__client__schema__node_info__tests__node_info_query_gql_output.snap index c2d466d7f8..1d538f92ad 100644 --- a/crates/client/src/client/schema/snapshots/fuel_core_client__client__schema__node_info__tests__node_info_query_gql_output.snap +++ b/crates/client/src/client/schema/snapshots/fuel_core_client__client__schema__node_info__tests__node_info_query_gql_output.snap @@ -6,7 +6,6 @@ query { nodeInfo { utxoValidation vmBacktrace - minGasPrice maxTx maxDepth nodeVersion diff --git a/crates/client/src/client/types/node_info.rs b/crates/client/src/client/types/node_info.rs index e0415508e9..3d9d7e36aa 100644 --- a/crates/client/src/client/types/node_info.rs +++ b/crates/client/src/client/types/node_info.rs @@ -3,7 +3,6 @@ use crate::client::schema; pub struct NodeInfo { pub utxo_validation: bool, pub vm_backtrace: bool, - pub min_gas_price: u64, pub max_tx: u64, pub max_depth: u64, pub node_version: String, @@ -16,7 +15,6 @@ impl From for NodeInfo { Self { utxo_validation: value.utxo_validation, vm_backtrace: value.vm_backtrace, - min_gas_price: value.min_gas_price.into(), max_tx: value.max_tx.into(), max_depth: value.max_depth.into(), node_version: value.node_version, diff --git a/crates/fuel-core/src/graphql_api.rs b/crates/fuel-core/src/graphql_api.rs index 34eb81e2c2..119016ca58 100644 --- a/crates/fuel-core/src/graphql_api.rs +++ b/crates/fuel-core/src/graphql_api.rs @@ -23,7 +23,6 @@ pub struct Config { pub utxo_validation: bool, pub debug: bool, pub vm_backtrace: bool, - pub min_gas_price: u64, pub max_tx: usize, pub max_depth: usize, pub chain_name: String, diff --git a/crates/fuel-core/src/graphql_api/api_service.rs b/crates/fuel-core/src/graphql_api/api_service.rs index 16f9522f59..ec3ea38d6d 100644 --- a/crates/fuel-core/src/graphql_api/api_service.rs +++ b/crates/fuel-core/src/graphql_api/api_service.rs @@ -4,6 +4,7 @@ use crate::{ ports::{ BlockProducerPort, ConsensusModulePort, + GraphQLGasPrice, OffChainDatabase, OnChainDatabase, P2pPort, @@ -88,6 +89,8 @@ pub type TxPool = Box; pub type ConsensusModule = Box; pub type P2pService = Box; +pub type GasPriceProvider = Box; + #[derive(Clone)] pub struct SharedState { pub bound_address: SocketAddr, @@ -173,6 +176,7 @@ pub fn new_service( producer: BlockProducer, consensus_module: ConsensusModule, p2p_service: P2pService, + gas_price_provider: GasPriceProvider, log_threshold_ms: Duration, request_timeout: Duration, ) -> anyhow::Result @@ -192,6 +196,7 @@ where .data(producer) .data(consensus_module) .data(p2p_service) + .data(gas_price_provider) .extension(async_graphql::extensions::Tracing) .extension(MetricsExtension::new(log_threshold_ms)) .extension(ViewExtension::new()) diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 069852e596..7b435c6559 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -197,6 +197,12 @@ pub trait P2pPort: Send + Sync { async fn all_peer_info(&self) -> anyhow::Result>; } +#[async_trait::async_trait] +pub trait GraphQLGasPrice: Send + Sync { + async fn known_gas_price(&self, height: BlockHeight) -> Option; + async fn worst_case_gas_price(&self, height: BlockHeight) -> u64; +} + pub mod worker { use super::super::storage::blocks::FuelBlockIdsToHeights; use crate::fuel_core_graphql_api::storage::{ diff --git a/crates/fuel-core/src/schema/gas_price.rs b/crates/fuel-core/src/schema/gas_price.rs index 29e80dae1a..b21eb33e8e 100644 --- a/crates/fuel-core/src/schema/gas_price.rs +++ b/crates/fuel-core/src/schema/gas_price.rs @@ -3,10 +3,8 @@ use super::scalars::{ U64, }; use crate::{ - fuel_core_graphql_api::{ - database::ReadView, - Config as GraphQLConfig, - }, + fuel_core_graphql_api::database::ReadView, + graphql_api::api_service::GasPriceProvider, query::BlockQueryData, }; use async_graphql::{ @@ -40,14 +38,20 @@ impl LatestGasPriceQuery { &self, ctx: &Context<'_>, ) -> async_graphql::Result { - let config = ctx.data_unchecked::(); - + let gas_price_provider = ctx.data_unchecked::(); let query: &ReadView = ctx.data_unchecked(); + let latest_block: Block<_> = query.latest_block()?; let block_height = u32::from(*latest_block.header().height()); + let gas_price = gas_price_provider + .known_gas_price(block_height.into()) + .await + .ok_or(async_graphql::Error::new(format!( + "Gas price not found for latest block:{block_height:?}" + )))?; Ok(LatestGasPrice { - gas_price: config.min_gas_price.into(), + gas_price: gas_price.into(), block_height: block_height.into(), }) } @@ -77,13 +81,23 @@ impl EstimateGasPriceQuery { )] block_horizon: Option, ) -> async_graphql::Result { - // TODO: implement dynamic calculation based on block horizon - // https://github.com/FuelLabs/fuel-core/issues/1653 - let _ = block_horizon; + let query: &ReadView = ctx.data_unchecked(); - let config = ctx.data_unchecked::(); - let gas_price = config.min_gas_price.into(); + let latest_block: Block<_> = query.latest_block()?; + let latest_block_height = u32::from(*latest_block.header().height()); + let target_block = block_horizon + .and_then(|h| h.0.checked_add(latest_block_height)) + .ok_or(async_graphql::Error::new(format!( + "Invalid block horizon. Overflows latest block :{latest_block_height:?}" + )))?; + + let gas_price_provider = ctx.data_unchecked::(); + let gas_price = gas_price_provider + .worst_case_gas_price(target_block.into()) + .await; - Ok(EstimateGasPrice { gas_price }) + Ok(EstimateGasPrice { + gas_price: gas_price.into(), + }) } } diff --git a/crates/fuel-core/src/schema/node_info.rs b/crates/fuel-core/src/schema/node_info.rs index 647b0c4215..1d8472d476 100644 --- a/crates/fuel-core/src/schema/node_info.rs +++ b/crates/fuel-core/src/schema/node_info.rs @@ -12,7 +12,6 @@ use std::time::UNIX_EPOCH; pub struct NodeInfo { utxo_validation: bool, vm_backtrace: bool, - min_gas_price: U64, max_tx: U64, max_depth: U64, node_version: String, @@ -28,10 +27,6 @@ impl NodeInfo { self.vm_backtrace } - async fn min_gas_price(&self) -> U64 { - self.min_gas_price - } - async fn max_tx(&self) -> U64 { self.max_tx } @@ -75,7 +70,6 @@ impl NodeQuery { Ok(NodeInfo { utxo_validation: config.utxo_validation, vm_backtrace: config.vm_backtrace, - min_gas_price: config.min_gas_price.into(), max_tx: (config.max_tx as u64).into(), max_depth: (config.max_depth as u64).into(), node_version: VERSION.to_owned(), diff --git a/crates/fuel-core/src/service/adapters.rs b/crates/fuel-core/src/service/adapters.rs index 7a3aa06f86..a019159c71 100644 --- a/crates/fuel-core/src/service/adapters.rs +++ b/crates/fuel-core/src/service/adapters.rs @@ -3,6 +3,7 @@ use crate::{ database_description::relayer::Relayer, Database, }, + graphql_api::ports::GraphQLGasPrice, service::sub_services::BlockProducerService, }; use fuel_core_consensus_module::{ @@ -63,6 +64,17 @@ impl ProducerGasPrice for StaticGasPrice { } } +#[async_trait::async_trait] +impl GraphQLGasPrice for StaticGasPrice { + async fn known_gas_price(&self, _height: BlockHeight) -> Option { + Some(self.gas_price) + } + + async fn worst_case_gas_price(&self, _height: BlockHeight) -> u64 { + self.gas_price + } +} + #[derive(Clone)] pub struct PoAAdapter { shared_state: Option, diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 754223afc9..9a7644070d 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -152,7 +152,7 @@ pub fn init_sub_services( executor: Arc::new(executor), relayer: Box::new(relayer_adapter.clone()), lock: Mutex::new(()), - gas_price_provider, + gas_price_provider: gas_price_provider.clone(), }; let producer_adapter = BlockProducerAdapter::new(block_producer); @@ -208,7 +208,6 @@ pub fn init_sub_services( utxo_validation: config.utxo_validation, debug: config.debug, vm_backtrace: config.vm.backtrace, - min_gas_price: config.static_gas_price, max_tx: config.txpool.max_tx, max_depth: config.txpool.max_depth, chain_name: config.chain_config.chain_name.clone(), @@ -225,6 +224,7 @@ pub fn init_sub_services( Box::new(producer_adapter), Box::new(poa_adapter.clone()), Box::new(p2p_adapter), + Box::new(gas_price_provider), config.query_log_threshold_time, config.api_request_timeout, )?; diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index 22c9a2b94b..3164ac2baf 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -28,20 +28,24 @@ pub trait ProducerGasPrice { fn gas_price(&self, params: GasPriceParams) -> Option; } -pub struct MockGasPrice { +pub struct MockProducerGasPrice { pub gas_price: Option, } -impl MockGasPrice { +impl MockProducerGasPrice { pub fn new(gas_price: u64) -> Self { Self { gas_price: Some(gas_price), } } + + pub fn new_none() -> Self { + Self { gas_price: None } + } } -impl ProducerGasPrice for MockGasPrice { +impl ProducerGasPrice for MockProducerGasPrice { fn gas_price(&self, _params: GasPriceParams) -> Option { - self.gas_price.clone() + self.gas_price } } diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index a7a7901f9d..6b0c386012 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -2,8 +2,8 @@ use crate::{ block_producer::{ gas_price::{ + MockProducerGasPrice, ProducerGasPrice, - StaticGasPrice, }, Error, }, @@ -247,6 +247,7 @@ struct TestContext { relayer: MockRelayer, executor: Arc, txpool: MockTxPool, + gas_price: u64, } impl TestContext { @@ -280,18 +281,22 @@ impl TestContext { let txpool = MockTxPool::default(); let relayer = MockRelayer::default(); let config = Config::default(); + let gas_price = 0; Self { config, db, relayer, executor: Arc::new(executor), txpool, + gas_price, } } - pub fn producer(self) -> Producer { - let gas_price = self.config.gas_price; - let static_gas_price = StaticGasPrice::new(gas_price); + pub fn producer( + self, + ) -> Producer { + let gas_price = self.gas_price; + let static_gas_price = MockProducerGasPrice::new(gas_price); self.producer_with_gas_price_provider(static_gas_price) } diff --git a/crates/services/txpool/src/config.rs b/crates/services/txpool/src/config.rs index 954d900362..d14033d65c 100644 --- a/crates/services/txpool/src/config.rs +++ b/crates/services/txpool/src/config.rs @@ -62,8 +62,6 @@ pub struct Config { pub max_tx: usize, /// max depth of connected UTXO excluding contracts pub max_depth: usize, - /// The minimum allowed gas price - // pub min_gas_price: u64, /// Flag to disable utxo existence and signature checks pub utxo_validation: bool, /// chain config diff --git a/crates/services/txpool/src/service/test_helpers.rs b/crates/services/txpool/src/service/test_helpers.rs index 1be18b1f32..0ec1c98e95 100644 --- a/crates/services/txpool/src/service/test_helpers.rs +++ b/crates/services/txpool/src/service/test_helpers.rs @@ -2,6 +2,7 @@ use super::*; use crate::{ mock_db::MockDBProvider, ports::BlockImporter, + txpool::MockTxPoolGasPrice, MockDb, }; use fuel_core_services::{ @@ -32,7 +33,7 @@ use std::cell::RefCell; type GossipedTransaction = GossipData; pub struct TestContext { - pub(crate) service: Service, + pub(crate) service: Service, mock_db: MockDb, rng: RefCell, } @@ -42,7 +43,7 @@ impl TestContext { TestContextBuilder::new().build_and_start().await } - pub fn service(&self) -> &Service { + pub fn service(&self) -> &Service { &self.service } @@ -188,6 +189,7 @@ impl TestContextBuilder { pub fn build(self) -> TestContext { let rng = RefCell::new(self.rng); + let gas_price = 0; let config = self.config.unwrap_or_default(); let mock_db = self.mock_db; @@ -202,6 +204,7 @@ impl TestContextBuilder { let importer = self .importer .unwrap_or_else(|| MockImporter::with_blocks(vec![])); + let gas_price_provider = MockTxPoolGasPrice::new(gas_price); let service = new_service( config, @@ -209,6 +212,7 @@ impl TestContextBuilder { importer, p2p, Default::default(), + gas_price_provider, ); TestContext { diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 7ff5312a48..cf22be487f 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -80,6 +80,29 @@ pub trait TxPoolGasPrice { fn gas_price(&self, block_height: BlockHeight) -> Option; } +#[derive(Debug, Clone)] +pub struct MockTxPoolGasPrice { + pub gas_price: Option, +} + +impl MockTxPoolGasPrice { + pub fn new(gas_price: GasPrice) -> Self { + Self { + gas_price: Some(gas_price), + } + } + + pub fn new_none() -> Self { + Self { gas_price: None } + } +} + +impl TxPoolGasPrice for MockTxPoolGasPrice { + fn gas_price(&self, _block_height: BlockHeight) -> Option { + self.gas_price + } +} + impl TxPool { pub fn new(config: Config, database: ViewProvider) -> Self { let max_depth = config.max_depth; diff --git a/crates/services/txpool/src/txpool/tests.rs b/crates/services/txpool/src/txpool/tests.rs index 8a0cad8c87..f6cd1c7c00 100644 --- a/crates/services/txpool/src/txpool/tests.rs +++ b/crates/services/txpool/src/txpool/tests.rs @@ -4,11 +4,14 @@ use crate::{ TextContext, TEST_COIN_AMOUNT, }, - txpool::test_helpers::{ - create_coin_output, - create_contract_input, - create_contract_output, - create_message_predicate_from_message, + txpool::{ + test_helpers::{ + create_coin_output, + create_contract_input, + create_contract_output, + create_message_predicate_from_message, + }, + MockTxPoolGasPrice, }, Config, Error, @@ -38,6 +41,7 @@ use fuel_core_types::{ }, }; +use crate::types::GasPrice; use fuel_core_types::fuel_tx::Finalizable; use std::{ cmp::Reverse, @@ -49,8 +53,13 @@ use super::check_single_tx; const GAS_LIMIT: Word = 1000; -async fn check_unwrap_tx(tx: Transaction, config: &Config) -> Checked { - check_single_tx(tx, Default::default(), config) +async fn check_unwrap_tx( + tx: Transaction, + config: &Config, + gas_price: GasPrice, +) -> Checked { + let gas_price_provider = MockTxPoolGasPrice::new(gas_price); + check_single_tx(tx, Default::default(), config, &gas_price_provider) .await .expect("Transaction should be checked") } @@ -58,13 +67,16 @@ async fn check_unwrap_tx(tx: Transaction, config: &Config) -> Checked Result, Error> { - check_single_tx(tx, Default::default(), config).await + let gas_price_provider = MockTxPoolGasPrice::new(gas_price); + check_single_tx(tx, Default::default(), config, &gas_price_provider).await } #[tokio::test] async fn insert_simple_tx_succeeds() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -73,7 +85,7 @@ async fn insert_simple_tx_succeeds() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; txpool .insert_single(tx) @@ -83,6 +95,7 @@ async fn insert_simple_tx_succeeds() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -90,7 +103,7 @@ async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { .add_input(gas_coin.clone()) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let utxo_id = *gas_coin.utxo_id().unwrap(); // Given @@ -110,6 +123,7 @@ async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_owner_fails() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -117,7 +131,7 @@ async fn insert_simple_tx_with_blacklisted_owner_fails() { .add_input(gas_coin.clone()) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let owner = *gas_coin.input_owner().unwrap(); // Given @@ -138,6 +152,7 @@ async fn insert_simple_tx_with_blacklisted_owner_fails() { async fn insert_simple_tx_with_blacklisted_contract_fails() { let mut context = TextContext::default(); let contract_id = Contract::EMPTY_CONTRACT_ID; + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -151,7 +166,7 @@ async fn insert_simple_tx_with_blacklisted_contract_fails() { .add_output(Output::contract(1, Default::default(), Default::default())) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; // Given txpool.config_mut().blacklist.contracts.insert(contract_id); @@ -170,6 +185,7 @@ async fn insert_simple_tx_with_blacklisted_contract_fails() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_message_fails() { let (message, input) = create_message_predicate_from_message(5000, 0); + let gas_price = 0; let tx = TransactionBuilder::script(vec![], vec![]) .script_gas_limit(GAS_LIMIT) @@ -181,7 +197,7 @@ async fn insert_simple_tx_with_blacklisted_message_fails() { context.database_mut().insert_message(message); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; // Given txpool.config_mut().blacklist.messages.insert(nonce); @@ -200,6 +216,7 @@ async fn insert_simple_tx_with_blacklisted_message_fails() { #[tokio::test] async fn insert_simple_tx_dependency_chain_succeeds() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(1); @@ -222,8 +239,8 @@ async fn insert_simple_tx_dependency_chain_succeeds() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; txpool .insert_single(tx1) @@ -236,6 +253,7 @@ async fn insert_simple_tx_dependency_chain_succeeds() { #[tokio::test] async fn faulty_t2_collided_on_contract_id_from_tx1() { let mut context = TextContext::default(); + let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; @@ -273,10 +291,10 @@ async fn faulty_t2_collided_on_contract_id_from_tx1() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; txpool.insert_single(tx).expect("Tx1 should be Ok, got Err"); - let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config).await; + let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config, gas_price).await; let err = txpool .insert_single(tx_faulty) @@ -290,6 +308,7 @@ async fn faulty_t2_collided_on_contract_id_from_tx1() { #[tokio::test] async fn fail_to_insert_tx_with_dependency_on_invalid_utxo_type() { let mut context = TextContext::default(); + let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_coin) = context.setup_coin(); @@ -318,13 +337,13 @@ async fn fail_to_insert_tx_with_dependency_on_invalid_utxo_type() { let mut txpool = context.build(); let tx_faulty_id = tx_faulty.id(&ChainId::default()); - let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config).await; + let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config, gas_price).await; txpool .insert_single(tx_faulty.clone()) .expect("Tx1 should be Ok, got Err"); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let err = txpool .insert_single(tx) @@ -341,6 +360,7 @@ async fn not_inserted_known_tx() { utxo_validation: false, ..Default::default() }; + let gas_price = 0; let context = TextContext::default().config(config); let mut txpool = context.build(); @@ -348,7 +368,7 @@ async fn not_inserted_known_tx() { .add_random_fee_input() .finalize() .into(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; txpool .insert_single(tx.clone()) @@ -363,6 +383,7 @@ async fn not_inserted_known_tx() { #[tokio::test] async fn try_to_insert_tx2_missing_utxo() { let mut context = TextContext::default(); + let gas_price = 0; let input = context.random_predicate(AssetId::BASE, TEST_COIN_AMOUNT, None); let tx = TransactionBuilder::script(vec![], vec![]) @@ -373,7 +394,7 @@ async fn try_to_insert_tx2_missing_utxo() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let err = txpool .insert_single(tx) @@ -387,6 +408,7 @@ async fn try_to_insert_tx2_missing_utxo() { #[tokio::test] async fn higher_priced_tx_removes_lower_priced_tx() { let mut context = TextContext::default(); + let gas_price = 0; let (_, coin_input) = context.setup_coin(); @@ -406,13 +428,13 @@ async fn higher_priced_tx_removes_lower_priced_tx() { let tx1_id = tx1.id(&ChainId::default()); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; txpool .insert_single(tx1.clone()) .expect("Tx1 should be Ok, got Err"); - let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; let vec = txpool .insert_single(tx2) @@ -423,6 +445,7 @@ async fn higher_priced_tx_removes_lower_priced_tx() { #[tokio::test] async fn underpriced_tx1_not_included_coin_collision() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(20); @@ -451,17 +474,17 @@ async fn underpriced_tx1_not_included_coin_collision() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1_checked = check_unwrap_tx(tx1.clone(), txpool.config()).await; + let tx1_checked = check_unwrap_tx(tx1.clone(), txpool.config(), gas_price).await; txpool .insert_single(tx1_checked) .expect("Tx1 should be Ok, got Err"); - let tx2_checked = check_unwrap_tx(tx2.clone(), txpool.config()).await; + let tx2_checked = check_unwrap_tx(tx2.clone(), txpool.config(), gas_price).await; txpool .insert_single(tx2_checked) .expect("Tx2 should be Ok, got Err"); - let tx3_checked = check_unwrap_tx(tx3, txpool.config()).await; + let tx3_checked = check_unwrap_tx(tx3, txpool.config(), gas_price).await; let err = txpool .insert_single(tx3_checked) .expect_err("Tx3 should be Err, got Ok"); @@ -474,6 +497,7 @@ async fn underpriced_tx1_not_included_coin_collision() { #[tokio::test] async fn overpriced_tx_contract_input_not_inserted() { let mut context = TextContext::default(); + let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_funds) = context.setup_coin(); @@ -503,12 +527,12 @@ async fn overpriced_tx_contract_input_not_inserted() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got err"); - let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; let err = txpool .insert_single(tx2) .expect_err("Tx2 should be Err, got Ok"); @@ -524,6 +548,7 @@ async fn overpriced_tx_contract_input_not_inserted() { #[tokio::test] async fn dependent_contract_input_inserted() { let mut context = TextContext::default(); + let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_funds) = context.setup_coin(); @@ -553,8 +578,8 @@ async fn dependent_contract_input_inserted() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got Err"); @@ -566,6 +591,7 @@ async fn dependent_contract_input_inserted() { #[tokio::test] async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); @@ -597,9 +623,9 @@ async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { let tx1_id = tx1.id(&ChainId::default()); let tx2_id = tx2.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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool .insert_single(tx1.clone()) @@ -622,6 +648,7 @@ async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { #[tokio::test] async fn more_priced_tx2_removes_tx1_and_more_priced_tx3_removes_tx2() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); @@ -647,9 +674,9 @@ async fn more_priced_tx2_removes_tx1_and_more_priced_tx3_removes_tx2() { .finalize_as_transaction(); 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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool .insert_single(tx1) @@ -674,6 +701,7 @@ async fn tx_limit_hit() { max_tx: 1, ..Default::default() }); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx1 = TransactionBuilder::script(vec![], vec![]) @@ -689,8 +717,8 @@ async fn tx_limit_hit() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got Err"); @@ -707,6 +735,7 @@ async fn tx_depth_hit() { max_depth: 2, ..Default::default() }); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(10_000); @@ -731,9 +760,9 @@ async fn tx_depth_hit() { .finalize_as_transaction(); 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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool .insert_single(tx1) @@ -780,10 +809,12 @@ async fn sorted_out_tx1_2_4() { let tx2_id = tx2.id(&ChainId::default()); let tx3_id = tx3.id(&ChainId::default()); + let gas_price = 0; + 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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool .insert_single(tx1) @@ -806,6 +837,7 @@ async fn sorted_out_tx1_2_4() { #[tokio::test] async fn find_dependent_tx1_tx2() { let mut context = TextContext::default(); + let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(10_000); @@ -840,9 +872,9 @@ async fn find_dependent_tx1_tx2() { 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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool .insert_single(tx1) @@ -870,8 +902,8 @@ async fn find_dependent_tx1_tx2() { #[tokio::test] async fn tx_at_least_min_gas_price_is_insertable() { + let gas_price = 10; let mut context = TextContext::default().config(Config { - min_gas_price: 10, ..Default::default() }); @@ -884,7 +916,7 @@ async fn tx_at_least_min_gas_price_is_insertable() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; txpool.insert_single(tx).expect("Tx should be Ok, got Err"); } @@ -899,13 +931,14 @@ async fn tx_below_min_gas_price_is_not_insertable() { .script_gas_limit(GAS_LIMIT) .add_input(gas_coin) .finalize_as_transaction(); + let gas_price = 11; let err = check_tx( tx, &Config { - min_gas_price: 11, ..Default::default() }, + gas_price, ) .await .expect_err("expected insertion failure"); @@ -930,8 +963,9 @@ async fn tx_inserted_into_pool_when_input_message_id_exists_in_db() { let tx1_id = tx.id(&ChainId::default()); let mut txpool = context.build(); + let gas_price = 0; - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; txpool.insert_single(tx).expect("should succeed"); let tx_info = txpool.find_one(&tx1_id).unwrap(); @@ -951,8 +985,9 @@ async fn tx_rejected_when_input_message_id_is_spent() { context.database_mut().insert_message(message.clone()); context.database_mut().spend_message(*message.id()); let mut txpool = context.build(); + let gas_price = 0; - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let err = txpool.insert_single(tx).expect_err("should fail"); // check error @@ -970,11 +1005,12 @@ async fn tx_rejected_from_pool_when_input_message_id_does_not_exist_in_db() { .script_gas_limit(GAS_LIMIT) .add_input(input) .finalize_as_transaction(); + let gas_price = 0; // Do not insert any messages into the DB to ensure there is no matching message for the // tx. let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config).await; + let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; let err = txpool.insert_single(tx).expect_err("should fail"); // check error @@ -1013,14 +1049,14 @@ async fn tx_rejected_from_pool_when_gas_price_is_lower_than_another_tx_with_same let mut txpool = context.build(); let tx_high_id = tx_high.id(&ChainId::default()); - let tx_high = check_unwrap_tx(tx_high, &txpool.config).await; + let tx_high = check_unwrap_tx(tx_high, &txpool.config, gas_price_high).await; // Insert a tx for the message id with a high gas amount txpool .insert_single(tx_high) .expect("expected successful insertion"); - let tx_low = check_unwrap_tx(tx_low, &txpool.config).await; + let tx_low = check_unwrap_tx(tx_low, &txpool.config, gas_price_low).await; // Insert a tx for the message id with a low gas amount // Because the new transaction's id matches an existing transaction, we compare the gas // prices of both the new and existing transactions. Since the existing transaction's gas @@ -1055,7 +1091,7 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { let mut txpool = context.build(); let tx_low_id = tx_low.id(&ChainId::default()); - let tx_low = check_unwrap_tx(tx_low, &txpool.config).await; + let tx_low = check_unwrap_tx(tx_low, &txpool.config, gas_price_low).await; txpool.insert_single(tx_low).expect("should succeed"); // Insert a tx for the message id with a high gas amount @@ -1068,7 +1104,7 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input) .finalize_as_transaction(); - let tx_high = check_unwrap_tx(tx_high, &txpool.config).await; + let tx_high = check_unwrap_tx(tx_high, &txpool.config, gas_price_high).await; let squeezed_out_txs = txpool.insert_single(tx_high).expect("should succeed"); assert_eq!(squeezed_out_txs.removed.len(), 1); @@ -1110,13 +1146,15 @@ async fn message_of_squeezed_out_tx_can_be_resubmitted_at_lower_gas_price() { .add_input(message_input_2) .finalize_as_transaction(); + let gas_price = 0; + context.database_mut().insert_message(message_1); context.database_mut().insert_message(message_2); 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; + let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; txpool.insert_single(tx1).expect("should succeed"); @@ -1138,7 +1176,9 @@ async fn predicates_with_incorrect_owner_fails() { .add_input(coin) .finalize_as_transaction(); - let err = check_tx(tx, &Default::default()) + let gas_price = 0; + + let err = check_tx(tx, &Default::default(), gas_price) .await .expect_err("Transaction should be err, got ok"); @@ -1177,7 +1217,9 @@ async fn predicate_without_enough_gas_returns_out_of_gas() { .add_input(coin) .finalize_as_transaction(); - let err = check_tx(tx, &Default::default()) + let gas_price = 0; + + let err = check_tx(tx, &Default::default(), gas_price) .await .expect_err("Transaction should be err, got ok"); @@ -1206,7 +1248,9 @@ async fn predicate_that_returns_false_is_invalid() { .add_input(coin) .finalize_as_transaction(); - let err = check_tx(tx, &Default::default()) + let gas_price = 0; + + let err = check_tx(tx, &Default::default(), gas_price) .await .expect_err("Transaction should be err, got ok"); diff --git a/tests/test-helpers/src/builder.rs b/tests/test-helpers/src/builder.rs index 0972e09fa0..a49b805432 100644 --- a/tests/test-helpers/src/builder.rs +++ b/tests/test-helpers/src/builder.rs @@ -212,6 +212,7 @@ impl TestSetupBuilder { chain_config: chain_conf, state_reader: StateReader::in_memory(state), block_production: self.trigger, + static_gas_price: self.min_gas_price, ..Config::local_node() }; diff --git a/tests/tests/gas_price.rs b/tests/tests/gas_price.rs index 08a13fc268..1ccf0e62a4 100644 --- a/tests/tests/gas_price.rs +++ b/tests/tests/gas_price.rs @@ -1,3 +1,4 @@ +#![allow(non_snake_case)] use fuel_core::service::{ Config, FuelService, @@ -9,24 +10,36 @@ use fuel_core_client::client::{ }; #[tokio::test] -async fn latest_gas_price() { +async fn latest_gas_price__should_be_static() { + // given let node_config = Config::local_node(); let srv = FuelService::new_node(node_config.clone()).await.unwrap(); let client = FuelClient::from(srv.bound_address); + // when let LatestGasPrice { gas_price, .. } = client.latest_gas_price().await.unwrap(); - assert_eq!(gas_price, node_config.txpool.min_gas_price); + + // then + let expected = node_config.static_gas_price; + let actual = gas_price; + assert_eq!(expected, actual) } #[tokio::test] -async fn estimate_gas_price() { +async fn estimate_gas_price__should_be_static() { + // given let node_config = Config::local_node(); let srv = FuelService::new_node(node_config.clone()).await.unwrap(); let client = FuelClient::from(srv.bound_address); + // when let arbitrary_horizon = 10; let EstimateGasPrice { gas_price } = client.estimate_gas_price(arbitrary_horizon).await.unwrap(); - assert_eq!(u64::from(gas_price), node_config.txpool.min_gas_price); + + // then + let expected = node_config.static_gas_price; + let actual = u64::from(gas_price); + assert_eq!(expected, actual); } diff --git a/tests/tests/node_info.rs b/tests/tests/node_info.rs index 5876be45ad..b5afb7dc91 100644 --- a/tests/tests/node_info.rs +++ b/tests/tests/node_info.rs @@ -16,7 +16,6 @@ async fn node_info() { let NodeInfo { utxo_validation, vm_backtrace, - min_gas_price, max_depth, max_tx, .. @@ -24,7 +23,6 @@ async fn node_info() { assert_eq!(utxo_validation, node_config.utxo_validation); assert_eq!(vm_backtrace, node_config.vm.backtrace); - assert_eq!(min_gas_price, node_config.txpool.min_gas_price); assert_eq!(max_depth, node_config.txpool.max_depth as u64); assert_eq!(max_tx, node_config.txpool.max_tx as u64); } From 08bbd4af7c44d75119546a75b1fac300599db989 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 14 Mar 2024 17:58:20 -0700 Subject: [PATCH 10/19] Fix tests with too low max fee limit --- crates/services/txpool/src/txpool/tests.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/services/txpool/src/txpool/tests.rs b/crates/services/txpool/src/txpool/tests.rs index f6cd1c7c00..a0b3e86840 100644 --- a/crates/services/txpool/src/txpool/tests.rs +++ b/crates/services/txpool/src/txpool/tests.rs @@ -1025,6 +1025,7 @@ async fn tx_rejected_from_pool_when_gas_price_is_lower_than_another_tx_with_same ) { let mut context = TextContext::default(); let message_amount = 10_000; + let max_fee_limit = 10u64; let gas_price_high = 2u64; let gas_price_low = 1u64; let (message, conflicting_message_input) = @@ -1032,14 +1033,14 @@ async fn tx_rejected_from_pool_when_gas_price_is_lower_than_another_tx_with_same let tx_high = TransactionBuilder::script(vec![], vec![]) .tip(gas_price_high) - .max_fee_limit(gas_price_high) + .max_fee_limit(max_fee_limit) .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input.clone()) .finalize_as_transaction(); let tx_low = TransactionBuilder::script(vec![], vec![]) .tip(gas_price_low) - .max_fee_limit(gas_price_low) + .max_fee_limit(max_fee_limit) .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input) .finalize_as_transaction(); @@ -1075,6 +1076,7 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { let mut context = TextContext::default(); let message_amount = 10_000; let gas_price_high = 2u64; + let max_fee_limit = 10u64; let gas_price_low = 1u64; let (message, conflicting_message_input) = create_message_predicate_from_message(message_amount, 0); @@ -1082,7 +1084,7 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { // Insert a tx for the message id with a low gas amount let tx_low = TransactionBuilder::script(vec![], vec![]) .tip(gas_price_low) - .max_fee_limit(gas_price_low) + .max_fee_limit(max_fee_limit) .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input.clone()) .finalize_as_transaction(); @@ -1100,7 +1102,7 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { // price is lower, we accept the new transaction and squeeze out the old transaction. let tx_high = TransactionBuilder::script(vec![], vec![]) .tip(gas_price_high) - .max_fee_limit(gas_price_high) + .max_fee_limit(max_fee_limit) .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input) .finalize_as_transaction(); From ab027a5b945e612f3d60929a35a8d2b8edd376d2 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Mar 2024 15:27:28 -0700 Subject: [PATCH 11/19] Use Mint for latest gas price, use block height directly, add test --- crates/fuel-core/src/schema/gas_price.rs | 34 ++++--- tests/tests/gas_price.rs | 115 +++++++++++++++++++++-- 2 files changed, 129 insertions(+), 20 deletions(-) diff --git a/crates/fuel-core/src/schema/gas_price.rs b/crates/fuel-core/src/schema/gas_price.rs index b21eb33e8e..1d78cbc40c 100644 --- a/crates/fuel-core/src/schema/gas_price.rs +++ b/crates/fuel-core/src/schema/gas_price.rs @@ -5,13 +5,22 @@ use super::scalars::{ use crate::{ fuel_core_graphql_api::database::ReadView, graphql_api::api_service::GasPriceProvider, - query::BlockQueryData, + query::{ + BlockQueryData, + SimpleTransactionData, + }, }; use async_graphql::{ Context, Object, }; -use fuel_core_types::blockchain::block::Block; +use fuel_core_types::{ + blockchain::block::Block, + fuel_tx::{ + field::MintGasPrice, + Transaction, + }, +}; pub struct LatestGasPrice { pub gas_price: U64, @@ -38,20 +47,18 @@ impl LatestGasPriceQuery { &self, ctx: &Context<'_>, ) -> async_graphql::Result { - let gas_price_provider = ctx.data_unchecked::(); let query: &ReadView = ctx.data_unchecked(); let latest_block: Block<_> = query.latest_block()?; - let block_height = u32::from(*latest_block.header().height()); - let gas_price = gas_price_provider - .known_gas_price(block_height.into()) - .await - .ok_or(async_graphql::Error::new(format!( - "Gas price not found for latest block:{block_height:?}" - )))?; - + let block_height: u32 = (*latest_block.header().height()).into(); + let mut gas_price: U64 = 0.into(); + if let Some(tx_id) = latest_block.transactions().last() { + if let Transaction::Mint(mint_tx) = query.transaction(tx_id)? { + gas_price = (*mint_tx.gas_price()).into(); + } + } Ok(LatestGasPrice { - gas_price: gas_price.into(), + gas_price, block_height: block_height.into(), }) } @@ -83,8 +90,7 @@ impl EstimateGasPriceQuery { ) -> async_graphql::Result { let query: &ReadView = ctx.data_unchecked(); - let latest_block: Block<_> = query.latest_block()?; - let latest_block_height = u32::from(*latest_block.header().height()); + let latest_block_height: u32 = query.latest_block_height()?.into(); let target_block = block_horizon .and_then(|h| h.0.checked_add(latest_block_height)) .ok_or(async_graphql::Error::new(format!( diff --git a/tests/tests/gas_price.rs b/tests/tests/gas_price.rs index 1ccf0e62a4..367f5b5bb5 100644 --- a/tests/tests/gas_price.rs +++ b/tests/tests/gas_price.rs @@ -1,18 +1,121 @@ #![allow(non_snake_case)] -use fuel_core::service::{ - Config, - FuelService, + +use fuel_core::{ + chain_config::{ + CoinConfig, + StateConfig, + StateReader, + }, + service::{ + Config, + FuelService, + }, }; use fuel_core_client::client::{ schema::gas_price::EstimateGasPrice, - types::gas_price::LatestGasPrice, + types::{ + gas_price::LatestGasPrice, + primitives::{ + Address, + AssetId, + }, + }, FuelClient, }; +use fuel_core_types::{ + fuel_crypto::{ + coins_bip32::ecdsa::signature::rand_core::SeedableRng, + SecretKey, + }, + fuel_tx::{ + Finalizable, + Input, + TransactionBuilder, + UtxoId, + }, +}; +use rand::prelude::StdRng; + +async fn setup_service_with_coin( + owner: Address, + amount: u64, + static_gas_price: u64, +) -> (FuelService, UtxoId) { + // setup config + let tx_id = [0u8; 32]; + let output_index = 0; + let coin_config = CoinConfig { + tx_id: tx_id.clone().into(), + output_index, + tx_pointer_block_height: Default::default(), + tx_pointer_tx_idx: 0, + owner, + amount, + asset_id: AssetId::BASE, + }; + let state = StateConfig { + coins: vec![coin_config], + ..Default::default() + }; + let config = Config { + state_reader: StateReader::in_memory(state), + static_gas_price, + ..Config::local_node() + }; + + // setup server & client + let srv = FuelService::new_node(config).await.unwrap(); + + let utxo_id = UtxoId::new(tx_id.into(), output_index); + + (srv, utxo_id) +} #[tokio::test] async fn latest_gas_price__should_be_static() { + // setup node with a block that has a non-mint transaction + let static_gas_price = 2; + let max_fee_limit = 100; + let mut rng = StdRng::seed_from_u64(1234); + let secret_key: SecretKey = SecretKey::random(&mut rng); + let pk = secret_key.public_key(); + let owner = Input::owner(&pk); + + let (srv, utxo_id) = + setup_service_with_coin(owner, max_fee_limit, static_gas_price).await; + + let client = FuelClient::from(srv.bound_address); + + let tx = TransactionBuilder::script(vec![], vec![]) + .max_fee_limit(1) + .add_unsigned_coin_input( + secret_key, + utxo_id, + max_fee_limit, + AssetId::BASE, + Default::default(), + ) + .finalize() + .into(); + + client.submit_and_await_commit(&tx).await.unwrap(); + // given - let node_config = Config::local_node(); + let expected = static_gas_price; + + // when + let LatestGasPrice { gas_price, .. } = client.latest_gas_price().await.unwrap(); + + // then + let actual = gas_price; + assert_eq!(expected, actual) +} + +#[tokio::test] +async fn latest_gas_price__if_no_mint_tx_in_previous_block_gas_price_is_zero() { + // given + let mut node_config = Config::local_node(); + node_config.static_gas_price = 100; let srv = FuelService::new_node(node_config.clone()).await.unwrap(); let client = FuelClient::from(srv.bound_address); @@ -20,7 +123,7 @@ async fn latest_gas_price__should_be_static() { let LatestGasPrice { gas_price, .. } = client.latest_gas_price().await.unwrap(); // then - let expected = node_config.static_gas_price; + let expected = 0; let actual = gas_price; assert_eq!(expected, actual) } From c3c2455550cdb9d0fe7cc2f59a2ae6707a78a41a Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Mar 2024 15:30:00 -0700 Subject: [PATCH 12/19] Remove unused trait method --- crates/fuel-core/src/graphql_api/api_service.rs | 4 ++-- crates/fuel-core/src/graphql_api/ports.rs | 3 +-- crates/fuel-core/src/service/adapters.rs | 8 ++------ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/api_service.rs b/crates/fuel-core/src/graphql_api/api_service.rs index ec3ea38d6d..222508af86 100644 --- a/crates/fuel-core/src/graphql_api/api_service.rs +++ b/crates/fuel-core/src/graphql_api/api_service.rs @@ -4,7 +4,7 @@ use crate::{ ports::{ BlockProducerPort, ConsensusModulePort, - GraphQLGasPrice, + GasPriceEstimate, OffChainDatabase, OnChainDatabase, P2pPort, @@ -89,7 +89,7 @@ pub type TxPool = Box; pub type ConsensusModule = Box; pub type P2pService = Box; -pub type GasPriceProvider = Box; +pub type GasPriceProvider = Box; #[derive(Clone)] pub struct SharedState { diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 7b435c6559..fa74969c97 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -198,8 +198,7 @@ pub trait P2pPort: Send + Sync { } #[async_trait::async_trait] -pub trait GraphQLGasPrice: Send + Sync { - async fn known_gas_price(&self, height: BlockHeight) -> Option; +pub trait GasPriceEstimate: Send + Sync { async fn worst_case_gas_price(&self, height: BlockHeight) -> u64; } diff --git a/crates/fuel-core/src/service/adapters.rs b/crates/fuel-core/src/service/adapters.rs index a019159c71..f980f7240b 100644 --- a/crates/fuel-core/src/service/adapters.rs +++ b/crates/fuel-core/src/service/adapters.rs @@ -3,7 +3,7 @@ use crate::{ database_description::relayer::Relayer, Database, }, - graphql_api::ports::GraphQLGasPrice, + graphql_api::ports::GasPriceEstimate, service::sub_services::BlockProducerService, }; use fuel_core_consensus_module::{ @@ -65,11 +65,7 @@ impl ProducerGasPrice for StaticGasPrice { } #[async_trait::async_trait] -impl GraphQLGasPrice for StaticGasPrice { - async fn known_gas_price(&self, _height: BlockHeight) -> Option { - Some(self.gas_price) - } - +impl GasPriceEstimate for StaticGasPrice { async fn worst_case_gas_price(&self, _height: BlockHeight) -> u64 { self.gas_price } From 05e2f023bc3969e777708b05db0ef35f188789b6 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Mar 2024 15:32:39 -0700 Subject: [PATCH 13/19] Add docs to traits --- crates/fuel-core/src/graphql_api/ports.rs | 2 ++ crates/services/txpool/src/txpool.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index fa74969c97..91736b18a9 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -197,8 +197,10 @@ pub trait P2pPort: Send + Sync { async fn all_peer_info(&self) -> anyhow::Result>; } +/// Trait for defining how to estimate gas price for future blocks #[async_trait::async_trait] pub trait GasPriceEstimate: Send + Sync { + /// The worst case scenario for gas price at a given horizon async fn worst_case_gas_price(&self, height: BlockHeight) -> u64; } diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index cf22be487f..ad2eb6278a 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -76,7 +76,9 @@ pub struct TxPool { database: ViewProvider, } +/// Trait for getting gas price for the Tx Pool code to look up the gas price for a given block height pub trait TxPoolGasPrice { + /// Get gas price for specific block height if it is known fn gas_price(&self, block_height: BlockHeight) -> Option; } From 1a8994bc78ea7a1e96d27a0016ea2a09e541f0db Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 15 Mar 2024 15:40:36 -0700 Subject: [PATCH 14/19] Remove `Clone` req and use `Arc` instead --- crates/services/txpool/src/service.rs | 6 ++---- crates/services/txpool/src/txpool.rs | 6 ++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs index 8e92295f7e..d9219e50e9 100644 --- a/crates/services/txpool/src/service.rs +++ b/crates/services/txpool/src/service.rs @@ -128,13 +128,11 @@ pub struct SharedState { consensus_params: ConsensusParameters, current_height: Arc>, config: Config, - gas_price_provider: GasPriceProvider, + gas_price_provider: Arc, } impl Clone for SharedState -where - GasPriceProvider: Clone, { fn clone(&self) -> Self { Self { @@ -505,7 +503,7 @@ where consensus_params, current_height: Arc::new(ParkingMutex::new(current_height)), config, - gas_price_provider, + gas_price_provider: Arc::new(gas_price_provider), }, ttl_timer, }; diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index ad2eb6278a..5151baaf82 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -82,6 +82,12 @@ pub trait TxPoolGasPrice { fn gas_price(&self, block_height: BlockHeight) -> Option; } +impl TxPoolGasPrice for Arc { + fn gas_price(&self, block_height: BlockHeight) -> Option { + self.deref().gas_price(block_height) + } +} + #[derive(Debug, Clone)] pub struct MockTxPoolGasPrice { pub gas_price: Option, From bced681f7ea4c7a2630b809831c677da180f3518 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 15 Mar 2024 16:22:29 -0700 Subject: [PATCH 15/19] Modify test helpers --- crates/services/txpool/src/txpool/tests.rs | 161 ++++++++++----------- 1 file changed, 73 insertions(+), 88 deletions(-) diff --git a/crates/services/txpool/src/txpool/tests.rs b/crates/services/txpool/src/txpool/tests.rs index a0b3e86840..7c56424920 100644 --- a/crates/services/txpool/src/txpool/tests.rs +++ b/crates/services/txpool/src/txpool/tests.rs @@ -53,7 +53,12 @@ use super::check_single_tx; const GAS_LIMIT: Word = 1000; -async fn check_unwrap_tx( +async fn check_unwrap_tx(tx: Transaction, config: &Config) -> Checked { + let gas_price = 0; + check_unwrap_tx_with_gas_price(tx, config, gas_price).await +} + +async fn check_unwrap_tx_with_gas_price( tx: Transaction, config: &Config, gas_price: GasPrice, @@ -67,7 +72,15 @@ async fn check_unwrap_tx( async fn check_tx( tx: Transaction, config: &Config, - gas_price: Word, +) -> Result, Error> { + let gas_price = 0; + check_tx_with_gas_price(tx, config, gas_price).await +} + +async fn check_tx_with_gas_price( + tx: Transaction, + config: &Config, + gas_price: GasPrice, ) -> Result, Error> { let gas_price_provider = MockTxPoolGasPrice::new(gas_price); check_single_tx(tx, Default::default(), config, &gas_price_provider).await @@ -76,7 +89,6 @@ async fn check_tx( #[tokio::test] async fn insert_simple_tx_succeeds() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -85,7 +97,7 @@ async fn insert_simple_tx_succeeds() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; txpool .insert_single(tx) @@ -95,7 +107,6 @@ async fn insert_simple_tx_succeeds() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -103,7 +114,7 @@ async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { .add_input(gas_coin.clone()) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let utxo_id = *gas_coin.utxo_id().unwrap(); // Given @@ -123,7 +134,6 @@ async fn insert_simple_tx_with_blacklisted_utxo_id_fails() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_owner_fails() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -131,7 +141,7 @@ async fn insert_simple_tx_with_blacklisted_owner_fails() { .add_input(gas_coin.clone()) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let owner = *gas_coin.input_owner().unwrap(); // Given @@ -152,7 +162,6 @@ async fn insert_simple_tx_with_blacklisted_owner_fails() { async fn insert_simple_tx_with_blacklisted_contract_fails() { let mut context = TextContext::default(); let contract_id = Contract::EMPTY_CONTRACT_ID; - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx = TransactionBuilder::script(vec![], vec![]) @@ -166,7 +175,7 @@ async fn insert_simple_tx_with_blacklisted_contract_fails() { .add_output(Output::contract(1, Default::default(), Default::default())) .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; // Given txpool.config_mut().blacklist.contracts.insert(contract_id); @@ -185,7 +194,6 @@ async fn insert_simple_tx_with_blacklisted_contract_fails() { #[tokio::test] async fn insert_simple_tx_with_blacklisted_message_fails() { let (message, input) = create_message_predicate_from_message(5000, 0); - let gas_price = 0; let tx = TransactionBuilder::script(vec![], vec![]) .script_gas_limit(GAS_LIMIT) @@ -197,7 +205,7 @@ async fn insert_simple_tx_with_blacklisted_message_fails() { context.database_mut().insert_message(message); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; // Given txpool.config_mut().blacklist.messages.insert(nonce); @@ -216,7 +224,6 @@ async fn insert_simple_tx_with_blacklisted_message_fails() { #[tokio::test] async fn insert_simple_tx_dependency_chain_succeeds() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(1); @@ -239,8 +246,8 @@ async fn insert_simple_tx_dependency_chain_succeeds() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; txpool .insert_single(tx1) @@ -253,7 +260,6 @@ async fn insert_simple_tx_dependency_chain_succeeds() { #[tokio::test] async fn faulty_t2_collided_on_contract_id_from_tx1() { let mut context = TextContext::default(); - let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; @@ -291,10 +297,10 @@ async fn faulty_t2_collided_on_contract_id_from_tx1() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; txpool.insert_single(tx).expect("Tx1 should be Ok, got Err"); - let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config, gas_price).await; + let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config).await; let err = txpool .insert_single(tx_faulty) @@ -308,7 +314,6 @@ async fn faulty_t2_collided_on_contract_id_from_tx1() { #[tokio::test] async fn fail_to_insert_tx_with_dependency_on_invalid_utxo_type() { let mut context = TextContext::default(); - let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_coin) = context.setup_coin(); @@ -337,13 +342,13 @@ async fn fail_to_insert_tx_with_dependency_on_invalid_utxo_type() { let mut txpool = context.build(); let tx_faulty_id = tx_faulty.id(&ChainId::default()); - let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config, gas_price).await; + let tx_faulty = check_unwrap_tx(tx_faulty, &txpool.config).await; txpool .insert_single(tx_faulty.clone()) .expect("Tx1 should be Ok, got Err"); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let err = txpool .insert_single(tx) @@ -360,7 +365,6 @@ async fn not_inserted_known_tx() { utxo_validation: false, ..Default::default() }; - let gas_price = 0; let context = TextContext::default().config(config); let mut txpool = context.build(); @@ -368,7 +372,7 @@ async fn not_inserted_known_tx() { .add_random_fee_input() .finalize() .into(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; txpool .insert_single(tx.clone()) @@ -383,7 +387,6 @@ async fn not_inserted_known_tx() { #[tokio::test] async fn try_to_insert_tx2_missing_utxo() { let mut context = TextContext::default(); - let gas_price = 0; let input = context.random_predicate(AssetId::BASE, TEST_COIN_AMOUNT, None); let tx = TransactionBuilder::script(vec![], vec![]) @@ -394,7 +397,7 @@ async fn try_to_insert_tx2_missing_utxo() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let err = txpool .insert_single(tx) @@ -408,7 +411,6 @@ async fn try_to_insert_tx2_missing_utxo() { #[tokio::test] async fn higher_priced_tx_removes_lower_priced_tx() { let mut context = TextContext::default(); - let gas_price = 0; let (_, coin_input) = context.setup_coin(); @@ -428,13 +430,13 @@ async fn higher_priced_tx_removes_lower_priced_tx() { let tx1_id = tx1.id(&ChainId::default()); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; txpool .insert_single(tx1.clone()) .expect("Tx1 should be Ok, got Err"); - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; let vec = txpool .insert_single(tx2) @@ -445,7 +447,6 @@ async fn higher_priced_tx_removes_lower_priced_tx() { #[tokio::test] async fn underpriced_tx1_not_included_coin_collision() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(20); @@ -474,17 +475,17 @@ async fn underpriced_tx1_not_included_coin_collision() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1_checked = check_unwrap_tx(tx1.clone(), txpool.config(), gas_price).await; + let tx1_checked = check_unwrap_tx(tx1.clone(), txpool.config()).await; txpool .insert_single(tx1_checked) .expect("Tx1 should be Ok, got Err"); - let tx2_checked = check_unwrap_tx(tx2.clone(), txpool.config(), gas_price).await; + let tx2_checked = check_unwrap_tx(tx2.clone(), txpool.config()).await; txpool .insert_single(tx2_checked) .expect("Tx2 should be Ok, got Err"); - let tx3_checked = check_unwrap_tx(tx3, txpool.config(), gas_price).await; + let tx3_checked = check_unwrap_tx(tx3, txpool.config()).await; let err = txpool .insert_single(tx3_checked) .expect_err("Tx3 should be Err, got Ok"); @@ -497,7 +498,6 @@ async fn underpriced_tx1_not_included_coin_collision() { #[tokio::test] async fn overpriced_tx_contract_input_not_inserted() { let mut context = TextContext::default(); - let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_funds) = context.setup_coin(); @@ -527,12 +527,12 @@ async fn overpriced_tx_contract_input_not_inserted() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got err"); - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; let err = txpool .insert_single(tx2) .expect_err("Tx2 should be Err, got Ok"); @@ -548,7 +548,6 @@ async fn overpriced_tx_contract_input_not_inserted() { #[tokio::test] async fn dependent_contract_input_inserted() { let mut context = TextContext::default(); - let gas_price = 0; let contract_id = Contract::EMPTY_CONTRACT_ID; let (_, gas_funds) = context.setup_coin(); @@ -578,8 +577,8 @@ async fn dependent_contract_input_inserted() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got Err"); @@ -591,7 +590,6 @@ async fn dependent_contract_input_inserted() { #[tokio::test] async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); @@ -623,9 +621,9 @@ async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { let tx1_id = tx1.id(&ChainId::default()); let tx2_id = tx2.id(&ChainId::default()); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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.clone()) @@ -648,7 +646,6 @@ async fn more_priced_tx3_removes_tx1_and_dependent_tx2() { #[tokio::test] async fn more_priced_tx2_removes_tx1_and_more_priced_tx3_removes_tx2() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); @@ -674,9 +671,9 @@ async fn more_priced_tx2_removes_tx1_and_more_priced_tx3_removes_tx2() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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) @@ -701,7 +698,6 @@ async fn tx_limit_hit() { max_tx: 1, ..Default::default() }); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let tx1 = TransactionBuilder::script(vec![], vec![]) @@ -717,8 +713,8 @@ async fn tx_limit_hit() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; + let tx1 = check_unwrap_tx(tx1, &txpool.config).await; + let tx2 = check_unwrap_tx(tx2, &txpool.config).await; txpool .insert_single(tx1) .expect("Tx1 should be Ok, got Err"); @@ -735,7 +731,6 @@ async fn tx_depth_hit() { max_depth: 2, ..Default::default() }); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(10_000); @@ -760,9 +755,9 @@ async fn tx_depth_hit() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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) @@ -809,12 +804,10 @@ async fn sorted_out_tx1_2_4() { let tx2_id = tx2.id(&ChainId::default()); let tx3_id = tx3.id(&ChainId::default()); - let gas_price = 0; - let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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) @@ -837,7 +830,6 @@ async fn sorted_out_tx1_2_4() { #[tokio::test] async fn find_dependent_tx1_tx2() { let mut context = TextContext::default(); - let gas_price = 0; let (_, gas_coin) = context.setup_coin(); let (output, unset_input) = context.create_output_and_input(10_000); @@ -872,9 +864,9 @@ async fn find_dependent_tx1_tx2() { let tx3_id = tx3.id(&ChainId::default()); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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) @@ -916,7 +908,7 @@ async fn tx_at_least_min_gas_price_is_insertable() { .finalize_as_transaction(); let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx_with_gas_price(tx, &txpool.config, gas_price).await; txpool.insert_single(tx).expect("Tx should be Ok, got Err"); } @@ -933,7 +925,7 @@ async fn tx_below_min_gas_price_is_not_insertable() { .finalize_as_transaction(); let gas_price = 11; - let err = check_tx( + let err = check_tx_with_gas_price( tx, &Config { ..Default::default() @@ -963,9 +955,8 @@ async fn tx_inserted_into_pool_when_input_message_id_exists_in_db() { let tx1_id = tx.id(&ChainId::default()); let mut txpool = context.build(); - let gas_price = 0; - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; txpool.insert_single(tx).expect("should succeed"); let tx_info = txpool.find_one(&tx1_id).unwrap(); @@ -985,9 +976,8 @@ async fn tx_rejected_when_input_message_id_is_spent() { context.database_mut().insert_message(message.clone()); context.database_mut().spend_message(*message.id()); let mut txpool = context.build(); - let gas_price = 0; - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let err = txpool.insert_single(tx).expect_err("should fail"); // check error @@ -1005,12 +995,11 @@ async fn tx_rejected_from_pool_when_input_message_id_does_not_exist_in_db() { .script_gas_limit(GAS_LIMIT) .add_input(input) .finalize_as_transaction(); - let gas_price = 0; // Do not insert any messages into the DB to ensure there is no matching message for the // tx. let mut txpool = context.build(); - let tx = check_unwrap_tx(tx, &txpool.config, gas_price).await; + let tx = check_unwrap_tx(tx, &txpool.config).await; let err = txpool.insert_single(tx).expect_err("should fail"); // check error @@ -1050,14 +1039,16 @@ async fn tx_rejected_from_pool_when_gas_price_is_lower_than_another_tx_with_same let mut txpool = context.build(); let tx_high_id = tx_high.id(&ChainId::default()); - let tx_high = check_unwrap_tx(tx_high, &txpool.config, gas_price_high).await; + let tx_high = + check_unwrap_tx_with_gas_price(tx_high, &txpool.config, gas_price_high).await; // Insert a tx for the message id with a high gas amount txpool .insert_single(tx_high) .expect("expected successful insertion"); - let tx_low = check_unwrap_tx(tx_low, &txpool.config, gas_price_low).await; + let tx_low = + check_unwrap_tx_with_gas_price(tx_low, &txpool.config, gas_price_low).await; // Insert a tx for the message id with a low gas amount // Because the new transaction's id matches an existing transaction, we compare the gas // prices of both the new and existing transactions. Since the existing transaction's gas @@ -1093,7 +1084,8 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { let mut txpool = context.build(); let tx_low_id = tx_low.id(&ChainId::default()); - let tx_low = check_unwrap_tx(tx_low, &txpool.config, gas_price_low).await; + let tx_low = + check_unwrap_tx_with_gas_price(tx_low, &txpool.config, gas_price_low).await; txpool.insert_single(tx_low).expect("should succeed"); // Insert a tx for the message id with a high gas amount @@ -1106,7 +1098,8 @@ async fn higher_priced_tx_squeezes_out_lower_priced_tx_with_same_message_id() { .script_gas_limit(GAS_LIMIT) .add_input(conflicting_message_input) .finalize_as_transaction(); - let tx_high = check_unwrap_tx(tx_high, &txpool.config, gas_price_high).await; + let tx_high = + check_unwrap_tx_with_gas_price(tx_high, &txpool.config, gas_price_high).await; let squeezed_out_txs = txpool.insert_single(tx_high).expect("should succeed"); assert_eq!(squeezed_out_txs.removed.len(), 1); @@ -1148,15 +1141,13 @@ async fn message_of_squeezed_out_tx_can_be_resubmitted_at_lower_gas_price() { .add_input(message_input_2) .finalize_as_transaction(); - let gas_price = 0; - context.database_mut().insert_message(message_1); context.database_mut().insert_message(message_2); let mut txpool = context.build(); - let tx1 = check_unwrap_tx(tx1, &txpool.config, gas_price).await; - let tx2 = check_unwrap_tx(tx2, &txpool.config, gas_price).await; - let tx3 = check_unwrap_tx(tx3, &txpool.config, gas_price).await; + 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("should succeed"); @@ -1178,9 +1169,7 @@ async fn predicates_with_incorrect_owner_fails() { .add_input(coin) .finalize_as_transaction(); - let gas_price = 0; - - let err = check_tx(tx, &Default::default(), gas_price) + let err = check_tx(tx, &Default::default()) .await .expect_err("Transaction should be err, got ok"); @@ -1219,9 +1208,7 @@ async fn predicate_without_enough_gas_returns_out_of_gas() { .add_input(coin) .finalize_as_transaction(); - let gas_price = 0; - - let err = check_tx(tx, &Default::default(), gas_price) + let err = check_tx(tx, &Default::default()) .await .expect_err("Transaction should be err, got ok"); @@ -1250,9 +1237,7 @@ async fn predicate_that_returns_false_is_invalid() { .add_input(coin) .finalize_as_transaction(); - let gas_price = 0; - - let err = check_tx(tx, &Default::default(), gas_price) + let err = check_tx(tx, &Default::default()) .await .expect_err("Transaction should be err, got ok"); From f238c3e26e4349b478657b6b9aae9323845c27b2 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 15 Mar 2024 16:31:05 -0700 Subject: [PATCH 16/19] Move Mock types closer to tests --- .../producer/src/block_producer/gas_price.rs | 22 ---------------- .../producer/src/block_producer/tests.rs | 20 ++++++++++++++- .../txpool/src/service/test_helpers.rs | 25 ++++++++++++++++++- crates/services/txpool/src/txpool.rs | 23 ----------------- crates/services/txpool/src/txpool/tests.rs | 14 +++++------ 5 files changed, 49 insertions(+), 55 deletions(-) diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index 3164ac2baf..d08fa9a655 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -27,25 +27,3 @@ pub trait ProducerGasPrice { /// The gas price for all transactions in the block. fn gas_price(&self, params: GasPriceParams) -> Option; } - -pub struct MockProducerGasPrice { - pub gas_price: Option, -} - -impl MockProducerGasPrice { - pub fn new(gas_price: u64) -> Self { - Self { - gas_price: Some(gas_price), - } - } - - pub fn new_none() -> Self { - Self { gas_price: None } - } -} - -impl ProducerGasPrice for MockProducerGasPrice { - fn gas_price(&self, _params: GasPriceParams) -> Option { - self.gas_price - } -} diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index 6b0c386012..ab0ae161f4 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -2,7 +2,7 @@ use crate::{ block_producer::{ gas_price::{ - MockProducerGasPrice, + GasPriceParams, ProducerGasPrice, }, Error, @@ -43,6 +43,24 @@ use std::sync::{ Mutex, }; +pub struct MockProducerGasPrice { + pub gas_price: Option, +} + +impl MockProducerGasPrice { + pub fn new(gas_price: u64) -> Self { + Self { + gas_price: Some(gas_price), + } + } +} + +impl ProducerGasPrice for MockProducerGasPrice { + fn gas_price(&self, _params: GasPriceParams) -> Option { + self.gas_price + } +} + #[tokio::test] async fn cant_produce_at_genesis_height() { let ctx = TestContext::default(); diff --git a/crates/services/txpool/src/service/test_helpers.rs b/crates/services/txpool/src/service/test_helpers.rs index 0ec1c98e95..cbcf963bed 100644 --- a/crates/services/txpool/src/service/test_helpers.rs +++ b/crates/services/txpool/src/service/test_helpers.rs @@ -2,13 +2,13 @@ use super::*; use crate::{ mock_db::MockDBProvider, ports::BlockImporter, - txpool::MockTxPoolGasPrice, MockDb, }; use fuel_core_services::{ stream::BoxStream, Service as ServiceTrait, }; +use fuel_core_txpool::types::GasPrice; use fuel_core_types::{ blockchain::SealedBlock, entities::coins::coin::Coin, @@ -38,6 +38,29 @@ pub struct TestContext { rng: RefCell, } +#[derive(Debug, Clone)] +pub struct MockTxPoolGasPrice { + pub gas_price: Option, +} + +impl MockTxPoolGasPrice { + pub fn new(gas_price: GasPrice) -> Self { + Self { + gas_price: Some(gas_price), + } + } + + pub fn new_none() -> Self { + Self { gas_price: None } + } +} + +impl TxPoolGasPrice for MockTxPoolGasPrice { + fn gas_price(&self, _block_height: BlockHeight) -> Option { + self.gas_price + } +} + impl TestContext { pub async fn new() -> Self { TestContextBuilder::new().build_and_start().await diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 5151baaf82..3859b1452f 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -88,29 +88,6 @@ impl TxPoolGasPrice for Arc { } } -#[derive(Debug, Clone)] -pub struct MockTxPoolGasPrice { - pub gas_price: Option, -} - -impl MockTxPoolGasPrice { - pub fn new(gas_price: GasPrice) -> Self { - Self { - gas_price: Some(gas_price), - } - } - - pub fn new_none() -> Self { - Self { gas_price: None } - } -} - -impl TxPoolGasPrice for MockTxPoolGasPrice { - fn gas_price(&self, _block_height: BlockHeight) -> Option { - self.gas_price - } -} - impl TxPool { pub fn new(config: Config, database: ViewProvider) -> Self { let max_depth = config.max_depth; diff --git a/crates/services/txpool/src/txpool/tests.rs b/crates/services/txpool/src/txpool/tests.rs index 7c56424920..d131ad64ae 100644 --- a/crates/services/txpool/src/txpool/tests.rs +++ b/crates/services/txpool/src/txpool/tests.rs @@ -1,17 +1,15 @@ use crate::{ + service::test_helpers::MockTxPoolGasPrice, test_helpers::{ IntoEstimated, TextContext, TEST_COIN_AMOUNT, }, - txpool::{ - test_helpers::{ - create_coin_output, - create_contract_input, - create_contract_output, - create_message_predicate_from_message, - }, - MockTxPoolGasPrice, + txpool::test_helpers::{ + create_coin_output, + create_contract_input, + create_contract_output, + create_message_predicate_from_message, }, Config, Error, From 56ebbdd001694e43b31e305759b7a8ef71a6a3ac Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 15 Mar 2024 17:23:19 -0700 Subject: [PATCH 17/19] Add placeholder test and implement failure mode test --- .../producer/src/block_producer/tests.rs | 64 ++++++++++++------- crates/services/producer/src/mocks.rs | 31 +++++++++ crates/types/src/services/block_producer.rs | 2 +- 3 files changed, 74 insertions(+), 23 deletions(-) diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index ab0ae161f4..cc9ef8c63f 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -11,6 +11,7 @@ use crate::{ FailingMockExecutor, MockDb, MockExecutor, + MockExecutorWithCapture, MockRelayer, MockTxPool, }, @@ -53,6 +54,10 @@ impl MockProducerGasPrice { gas_price: Some(gas_price), } } + + pub fn new_none() -> Self { + Self { gas_price: None } + } } impl ProducerGasPrice for MockProducerGasPrice { @@ -232,31 +237,46 @@ async fn production_fails_on_execution_error() { // https://github.com/FuelLabs/fuel-core/issues/1751 #[ignore] #[tokio::test] -async fn produce_and_execute_block_txpool__includes_gas_price_provided_on_mint_tx() { - todo!() - // // given - // let gas_price = 1_000; - // let gas_price_provider = StaticGasPrice::new(gas_price); - // let ctx = TestContext::default(); - // let producer = ctx.producer_with_gas_price_provider(gas_price_provider); - // - // // when - // let changes = producer - // .produce_and_execute_block_txpool(1u32.into(), Tai64::now(), 1_000_000_000) - // .await - // .unwrap(); - // - // // then - // let mint_tx = changes.get_mint_tx(); - // assert_eq!(mint_tx.gas_price, gas_price); +async fn produce_and_execute_block_txpool__executor_receives_gas_price_provided() { + // given + let gas_price = 1_000; + let gas_price_provider = MockProducerGasPrice::new(gas_price); + let executor = MockExecutorWithCapture::default(); + let ctx = TestContext::default_from_executor(executor.clone()); + + let producer = ctx.producer_with_gas_price_provider(gas_price_provider); + + // when + let _ = producer + .produce_and_execute_block_txpool(1u32.into(), Tai64::now(), 1_000_000_000) + .await + .unwrap(); + + // then + let captured = executor.captured.lock().unwrap(); + let expected = gas_price; + let actual = captured + .as_ref() + .expect("expected executor to be called") + .gas_price; + assert_eq!(expected, actual); } -// TODO: Add test that checks the failure case for gas_price after `Executor` refactor -// https://github.com/FuelLabs/fuel-core/issues/1751 -#[ignore] #[tokio::test] -async fn produce_and_execute_block_txpool__block_production_fails() { - todo!() +async fn produce_and_execute_block_txpool__missing_gas_price_causes_block_production_to_fail( +) { + // given + let gas_price_provider = MockProducerGasPrice::new_none(); + let ctx = TestContext::default(); + let producer = ctx.producer_with_gas_price_provider(gas_price_provider); + + // when + let result = producer + .produce_and_execute_block_txpool(1u32.into(), Tai64::now(), 1_000_000_000) + .await; + + // then + assert!(result.is_err()); } struct TestContext { diff --git a/crates/services/producer/src/mocks.rs b/crates/services/producer/src/mocks.rs index 1409bbabc6..648b85aa8d 100644 --- a/crates/services/producer/src/mocks.rs +++ b/crates/services/producer/src/mocks.rs @@ -169,6 +169,37 @@ impl Executor> for FailingMockExecutor { } } +#[derive(Clone)] +pub struct MockExecutorWithCapture { + pub captured: Arc>>>>, +} + +impl Executor> for MockExecutorWithCapture { + fn execute_without_commit( + &self, + component: Components>, + ) -> ExecutorResult> { + *self.captured.lock().unwrap() = Some(component.clone()); + Ok(UncommittedResult::new( + ExecutionResult { + block: to_block(component), + skipped_transactions: vec![], + tx_status: vec![], + events: vec![], + }, + Default::default(), + )) + } +} + +impl Default for MockExecutorWithCapture { + fn default() -> Self { + Self { + captured: Arc::new(Mutex::new(None)), + } + } +} + #[derive(Clone, Default, Debug)] pub struct MockDb { pub blocks: Arc>>, diff --git a/crates/types/src/services/block_producer.rs b/crates/types/src/services/block_producer.rs index 586ea00887..a03ae753a4 100644 --- a/crates/types/src/services/block_producer.rs +++ b/crates/types/src/services/block_producer.rs @@ -3,7 +3,7 @@ use crate::blockchain::header::PartialBlockHeader; /// The components required to produce a block. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Components { /// The partial block header of the future block without transactions related information. pub header_to_produce: PartialBlockHeader, From e13dada9a98334dbbdcf00f0f0c533a4f23170c1 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 15 Mar 2024 17:25:31 -0700 Subject: [PATCH 18/19] Appease Clippy-sama --- tests/tests/gas_price.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/gas_price.rs b/tests/tests/gas_price.rs index 367f5b5bb5..9e76366458 100644 --- a/tests/tests/gas_price.rs +++ b/tests/tests/gas_price.rs @@ -45,7 +45,7 @@ async fn setup_service_with_coin( let tx_id = [0u8; 32]; let output_index = 0; let coin_config = CoinConfig { - tx_id: tx_id.clone().into(), + tx_id: tx_id.into(), output_index, tx_pointer_block_height: Default::default(), tx_pointer_tx_idx: 0, From 9a4c9d8f32c3c919446985cc0d3560eba97eeea7 Mon Sep 17 00:00:00 2001 From: xgreenx Date: Sat, 16 Mar 2024 12:36:09 +0100 Subject: [PATCH 19/19] Small renaming and removed `Clone` from `Components` --- crates/fuel-core/src/service/adapters.rs | 29 +------------------ .../src/service/adapters/graphql_api.rs | 9 ++++++ .../src/service/adapters/producer.rs | 15 +++++++++- .../fuel-core/src/service/adapters/txpool.rs | 13 ++++++++- .../services/producer/src/block_producer.rs | 10 +++---- .../producer/src/block_producer/gas_price.rs | 2 +- .../producer/src/block_producer/tests.rs | 13 ++++----- crates/services/producer/src/mocks.rs | 14 +++++---- crates/services/txpool/src/service.rs | 10 +++---- .../txpool/src/service/test_helpers.rs | 2 +- crates/services/txpool/src/txpool.rs | 15 ++++++---- crates/types/src/services/block_producer.rs | 2 +- 12 files changed, 72 insertions(+), 62 deletions(-) diff --git a/crates/fuel-core/src/service/adapters.rs b/crates/fuel-core/src/service/adapters.rs index f980f7240b..58f0405f1e 100644 --- a/crates/fuel-core/src/service/adapters.rs +++ b/crates/fuel-core/src/service/adapters.rs @@ -3,7 +3,6 @@ use crate::{ database_description::relayer::Relayer, Database, }, - graphql_api::ports::GasPriceEstimate, service::sub_services::BlockProducerService, }; use fuel_core_consensus_module::{ @@ -11,15 +10,8 @@ use fuel_core_consensus_module::{ RelayerConsensusConfig, }; use fuel_core_executor::executor::Executor; -use fuel_core_producer::block_producer::gas_price::{ - GasPriceParams, - ProducerGasPrice, -}; use fuel_core_services::stream::BoxStream; -use fuel_core_txpool::{ - service::SharedState as TxPoolSharedState, - txpool::TxPoolGasPrice, -}; +use fuel_core_txpool::service::SharedState as TxPoolSharedState; #[cfg(feature = "p2p")] use fuel_core_types::services::p2p::peer_reputation::AppScore; use fuel_core_types::{ @@ -52,25 +44,6 @@ impl StaticGasPrice { } } -impl TxPoolGasPrice for StaticGasPrice { - fn gas_price(&self, _block_height: BlockHeight) -> Option { - Some(self.gas_price) - } -} - -impl ProducerGasPrice for StaticGasPrice { - fn gas_price(&self, _block_height: GasPriceParams) -> Option { - Some(self.gas_price) - } -} - -#[async_trait::async_trait] -impl GasPriceEstimate for StaticGasPrice { - async fn worst_case_gas_price(&self, _height: BlockHeight) -> u64 { - self.gas_price - } -} - #[derive(Clone)] pub struct PoAAdapter { shared_state: Option, diff --git a/crates/fuel-core/src/service/adapters/graphql_api.rs b/crates/fuel-core/src/service/adapters/graphql_api.rs index b3ef02fc74..0551884f9d 100644 --- a/crates/fuel-core/src/service/adapters/graphql_api.rs +++ b/crates/fuel-core/src/service/adapters/graphql_api.rs @@ -1,6 +1,7 @@ use super::{ BlockImporterAdapter, BlockProducerAdapter, + StaticGasPrice, }; use crate::{ database::Database, @@ -8,6 +9,7 @@ use crate::{ worker, BlockProducerPort, DatabaseMessageProof, + GasPriceEstimate, P2pPort, TxPoolPort, }, @@ -161,3 +163,10 @@ impl worker::TxPool for TxPoolAdapter { self.service.send_complete(id, block_height, status) } } + +#[async_trait::async_trait] +impl GasPriceEstimate for StaticGasPrice { + async fn worst_case_gas_price(&self, _height: BlockHeight) -> u64 { + self.gas_price + } +} diff --git a/crates/fuel-core/src/service/adapters/producer.rs b/crates/fuel-core/src/service/adapters/producer.rs index 573bc66879..3c131a96d2 100644 --- a/crates/fuel-core/src/service/adapters/producer.rs +++ b/crates/fuel-core/src/service/adapters/producer.rs @@ -5,6 +5,7 @@ use crate::{ BlockProducerAdapter, ExecutorAdapter, MaybeRelayerAdapter, + StaticGasPrice, TransactionsSource, TxPoolAdapter, }, @@ -12,7 +13,13 @@ use crate::{ }, }; use fuel_core_executor::executor::OnceTransactionsSource; -use fuel_core_producer::ports::TxPool; +use fuel_core_producer::{ + block_producer::gas_price::{ + GasPriceParams, + GasPriceProvider, + }, + ports::TxPool, +}; use fuel_core_storage::{ not_found, tables::FuelBlocks, @@ -140,3 +147,9 @@ impl fuel_core_producer::ports::BlockProducerDatabase for Database { self.storage::().root(height).map(Into::into) } } + +impl GasPriceProvider for StaticGasPrice { + fn gas_price(&self, _block_height: GasPriceParams) -> Option { + Some(self.gas_price) + } +} diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs index 02914e0f5d..6b243a3b91 100644 --- a/crates/fuel-core/src/service/adapters/txpool.rs +++ b/crates/fuel-core/src/service/adapters/txpool.rs @@ -3,6 +3,7 @@ use crate::{ service::adapters::{ BlockImporterAdapter, P2PAdapter, + StaticGasPrice, }, }; use fuel_core_services::stream::BoxStream; @@ -16,7 +17,10 @@ use fuel_core_storage::{ Result as StorageResult, StorageAsRef, }; -use fuel_core_txpool::ports::BlockImporter; +use fuel_core_txpool::{ + ports::BlockImporter, + txpool::GasPriceProvider, +}; use fuel_core_types::{ entities::{ coins::coin::CompressedCoin, @@ -27,6 +31,7 @@ use fuel_core_types::{ UtxoId, }, fuel_types::{ + BlockHeight, ContractId, Nonce, }, @@ -132,3 +137,9 @@ impl fuel_core_txpool::ports::TxPoolDb for Database { self.storage::().contains_key(id) } } + +impl GasPriceProvider for StaticGasPrice { + fn gas_price(&self, _block_height: BlockHeight) -> Option { + Some(self.gas_price) + } +} diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index be56e675fa..a03914cdd5 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -1,5 +1,5 @@ use crate::{ - block_producer::gas_price::ProducerGasPrice, + block_producer::gas_price::GasPriceProvider as GasPriceProviderConstraint, ports, ports::BlockProducerDatabase, Config, @@ -90,7 +90,7 @@ impl where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, - GasPriceProvider: ProducerGasPrice, + GasPriceProvider: GasPriceProviderConstraint, { /// Produces and execute block for the specified height. async fn produce_and_execute( @@ -151,7 +151,7 @@ where ViewProvider::View: BlockProducerDatabase, TxPool: ports::TxPool + 'static, Executor: ports::Executor + 'static, - GasPriceProvider: ProducerGasPrice, + GasPriceProvider: GasPriceProviderConstraint, { /// Produces and execute block for the specified height with transactions from the `TxPool`. pub async fn produce_and_execute_block_txpool( @@ -176,7 +176,7 @@ where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, Executor: ports::Executor> + 'static, - GasPriceProvider: ProducerGasPrice, + GasPriceProvider: GasPriceProviderConstraint, { /// Produces and execute block for the specified height with `transactions`. pub async fn produce_and_execute_block_transactions( @@ -197,7 +197,7 @@ where ViewProvider: AtomicView + 'static, ViewProvider::View: BlockProducerDatabase, Executor: ports::DryRunner + 'static, - GasPriceProvider: ProducerGasPrice, + GasPriceProvider: GasPriceProviderConstraint, { // TODO: Support custom `block_time` for `dry_run`. /// Simulates multiple transactions without altering any state. Does not acquire the production lock. diff --git a/crates/services/producer/src/block_producer/gas_price.rs b/crates/services/producer/src/block_producer/gas_price.rs index d08fa9a655..c5d186481c 100644 --- a/crates/services/producer/src/block_producer/gas_price.rs +++ b/crates/services/producer/src/block_producer/gas_price.rs @@ -23,7 +23,7 @@ impl From for GasPriceParams { } /// Interface for retrieving the gas price for a block -pub trait ProducerGasPrice { +pub trait GasPriceProvider { /// The gas price for all transactions in the block. fn gas_price(&self, params: GasPriceParams) -> Option; } diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index cc9ef8c63f..41caa0af32 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -3,7 +3,7 @@ use crate::{ block_producer::{ gas_price::{ GasPriceParams, - ProducerGasPrice, + GasPriceProvider, }, Error, }, @@ -60,7 +60,7 @@ impl MockProducerGasPrice { } } -impl ProducerGasPrice for MockProducerGasPrice { +impl GasPriceProvider for MockProducerGasPrice { fn gas_price(&self, _params: GasPriceParams) -> Option { self.gas_price } @@ -235,7 +235,6 @@ async fn production_fails_on_execution_error() { // TODO: Add test that checks the gas price on the mint tx after `Executor` refactor // https://github.com/FuelLabs/fuel-core/issues/1751 -#[ignore] #[tokio::test] async fn produce_and_execute_block_txpool__executor_receives_gas_price_provided() { // given @@ -338,12 +337,12 @@ impl TestContext { self.producer_with_gas_price_provider(static_gas_price) } - pub fn producer_with_gas_price_provider( + pub fn producer_with_gas_price_provider( self, - gas_price_provider: GasPriceProvider, - ) -> Producer + gas_price_provider: GasPrice, + ) -> Producer where - GasPriceProvider: ProducerGasPrice, + GasPrice: GasPriceProvider, { Producer { config: self.config, diff --git a/crates/services/producer/src/mocks.rs b/crates/services/producer/src/mocks.rs index 648b85aa8d..b909ed588e 100644 --- a/crates/services/producer/src/mocks.rs +++ b/crates/services/producer/src/mocks.rs @@ -110,13 +110,14 @@ impl AsRef for MockDb { } } -fn to_block(component: Components>) -> Block { +fn to_block(component: &Components>) -> Block { let transactions = component .transactions_source + .clone() .into_iter() .map(|tx| tx.as_ref().into()) .collect(); - Block::new(component.header_to_produce, transactions, &[]) + Block::new(component.header_to_produce.clone(), transactions, &[]) } impl Executor> for MockExecutor { @@ -124,7 +125,7 @@ impl Executor> for MockExecutor { &self, component: Components>, ) -> ExecutorResult> { - let block = to_block(component); + let block = to_block(&component); // simulate executor inserting a block let mut block_db = self.0.blocks.lock().unwrap(); block_db.insert( @@ -155,7 +156,7 @@ impl Executor> for FailingMockExecutor { if let Some(err) = err.take() { Err(err) } else { - let block = to_block(component); + let block = to_block(&component); Ok(UncommittedResult::new( ExecutionResult { block, @@ -179,10 +180,11 @@ impl Executor> for MockExecutorWithCapture { &self, component: Components>, ) -> ExecutorResult> { - *self.captured.lock().unwrap() = Some(component.clone()); + let block = to_block(&component); + *self.captured.lock().unwrap() = Some(component); Ok(UncommittedResult::new( ExecutionResult { - block: to_block(component), + block, skipped_transactions: vec![], tx_status: vec![], events: vec![], diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs index d9219e50e9..f6ac475c35 100644 --- a/crates/services/txpool/src/service.rs +++ b/crates/services/txpool/src/service.rs @@ -8,6 +8,7 @@ use crate::{ txpool::{ check_single_tx, check_transactions, + GasPriceProvider as GasPriceProviderConstraint, }, Config, Error as TxPoolError, @@ -50,7 +51,6 @@ use fuel_core_types::{ tai64::Tai64, }; -use crate::txpool::TxPoolGasPrice; use anyhow::anyhow; use fuel_core_storage::transactional::AtomicView; use fuel_core_types::services::block_importer::SharedImportResult; @@ -161,7 +161,7 @@ where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, - GasPriceProvider: TxPoolGasPrice + Send + Sync + Clone, + GasPriceProvider: GasPriceProviderConstraint + Send + Sync + Clone, { const NAME: &'static str = "TxPool"; @@ -190,7 +190,7 @@ where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, - GasPriceProvider: TxPoolGasPrice + Send + Sync, + GasPriceProvider: GasPriceProviderConstraint + Send + Sync, { async fn run(&mut self, watcher: &mut StateWatcher) -> anyhow::Result { let should_continue; @@ -371,7 +371,7 @@ where P2P: PeerToPeer, ViewProvider: AtomicView, View: TxPoolDb, - GasPriceProvider: TxPoolGasPrice, + GasPriceProvider: GasPriceProviderConstraint, { #[tracing::instrument(name = "insert_submitted_txn", skip_all)] pub async fn insert( @@ -476,7 +476,7 @@ where P2P: PeerToPeer + 'static, ViewProvider: AtomicView, ViewProvider::View: TxPoolDb, - GasPriceProvider: TxPoolGasPrice + Send + Sync + Clone, + GasPriceProvider: GasPriceProviderConstraint + Send + Sync + Clone, { let p2p = Arc::new(p2p); let gossiped_tx_stream = p2p.gossiped_transaction_events(); diff --git a/crates/services/txpool/src/service/test_helpers.rs b/crates/services/txpool/src/service/test_helpers.rs index cbcf963bed..63e2459ba1 100644 --- a/crates/services/txpool/src/service/test_helpers.rs +++ b/crates/services/txpool/src/service/test_helpers.rs @@ -55,7 +55,7 @@ impl MockTxPoolGasPrice { } } -impl TxPoolGasPrice for MockTxPoolGasPrice { +impl GasPriceProviderConstraint for MockTxPoolGasPrice { fn gas_price(&self, _block_height: BlockHeight) -> Option { self.gas_price } diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index 3859b1452f..4e575f41aa 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -77,12 +77,12 @@ pub struct TxPool { } /// Trait for getting gas price for the Tx Pool code to look up the gas price for a given block height -pub trait TxPoolGasPrice { +pub trait GasPriceProvider { /// Get gas price for specific block height if it is known fn gas_price(&self, block_height: BlockHeight) -> Option; } -impl TxPoolGasPrice for Arc { +impl GasPriceProvider for Arc { fn gas_price(&self, block_height: BlockHeight) -> Option { self.deref().gas_price(block_height) } @@ -475,12 +475,15 @@ where } } -pub async fn check_transactions( +pub async fn check_transactions( txs: &[Arc], current_height: BlockHeight, config: &Config, - gas_price_provider: &GasPriceProvider, -) -> Vec, Error>> { + gas_price_provider: &Provider, +) -> Vec, Error>> +where + Provider: GasPriceProvider, +{ let mut checked_txs = Vec::with_capacity(txs.len()); for tx in txs.iter() { @@ -498,7 +501,7 @@ pub async fn check_transactions( checked_txs } -pub async fn check_single_tx( +pub async fn check_single_tx( tx: Transaction, current_height: BlockHeight, config: &Config, diff --git a/crates/types/src/services/block_producer.rs b/crates/types/src/services/block_producer.rs index a03ae753a4..586ea00887 100644 --- a/crates/types/src/services/block_producer.rs +++ b/crates/types/src/services/block_producer.rs @@ -3,7 +3,7 @@ use crate::blockchain::header::PartialBlockHeader; /// The components required to produce a block. -#[derive(Debug, Clone)] +#[derive(Debug)] pub struct Components { /// The partial block header of the future block without transactions related information. pub header_to_produce: PartialBlockHeader,