diff --git a/CHANGELOG.md b/CHANGELOG.md index e652cf7af33..8bf912c3c7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [#1948](https://github.com/FuelLabs/fuel-core/pull/1948): Add new `AlgorithmV1` and `AlgorithmUpdaterV1` for the gas price. Include tools for analysis ### Changed +- [#2002](https://github.com/FuelLabs/fuel-core/pull/2002): Adapted the block producer to react to checked transactions that were using another version of consensus parameters during validation in the TxPool. After an upgrade of the consensus parameters of the network, TxPool could store invalid `Checked` transactions. This change fixes that by tracking the version that was used to validate the transactions. - [#1999](https://github.com/FuelLabs/fuel-core/pull/1999): Minimize the number of panics in the codebase. - [#1990](https://github.com/FuelLabs/fuel-core/pull/1990): Use latest view for mutate GraphQL queries after modification of the node. - [#1992](https://github.com/FuelLabs/fuel-core/pull/1992): Parse multiple relayer contracts, `RELAYER-V2-LISTENING-CONTRACTS` env variable using a `,` delimiter. diff --git a/bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm b/bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm index bcf7a7cc9f2..334fbfda3dc 100755 Binary files a/bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm and b/bin/fuel-core/chainspec/local-testnet/state_transition_bytecode.wasm differ diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 7a1bf096799..151b6131d41 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -7,7 +7,10 @@ mod tests { use fuel_core::database::Database; use fuel_core_executor::{ executor::OnceTransactionsSource, - ports::RelayerPort, + ports::{ + MaybeCheckedTransaction, + RelayerPort, + }, refs::ContractRef, }; use fuel_core_storage::{ @@ -32,6 +35,7 @@ mod tests { PartialFuelBlock, }, header::{ + ApplicationHeader, ConsensusHeader, PartialBlockHeader, }, @@ -52,6 +56,7 @@ mod tests { fuel_crypto::SecretKey, fuel_merkle::sparse, fuel_tx::{ + consensus_parameters::gas::GasCostsValuesV1, field::{ InputContract, Inputs, @@ -76,8 +81,10 @@ mod tests { Cacheable, ConsensusParameters, Create, + DependentCost, FeeParameters, Finalizable, + GasCostsValues, Output, Receipt, Script, @@ -104,6 +111,7 @@ mod tests { checked_transaction::{ CheckError, EstimatePredicates, + IntoChecked, }, interpreter::{ ExecutableTransaction, @@ -2730,6 +2738,90 @@ mod tests { assert!(result.is_ok(), "{result:?}") } + #[test] + fn verifying_during_production_consensus_parameters_version_works() { + let mut rng = StdRng::seed_from_u64(2322u64); + let predicate: Vec = vec![op::ret(RegId::ONE)].into_iter().collect(); + let owner = Input::predicate_owner(&predicate); + let amount = 1000; + let cheap_consensus_parameters = ConsensusParameters::default(); + + let mut tx = TransactionBuilder::script(vec![], vec![]) + .max_fee_limit(amount) + .add_input(Input::coin_predicate( + rng.gen(), + owner, + amount, + AssetId::BASE, + rng.gen(), + 0, + predicate, + vec![], + )) + .finalize(); + tx.estimate_predicates( + &cheap_consensus_parameters.clone().into(), + MemoryInstance::new(), + ) + .unwrap(); + + // Given + let gas_costs: GasCostsValues = GasCostsValuesV1 { + vm_initialization: DependentCost::HeavyOperation { + base: u32::MAX as u64, + gas_per_unit: 0, + }, + ..GasCostsValuesV1::free() + } + .into(); + let expensive_consensus_parameters_version = 0; + let mut expensive_consensus_parameters = ConsensusParameters::default(); + expensive_consensus_parameters.set_gas_costs(gas_costs.into()); + let config = Config { + consensus_parameters: expensive_consensus_parameters.clone(), + ..Default::default() + }; + let producer = create_executor(Database::default(), config.clone()); + + let cheap_consensus_parameters_version = 1; + let cheaply_checked_tx = MaybeCheckedTransaction::CheckedTransaction( + tx.into_checked_basic(0u32.into(), &cheap_consensus_parameters) + .unwrap() + .into(), + cheap_consensus_parameters_version, + ); + + // When + let ExecutionResult { + skipped_transactions, + .. + } = producer + .produce_without_commit_with_source(Components { + header_to_produce: PartialBlockHeader { + application: ApplicationHeader { + consensus_parameters_version: + expensive_consensus_parameters_version, + ..Default::default() + }, + ..Default::default() + }, + transactions_source: OnceTransactionsSource::new_maybe_checked(vec![ + cheaply_checked_tx, + ]), + coinbase_recipient: Default::default(), + gas_price: 1, + }) + .unwrap() + .into_result(); + + // Then + assert_eq!(skipped_transactions.len(), 1); + assert!(matches!( + skipped_transactions[0].1, + ExecutorError::InvalidTransaction(_) + )); + } + #[cfg(feature = "relayer")] mod relayer { use super::*; diff --git a/crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs b/crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs index 02e8815009d..64bf8ef40ae 100644 --- a/crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs +++ b/crates/fuel-core/src/service/adapters/consensus_parameters_provider.rs @@ -95,9 +95,16 @@ impl SharedState { } pub fn latest_consensus_parameters(&self) -> Arc { + self.latest_consensus_parameters_with_version().1 + } + + pub fn latest_consensus_parameters_with_version( + &self, + ) -> (ConsensusParametersVersion, Arc) { let version = *self.latest_consensus_parameters_version.lock(); - self.get_consensus_parameters(&version) - .expect("The latest consensus parameters always are available unless this function was called before regenesis.") + let params = self.get_consensus_parameters(&version) + .expect("The latest consensus parameters always are available unless this function was called before regenesis."); + (version, params) } } diff --git a/crates/fuel-core/src/service/adapters/executor.rs b/crates/fuel-core/src/service/adapters/executor.rs index e9a4739aa38..787e0db6558 100644 --- a/crates/fuel-core/src/service/adapters/executor.rs +++ b/crates/fuel-core/src/service/adapters/executor.rs @@ -13,7 +13,12 @@ impl fuel_core_executor::ports::TransactionsSource for TransactionsSource { self.txpool .select_transactions(gas_limit) .into_iter() - .map(|tx| MaybeCheckedTransaction::CheckedTransaction(tx.as_ref().into())) + .map(|tx| { + MaybeCheckedTransaction::CheckedTransaction( + tx.as_ref().into(), + tx.used_consensus_parameters_version(), + ) + }) .collect() } } diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs index d9314ecdad3..99544558e76 100644 --- a/crates/fuel-core/src/service/adapters/txpool.rs +++ b/crates/fuel-core/src/service/adapters/txpool.rs @@ -28,6 +28,7 @@ use fuel_core_txpool::ports::{ MemoryPool, }; use fuel_core_types::{ + blockchain::header::ConsensusParametersVersion, entities::{ coins::coin::CompressedCoin, relayer::message::Message, @@ -149,8 +150,10 @@ impl GasPriceProvider for StaticGasPrice { } impl ConsensusParametersProviderTrait for ConsensusParametersProvider { - fn latest_consensus_parameters(&self) -> Arc { - self.shared_state.latest_consensus_parameters() + fn latest_consensus_parameters( + &self, + ) -> (ConsensusParametersVersion, Arc) { + self.shared_state.latest_consensus_parameters_with_version() } } diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index f9170b6bcdd..d890b1720f7 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -164,6 +164,12 @@ impl OnceTransactionsSource { ), } } + + pub fn new_maybe_checked(transactions: Vec) -> Self { + Self { + transactions: ParkingMutex::new(transactions), + } + } } impl TransactionsSource for OnceTransactionsSource { @@ -697,10 +703,13 @@ where { let block_header = partial_block.header; let block_height = block_header.height(); + let consensus_parameters_version = block_header.consensus_parameters_version; let relayed_tx_iter = forced_transactions.into_iter(); - for transaction in relayed_tx_iter { - let maybe_checked_transaction = - MaybeCheckedTransaction::CheckedTransaction(transaction); + for checked in relayed_tx_iter { + let maybe_checked_transaction = MaybeCheckedTransaction::CheckedTransaction( + checked, + consensus_parameters_version, + ); let tx_id = maybe_checked_transaction.id(&self.consensus_params.chain_id()); match self.execute_transaction_and_commit( partial_block, @@ -1038,11 +1047,21 @@ where header: &PartialBlockHeader, ) -> ExecutorResult { let block_height = *header.height(); + let actual_version = header.consensus_parameters_version; let checked_tx = match tx { MaybeCheckedTransaction::Transaction(tx) => tx .into_checked_basic(block_height, &self.consensus_params)? .into(), - MaybeCheckedTransaction::CheckedTransaction(checked_tx) => checked_tx, + MaybeCheckedTransaction::CheckedTransaction(checked_tx, checked_version) => { + if actual_version == checked_version { + checked_tx + } else { + let checked_tx: Checked = checked_tx.into(); + let (tx, _) = checked_tx.into(); + tx.into_checked_basic(block_height, &self.consensus_params)? + .into() + } + } }; Ok(checked_tx) } diff --git a/crates/services/executor/src/ports.rs b/crates/services/executor/src/ports.rs index 74d169e62c4..29ac0d07302 100644 --- a/crates/services/executor/src/ports.rs +++ b/crates/services/executor/src/ports.rs @@ -1,5 +1,8 @@ use fuel_core_types::{ - blockchain::primitives::DaBlockHeight, + blockchain::{ + header::ConsensusParametersVersion, + primitives::DaBlockHeight, + }, fuel_tx, fuel_tx::{ TxId, @@ -12,28 +15,33 @@ use fuel_core_types::{ /// The wrapper around either `Transaction` or `CheckedTransaction`. pub enum MaybeCheckedTransaction { - CheckedTransaction(CheckedTransaction), + CheckedTransaction(CheckedTransaction, ConsensusParametersVersion), Transaction(fuel_tx::Transaction), } impl MaybeCheckedTransaction { pub fn id(&self, chain_id: &ChainId) -> TxId { match self { - MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Script( - tx, - )) => tx.id(), - MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Create( - tx, - )) => tx.id(), - MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Mint(tx)) => { - tx.id() - } - MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Upgrade( - tx, - )) => tx.id(), - MaybeCheckedTransaction::CheckedTransaction(CheckedTransaction::Upload( - tx, - )) => tx.id(), + MaybeCheckedTransaction::CheckedTransaction( + CheckedTransaction::Script(tx), + _, + ) => tx.id(), + MaybeCheckedTransaction::CheckedTransaction( + CheckedTransaction::Create(tx), + _, + ) => tx.id(), + MaybeCheckedTransaction::CheckedTransaction( + CheckedTransaction::Mint(tx), + _, + ) => tx.id(), + MaybeCheckedTransaction::CheckedTransaction( + CheckedTransaction::Upgrade(tx), + _, + ) => tx.id(), + MaybeCheckedTransaction::CheckedTransaction( + CheckedTransaction::Upload(tx), + _, + ) => tx.id(), MaybeCheckedTransaction::Transaction(tx) => tx.id(chain_id), } } diff --git a/crates/services/txpool/src/ports.rs b/crates/services/txpool/src/ports.rs index 9931b8e3160..2cdeec3aca1 100644 --- a/crates/services/txpool/src/ports.rs +++ b/crates/services/txpool/src/ports.rs @@ -2,6 +2,7 @@ use crate::types::GasPrice; use fuel_core_services::stream::BoxStream; use fuel_core_storage::Result as StorageResult; use fuel_core_types::{ + blockchain::header::ConsensusParametersVersion, entities::{ coins::coin::CompressedCoin, relayer::message::Message, @@ -81,7 +82,9 @@ pub trait MemoryPool { #[cfg_attr(feature = "test-helpers", mockall::automock)] pub trait ConsensusParametersProvider { /// Get latest consensus parameters. - fn latest_consensus_parameters(&self) -> Arc; + fn latest_consensus_parameters( + &self, + ) -> (ConsensusParametersVersion, Arc); } #[async_trait::async_trait] diff --git a/crates/services/txpool/src/service.rs b/crates/services/txpool/src/service.rs index fcd5a31e82c..05f103a4236 100644 --- a/crates/services/txpool/src/service.rs +++ b/crates/services/txpool/src/service.rs @@ -240,7 +240,7 @@ where new_transaction = self.gossiped_tx_stream.next() => { if let Some(GossipData { data: Some(tx), message_id, peer_id }) = new_transaction { let current_height = *self.tx_pool_shared_state.current_height.lock(); - let params = self + let (version, params) = self .tx_pool_shared_state .consensus_parameters_provider .latest_consensus_parameters(); @@ -265,6 +265,7 @@ where .in_scope(|| { self.tx_pool_shared_state.txpool.lock().insert( &self.tx_pool_shared_state.tx_status_sender, + version, txs ) }); @@ -400,7 +401,7 @@ where ) -> Vec> { // verify txs let current_height = *self.current_height.lock(); - let params = self + let (version, params) = self .consensus_parameters_provider .latest_consensus_parameters(); @@ -428,7 +429,11 @@ where .collect(); // insert txs - let insertion = { self.txpool.lock().insert(&self.tx_status_sender, valid_txs) }; + let insertion = { + self.txpool + .lock() + .insert(&self.tx_status_sender, version, valid_txs) + }; for (ret, tx) in insertion.iter().zip(txs.into_iter()) { match ret { diff --git a/crates/services/txpool/src/service/test_helpers.rs b/crates/services/txpool/src/service/test_helpers.rs index 0e76a51fede..2c6643a4008 100644 --- a/crates/services/txpool/src/service/test_helpers.rs +++ b/crates/services/txpool/src/service/test_helpers.rs @@ -265,7 +265,7 @@ impl TestContextBuilder { MockConsensusParametersProvider::default(); consensus_parameters_provider .expect_latest_consensus_parameters() - .returning(|| Arc::new(Default::default())); + .returning(|| (0, Arc::new(Default::default()))); let service = new_service( config, diff --git a/crates/services/txpool/src/transaction_selector.rs b/crates/services/txpool/src/transaction_selector.rs index ef4ab001b83..80158172714 100644 --- a/crates/services/txpool/src/transaction_selector.rs +++ b/crates/services/txpool/src/transaction_selector.rs @@ -43,6 +43,7 @@ pub fn select_transactions( mod tests { use fuel_core_txpool as _; use fuel_core_types::{ + blockchain::header::ConsensusParametersVersion, fuel_asm::{ op, RegId, @@ -61,6 +62,7 @@ mod tests { checked_transaction::builder::TransactionBuilderExt, SecretKey, }, + services::txpool::PoolTransaction, }; use itertools::Itertools; use std::sync::Arc; @@ -85,7 +87,7 @@ mod tests { let mut txs = txs .iter() .map(|tx_gas| { - TransactionBuilder::script( + let script = TransactionBuilder::script( vec![op::ret(RegId::ONE)].into_iter().collect(), vec![], ) @@ -107,7 +109,9 @@ mod tests { .with_gas_costs(GasCosts::free()) // The block producer assumes transactions are already checked // so it doesn't need to compute valid sigs for tests - .finalize_checked_basic(Default::default()).into() + .finalize_checked_basic(Default::default()); + + PoolTransaction::Script(script, ConsensusParametersVersion::MIN) }) .map(Arc::new) .collect::>(); diff --git a/crates/services/txpool/src/txpool.rs b/crates/services/txpool/src/txpool.rs index eab09da10ac..0afaa8216dd 100644 --- a/crates/services/txpool/src/txpool.rs +++ b/crates/services/txpool/src/txpool.rs @@ -36,6 +36,7 @@ use crate::ports::{ use fuel_core_metrics::txpool_metrics::txpool_metrics; use fuel_core_storage::transactional::AtomicView; use fuel_core_types::{ + blockchain::header::ConsensusParametersVersion, fuel_tx::{ input::{ coin::{ @@ -343,7 +344,7 @@ where tx: Checked, ) -> Result { let view = self.database.latest_view().unwrap(); - self.insert_inner(tx, &view) + self.insert_inner(tx, ConsensusParametersVersion::MIN, &view) } #[tracing::instrument(level = "debug", skip_all, fields(tx_id = %tx.id()), ret, err)] @@ -351,16 +352,17 @@ where fn insert_inner( &mut self, tx: Checked, + version: ConsensusParametersVersion, view: &View, ) -> Result { let tx: CheckedTransaction = tx.into(); let tx = Arc::new(match tx { - CheckedTransaction::Script(tx) => PoolTransaction::Script(tx), - CheckedTransaction::Create(tx) => PoolTransaction::Create(tx), + CheckedTransaction::Script(tx) => PoolTransaction::Script(tx, version), + CheckedTransaction::Create(tx) => PoolTransaction::Create(tx, version), CheckedTransaction::Mint(_) => return Err(Error::MintIsDisallowed), - CheckedTransaction::Upgrade(tx) => PoolTransaction::Upgrade(tx), - CheckedTransaction::Upload(tx) => PoolTransaction::Upload(tx), + CheckedTransaction::Upgrade(tx) => PoolTransaction::Upgrade(tx, version), + CheckedTransaction::Upload(tx) => PoolTransaction::Upload(tx, version), }); self.check_blacklisting(tx.as_ref())?; @@ -427,6 +429,7 @@ where pub fn insert( &mut self, tx_status_sender: &TxStatusChange, + version: ConsensusParametersVersion, txs: Vec>, ) -> Vec> { // Check if that data is okay (witness match input/output, and if recovered signatures ara valid). @@ -438,7 +441,7 @@ where }; for tx in txs.into_iter() { - res.push(self.insert_inner(tx, &view)); + res.push(self.insert_inner(tx, version, &view)); } // announce to subscribers diff --git a/crates/services/upgradable-executor/src/instance.rs b/crates/services/upgradable-executor/src/instance.rs index 3fade8a2efa..01f5ba41a23 100644 --- a/crates/services/upgradable-executor/src/instance.rs +++ b/crates/services/upgradable-executor/src/instance.rs @@ -179,7 +179,7 @@ impl Instance { .next(gas_limit) .into_iter() .map(|tx| match tx { - MaybeCheckedTransaction::CheckedTransaction(checked) => { + MaybeCheckedTransaction::CheckedTransaction(checked, _) => { let checked: Checked = checked.into(); let (tx, _) = checked.into(); tx diff --git a/crates/types/src/services/txpool.rs b/crates/types/src/services/txpool.rs index a301be8b952..7699c744f11 100644 --- a/crates/types/src/services/txpool.rs +++ b/crates/types/src/services/txpool.rs @@ -1,7 +1,10 @@ //! Types for interoperability with the txpool service use crate::{ - blockchain::block::Block, + blockchain::{ + block::Block, + header::ConsensusParametersVersion, + }, fuel_asm::Word, fuel_tx::{ field::{ @@ -57,43 +60,53 @@ pub type ArcPoolTx = Arc; #[derive(Debug, Eq, PartialEq)] pub enum PoolTransaction { /// Script - Script(Checked