From 64e8f3f70c235aa788260dff3a41a63be3e2f685 Mon Sep 17 00:00:00 2001 From: Brandon Kite Date: Fri, 29 Mar 2024 00:12:27 -0700 Subject: [PATCH 01/56] fti processing wip --- .../src/graphql_api/worker_service.rs | 22 ++++ crates/services/executor/src/executor.rs | 120 ++++++++++++++++-- crates/services/executor/src/ports.rs | 14 ++ crates/types/src/services/executor.rs | 40 +++++- crates/types/src/services/txpool.rs | 2 +- 5 files changed, 183 insertions(+), 15 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index 4fdc750197d..7261f0f6fdb 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -165,6 +165,28 @@ where .storage_as_mut::() .remove(&key)?; } + Event::ForcedTransactionFailed { + id: _, + block_height, + block_time, + failure: _, + } => { + let _status = + fuel_core_types::services::txpool::TransactionStatus::Failed { + block_height: *block_height, + time: *block_time, + result: None, + receipts: vec![], + }; + + // TODO: figure out how to report the failed relayed tx status + // block_st_transaction + // .storage_as_mut::() + // .insert(id.into(), &status)?; + } + _ => { + // unknown execution event (likely due to a runtime upgrade) + } } } Ok(()) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 9daf8ae0974..25806c5e812 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -48,6 +48,8 @@ use fuel_core_types::{ CompressedCoinV1, }, contract::ContractUtxoInfo, + relayer::transaction::RelayedTransactionId, + RelayedTransaction, }, fuel_asm::{ RegId, @@ -95,6 +97,7 @@ use fuel_core_types::{ UtxoId, }, fuel_types::{ + canonical::Deserialize, BlockHeight, ContractId, MessageId, @@ -127,6 +130,7 @@ use fuel_core_types::{ ExecutionResult, ExecutionType, ExecutionTypes, + ForcedTransactionFailure, Result as ExecutorResult, TransactionExecutionResult, TransactionExecutionStatus, @@ -470,12 +474,13 @@ where let source = component.transactions_source; let gas_price = component.gas_price; let coinbase_contract_id = component.coinbase_contract_id; - let mut remaining_gas_limit = block_gas_limit; let block_height = *block.header.height(); - if self.relayer.enabled() { - self.process_da(&block.header, execution_data)?; - } + let forced_transactions = if self.relayer.enabled() { + self.process_da(&block.header, execution_data)? + } else { + Vec::with_capacity(0) + }; // The block level storage transaction that also contains data from the relayer. // Starting from this point, modifications from each thread should be independent @@ -490,10 +495,11 @@ where .read_transaction() .with_policy(ConflictPolicy::Overwrite); - // ALl transactions should be in the `TxSource`. - // We use `block.transactions` to store executed transactions. debug_assert!(block.transactions.is_empty()); - let mut iter = source.next(remaining_gas_limit).into_iter().peekable(); + // initiate transaction stream with relayed (forced) transactions first, + // and pull the rest from the TxSource (txpool) if there is remaining blockspace available. + // We use `block.transactions` to store executed transactions. + let mut iter = forced_transactions.into_iter().peekable(); let mut execute_transaction = |execution_data: &mut ExecutionData, tx: MaybeCheckedTransaction| @@ -516,6 +522,20 @@ where ); let tx = match result { + Err(ExecutorError::RelayedTransactionFailed(id, err)) => { + // if it was a relayed tx that failed, we need to record the reason + // and all nodes should process this event the same way + execution_data.events.push( + ExecutorEvent::ForcedTransactionFailed { + id, + block_height, + block_time: *block.header.time(), + failure: ForcedTransactionFailure::ExecutionError(*err) + .to_string(), + }, + ); + return Ok(()) + } Err(err) => { return match execution_kind { ExecutionKind::Production => { @@ -554,8 +574,11 @@ where execute_transaction(&mut *execution_data, transaction)?; } - remaining_gas_limit = block_gas_limit.saturating_sub(execution_data.used_gas); + let remaining_gas_limit = + block_gas_limit.saturating_sub(execution_data.used_gas); + // L2 originated transactions should be in the `TxSource`. This will be triggered after + // all relayed transactions are processed. iter = source.next(remaining_gas_limit).into_iter().peekable(); } @@ -610,7 +633,7 @@ where &mut self, header: &PartialBlockHeader, execution_data: &mut ExecutionData, - ) -> ExecutorResult<()> { + ) -> ExecutorResult> { let block_height = *header.height(); let prev_block_height = block_height .pred() @@ -628,6 +651,8 @@ where let mut root_calculator = MerkleRootCalculator::new(); + let mut checked_forced_txs = vec![]; + for da_height in next_unprocessed_da_height..=header.da_height.0 { let da_height = da_height.into(); let events = self @@ -648,8 +673,30 @@ where .events .push(ExecutorEvent::MessageImported(message)); } - Event::Transaction(_) => { - // TODO: implement handling of forced transactions in a later PR + Event::Transaction(relayed_tx) => { + let id = relayed_tx.id(); + // perform basic checks + let checked_tx_res = Self::validate_forced_tx( + relayed_tx, + header, + &self.consensus_params, + ); + // handle the result + match checked_tx_res { + Ok(checked_tx) => { + checked_forced_txs.push(checked_tx); + } + Err(err) => { + execution_data.events.push( + ExecutorEvent::ForcedTransactionFailed { + id, + block_height, + block_time: header.consensus.time, + failure: err.to_string(), + }, + ); + } + } } } } @@ -657,7 +704,41 @@ where execution_data.event_inbox_root = root_calculator.root().into(); - Ok(()) + Ok(checked_forced_txs) + } + + /// Parse forced transaction payloads and perform basic checks + fn validate_forced_tx( + relayed_tx: RelayedTransaction, + header: &PartialBlockHeader, + consensus_params: &ConsensusParameters, + ) -> Result { + let parsed_tx = Transaction::from_bytes(relayed_tx.serialized_transaction()) + .map_err(|_| ForcedTransactionFailure::CodecError)?; + + let check_tx_res = parsed_tx + .into_checked(header.consensus.height, consensus_params) + .map_err(|check_error| ForcedTransactionFailure::CheckError(check_error))?; + + let checked_tx = check_tx_res + .check_predicates(&CheckPredicateParams::from(consensus_params)) + .map_err(|err| ForcedTransactionFailure::CheckError(err))?; + + // Fail if transaction is not a script or create + // todo: Ideally we'd have a helper such as `.is_chargeable()` to be more future proof in + // case new tx types are added in the future. + match checked_tx.transaction() { + Transaction::Script(_) => {} + Transaction::Create(_) => {} + Transaction::Mint(_) => { + return Err(ForcedTransactionFailure::InvalidTransactionType) + } + } + + Ok(MaybeCheckedTransaction::RelayedCheckedTransaction( + relayed_tx.id(), + CheckedTransaction::from(checked_tx), + )) } #[allow(clippy::too_many_arguments)] @@ -688,14 +769,19 @@ where } let block_height = *header.height(); + let mut relayed_tx: Option = None; 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::RelayedCheckedTransaction(id, checked_tx) => { + relayed_tx = Some(id); + checked_tx + } }; - match checked_tx { + let mut res = match checked_tx { CheckedTransaction::Script(script) => self.execute_create_or_script( script, header, @@ -723,7 +809,15 @@ where tx_st_transaction, execution_kind, ), + }; + + // if it's a relayed tx, wrap the error for proper handling + if let Some(id) = &relayed_tx { + res = res.map_err(|err| { + ExecutorError::RelayedTransactionFailed(id.clone(), Box::new(err)) + }); } + res } #[allow(clippy::too_many_arguments)] diff --git a/crates/services/executor/src/ports.rs b/crates/services/executor/src/ports.rs index 40304a4d375..49edfe44efb 100644 --- a/crates/services/executor/src/ports.rs +++ b/crates/services/executor/src/ports.rs @@ -1,5 +1,6 @@ use fuel_core_types::{ blockchain::primitives::DaBlockHeight, + entities::relayer::transaction::RelayedTransactionId, fuel_tx, fuel_tx::{ TxId, @@ -14,6 +15,7 @@ use fuel_core_types::{ pub enum MaybeCheckedTransaction { CheckedTransaction(CheckedTransaction), Transaction(fuel_tx::Transaction), + RelayedCheckedTransaction(RelayedTransactionId, CheckedTransaction), } impl MaybeCheckedTransaction { @@ -29,6 +31,18 @@ impl MaybeCheckedTransaction { tx.id() } MaybeCheckedTransaction::Transaction(tx) => tx.id(chain_id), + MaybeCheckedTransaction::RelayedCheckedTransaction( + _, + CheckedTransaction::Script(tx), + ) => tx.id(), + MaybeCheckedTransaction::RelayedCheckedTransaction( + _, + CheckedTransaction::Create(tx), + ) => tx.id(), + MaybeCheckedTransaction::RelayedCheckedTransaction( + _, + CheckedTransaction::Mint(tx), + ) => tx.id(), } } } diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index d8c10c1f17c..37a9f31ad93 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -11,7 +11,10 @@ use crate::{ }, entities::{ coins::coin::Coin, - relayer::message::Message, + relayer::{ + message::Message, + transaction::RelayedTransactionId, + }, }, fuel_tx::{ Receipt, @@ -20,6 +23,7 @@ use crate::{ ValidityError, }, fuel_types::{ + BlockHeight, Bytes32, ContractId, Nonce, @@ -30,6 +34,7 @@ use crate::{ }, services::Uncommitted, }; +use tai64::Tai64; /// The alias for executor result. pub type Result = core::result::Result; @@ -55,6 +60,7 @@ pub struct ExecutionResult { /// The event represents some internal state changes caused by the block execution. #[derive(Debug, Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[non_exhaustive] pub enum Event { /// Imported a new spendable message from the relayer. MessageImported(Message), @@ -64,6 +70,36 @@ pub enum Event { CoinCreated(Coin), /// The coin was consumed by the transaction. CoinConsumed(Coin), + /// Failed transaction inclusion + ForcedTransactionFailed { + /// The hash of the relayed transaction + id: RelayedTransactionId, + /// The height of the block that processed this transaction + block_height: BlockHeight, + /// The time of the block that processed this transaction + block_time: Tai64, + /// The actual failure reason for why the forced transaction was not included + failure: String, + }, +} + +/// Known failure modes for processing forced transactions +#[derive(Debug, derive_more::Display)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[non_exhaustive] +pub enum ForcedTransactionFailure { + /// Failed to decode transaction to a valid fuel_tx::Transaction + #[display(fmt = "Failed to decode transaction")] + CodecError, + /// Transaction failed basic checks + #[display(fmt = "Failed validity checks")] + CheckError(CheckError), + /// Invalid transaction type + #[display(fmt = "Transaction type is not accepted")] + InvalidTransactionType, + /// Execution error which failed to include + #[display(fmt = "Transaction inclusion failed {_0}")] + ExecutionError(Error), } /// The status of a transaction after it is executed. @@ -358,6 +394,8 @@ pub enum Error { RelayerGivesIncorrectMessages, #[display(fmt = "Consensus parameters not found for version {_0}")] ConsensusParametersNotFound(ConsensusParametersVersion), + #[display(fmt = "Failed to execute relayed transaction {_0}")] + RelayedTransactionFailed(RelayedTransactionId, Box), /// It is possible to occur untyped errors in the case of the upgrade. #[display(fmt = "Occurred untyped error: {_0}")] Other(String), diff --git a/crates/types/src/services/txpool.rs b/crates/types/src/services/txpool.rs index 573a44967da..3e0f2a12927 100644 --- a/crates/types/src/services/txpool.rs +++ b/crates/types/src/services/txpool.rs @@ -197,7 +197,7 @@ pub enum TransactionStatus { /// Why this happened reason: String, }, - /// Transaction was included in a block, but the exection was reverted + /// Transaction was included in a block, but the execution was reverted Failed { /// Included in this block block_height: BlockHeight, From 5dbccbb61a7361b8e9cffa31bddafa24c3b96665 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 2 Apr 2024 11:07:33 -0700 Subject: [PATCH 02/56] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b00289e0573..847676cbdb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Description of the upcoming release here. ### Added +- [#1787](https://github.com/FuelLabs/fuel-core/pull/1787): Handle processing of relayed (forced) transactions - [#1786](https://github.com/FuelLabs/fuel-core/pull/1786): Regenesis now includes off-chain tables. - [#1716](https://github.com/FuelLabs/fuel-core/pull/1716): Added support of WASM state transition along with upgradable execution that works with native(std) and WASM(non-std) executors. The `fuel-core` now requires a `wasm32-unknown-unknown` target to build. - [#1770](https://github.com/FuelLabs/fuel-core/pull/1770): Add the new L1 event type for forced transactions. From 4868e5ebb34b8d5ca831b468f55417ca85c71f6a Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 2 Apr 2024 12:37:34 -0700 Subject: [PATCH 03/56] WIP add storage for L1 tx status --- crates/fuel-core/src/graphql_api/storage.rs | 4 + .../storage/layer_one_transactions.rs | 113 ++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs diff --git a/crates/fuel-core/src/graphql_api/storage.rs b/crates/fuel-core/src/graphql_api/storage.rs index 401fbfb4a72..9214e52a5a0 100644 --- a/crates/fuel-core/src/graphql_api/storage.rs +++ b/crates/fuel-core/src/graphql_api/storage.rs @@ -42,6 +42,8 @@ pub mod messages; pub mod statistic; pub mod transactions; +pub mod layer_one_transactions; + /// GraphQL database tables column ids to the corresponding [`fuel_core_storage::Mappable`] table. #[repr(u32)] #[derive( @@ -74,6 +76,8 @@ pub enum Column { FuelBlockIdsToHeights = 7, /// See [`ContractsInfo`](contracts::ContractsInfo) ContractsInfo = 8, + /// ID for Layer 1 Transaction status + LayerOneTransactionStatus = 9, } impl Column { diff --git a/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs b/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs new file mode 100644 index 00000000000..f427dbb3017 --- /dev/null +++ b/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs @@ -0,0 +1,113 @@ +use fuel_core_chain_config::{ + AddTable, + AsTable, + StateConfig, + StateConfigBuilder, + TableEntry, +}; +use fuel_core_storage::{ + blueprint::plain::Plain, + codec::{ + postcard::Postcard, + raw::Raw, + }, + structured_storage::TableWithBlueprint, + Mappable, +}; +use fuel_core_types::{ + fuel_tx::{ + Bytes32, + // Receipt, + }, + // fuel_types::BlockHeight, + // fuel_vm::ProgramState, + services::txpool::TransactionStatus, + // tai64::Tai64, +}; + +pub struct LayerOneTransactionStatuses; + +// #[derive(Clone, Debug)] +// pub struct LayerOneTransactionIdentifier(Bytes32); +// +// impl From for LayerOneTransactionIdentifier { +// fn from(bytes: Bytes32) -> Self { +// Self(bytes) +// } +// } +// +// impl From for Bytes32 { +// fn from(identifier: LayerOneTransactionIdentifier) -> Self { +// identifier.0 +// } +// } + +// #[derive(Clone, Debug, PartialEq, Eq)] +// pub enum LayerOneTransactionStatus { +// /// Transaction was included in a block, but the execution was reverted +// Failed { +// /// Included in this block +// block_height: BlockHeight, +// /// Time when the block was generated +// time: Tai64, +// /// Result of executing the transaction for scripts +// result: Option, +// /// The receipts generated during execution of the transaction. +// receipts: Vec, +// }, +// } + +impl Mappable for LayerOneTransactionStatuses { + type Key = Bytes32; + type OwnedKey = Self::Key; + type Value = TransactionStatus; + type OwnedValue = Self::Value; +} + +impl TableWithBlueprint for LayerOneTransactionStatuses { + type Blueprint = Plain; + + type Column = super::Column; + + fn column() -> Self::Column { + Self::Column::LayerOneTransactionStatus + } +} + +impl AsTable for StateConfig { + fn as_table(&self) -> Vec> { + Vec::new() // Do not include these for now + } +} + +impl AddTable for StateConfigBuilder { + fn add(&mut self, _entries: Vec>) { + // Do not include these for now + } +} + +#[cfg(test)] +mod tests { + use super::*; + use fuel_core_types::{ + services::txpool::TransactionStatus, + tai64::Tai64, + }; + + fuel_core_storage::basic_storage_tests!( + LayerOneTransactionStatuses, + ::Key::from(Bytes32::default()), + // LayerOneTransactionStatus::Failed { + // block_height: 0.into(), + // time: Tai64::UNIX_EPOCH, + // result: None, + // receipts: Vec::new(), + // } + TransactionStatus::Failed { + block_height: 0.into(), + time: Tai64::UNIX_EPOCH, + result: None, + receipts: Vec::new(), + } + ); +} From ca2e14b7fab6310f4a61da52f050377ce857d9f5 Mon Sep 17 00:00:00 2001 From: Turner Date: Tue, 2 Apr 2024 16:32:58 -0700 Subject: [PATCH 04/56] Use types specific to relayed txs --- .../storage/layer_one_transactions.rs | 76 +++++-------------- .../types/src/entities/relayer/transaction.rs | 17 +++++ 2 files changed, 35 insertions(+), 58 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs b/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs index f427dbb3017..66dc056b03c 100644 --- a/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs @@ -15,56 +15,23 @@ use fuel_core_storage::{ Mappable, }; use fuel_core_types::{ - fuel_tx::{ - Bytes32, - // Receipt, + entities::relayer::transaction::{ + // RelayedTransactionId, + RelayedTransactionStatus, }, - // fuel_types::BlockHeight, - // fuel_vm::ProgramState, - services::txpool::TransactionStatus, - // tai64::Tai64, + fuel_tx::Bytes32, }; -pub struct LayerOneTransactionStatuses; +pub struct RelayedTransactionStatuses; -// #[derive(Clone, Debug)] -// pub struct LayerOneTransactionIdentifier(Bytes32); -// -// impl From for LayerOneTransactionIdentifier { -// fn from(bytes: Bytes32) -> Self { -// Self(bytes) -// } -// } -// -// impl From for Bytes32 { -// fn from(identifier: LayerOneTransactionIdentifier) -> Self { -// identifier.0 -// } -// } - -// #[derive(Clone, Debug, PartialEq, Eq)] -// pub enum LayerOneTransactionStatus { -// /// Transaction was included in a block, but the execution was reverted -// Failed { -// /// Included in this block -// block_height: BlockHeight, -// /// Time when the block was generated -// time: Tai64, -// /// Result of executing the transaction for scripts -// result: Option, -// /// The receipts generated during execution of the transaction. -// receipts: Vec, -// }, -// } - -impl Mappable for LayerOneTransactionStatuses { +impl Mappable for RelayedTransactionStatuses { type Key = Bytes32; type OwnedKey = Self::Key; - type Value = TransactionStatus; + type Value = RelayedTransactionStatus; type OwnedValue = Self::Value; } -impl TableWithBlueprint for LayerOneTransactionStatuses { +impl TableWithBlueprint for RelayedTransactionStatuses { type Blueprint = Plain; type Column = super::Column; @@ -74,14 +41,14 @@ impl TableWithBlueprint for LayerOneTransactionStatuses { } } -impl AsTable for StateConfig { - fn as_table(&self) -> Vec> { +impl AsTable for StateConfig { + fn as_table(&self) -> Vec> { Vec::new() // Do not include these for now } } -impl AddTable for StateConfigBuilder { - fn add(&mut self, _entries: Vec>) { +impl AddTable for StateConfigBuilder { + fn add(&mut self, _entries: Vec>) { // Do not include these for now } } @@ -90,24 +57,17 @@ impl AddTable for StateConfigBuilder { mod tests { use super::*; use fuel_core_types::{ - services::txpool::TransactionStatus, + fuel_tx::Bytes32, tai64::Tai64, }; fuel_core_storage::basic_storage_tests!( - LayerOneTransactionStatuses, - ::Key::from(Bytes32::default()), - // LayerOneTransactionStatus::Failed { - // block_height: 0.into(), - // time: Tai64::UNIX_EPOCH, - // result: None, - // receipts: Vec::new(), - // } - TransactionStatus::Failed { + RelayedTransactionStatuses, + ::Key::from(Bytes32::default()), + RelayedTransactionStatus::Failed { block_height: 0.into(), - time: Tai64::UNIX_EPOCH, - result: None, - receipts: Vec::new(), + block_time: Tai64::UNIX_EPOCH, + failure: "Some reason".to_string(), } ); } diff --git a/crates/types/src/entities/relayer/transaction.rs b/crates/types/src/entities/relayer/transaction.rs index cdb3eaa8494..1a809d4f715 100644 --- a/crates/types/src/entities/relayer/transaction.rs +++ b/crates/types/src/entities/relayer/transaction.rs @@ -4,10 +4,12 @@ use crate::{ blockchain::primitives::DaBlockHeight, fuel_crypto, fuel_types::{ + BlockHeight, Bytes32, Nonce, }, }; +use tai64::Tai64; /// Transaction sent from the DA layer to fuel by the relayer #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -149,3 +151,18 @@ impl From for RelayedTransaction { RelayedTransaction::V1(relayed_transaction) } } + +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Clone, Debug, PartialEq, Eq)] +/// Potential states for the relayed transaction +pub enum RelayedTransactionStatus { + /// Transaction was included in a block, but the execution was reverted + Failed { + /// The height of the block that processed this transaction + block_height: BlockHeight, + /// The time of the block that processed this transaction + block_time: Tai64, + /// The actual failure reason for why the forced transaction was not included + failure: String, + }, +} From 2b0fc508da7e398a843d4917fcff314734c1958e Mon Sep 17 00:00:00 2001 From: Turner Date: Tue, 2 Apr 2024 18:07:23 -0700 Subject: [PATCH 05/56] Add failing test for checking the events are stored in db --- crates/fuel-core/src/graphql_api/storage.rs | 2 +- ...ransactions.rs => relayed_transactions.rs} | 0 .../src/graphql_api/worker_service.rs | 53 ++++++------ .../src/graphql_api/worker_service/tests.rs | 74 +++++++++++++++++ crates/services/executor/src/executor.rs | 80 +++++++++---------- .../upgradable-executor/src/instance.rs | 5 ++ .../types/src/entities/relayer/transaction.rs | 1 - 7 files changed, 149 insertions(+), 66 deletions(-) rename crates/fuel-core/src/graphql_api/storage/{layer_one_transactions.rs => relayed_transactions.rs} (100%) create mode 100644 crates/fuel-core/src/graphql_api/worker_service/tests.rs diff --git a/crates/fuel-core/src/graphql_api/storage.rs b/crates/fuel-core/src/graphql_api/storage.rs index 9214e52a5a0..8bb7a4ac245 100644 --- a/crates/fuel-core/src/graphql_api/storage.rs +++ b/crates/fuel-core/src/graphql_api/storage.rs @@ -42,7 +42,7 @@ pub mod messages; pub mod statistic; pub mod transactions; -pub mod layer_one_transactions; +pub mod relayed_transactions; /// GraphQL database tables column ids to the corresponding [`fuel_core_storage::Mappable`] table. #[repr(u32)] diff --git a/crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs similarity index 100% rename from crates/fuel-core/src/graphql_api/storage/layer_one_transactions.rs rename to crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index 7261f0f6fdb..cfd4e7237d0 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -1,18 +1,21 @@ -use crate::fuel_core_graphql_api::{ - ports, - ports::worker::OffChainDatabase, - storage::{ - blocks::FuelBlockIdsToHeights, - coins::{ - owner_coin_id_key, - OwnedCoins, - }, - contracts::ContractsInfo, - messages::{ - OwnedMessageIds, - OwnedMessageKey, +use crate::{ + fuel_core_graphql_api::{ + ports, + ports::worker::OffChainDatabase, + storage::{ + blocks::FuelBlockIdsToHeights, + coins::{ + owner_coin_id_key, + OwnedCoins, + }, + contracts::ContractsInfo, + messages::{ + OwnedMessageIds, + OwnedMessageKey, + }, }, }, + // graphql_api::storage::layer_one_transactions::RelayedTransactionStatuses, }; use fuel_core_metrics::graphql_metrics::graphql_metrics; use fuel_core_services::{ @@ -29,6 +32,7 @@ use fuel_core_storage::{ }; use fuel_core_types::{ blockchain::block::Block, + entities::relayer::transaction::RelayedTransactionStatus, fuel_tx::{ field::{ Inputs, @@ -70,6 +74,9 @@ use std::{ ops::Deref, }; +#[cfg(test)] +mod tests; + /// The off-chain GraphQL API worker task processes the imported blocks /// and actualize the information used by the GraphQL service. pub struct Task { @@ -169,19 +176,17 @@ where id: _, block_height, block_time, - failure: _, + failure, } => { - let _status = - fuel_core_types::services::txpool::TransactionStatus::Failed { - block_height: *block_height, - time: *block_time, - result: None, - receipts: vec![], - }; - - // TODO: figure out how to report the failed relayed tx status + let _status = RelayedTransactionStatus::Failed { + block_height: *block_height, + block_time: *block_time, + failure: failure.clone(), + }; + + todo!(); // block_st_transaction - // .storage_as_mut::() + // .storage_as_mut::() // .insert(id.into(), &status)?; } _ => { diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs new file mode 100644 index 00000000000..249d230ccbb --- /dev/null +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -0,0 +1,74 @@ +#![allow(non_snake_case)] + +use super::*; +use crate::{ + database::Database, + graphql_api::storage::relayed_transactions::RelayedTransactionStatuses, +}; +use fuel_core_services::{ + stream::IntoBoxStream, + State, +}; +use fuel_core_storage::StorageAsRef; +use fuel_core_types::{ + fuel_tx::Bytes32, + fuel_types::BlockHeight, + services::txpool::TransactionStatus, + tai64::Tai64, +}; +use std::sync::Arc; + +struct MockTxPool; + +impl ports::worker::TxPool for MockTxPool { + fn send_complete( + &self, + _id: Bytes32, + _block_height: &BlockHeight, + _status: TransactionStatus, + ) { + // Do nothing + () + } +} + +#[tokio::test] +async fn run__relayed_transaction_events_are_added_to_storage() { + let tx_pool = MockTxPool; + let blocks: Vec + Send + Sync>> = vec![]; + let block_importer = tokio_stream::iter(blocks).into_boxed(); + let database = Database::in_memory(); + let chain_id = Default::default(); + let mut task = Task { + tx_pool, + block_importer, + database: database.clone(), + chain_id, + }; + // given + let tx_id: Bytes32 = [1; 32].into(); + let block_height = 8.into(); + let block_time = Tai64::UNIX_EPOCH; + let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); + let _event = Event::ForcedTransactionFailed { + id: tx_id.into(), + block_height, + block_time: Tai64::UNIX_EPOCH, + failure: failure.clone(), + }; + let expected = RelayedTransactionStatus::Failed { + block_height, + block_time, + failure, + }; + + // when + let (_sender, receiver) = tokio::sync::watch::channel(State::Started); + let mut state_watcher = receiver.into(); + task.run(&mut state_watcher).await.unwrap(); + + // then + let storage = database.storage_as_ref::(); + let actual = storage.get(&tx_id).unwrap().unwrap(); + assert_eq!(*actual, expected); +} diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 25806c5e812..726e7e7be3c 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -383,7 +383,7 @@ where if let Some(pre_exec_block_id) = pre_exec_block_id { // The block id comparison compares the whole blocks including all fields. if pre_exec_block_id != finalized_block_id { - return Err(ExecutorError::InvalidBlockId) + return Err(ExecutorError::InvalidBlockId); } } @@ -486,8 +486,8 @@ where // Starting from this point, modifications from each thread should be independent // and shouldn't touch the same data. let mut block_with_relayer_data_transaction = self.block_st_transaction.read_transaction() - // Enforces independent changes from each thread. - .with_policy(ConflictPolicy::Fail); + // Enforces independent changes from each thread. + .with_policy(ConflictPolicy::Fail); // We execute transactions in a single thread right now, but later, // we will execute them in parallel with a separate independent storage transaction per thread. @@ -534,7 +534,7 @@ where .to_string(), }, ); - return Ok(()) + return Ok(()); } Err(err) => { return match execution_kind { @@ -550,13 +550,13 @@ where Ok(()) } ExecutionKind::DryRun | ExecutionKind::Validation => Err(err), - } + }; } Ok(tx) => tx, }; if let Err(err) = tx_st_transaction.commit() { - return Err(err.into()) + return Err(err.into()); } tx }; @@ -621,7 +621,7 @@ where .commit_changes(block_with_relayer_data_transaction.into_changes())?; if execution_kind != ExecutionKind::DryRun && !data.found_mint { - return Err(ExecutorError::MintMissing) + return Err(ExecutorError::MintMissing); } data.changes = self.block_st_transaction.into_changes(); @@ -646,7 +646,7 @@ where .ok_or(ExecutorError::PreviousBlockIsNotFound)?; let previous_da_height = prev_block_header.header().da_height; let Some(next_unprocessed_da_height) = previous_da_height.0.checked_add(1) else { - return Err(ExecutorError::DaHeightExceededItsLimit) + return Err(ExecutorError::DaHeightExceededItsLimit); }; let mut root_calculator = MerkleRootCalculator::new(); @@ -664,7 +664,7 @@ where match event { Event::Message(message) => { if message.da_height() != da_height { - return Err(ExecutorError::RelayerGivesIncorrectMessages) + return Err(ExecutorError::RelayerGivesIncorrectMessages); } self.block_st_transaction .storage_as_mut::() @@ -718,11 +718,11 @@ where let check_tx_res = parsed_tx .into_checked(header.consensus.height, consensus_params) - .map_err(|check_error| ForcedTransactionFailure::CheckError(check_error))?; + .map_err(ForcedTransactionFailure::CheckError)?; let checked_tx = check_tx_res .check_predicates(&CheckPredicateParams::from(consensus_params)) - .map_err(|err| ForcedTransactionFailure::CheckError(err))?; + .map_err(ForcedTransactionFailure::CheckError)?; // Fail if transaction is not a script or create // todo: Ideally we'd have a helper such as `.is_chargeable()` to be more future proof in @@ -731,7 +731,7 @@ where Transaction::Script(_) => {} Transaction::Create(_) => {} Transaction::Mint(_) => { - return Err(ForcedTransactionFailure::InvalidTransactionType) + return Err(ForcedTransactionFailure::InvalidTransactionType); } } @@ -757,7 +757,7 @@ where T: KeyValueInspect, { if execution_data.found_mint { - return Err(ExecutorError::MintIsNotLastTransaction) + return Err(ExecutorError::MintIsNotLastTransaction); } // Throw a clear error if the transaction id is a duplicate @@ -765,7 +765,7 @@ where .storage::() .contains_key(tx_id)? { - return Err(ExecutorError::TransactionIdCollision(*tx_id)) + return Err(ExecutorError::TransactionIdCollision(*tx_id)); } let block_height = *header.height(); @@ -837,7 +837,7 @@ where execution_data.found_mint = true; if checked_mint.transaction().tx_pointer().tx_index() != execution_data.tx_count { - return Err(ExecutorError::MintHasUnexpectedIndex) + return Err(ExecutorError::MintHasUnexpectedIndex); } let coinbase_id = checked_mint.id(); @@ -845,7 +845,7 @@ where fn verify_mint_for_empty_contract(mint: &Mint) -> ExecutorResult<()> { if *mint.mint_amount() != 0 { - return Err(ExecutorError::CoinbaseAmountMismatch) + return Err(ExecutorError::CoinbaseAmountMismatch); } let input = input::contract::Contract { @@ -861,7 +861,7 @@ where state_root: Bytes32::zeroed(), }; if mint.input_contract() != &input || mint.output_contract() != &output { - return Err(ExecutorError::MintMismatch) + return Err(ExecutorError::MintMismatch); } Ok(()) } @@ -870,10 +870,10 @@ where verify_mint_for_empty_contract(&mint)?; } else { if *mint.mint_amount() != execution_data.coinbase { - return Err(ExecutorError::CoinbaseAmountMismatch) + return Err(ExecutorError::CoinbaseAmountMismatch); } if *mint.gas_price() != gas_price { - return Err(ExecutorError::CoinbaseGasPriceMismatch) + return Err(ExecutorError::CoinbaseGasPriceMismatch); } let block_height = *header.height(); @@ -948,7 +948,7 @@ where if execution_kind == ExecutionKind::Validation { if mint.input_contract() != &input || mint.output_contract() != &output { - return Err(ExecutorError::MintMismatch) + return Err(ExecutorError::MintMismatch); } } else { *mint.input_contract_mut() = input; @@ -971,7 +971,7 @@ where .insert(&coinbase_id, &())? .is_some() { - return Err(ExecutorError::TransactionIdCollision(coinbase_id)) + return Err(ExecutorError::TransactionIdCollision(coinbase_id)); } Ok(tx) } @@ -1095,7 +1095,7 @@ where or we added a new predicate inputs."); return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } } } @@ -1144,7 +1144,7 @@ where { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } let final_tx = tx.into(); @@ -1206,12 +1206,12 @@ where { return Err( TransactionValidityError::CoinMismatch(*utxo_id).into() - ) + ); } } else { return Err( TransactionValidityError::CoinDoesNotExist(*utxo_id).into() - ) + ); } } Input::Contract(contract) => { @@ -1222,7 +1222,7 @@ where return Err(TransactionValidityError::ContractDoesNotExist( contract.contract_id, ) - .into()) + .into()); } } Input::MessageCoinSigned(MessageCoinSigned { nonce, .. }) @@ -1233,14 +1233,14 @@ where if db.storage::().contains_key(nonce)? { return Err( TransactionValidityError::MessageAlreadySpent(*nonce).into() - ) + ); } if let Some(message) = db.storage::().get(nonce)? { if message.da_height() > block_da_height { return Err(TransactionValidityError::MessageSpendTooEarly( *nonce, ) - .into()) + .into()); } if !message @@ -1249,12 +1249,12 @@ where { return Err( TransactionValidityError::MessageMismatch(*nonce).into() - ) + ); } } else { return Err( TransactionValidityError::MessageDoesNotExist(*nonce).into() - ) + ); } } } @@ -1312,7 +1312,7 @@ where if reverted => { // Don't spend the retryable messages if transaction is reverted - continue + continue; } Input::MessageCoinSigned(MessageCoinSigned { nonce, .. }) | Input::MessageCoinPredicate(MessageCoinPredicate { nonce, .. }) @@ -1324,7 +1324,7 @@ where db.storage::().insert(nonce, &())?; // ensure message wasn't already marked as spent if was_already_spent.is_some() { - return Err(ExecutorError::MessageAlreadySpent(*nonce)) + return Err(ExecutorError::MessageAlreadySpent(*nonce)); } // cleanup message contents let message = db @@ -1352,7 +1352,7 @@ where for r in receipts { if let Receipt::ScriptResult { gas_used, .. } = r { used_gas = *gas_used; - break + break; } } @@ -1461,7 +1461,7 @@ where if tx_pointer != coin.tx_pointer() { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } } Input::Contract(Contract { @@ -1481,17 +1481,17 @@ where { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } if balance_root != &contract.balance_root()? { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } if state_root != &contract.state_root()? { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); } } _ => {} @@ -1525,7 +1525,7 @@ where } else { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }) + }); }; let contract = ContractRef::new(StructuredStorage::new(db), *contract_id); @@ -1641,7 +1641,7 @@ where } else { return Err(ExecutorError::TransactionValidity( TransactionValidityError::InvalidContractInputIndex(utxo_id), - )) + )); } } Output::Change { @@ -1707,7 +1707,7 @@ where .into(); if db.storage::().insert(&utxo_id, &coin)?.is_some() { - return Err(ExecutorError::OutputAlreadyExists) + return Err(ExecutorError::OutputAlreadyExists); } execution_data .events diff --git a/crates/services/upgradable-executor/src/instance.rs b/crates/services/upgradable-executor/src/instance.rs index 7f26dcecdf0..27c0a3492ea 100644 --- a/crates/services/upgradable-executor/src/instance.rs +++ b/crates/services/upgradable-executor/src/instance.rs @@ -166,6 +166,11 @@ impl Instance { tx } MaybeCheckedTransaction::Transaction(tx) => tx, + MaybeCheckedTransaction::RelayedCheckedTransaction(_, checked) => { + let checked: Checked = checked.into(); + let (tx, _) = checked.into(); + tx + } }) .collect(); diff --git a/crates/types/src/entities/relayer/transaction.rs b/crates/types/src/entities/relayer/transaction.rs index 1a809d4f715..eed3b5d48d8 100644 --- a/crates/types/src/entities/relayer/transaction.rs +++ b/crates/types/src/entities/relayer/transaction.rs @@ -116,7 +116,6 @@ impl RelayedTransaction { } } - #[cfg(any(test, feature = "test-helpers"))] /// Get the canonically serialized transaction pub fn serialized_transaction(&self) -> &[u8] { match self { From 40dd37fd5ff33ecfc94ce3e9be557c8819afd9a2 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 3 Apr 2024 11:09:19 -0700 Subject: [PATCH 06/56] Implement code so test passes :) --- crates/fuel-core/src/graphql_api/ports.rs | 12 +++++--- .../src/graphql_api/worker_service.rs | 12 ++++---- .../src/graphql_api/worker_service/tests.rs | 29 ++++++++++++------- 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 53662e3d2c3..a5265924b29 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -207,10 +207,13 @@ pub trait GasPriceEstimate: Send + Sync { pub mod worker { use super::super::storage::blocks::FuelBlockIdsToHeights; - use crate::fuel_core_graphql_api::storage::{ - coins::OwnedCoins, - contracts::ContractsInfo, - messages::OwnedMessageIds, + use crate::{ + fuel_core_graphql_api::storage::{ + coins::OwnedCoins, + contracts::ContractsInfo, + messages::OwnedMessageIds, + }, + graphql_api::storage::relayed_transactions::RelayedTransactionStatuses, }; use fuel_core_services::stream::BoxStream; use fuel_core_storage::{ @@ -244,6 +247,7 @@ pub mod worker { + StorageMutate + StorageMutate + StorageMutate + + StorageMutate { fn record_tx_id_owner( &mut self, diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index cfd4e7237d0..da28cbea9f1 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -1,3 +1,4 @@ +use crate::graphql_api::storage::relayed_transactions::RelayedTransactionStatuses; use crate::{ fuel_core_graphql_api::{ ports, @@ -173,21 +174,20 @@ where .remove(&key)?; } Event::ForcedTransactionFailed { - id: _, + id, block_height, block_time, failure, } => { - let _status = RelayedTransactionStatus::Failed { + let status = RelayedTransactionStatus::Failed { block_height: *block_height, block_time: *block_time, failure: failure.clone(), }; - todo!(); - // block_st_transaction - // .storage_as_mut::() - // .insert(id.into(), &status)?; + block_st_transaction + .storage_as_mut::() + .insert(&Bytes32::from(id.to_owned()), &status)?; } _ => { // unknown execution event (likely due to a runtime upgrade) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index 249d230ccbb..9f1618db1b7 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -34,8 +34,24 @@ impl ports::worker::TxPool for MockTxPool { #[tokio::test] async fn run__relayed_transaction_events_are_added_to_storage() { + let tx_id: Bytes32 = [1; 32].into(); + let block_height = 8.into(); + let block_time = Tai64::UNIX_EPOCH; + let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); + let event = Event::ForcedTransactionFailed { + id: tx_id.into(), + block_height, + block_time: Tai64::UNIX_EPOCH, + failure: failure.clone(), + }; let tx_pool = MockTxPool; - let blocks: Vec + Send + Sync>> = vec![]; + let block = Arc::new(ImportResult { + sealed_block: Default::default(), + tx_status: vec![], + events: vec![event], + source: Default::default(), + }); + let blocks: Vec + Send + Sync>> = vec![block]; let block_importer = tokio_stream::iter(blocks).into_boxed(); let database = Database::in_memory(); let chain_id = Default::default(); @@ -46,16 +62,7 @@ async fn run__relayed_transaction_events_are_added_to_storage() { chain_id, }; // given - let tx_id: Bytes32 = [1; 32].into(); - let block_height = 8.into(); - let block_time = Tai64::UNIX_EPOCH; - let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); - let _event = Event::ForcedTransactionFailed { - id: tx_id.into(), - block_height, - block_time: Tai64::UNIX_EPOCH, - failure: failure.clone(), - }; + let expected = RelayedTransactionStatus::Failed { block_height, block_time, From bca0830b5f70a656c3515dcf6d81c61919d7b959 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 3 Apr 2024 13:39:43 -0700 Subject: [PATCH 07/56] WIP add graphql endpoints + client method --- crates/client/src/client.rs | 16 +++++++++ crates/client/src/client/schema.rs | 2 ++ crates/client/src/client/schema/primitives.rs | 1 + crates/client/src/client/schema/relayed_tx.rs | 34 +++++++++++++++++++ crates/fuel-core/src/schema.rs | 2 ++ crates/fuel-core/src/schema/relayed_tx.rs | 27 +++++++++++++++ tests/tests/relayer.rs | 14 ++++++++ 7 files changed, 96 insertions(+) create mode 100644 crates/client/src/client/schema/relayed_tx.rs create mode 100644 crates/fuel-core/src/schema/relayed_tx.rs diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index ccdd4cadb73..098af9b26bc 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -8,6 +8,7 @@ use crate::client::{ contract::ContractBalanceQueryArgs, gas_price::EstimateGasPrice, message::MessageStatusArgs, + relayed_tx::RelayedTransactionStatusArgs, tx::DryRunArg, Tai64Timestamp, TransactionId, @@ -36,11 +37,13 @@ use cynic::{ QueryBuilder, }; use fuel_core_types::{ + entities::relayer::transaction::RelayedTransactionStatus, fuel_asm::{ Instruction, Word, }, fuel_tx::{ + Bytes32, Receipt, Transaction, TxId, @@ -968,6 +971,19 @@ impl FuelClient { Ok(proof) } + + pub async fn relayed_transaction_status( + &self, + id: &Bytes32, + ) -> io::Result> { + let query = schema::relayed_tx::RelayedTransactionStatusQuery::build( + RelayedTransactionStatusArgs { + id: id.to_owned().into(), + }, + ); + let status = self.query(query).await?; + Ok(status) + } } #[cfg(any(test, feature = "test-helpers"))] diff --git a/crates/client/src/client/schema.rs b/crates/client/src/client/schema.rs index 36f24e9444b..cc470c28786 100644 --- a/crates/client/src/client/schema.rs +++ b/crates/client/src/client/schema.rs @@ -38,6 +38,8 @@ pub mod gas_price; pub mod primitives; pub mod tx; +pub mod relayed_tx; + #[derive(cynic::QueryFragment, Clone, Debug)] #[cynic(schema_path = "./assets/schema.sdl", graphql_type = "Query")] pub struct Health { diff --git a/crates/client/src/client/schema/primitives.rs b/crates/client/src/client/schema/primitives.rs index db41e3ff0ba..c9a6911349e 100644 --- a/crates/client/src/client/schema/primitives.rs +++ b/crates/client/src/client/schema/primitives.rs @@ -112,6 +112,7 @@ fuel_type_scalar!(AssetId, AssetId); fuel_type_scalar!(ContractId, ContractId); fuel_type_scalar!(Salt, Salt); fuel_type_scalar!(TransactionId, Bytes32); +fuel_type_scalar!(RelayedTransactionId, Bytes32); fuel_type_scalar!(Signature, Bytes64); fuel_type_scalar!(Nonce, Nonce); diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs new file mode 100644 index 00000000000..1c7526fe1a1 --- /dev/null +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -0,0 +1,34 @@ +use crate::client::schema::{ + schema, + RelayedTransactionId, +}; + +#[derive(cynic::QueryVariables, Debug)] +pub struct RelayedTransactionStatusArgs { + /// Transaction id that contains the output message. + pub id: RelayedTransactionId, +} + +#[derive(cynic::QueryFragment, Clone, Debug)] +#[cynic( + schema_path = "./assets/schema.sdl", + graphql_type = "Query", + variables = "RelayedTransactionStatusArgs" +)] +pub struct RelayedTransactionStatusQuery { + #[arguments(id: $id)] + pub message_proof: Option, +} + +#[derive(cynic::QueryFragment, Clone, Debug)] +#[cynic(schema_path = "./assets/schema.sdl")] +pub struct RelayedTransactionStatus { + pub(crate) state: RelayedTransactionState, +} + +#[derive(cynic::Enum, Copy, Clone, Debug)] +#[cynic(schema_path = "./assets/schema.sdl")] +pub enum RelayedTransactionState { + /// Transaction was included in a block, but the execution was reverted + Failed, +} diff --git a/crates/fuel-core/src/schema.rs b/crates/fuel-core/src/schema.rs index f1df9c6d4fd..4c8a6eedc7e 100644 --- a/crates/fuel-core/src/schema.rs +++ b/crates/fuel-core/src/schema.rs @@ -33,6 +33,8 @@ pub mod gas_price; pub mod scalars; pub mod tx; +pub mod relayed_tx; + #[derive(MergedObject, Default)] pub struct Query( dap::DapQuery, diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs new file mode 100644 index 00000000000..e8a62c6643e --- /dev/null +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -0,0 +1,27 @@ +use async_graphql::{ + Enum, + Object, +}; +use fuel_core_types::entities; + +pub struct RelayedTransactionStatus( + pub(crate) entities::relayer::transaction::RelayedTransactionStatus, +); + +#[derive(Enum, Copy, Clone, Eq, PartialEq)] +enum RelayedTransactionState { + Failed, +} + +#[Object] +impl RelayedTransactionStatus { + async fn state(&self) -> RelayedTransactionState { + match &self.0 { + entities::relayer::transaction::RelayedTransactionStatus::Failed { + block_height: _, + block_time: _, + failure: _, + } => RelayedTransactionState::Failed, + } + } +} diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index ecf010e990f..e14e7ad7641 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -252,6 +252,20 @@ async fn messages_are_spendable_after_relayer_is_synced() { eth_node_handle.shutdown.send(()).unwrap(); } +#[tokio::test(flavor = "multi_thread")] +async fn can_find_failed_relayed_tx() { + let db = Database::in_memory(); + let srv = FuelService::from_database(db.clone(), Config::local_node()) + .await + .unwrap(); + + let client = FuelClient::from(srv.bound_address); + + let tx_id = [1; 32].into(); + let query = client.relayed_transaction_status(&tx_id).await.unwrap(); + assert!(query.is_some()); +} + #[allow(clippy::too_many_arguments)] fn make_message_event( nonce: Nonce, From f62694925c01f100095f10028a74289d8bd45dc3 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 3 Apr 2024 15:56:09 -0700 Subject: [PATCH 08/56] Get graphql compiling --- crates/client/assets/schema.sdl | 9 ++++++ crates/client/src/client.rs | 10 ++++--- crates/client/src/client/schema/primitives.rs | 1 - crates/client/src/client/schema/relayed_tx.rs | 18 +++++------ crates/fuel-core/src/graphql_api/ports.rs | 20 +++++++++++-- crates/fuel-core/src/schema.rs | 13 ++++---- crates/fuel-core/src/schema/relayed_tx.rs | 30 +++++++++++++++++++ 7 files changed, 78 insertions(+), 23 deletions(-) diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 3c848ae8a0f..4cfdf72bdf8 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -850,6 +850,7 @@ type Query { messages(owner: Address, first: Int, after: String, last: Int, before: String): MessageConnection! messageProof(transactionId: TransactionId!, nonce: Nonce!, commitBlockId: BlockId, commitBlockHeight: U32): MessageProof messageStatus(nonce: Nonce!): MessageStatus! + status(id: Bytes32!): RelayedTransactionStatus } type Receipt { @@ -902,6 +903,14 @@ enum ReceiptType { BURN } +enum RelayedTransactionState { + FAILED +} + +type RelayedTransactionStatus { + state: RelayedTransactionState! +} + enum ReturnType { RETURN RETURN_DATA diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 098af9b26bc..21b6ca46857 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -8,7 +8,10 @@ use crate::client::{ contract::ContractBalanceQueryArgs, gas_price::EstimateGasPrice, message::MessageStatusArgs, - relayed_tx::RelayedTransactionStatusArgs, + relayed_tx::{ + RelayedTransactionState, + RelayedTransactionStatusArgs, + }, tx::DryRunArg, Tai64Timestamp, TransactionId, @@ -37,7 +40,6 @@ use cynic::{ QueryBuilder, }; use fuel_core_types::{ - entities::relayer::transaction::RelayedTransactionStatus, fuel_asm::{ Instruction, Word, @@ -975,13 +977,13 @@ impl FuelClient { pub async fn relayed_transaction_status( &self, id: &Bytes32, - ) -> io::Result> { + ) -> io::Result> { let query = schema::relayed_tx::RelayedTransactionStatusQuery::build( RelayedTransactionStatusArgs { id: id.to_owned().into(), }, ); - let status = self.query(query).await?; + let status = self.query(query).await?.status.map(|inner| inner.state); Ok(status) } } diff --git a/crates/client/src/client/schema/primitives.rs b/crates/client/src/client/schema/primitives.rs index c9a6911349e..db41e3ff0ba 100644 --- a/crates/client/src/client/schema/primitives.rs +++ b/crates/client/src/client/schema/primitives.rs @@ -112,7 +112,6 @@ fuel_type_scalar!(AssetId, AssetId); fuel_type_scalar!(ContractId, ContractId); fuel_type_scalar!(Salt, Salt); fuel_type_scalar!(TransactionId, Bytes32); -fuel_type_scalar!(RelayedTransactionId, Bytes32); fuel_type_scalar!(Signature, Bytes64); fuel_type_scalar!(Nonce, Nonce); diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index 1c7526fe1a1..4b2f8f00f3e 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -1,14 +1,8 @@ use crate::client::schema::{ schema, - RelayedTransactionId, + Bytes32, }; -#[derive(cynic::QueryVariables, Debug)] -pub struct RelayedTransactionStatusArgs { - /// Transaction id that contains the output message. - pub id: RelayedTransactionId, -} - #[derive(cynic::QueryFragment, Clone, Debug)] #[cynic( schema_path = "./assets/schema.sdl", @@ -16,8 +10,14 @@ pub struct RelayedTransactionStatusArgs { variables = "RelayedTransactionStatusArgs" )] pub struct RelayedTransactionStatusQuery { - #[arguments(id: $id)] - pub message_proof: Option, + #[arguments(id: $ id)] + pub status: Option, +} + +#[derive(cynic::QueryVariables, Debug)] +pub struct RelayedTransactionStatusArgs { + /// Transaction id that contains the output message. + pub id: Bytes32, } #[derive(cynic::QueryFragment, Clone, Debug)] diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index a5265924b29..d55bcd41e6f 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -1,3 +1,4 @@ +use crate::graphql_api::storage::relayed_transactions::RelayedTransactionStatuses; use async_trait::async_trait; use fuel_core_services::stream::BoxStream; use fuel_core_storage::{ @@ -27,11 +28,15 @@ use fuel_core_types::{ DaBlockHeight, }, }, - entities::relayer::message::{ - MerkleProof, - Message, + entities::relayer::{ + message::{ + MerkleProof, + Message, + }, + transaction::RelayedTransactionStatus, }, fuel_tx::{ + Bytes32, Salt, Transaction, TxId, @@ -128,6 +133,15 @@ pub trait DatabaseMessages: StorageInspect { fn message_exists(&self, nonce: &Nonce) -> StorageResult; } +pub trait DatabaseRelayedTransactions: + StorageInspect +{ + fn transaction_status( + &self, + id: Bytes32, + ) -> StorageResult>; +} + /// Trait that specifies all the getters required for contract. pub trait DatabaseContracts: StorageInspect diff --git a/crates/fuel-core/src/schema.rs b/crates/fuel-core/src/schema.rs index 4c8a6eedc7e..ca122781738 100644 --- a/crates/fuel-core/src/schema.rs +++ b/crates/fuel-core/src/schema.rs @@ -50,6 +50,7 @@ pub struct Query( gas_price::LatestGasPriceQuery, gas_price::EstimateGasPriceQuery, message::MessageQuery, + relayed_tx::RelayedTransactionQuery, ); #[derive(MergedObject, Default)] @@ -93,22 +94,22 @@ where return Err(anyhow!( "Either first `{first}` or latest `{last}` elements, not both" ) - .into()) + .into()); } (Some(after), _, _, Some(last)) => { return Err(anyhow!( "After `{after:?}` with last `{last}` elements is not supported" ) - .into()) + .into()); } (_, Some(before), Some(first), _) => { return Err(anyhow!( "Before `{before:?}` with first `{first}` elements is not supported" ) - .into()) + .into()); } (None, None, None, None) => { - return Err(anyhow!("The queries for the whole range is not supported").into()) + return Err(anyhow!("The queries for the whole range is not supported").into()); } (_, _, _, _) => { /* Other combinations are allowed */ } }; @@ -153,7 +154,7 @@ where // Skip until start + 1 if key == start { has_previous_page = true; - return true + return true; } } } @@ -167,7 +168,7 @@ where // take until we've reached the end if key == end { has_next_page = true; - return false + return false; } } count = count.saturating_sub(1); diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index e8a62c6643e..5671e8e3b40 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -1,9 +1,39 @@ +// TODO: Remove all these before merging +#![allow(unused_imports)] +#![allow(unreachable_code)] +#![allow(unused_variables)] + +use crate::{ + fuel_core_graphql_api::database::ReadView, + schema::scalars::{ + Bytes32, + Nonce, + }, +}; use async_graphql::{ + Context, Enum, Object, }; use fuel_core_types::entities; +#[derive(Default)] +pub struct RelayedTransactionQuery {} + +#[Object] +impl RelayedTransactionQuery { + async fn status( + &self, + ctx: &Context<'_>, + #[graphql(desc = "The id of the relayed tx")] id: Bytes32, + ) -> async_graphql::Result> { + // let query: &ReadView = ctx.data_unchecked(); + // let status = query.relayed_transaction_status(&id).into_api_result()?; + // Ok(status.into()) + todo!() + } +} + pub struct RelayedTransactionStatus( pub(crate) entities::relayer::transaction::RelayedTransactionStatus, ); From 611f9ff065dbd5c708ec742cfb58dcf5017117a1 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 3 Apr 2024 17:30:21 -0700 Subject: [PATCH 09/56] Get failing test compiling --- crates/fuel-core/src/graphql_api/database.rs | 28 +++++++++++++++++-- crates/fuel-core/src/graphql_api/ports.rs | 10 ++++--- crates/fuel-core/src/graphql_api/storage.rs | 4 +-- .../storage/relayed_transactions.rs | 2 +- crates/fuel-core/src/query.rs | 3 ++ crates/fuel-core/src/query/relayed_tx.rs | 17 +++++++++++ crates/fuel-core/src/schema/relayed_tx.rs | 7 ++--- .../service/adapters/graphql_api/off_chain.rs | 15 ++++++++++ 8 files changed, 72 insertions(+), 14 deletions(-) create mode 100644 crates/fuel-core/src/query/relayed_tx.rs diff --git a/crates/fuel-core/src/graphql_api/database.rs b/crates/fuel-core/src/graphql_api/database.rs index 35706a61412..30a3c619b31 100644 --- a/crates/fuel-core/src/graphql_api/database.rs +++ b/crates/fuel-core/src/graphql_api/database.rs @@ -6,6 +6,7 @@ use crate::fuel_core_graphql_api::{ DatabaseContracts, DatabaseMessageProof, DatabaseMessages, + DatabaseRelayedTransactions, OffChainDatabase, OnChainDatabase, }, @@ -33,13 +34,17 @@ use fuel_core_types::{ DaBlockHeight, }, }, - entities::relayer::message::{ - MerkleProof, - Message, + entities::relayer::{ + message::{ + MerkleProof, + Message, + }, + transaction::RelayedTransactionStatus, }, fuel_tx::{ Address, AssetId, + Bytes32, Salt, TxPointer, UtxoId, @@ -154,6 +159,16 @@ impl DatabaseMessages for ReadView { } } +impl DatabaseRelayedTransactions for ReadView { + fn transaction_status( + &self, + id: Bytes32, + ) -> StorageResult> { + let maybe_status = self.off_chain.relayed_tx_status(id)?; + Ok(maybe_status) + } +} + impl DatabaseContracts for ReadView { fn contract_balances( &self, @@ -226,4 +241,11 @@ impl OffChainDatabase for ReadView { fn contract_salt(&self, contract_id: &ContractId) -> StorageResult { self.off_chain.contract_salt(contract_id) } + + fn relayed_tx_status( + &self, + id: Bytes32, + ) -> StorageResult> { + self.off_chain.relayed_tx_status(id) + } } diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index d55bcd41e6f..450828dd321 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -1,4 +1,3 @@ -use crate::graphql_api::storage::relayed_transactions::RelayedTransactionStatuses; use async_trait::async_trait; use fuel_core_services::stream::BoxStream; use fuel_core_storage::{ @@ -90,6 +89,11 @@ pub trait OffChainDatabase: Send + Sync { ) -> BoxedIter>; fn contract_salt(&self, contract_id: &ContractId) -> StorageResult; + + fn relayed_tx_status( + &self, + id: Bytes32, + ) -> StorageResult>; } /// The on chain database port expected by GraphQL API service. @@ -133,9 +137,7 @@ pub trait DatabaseMessages: StorageInspect { fn message_exists(&self, nonce: &Nonce) -> StorageResult; } -pub trait DatabaseRelayedTransactions: - StorageInspect -{ +pub trait DatabaseRelayedTransactions { fn transaction_status( &self, id: Bytes32, diff --git a/crates/fuel-core/src/graphql_api/storage.rs b/crates/fuel-core/src/graphql_api/storage.rs index 8bb7a4ac245..91523215af3 100644 --- a/crates/fuel-core/src/graphql_api/storage.rs +++ b/crates/fuel-core/src/graphql_api/storage.rs @@ -76,8 +76,8 @@ pub enum Column { FuelBlockIdsToHeights = 7, /// See [`ContractsInfo`](contracts::ContractsInfo) ContractsInfo = 8, - /// ID for Layer 1 Transaction status - LayerOneTransactionStatus = 9, + /// Relayed Tx ID to Layer 1 Relayed Transaction status + RelayedTransactionStatus = 9, } impl Column { diff --git a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs index 66dc056b03c..d20ac6ec8f1 100644 --- a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs @@ -37,7 +37,7 @@ impl TableWithBlueprint for RelayedTransactionStatuses { type Column = super::Column; fn column() -> Self::Column { - Self::Column::LayerOneTransactionStatus + Self::Column::RelayedTransactionStatus } } diff --git a/crates/fuel-core/src/query.rs b/crates/fuel-core/src/query.rs index 50283d1a275..82f3212759c 100644 --- a/crates/fuel-core/src/query.rs +++ b/crates/fuel-core/src/query.rs @@ -7,6 +7,8 @@ mod message; mod subscriptions; mod tx; +mod relayed_tx; + // TODO: Remove reexporting of everything pub use balance::*; pub use block::*; @@ -14,5 +16,6 @@ pub use chain::*; pub use coin::*; pub use contract::*; pub use message::*; +pub use relayed_tx::*; pub(crate) use subscriptions::*; pub use tx::*; diff --git a/crates/fuel-core/src/query/relayed_tx.rs b/crates/fuel-core/src/query/relayed_tx.rs new file mode 100644 index 00000000000..e6340e7f263 --- /dev/null +++ b/crates/fuel-core/src/query/relayed_tx.rs @@ -0,0 +1,17 @@ +use crate::{ + fuel_core_graphql_api::ports::DatabaseRelayedTransactions, + schema::relayed_tx::RelayedTransactionStatus, +}; +use fuel_core_storage::Result as StorageResult; +use fuel_core_types::fuel_tx::Bytes32; + +pub fn relayed_tx_status( + db: &DB, + id: Bytes32, +) -> StorageResult> { + let status = db.transaction_status(id)?; + match status { + Some(status) => Ok(Some(RelayedTransactionStatus(status))), + None => Ok(None), + } +} diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index 5671e8e3b40..47d6101ab75 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -27,10 +27,9 @@ impl RelayedTransactionQuery { ctx: &Context<'_>, #[graphql(desc = "The id of the relayed tx")] id: Bytes32, ) -> async_graphql::Result> { - // let query: &ReadView = ctx.data_unchecked(); - // let status = query.relayed_transaction_status(&id).into_api_result()?; - // Ok(status.into()) - todo!() + let query: &ReadView = ctx.data_unchecked(); + let status = crate::query::relayed_tx_status(query, id.0)?; + Ok(status.into()) } } diff --git a/crates/fuel-core/src/service/adapters/graphql_api/off_chain.rs b/crates/fuel-core/src/service/adapters/graphql_api/off_chain.rs index ce5645a3e6d..887ddaccc39 100644 --- a/crates/fuel-core/src/service/adapters/graphql_api/off_chain.rs +++ b/crates/fuel-core/src/service/adapters/graphql_api/off_chain.rs @@ -10,6 +10,7 @@ use crate::{ }, storage::{ contracts::ContractsInfo, + relayed_transactions::RelayedTransactionStatuses, transactions::OwnedTransactionIndexCursor, }, }, @@ -35,8 +36,10 @@ use fuel_core_txpool::types::{ }; use fuel_core_types::{ blockchain::primitives::BlockId, + entities::relayer::transaction::RelayedTransactionStatus, fuel_tx::{ Address, + Bytes32, Salt, TxPointer, UtxoId, @@ -106,6 +109,18 @@ impl OffChainDatabase for Database { Ok(salt) } + + fn relayed_tx_status( + &self, + id: Bytes32, + ) -> StorageResult> { + let status = self + .storage_as_ref::() + .get(&id) + .map_err(StorageError::from)? + .map(|cow| cow.into_owned()); + Ok(status) + } } impl Transactional for Database { From 2c26842938bc40af4d1f7cb4695088b628570ab4 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 3 Apr 2024 17:43:12 -0700 Subject: [PATCH 10/56] Add actual value to test --- crates/client/src/client/schema/relayed_tx.rs | 2 +- tests/tests/relayer.rs | 27 +++++++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index 4b2f8f00f3e..352927c9a1a 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -26,7 +26,7 @@ pub struct RelayedTransactionStatus { pub(crate) state: RelayedTransactionState, } -#[derive(cynic::Enum, Copy, Clone, Debug)] +#[derive(cynic::Enum, Copy, Clone, Debug, PartialEq, Eq)] #[cynic(schema_path = "./assets/schema.sdl")] pub enum RelayedTransactionState { /// Transaction was included in a block, but the execution was reverted diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index e14e7ad7641..213d6c3ea25 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -7,7 +7,9 @@ use ethers::{ }, }; use fuel_core::{ + combined_database::CombinedDatabase, database::Database, + fuel_core_graphql_api::storage::relayed_transactions::RelayedTransactionStatuses, relayer, service::{ Config, @@ -20,6 +22,7 @@ use fuel_core_client::client::{ PageDirection, PaginationRequest, }, + schema::relayed_tx::RelayedTransactionState, types::TransactionStatus, FuelClient, }; @@ -34,13 +37,16 @@ use fuel_core_relayer::{ }; use fuel_core_storage::{ tables::Messages, + StorageAsMut, StorageAsRef, }; use fuel_core_types::{ + entities::relayer::transaction::RelayedTransactionStatus, fuel_asm::*, fuel_crypto::*, fuel_tx::*, fuel_types::Nonce, + tai64::Tai64, }; use hyper::{ service::{ @@ -254,16 +260,27 @@ async fn messages_are_spendable_after_relayer_is_synced() { #[tokio::test(flavor = "multi_thread")] async fn can_find_failed_relayed_tx() { - let db = Database::in_memory(); - let srv = FuelService::from_database(db.clone(), Config::local_node()) + let mut db = CombinedDatabase::in_memory(); + let id = [1; 32].into(); + let status = RelayedTransactionStatus::Failed { + block_height: 999.into(), + block_time: Tai64::UNIX_EPOCH, + failure: "lolz".to_string(), + }; + db.off_chain_mut() + .storage_as_mut::() + .insert(&id, &status) + .unwrap(); + + let srv = FuelService::from_combined_database(db.clone(), Config::local_node()) .await .unwrap(); let client = FuelClient::from(srv.bound_address); - let tx_id = [1; 32].into(); - let query = client.relayed_transaction_status(&tx_id).await.unwrap(); - assert!(query.is_some()); + let expected = Some(RelayedTransactionState::Failed); + let actual = client.relayed_transaction_status(&id).await.unwrap(); + assert_eq!(expected, actual); } #[allow(clippy::too_many_arguments)] From 63ed88db787bfb573f998b693230a6f6adc4557a Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 4 Apr 2024 13:55:24 -0700 Subject: [PATCH 11/56] Use union instead of enum for status --- crates/client/assets/schema.sdl | 10 +-- crates/client/src/client.rs | 15 ++-- crates/client/src/client/schema/relayed_tx.rs | 21 +++-- crates/client/src/client/types.rs | 33 +++++++- crates/fuel-core/src/query/relayed_tx.rs | 16 ++-- crates/fuel-core/src/schema/relayed_tx.rs | 84 ++++++++++++++----- tests/tests/relayer.rs | 30 +++++-- 7 files changed, 150 insertions(+), 59 deletions(-) diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 4cfdf72bdf8..233895e373b 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -903,13 +903,13 @@ enum ReceiptType { BURN } -enum RelayedTransactionState { - FAILED +type RelayedTransactionFailed { + blockHeight: U32! + blockTime: Tai64Timestamp! + failure: String! } -type RelayedTransactionStatus { - state: RelayedTransactionState! -} +union RelayedTransactionStatus = RelayedTransactionFailed enum ReturnType { RETURN diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 21b6ca46857..4c9933cec29 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -8,10 +8,7 @@ use crate::client::{ contract::ContractBalanceQueryArgs, gas_price::EstimateGasPrice, message::MessageStatusArgs, - relayed_tx::{ - RelayedTransactionState, - RelayedTransactionStatusArgs, - }, + relayed_tx::RelayedTransactionStatusArgs, tx::DryRunArg, Tai64Timestamp, TransactionId, @@ -26,6 +23,7 @@ use crate::client::{ ContractId, UtxoId, }, + RelayedTransactionStatus, }, }; use anyhow::Context; @@ -977,13 +975,18 @@ impl FuelClient { pub async fn relayed_transaction_status( &self, id: &Bytes32, - ) -> io::Result> { + ) -> io::Result> { let query = schema::relayed_tx::RelayedTransactionStatusQuery::build( RelayedTransactionStatusArgs { id: id.to_owned().into(), }, ); - let status = self.query(query).await?.status.map(|inner| inner.state); + let status = self + .query(query) + .await? + .status + .map(|status| status.try_into()) + .transpose()?; Ok(status) } } diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index 352927c9a1a..f719d06571f 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -1,6 +1,8 @@ use crate::client::schema::{ schema, Bytes32, + Tai64Timestamp, + U32, }; #[derive(cynic::QueryFragment, Clone, Debug)] @@ -20,15 +22,20 @@ pub struct RelayedTransactionStatusArgs { pub id: Bytes32, } -#[derive(cynic::QueryFragment, Clone, Debug)] +#[allow(clippy::enum_variant_names)] +#[derive(cynic::InlineFragments, Clone, Debug)] #[cynic(schema_path = "./assets/schema.sdl")] -pub struct RelayedTransactionStatus { - pub(crate) state: RelayedTransactionState, +pub enum RelayedTransactionStatus { + /// Transaction was included in a block, but the execution was reverted + Failed(RelayedTransactionFailed), + #[cynic(fallback)] + Unknown, } -#[derive(cynic::Enum, Copy, Clone, Debug, PartialEq, Eq)] +#[derive(cynic::QueryFragment, Clone, Debug, PartialEq, Eq)] #[cynic(schema_path = "./assets/schema.sdl")] -pub enum RelayedTransactionState { - /// Transaction was included in a block, but the execution was reverted - Failed, +pub struct RelayedTransactionFailed { + pub block_height: U32, + pub block_time: Tai64Timestamp, + pub failure: String, } diff --git a/crates/client/src/client/types.rs b/crates/client/src/client/types.rs index f0db1e76aa9..e9543407268 100644 --- a/crates/client/src/client/types.rs +++ b/crates/client/src/client/types.rs @@ -37,6 +37,7 @@ pub use message::{ pub use node_info::NodeInfo; use crate::client::schema::{ + relayed_tx::RelayedTransactionStatus as SchemaRelayedTransactionStatus, tx::{ OpaqueTransaction, TransactionStatus as SchemaTxStatus, @@ -145,7 +146,7 @@ impl TryFrom for TransactionStatus { TransactionStatus::SqueezedOut { reason: s.reason } } SchemaTxStatus::Unknown => { - return Err(Self::Error::UnknownVariant("SchemaTxStatus")) + return Err(Self::Error::UnknownVariant("SchemaTxStatus")); } }) } @@ -169,3 +170,33 @@ impl TryFrom for TransactionResponse { }) } } + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RelayedTransactionStatus { + Failed { + block_height: BlockHeight, + block_time: Tai64, + failure: String, + }, +} + +impl TryFrom for RelayedTransactionStatus { + type Error = ConversionError; + + fn try_from(status: SchemaRelayedTransactionStatus) -> Result { + Ok(match status { + SchemaRelayedTransactionStatus::Failed(s) => { + RelayedTransactionStatus::Failed { + block_height: s.block_height.into(), + block_time: s.block_time.0, + failure: s.failure, + } + } + SchemaRelayedTransactionStatus::Unknown => { + return Err(Self::Error::UnknownVariant( + "SchemaRelayedTransactionStatus", + )); + } + }) + } +} diff --git a/crates/fuel-core/src/query/relayed_tx.rs b/crates/fuel-core/src/query/relayed_tx.rs index e6340e7f263..a272458c0ea 100644 --- a/crates/fuel-core/src/query/relayed_tx.rs +++ b/crates/fuel-core/src/query/relayed_tx.rs @@ -1,17 +1,13 @@ -use crate::{ - fuel_core_graphql_api::ports::DatabaseRelayedTransactions, - schema::relayed_tx::RelayedTransactionStatus, -}; +use crate::fuel_core_graphql_api::ports::DatabaseRelayedTransactions; use fuel_core_storage::Result as StorageResult; -use fuel_core_types::fuel_tx::Bytes32; +use fuel_core_types::{ + entities::relayer::transaction::RelayedTransactionStatus, + fuel_tx::Bytes32, +}; pub fn relayed_tx_status( db: &DB, id: Bytes32, ) -> StorageResult> { - let status = db.transaction_status(id)?; - match status { - Some(status) => Ok(Some(RelayedTransactionStatus(status))), - None => Ok(None), - } + db.transaction_status(id) } diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index 47d6101ab75..32013c6223f 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -4,18 +4,37 @@ #![allow(unused_variables)] use crate::{ - fuel_core_graphql_api::database::ReadView, - schema::scalars::{ - Bytes32, - Nonce, + fuel_core_graphql_api::{ + database::ReadView, + IntoApiResult, + }, + schema::{ + scalars::{ + Bytes32, + Nonce, + Tai64Timestamp, + U32, + }, + tx::types::{ + FailureStatus, + SqueezedOutStatus, + SubmittedStatus, + SuccessStatus, + }, }, }; use async_graphql::{ Context, Enum, Object, + Union, +}; +use fuel_core_types::{ + entities, + entities::relayer::transaction::RelayedTransactionStatus as FuelRelayedTransactionStatus, + fuel_types::BlockHeight, + tai64::Tai64, }; -use fuel_core_types::entities; #[derive(Default)] pub struct RelayedTransactionQuery {} @@ -28,29 +47,52 @@ impl RelayedTransactionQuery { #[graphql(desc = "The id of the relayed tx")] id: Bytes32, ) -> async_graphql::Result> { let query: &ReadView = ctx.data_unchecked(); - let status = crate::query::relayed_tx_status(query, id.0)?; - Ok(status.into()) + let status = + crate::query::relayed_tx_status(query, id.0)?.map(|status| status.into()); + Ok(status) } } -pub struct RelayedTransactionStatus( - pub(crate) entities::relayer::transaction::RelayedTransactionStatus, -); +#[derive(Union, Debug)] +pub enum RelayedTransactionStatus { + Failed(RelayedTransactionFailed), +} -#[derive(Enum, Copy, Clone, Eq, PartialEq)] -enum RelayedTransactionState { - Failed, +#[derive(Debug)] +pub struct RelayedTransactionFailed { + pub block_height: BlockHeight, + pub block_time: Tai64, + pub failure: String, } #[Object] -impl RelayedTransactionStatus { - async fn state(&self) -> RelayedTransactionState { - match &self.0 { - entities::relayer::transaction::RelayedTransactionStatus::Failed { - block_height: _, - block_time: _, - failure: _, - } => RelayedTransactionState::Failed, +impl RelayedTransactionFailed { + async fn block_height(&self) -> U32 { + let as_u32: u32 = self.block_height.into(); + as_u32.into() + } + + async fn block_time(&self) -> Tai64Timestamp { + self.block_time.into() + } + + async fn failure(&self) -> String { + self.failure.clone() + } +} + +impl From for RelayedTransactionStatus { + fn from(status: FuelRelayedTransactionStatus) -> Self { + match status { + FuelRelayedTransactionStatus::Failed { + block_height, + block_time, + failure, + } => RelayedTransactionStatus::Failed(RelayedTransactionFailed { + block_height, + block_time, + failure, + }), } } } diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index 213d6c3ea25..b9c2ea8f626 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -22,8 +22,10 @@ use fuel_core_client::client::{ PageDirection, PaginationRequest, }, - schema::relayed_tx::RelayedTransactionState, - types::TransactionStatus, + types::{ + RelayedTransactionStatus as ClientRelayedTransactionStatus, + TransactionStatus, + }, FuelClient, }; use fuel_core_poa::service::Mode; @@ -41,11 +43,14 @@ use fuel_core_storage::{ StorageAsRef, }; use fuel_core_types::{ - entities::relayer::transaction::RelayedTransactionStatus, + entities::relayer::transaction::RelayedTransactionStatus as FuelRelayedTransactionStatus, fuel_asm::*, fuel_crypto::*, fuel_tx::*, - fuel_types::Nonce, + fuel_types::{ + BlockHeight, + Nonce, + }, tai64::Tai64, }; use hyper::{ @@ -262,10 +267,13 @@ async fn messages_are_spendable_after_relayer_is_synced() { async fn can_find_failed_relayed_tx() { let mut db = CombinedDatabase::in_memory(); let id = [1; 32].into(); - let status = RelayedTransactionStatus::Failed { - block_height: 999.into(), - block_time: Tai64::UNIX_EPOCH, - failure: "lolz".to_string(), + let block_height: BlockHeight = 999.into(); + let block_time = Tai64::UNIX_EPOCH; + let failure = "lolz".to_string(); + let status = FuelRelayedTransactionStatus::Failed { + block_height: block_height.clone(), + block_time: block_time.clone(), + failure: failure.clone(), }; db.off_chain_mut() .storage_as_mut::() @@ -278,7 +286,11 @@ async fn can_find_failed_relayed_tx() { let client = FuelClient::from(srv.bound_address); - let expected = Some(RelayedTransactionState::Failed); + let expected = Some(ClientRelayedTransactionStatus::Failed { + block_height, + block_time, + failure, + }); let actual = client.relayed_transaction_status(&id).await.unwrap(); assert_eq!(expected, actual); } From a672cd00d44af014df57846c852ea0026ad275d7 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 4 Apr 2024 14:28:12 -0700 Subject: [PATCH 12/56] Cleanup --- .../src/graphql_api/worker_service.rs | 3 +- crates/fuel-core/src/schema/relayed_tx.rs | 29 ++++--------------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index da28cbea9f1..7e1cde5c34c 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -1,4 +1,3 @@ -use crate::graphql_api::storage::relayed_transactions::RelayedTransactionStatuses; use crate::{ fuel_core_graphql_api::{ ports, @@ -16,7 +15,7 @@ use crate::{ }, }, }, - // graphql_api::storage::layer_one_transactions::RelayedTransactionStatuses, + graphql_api::storage::relayed_transactions::RelayedTransactionStatuses, }; use fuel_core_metrics::graphql_metrics::graphql_metrics; use fuel_core_services::{ diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index 32013c6223f..a90926e9b5d 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -1,36 +1,17 @@ -// TODO: Remove all these before merging -#![allow(unused_imports)] -#![allow(unreachable_code)] -#![allow(unused_variables)] - use crate::{ - fuel_core_graphql_api::{ - database::ReadView, - IntoApiResult, - }, - schema::{ - scalars::{ - Bytes32, - Nonce, - Tai64Timestamp, - U32, - }, - tx::types::{ - FailureStatus, - SqueezedOutStatus, - SubmittedStatus, - SuccessStatus, - }, + fuel_core_graphql_api::database::ReadView, + schema::scalars::{ + Bytes32, + Tai64Timestamp, + U32, }, }; use async_graphql::{ Context, - Enum, Object, Union, }; use fuel_core_types::{ - entities, entities::relayer::transaction::RelayedTransactionStatus as FuelRelayedTransactionStatus, fuel_types::BlockHeight, tai64::Tai64, From 065e93fc534945178f91f530a8b05fabab20dc47 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 4 Apr 2024 15:11:23 -0700 Subject: [PATCH 13/56] Add test for including relayed txs in block --- crates/fuel-core/src/executor.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index c10836495b5..55978a5104f 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3038,6 +3038,34 @@ mod tests { assert_eq!(actual, expected); } + #[test] + fn execute_without_commit__relayed_txs_included_in_block() { + let genesis_da_height = 3u64; + let on_chain_db = database_with_genesis_block(genesis_da_height); + let mut relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let relayed_tx = RelayedTransaction::default(); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[relayed_tx.clone().into()], + ); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + + // when + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + } + #[test] fn block_producer_does_not_take_messages_for_the_same_height() { let genesis_da_height = 1u64; From 21181a133d3a7b5019133279ea19e134f3176744 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 4 Apr 2024 16:05:23 -0700 Subject: [PATCH 14/56] Fix test for relayed tx inclusion --- crates/fuel-core/src/executor.rs | 64 ++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 55978a5104f..dd16e4ed4d1 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -284,19 +284,7 @@ mod tests { da_block_height: DaBlockHeight, num_txs: usize, ) -> Block { - let transactions = (1..num_txs + 1) - .map(|i| { - TxBuilder::new(2322u64) - .script_gas_limit(10) - .coin_input(AssetId::default(), (i as Word) * 100) - .coin_output(AssetId::default(), (i as Word) * 50) - .change_output(AssetId::default()) - .build() - .transaction() - .clone() - .into() - }) - .collect_vec(); + let transactions = (1..num_txs + 1).map(script_tx_for_amount).collect_vec(); let mut block = Block::default(); block.header_mut().set_block_height(block_height); @@ -305,6 +293,18 @@ mod tests { block } + fn script_tx_for_amount(amount: usize) -> Transaction { + TxBuilder::new(2322u64) + .script_gas_limit(10) + .coin_input(AssetId::default(), (amount as Word) * 100) + .coin_output(AssetId::default(), (amount as Word) * 50) + .change_output(AssetId::default()) + .build() + .transaction() + .to_owned() + .into() + } + pub(crate) fn create_contract( contract_code: Vec, rng: &mut R, @@ -3039,14 +3039,17 @@ mod tests { } #[test] - fn execute_without_commit__relayed_txs_included_in_block() { + fn execute_without_commit__relayed_tx_included_in_block() { let genesis_da_height = 3u64; let on_chain_db = database_with_genesis_block(genesis_da_height); let mut relayer_db = Database::::default(); // given let block_height = 1u32; let da_height = 10u64; - let relayed_tx = RelayedTransaction::default(); + let mut relayed_tx = RelayedTransaction::default(); + let tx = script_tx_for_amount(100); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); add_events_to_relayer( &mut relayer_db, da_height.into(), @@ -3063,9 +3066,38 @@ mod tests { // then let txs = result.block.transactions(); - assert_eq!(txs.len(), 1); + dbg!(&txs); + assert_eq!(txs.len(), 2); } + // #[test] + // fn execute_without_commit__relayed_mint_tx_not_included_in_block() { + // let genesis_da_height = 3u64; + // let on_chain_db = database_with_genesis_block(genesis_da_height); + // let mut relayer_db = Database::::default(); + // // given + // let block_height = 1u32; + // let da_height = 10u64; + // let relayed_tx = RelayedTransaction::default(); + // add_events_to_relayer( + // &mut relayer_db, + // da_height.into(), + // &[relayed_tx.clone().into()], + // ); + // let producer = create_relayer_executor(on_chain_db, relayer_db); + // let block = test_block(block_height.into(), da_height.into(), 0); + // + // // when + // let (result, _) = producer + // .execute_without_commit(ExecutionTypes::Production(block.into())) + // .unwrap() + // .into(); + // + // // then + // let txs = result.block.transactions(); + // assert_eq!(txs.len(), 1); + // } + #[test] fn block_producer_does_not_take_messages_for_the_same_height() { let genesis_da_height = 1u64; From bdc61037cec1274691b94033908075fde1acd207 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 4 Apr 2024 16:33:31 -0700 Subject: [PATCH 15/56] Add test to show mints are not allowed --- crates/fuel-core/src/executor.rs | 89 ++++++++++++++++++++++---------- 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index dd16e4ed4d1..d8be25127a0 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2837,6 +2837,7 @@ mod tests { use fuel_core_types::{ entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, + fuel_tx::output, }; fn database_with_genesis_block(da_block_height: u64) -> Database { @@ -3070,33 +3071,67 @@ mod tests { assert_eq!(txs.len(), 2); } - // #[test] - // fn execute_without_commit__relayed_mint_tx_not_included_in_block() { - // let genesis_da_height = 3u64; - // let on_chain_db = database_with_genesis_block(genesis_da_height); - // let mut relayer_db = Database::::default(); - // // given - // let block_height = 1u32; - // let da_height = 10u64; - // let relayed_tx = RelayedTransaction::default(); - // add_events_to_relayer( - // &mut relayer_db, - // da_height.into(), - // &[relayed_tx.clone().into()], - // ); - // let producer = create_relayer_executor(on_chain_db, relayer_db); - // let block = test_block(block_height.into(), da_height.into(), 0); - // - // // when - // let (result, _) = producer - // .execute_without_commit(ExecutionTypes::Production(block.into())) - // .unwrap() - // .into(); - // - // // then - // let txs = result.block.transactions(); - // assert_eq!(txs.len(), 1); - // } + #[test] + fn execute_without_commit__relayed_mint_tx_not_included_in_block() { + let genesis_da_height = 3u64; + let on_chain_db = database_with_genesis_block(genesis_da_height); + let mut relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let mut relayed_tx = RelayedTransaction::default(); + let base_asset_id = AssetId::BASE; + let tx_count = 0; + let mint = Transaction::mint( + TxPointer::new(block_height.into(), tx_count), + contract::Contract { + utxo_id: UtxoId::new(Bytes32::zeroed(), 0), + balance_root: Bytes32::zeroed(), + state_root: Bytes32::zeroed(), + tx_pointer: TxPointer::new(BlockHeight::new(0), 0), + contract_id: ContractId::zeroed(), + }, + output::contract::Contract { + input_index: 0, + balance_root: Bytes32::zeroed(), + state_root: Bytes32::zeroed(), + }, + 0, + base_asset_id, + 0, + ); + let tx = Transaction::Mint(mint.into()); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[relayed_tx.clone().into()], + ); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = + test_block(block_height.into(), da_height.into(), tx_count as usize); + + // when + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[0] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Transaction type is not accepted"; + assert_eq!(expected, actual); + } #[test] fn block_producer_does_not_take_messages_for_the_same_height() { From a96c787049d543517c87b49a42e5219b1a4a0e9b Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 5 Apr 2024 11:37:32 -0700 Subject: [PATCH 16/56] Add test for duplicate relayed txs --- crates/fuel-core/src/executor.rs | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index d8be25127a0..59047124544 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3071,6 +3071,39 @@ mod tests { assert_eq!(txs.len(), 2); } + #[test] + fn execute_without_commit__duplicated_relayed_tx_not_included_in_block() { + let genesis_da_height = 3u64; + let on_chain_db = database_with_genesis_block(genesis_da_height); + let mut relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let mut relayed_tx = RelayedTransaction::default(); + let tx = script_tx_for_amount(100); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + let duplicate_count = 10; + let events = std::iter::repeat(relayed_tx.clone().into()) + .take(duplicate_count + 1) + .collect::>(); + add_events_to_relayer(&mut relayer_db, da_height.into(), &events); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + + // when + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 2); + let skipped_txs = result.skipped_transactions; + assert_eq!(skipped_txs.len(), duplicate_count); + } + #[test] fn execute_without_commit__relayed_mint_tx_not_included_in_block() { let genesis_da_height = 3u64; From a2a9576271aa180dff1602bf13b2c0be1d10c287 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 5 Apr 2024 12:15:16 -0700 Subject: [PATCH 17/56] Add test for invalid relayed tx --- crates/fuel-core/src/executor.rs | 43 ++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 59047124544..b033cad7d22 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3104,6 +3104,49 @@ mod tests { assert_eq!(skipped_txs.len(), duplicate_count); } + #[test] + fn execute_without_commit__invalid_relayed_txs_are_not_included_and_are_reported() + { + let genesis_da_height = 3u64; + let on_chain_db = database_with_genesis_block(genesis_da_height); + let mut relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let mut invalid_relayed_tx = RelayedTransaction::default(); + let mut tx = script_tx_for_amount(100); + tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) + let tx_bytes = tx.to_bytes(); + invalid_relayed_tx.set_serialized_transaction(tx_bytes); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[invalid_relayed_tx.clone().into()], + ); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + + // when + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[0] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Failed validity checks"; + assert_eq!(expected, actual); + } + #[test] fn execute_without_commit__relayed_mint_tx_not_included_in_block() { let genesis_da_height = 3u64; From 4b4d343d3c31ca2e2eb6cda2d96e44ec52f41a8c Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 5 Apr 2024 13:05:35 -0700 Subject: [PATCH 18/56] Add test for verifier to see failed relayed tx events --- crates/fuel-core/src/executor.rs | 49 ++++++++++++++++++++++++ crates/services/executor/src/executor.rs | 21 ++++++---- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index b033cad7d22..75c47ee012e 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3147,6 +3147,55 @@ mod tests { assert_eq!(expected, actual); } + #[test] + fn execute_without_commit__validation__includes_status_of_failed_relayed_tx() { + let genesis_da_height = 3u64; + let producer_db = database_with_genesis_block(genesis_da_height); + let verifyer_db = database_with_genesis_block(genesis_da_height); + let mut producer_relayer_db = Database::::default(); + let mut verifier_relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let mut invalid_relayed_tx = RelayedTransaction::default(); + let mut tx = script_tx_for_amount(100); + tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) + let tx_bytes = tx.to_bytes(); + invalid_relayed_tx.set_serialized_transaction(tx_bytes); + let events = vec![invalid_relayed_tx.clone().into()]; + add_events_to_relayer(&mut producer_relayer_db, da_height.into(), &events); + add_events_to_relayer(&mut verifier_relayer_db, da_height.into(), &events); + + let producer = create_relayer_executor(producer_db, producer_relayer_db); + let verifier = create_relayer_executor(verifyer_db, verifier_relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + let (produced_result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + let produced_block = produced_result.block; + + // when + let (result, _) = verifier + .execute_without_commit(ExecutionTypes::Validation(produced_block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[0] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Failed validity checks"; + assert_eq!(expected, actual); + } + #[test] fn execute_without_commit__relayed_mint_tx_not_included_in_block() { let genesis_da_height = 3u64; diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 726e7e7be3c..f41c45e1d56 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -499,7 +499,7 @@ where // initiate transaction stream with relayed (forced) transactions first, // and pull the rest from the TxSource (txpool) if there is remaining blockspace available. // We use `block.transactions` to store executed transactions. - let mut iter = forced_transactions.into_iter().peekable(); + let relayed_tx_iter = forced_transactions.into_iter().peekable(); let mut execute_transaction = |execution_data: &mut ExecutionData, tx: MaybeCheckedTransaction| @@ -569,17 +569,24 @@ where Ok(()) }; - while iter.peek().is_some() { - for transaction in iter { + for transaction in relayed_tx_iter { + execute_transaction(&mut *execution_data, transaction)?; + } + + let remaining_gas_limit = block_gas_limit.saturating_sub(execution_data.used_gas); + + // L2 originated transactions should be in the `TxSource`. This will be triggered after + // all relayed transactions are processed. + let mut regular_tx_iter = source.next(remaining_gas_limit).into_iter().peekable(); + while regular_tx_iter.peek().is_some() { + for transaction in regular_tx_iter { execute_transaction(&mut *execution_data, transaction)?; } - let remaining_gas_limit = + let new_remaining_gas_limit = block_gas_limit.saturating_sub(execution_data.used_gas); - // L2 originated transactions should be in the `TxSource`. This will be triggered after - // all relayed transactions are processed. - iter = source.next(remaining_gas_limit).into_iter().peekable(); + regular_tx_iter = source.next(new_remaining_gas_limit).into_iter().peekable(); } // After the execution of all transactions in production mode, we can set the final fee. From 8fbda0f8127eeeaceb12b6ca5037a06a01ccc28e Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 5 Apr 2024 17:02:01 -0700 Subject: [PATCH 19/56] Add test for not charging for L1 txs on L2 --- crates/fuel-core/src/executor.rs | 153 +++++++++++++++-------- crates/services/executor/src/executor.rs | 9 +- 2 files changed, 107 insertions(+), 55 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 75c47ee012e..f272bbe504f 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -284,7 +284,9 @@ mod tests { da_block_height: DaBlockHeight, num_txs: usize, ) -> Block { - let transactions = (1..num_txs + 1).map(script_tx_for_amount).collect_vec(); + let transactions = (1..num_txs + 1) + .map(script_tx_for_amount_of_base_asset) + .collect_vec(); let mut block = Block::default(); block.header_mut().set_block_height(block_height); @@ -293,12 +295,17 @@ mod tests { block } - fn script_tx_for_amount(amount: usize) -> Transaction { + fn script_tx_for_amount_of_base_asset(amount: usize) -> Transaction { + let asset = AssetId::BASE; + script_tx_for_amount(amount, asset) + } + + fn script_tx_for_amount(amount: usize, asset: AssetId) -> Transaction { TxBuilder::new(2322u64) .script_gas_limit(10) - .coin_input(AssetId::default(), (amount as Word) * 100) - .coin_output(AssetId::default(), (amount as Word) * 50) - .change_output(AssetId::default()) + .coin_input(asset, (amount as Word) * 100) + .coin_output(asset, (amount as Word) * 50) + .change_output(asset) .build() .transaction() .to_owned() @@ -761,7 +768,7 @@ mod tests { // register `0x13`(1 - true, 0 - false). op::meq(0x13, 0x10, 0x12, 0x11), // Return the result of the comparison as a receipt. - op::ret(0x13) + op::ret(0x13), ], expected_in_tx_coinbase.to_vec() /* pass expected address as script data */) .coin_input(AssetId::BASE, 1000) .variable_output(Default::default()) @@ -800,19 +807,19 @@ mod tests { assert!(compare_coinbase_addresses( ContractId::from([1u8; 32]), - ContractId::from([1u8; 32]) + ContractId::from([1u8; 32]), )); assert!(!compare_coinbase_addresses( ContractId::from([9u8; 32]), - ContractId::from([1u8; 32]) + ContractId::from([1u8; 32]), )); assert!(!compare_coinbase_addresses( ContractId::from([1u8; 32]), - ContractId::from([9u8; 32]) + ContractId::from([9u8; 32]), )); assert!(compare_coinbase_addresses( ContractId::from([9u8; 32]), - ContractId::from([9u8; 32]) + ContractId::from([9u8; 32]), )); } @@ -2837,7 +2844,10 @@ mod tests { use fuel_core_types::{ entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, - fuel_tx::output, + fuel_tx::{ + field::MintGasPrice, + output, + }, }; fn database_with_genesis_block(da_block_height: u64) -> Database { @@ -2897,52 +2907,52 @@ mod tests { } #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(0), - } => matches Ok(()) ; "block producer takes all 10 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(0), + } => matches Ok(()); "block producer takes all 10 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 5, - genesis_da_height: Some(0), - } => matches Ok(()) ; "block producer takes first 5 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 5, + genesis_da_height: Some(0), + } => matches Ok(()); "block producer takes first 5 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(5), - } => matches Ok(()) ; "block producer takes last 5 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(5), + } => matches Ok(()); "block producer takes last 5 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(u64::MAX), - } => matches Err(ExecutorError::DaHeightExceededItsLimit) ; "block producer fails when previous block exceeds `u64::MAX`" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(u64::MAX), + } => matches Err(ExecutorError::DaHeightExceededItsLimit); "block producer fails when previous block exceeds `u64::MAX`" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: None, - } => matches Err(ExecutorError::PreviousBlockIsNotFound) ; "block producer fails when previous block doesn't exist" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: None, + } => matches Err(ExecutorError::PreviousBlockIsNotFound); "block producer fails when previous block doesn't exist" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 0, - block_da_height: 10, - genesis_da_height: Some(0), - } => matches Err(ExecutorError::ExecutingGenesisBlock) ; "block producer fails when block height is zero" + Input { + relayer_da_height: 10, + block_height: 0, + block_da_height: 10, + genesis_da_height: Some(0), + } => matches Err(ExecutorError::ExecutingGenesisBlock); "block producer fails when block height is zero" )] fn block_producer_takes_messages_from_the_relayer( input: Input, @@ -3048,7 +3058,7 @@ mod tests { let block_height = 1u32; let da_height = 10u64; let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount(100); + let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); add_events_to_relayer( @@ -3067,10 +3077,49 @@ mod tests { // then let txs = result.block.transactions(); - dbg!(&txs); assert_eq!(txs.len(), 2); } + #[test] + fn execute_without_commit_with_coinbase__relayed_tx_can_have_no_base_asset_and_mint_will_have_no_fees( + ) { + let genesis_da_height = 3u64; + let on_chain_db = database_with_genesis_block(genesis_da_height); + let mut relayer_db = Database::::default(); + // given + let block_height = 1u32; + let da_height = 10u64; + let mut relayed_tx = RelayedTransaction::default(); + let not_base_asset = AssetId::new([5; 32]); + let tx = script_tx_for_amount(100, not_base_asset); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[relayed_tx.clone().into()], + ); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + + // when + let gas_price = 1; + let (result, _) = producer + .execute_without_commit_with_coinbase( + ExecutionTypes::Production(block.into()), + Default::default(), + gas_price, + ) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 2); + let mint = txs[1].as_mint().unwrap(); + assert_eq!(*mint.mint_amount(), 0); + } + #[test] fn execute_without_commit__duplicated_relayed_tx_not_included_in_block() { let genesis_da_height = 3u64; @@ -3080,7 +3129,7 @@ mod tests { let block_height = 1u32; let da_height = 10u64; let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount(100); + let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); let duplicate_count = 10; @@ -3114,7 +3163,7 @@ mod tests { let block_height = 1u32; let da_height = 10u64; let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount(100); + let mut tx = script_tx_for_amount_of_base_asset(100); tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) let tx_bytes = tx.to_bytes(); invalid_relayed_tx.set_serialized_transaction(tx_bytes); @@ -3158,7 +3207,7 @@ mod tests { let block_height = 1u32; let da_height = 10u64; let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount(100); + let mut tx = script_tx_for_amount_of_base_asset(100); tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) let tx_bytes = tx.to_bytes(); invalid_relayed_tx.set_serialized_transaction(tx_bytes); diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index f41c45e1d56..4370ab07d13 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -502,7 +502,8 @@ where let relayed_tx_iter = forced_transactions.into_iter().peekable(); let mut execute_transaction = |execution_data: &mut ExecutionData, - tx: MaybeCheckedTransaction| + tx: MaybeCheckedTransaction, + gas_price: Word| -> ExecutorResult<()> { let tx_count = execution_data.tx_count; let tx = { @@ -570,7 +571,8 @@ where }; for transaction in relayed_tx_iter { - execute_transaction(&mut *execution_data, transaction)?; + const RELAYED_GAS_PRICE: Word = 0; + execute_transaction(&mut *execution_data, transaction, RELAYED_GAS_PRICE)?; } let remaining_gas_limit = block_gas_limit.saturating_sub(execution_data.used_gas); @@ -580,7 +582,7 @@ where let mut regular_tx_iter = source.next(remaining_gas_limit).into_iter().peekable(); while regular_tx_iter.peek().is_some() { for transaction in regular_tx_iter { - execute_transaction(&mut *execution_data, transaction)?; + execute_transaction(&mut *execution_data, transaction, gas_price)?; } let new_remaining_gas_limit = @@ -619,6 +621,7 @@ where execute_transaction( execution_data, MaybeCheckedTransaction::Transaction(coinbase_tx.into()), + gas_price, )?; } From 9422061321ea04ad7770c10228002f8e46148718 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 5 Apr 2024 18:04:32 -0700 Subject: [PATCH 20/56] WIP cleanup --- crates/fuel-core/src/executor.rs | 152 +++++++++++++++++++------------ 1 file changed, 93 insertions(+), 59 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index f272bbe504f..b6f0a4e9f40 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2844,10 +2844,7 @@ mod tests { use fuel_core_types::{ entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, - fuel_tx::{ - field::MintGasPrice, - output, - }, + fuel_tx::output, }; fn database_with_genesis_block(da_block_height: u64) -> Database { @@ -3052,24 +3049,16 @@ mod tests { #[test] fn execute_without_commit__relayed_tx_included_in_block() { let genesis_da_height = 3u64; - let on_chain_db = database_with_genesis_block(genesis_da_height); - let mut relayer_db = Database::::default(); - // given let block_height = 1u32; let da_height = 10u64; - let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount_of_base_asset(100); - let tx_bytes = tx.to_bytes(); - relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[relayed_tx.clone().into()], - ); - let producer = create_relayer_executor(on_chain_db, relayer_db); - let block = test_block(block_height.into(), da_height.into(), 0); + + // given + let relayer_db = relayer_db_with_valid_relayed_txs(da_height); // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); let (result, _) = producer .execute_without_commit(ExecutionTypes::Production(block.into())) .unwrap() @@ -3080,18 +3069,10 @@ mod tests { assert_eq!(txs.len(), 2); } - #[test] - fn execute_without_commit_with_coinbase__relayed_tx_can_have_no_base_asset_and_mint_will_have_no_fees( - ) { - let genesis_da_height = 3u64; - let on_chain_db = database_with_genesis_block(genesis_da_height); + fn relayer_db_with_valid_relayed_txs(da_height: u64) -> Database { let mut relayer_db = Database::::default(); - // given - let block_height = 1u32; - let da_height = 10u64; let mut relayed_tx = RelayedTransaction::default(); - let not_base_asset = AssetId::new([5; 32]); - let tx = script_tx_for_amount(100, not_base_asset); + let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); add_events_to_relayer( @@ -3099,11 +3080,25 @@ mod tests { da_height.into(), &[relayed_tx.clone().into()], ); - let producer = create_relayer_executor(on_chain_db, relayer_db); - let block = test_block(block_height.into(), da_height.into(), 0); + relayer_db + } - // when + #[test] + fn execute_without_commit_with_coinbase__relayed_tx_can_have_no_base_asset_and_mint_will_have_no_fees( + ) { + let genesis_da_height = 3u64; + let block_height = 1u32; + let da_height = 10u64; let gas_price = 1; + + // given + let relayer_db = + relayer_db_with_valid_relayed_tx_with_no_base_assets(da_height); + + // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); let (result, _) = producer .execute_without_commit_with_coinbase( ExecutionTypes::Production(block.into()), @@ -3116,31 +3111,44 @@ mod tests { // then let txs = result.block.transactions(); assert_eq!(txs.len(), 2); + + // and let mint = txs[1].as_mint().unwrap(); assert_eq!(*mint.mint_amount(), 0); } + fn relayer_db_with_valid_relayed_tx_with_no_base_assets( + da_height: u64, + ) -> Database { + let mut relayer_db = Database::::default(); + let mut relayed_tx = RelayedTransaction::default(); + let not_base_asset = AssetId::new([5; 32]); + let tx = script_tx_for_amount(100, not_base_asset); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[relayed_tx.clone().into()], + ); + relayer_db + } + #[test] fn execute_without_commit__duplicated_relayed_tx_not_included_in_block() { let genesis_da_height = 3u64; - let on_chain_db = database_with_genesis_block(genesis_da_height); - let mut relayer_db = Database::::default(); - // given let block_height = 1u32; let da_height = 10u64; - let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount_of_base_asset(100); - let tx_bytes = tx.to_bytes(); - relayed_tx.set_serialized_transaction(tx_bytes); let duplicate_count = 10; - let events = std::iter::repeat(relayed_tx.clone().into()) - .take(duplicate_count + 1) - .collect::>(); - add_events_to_relayer(&mut relayer_db, da_height.into(), &events); - let producer = create_relayer_executor(on_chain_db, relayer_db); - let block = test_block(block_height.into(), da_height.into(), 0); + + // given + let relayer_db = + relayer_db_with_duplicate_valid_relayed_txs(da_height, duplicate_count); // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); let (result, _) = producer .execute_without_commit(ExecutionTypes::Production(block.into())) .unwrap() @@ -3149,33 +3157,42 @@ mod tests { // then let txs = result.block.transactions(); assert_eq!(txs.len(), 2); + + // and let skipped_txs = result.skipped_transactions; assert_eq!(skipped_txs.len(), duplicate_count); } + fn relayer_db_with_duplicate_valid_relayed_txs( + da_height: u64, + duplicate_count: usize, + ) -> Database { + let mut relayer_db = Database::::default(); + let mut relayed_tx = RelayedTransaction::default(); + let tx = script_tx_for_amount_of_base_asset(100); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + let events = std::iter::repeat(relayed_tx.clone().into()) + .take(duplicate_count + 1) + .collect::>(); + add_events_to_relayer(&mut relayer_db, da_height.into(), &events); + relayer_db + } + #[test] fn execute_without_commit__invalid_relayed_txs_are_not_included_and_are_reported() { let genesis_da_height = 3u64; - let on_chain_db = database_with_genesis_block(genesis_da_height); - let mut relayer_db = Database::::default(); - // given let block_height = 1u32; let da_height = 10u64; - let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount_of_base_asset(100); - tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) - let tx_bytes = tx.to_bytes(); - invalid_relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[invalid_relayed_tx.clone().into()], - ); - let producer = create_relayer_executor(on_chain_db, relayer_db); - let block = test_block(block_height.into(), da_height.into(), 0); + + // given + let relayer_db = relayer_db_with_invalid_relayed_txs(da_height); // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); let (result, _) = producer .execute_without_commit(ExecutionTypes::Production(block.into())) .unwrap() @@ -3184,6 +3201,8 @@ mod tests { // then let txs = result.block.transactions(); assert_eq!(txs.len(), 1); + + // and let events = result.events; let fuel_core_types::services::executor::Event::ForcedTransactionFailed { failure: actual, @@ -3196,6 +3215,21 @@ mod tests { assert_eq!(expected, actual); } + fn relayer_db_with_invalid_relayed_txs(da_height: u64) -> Database { + let mut relayer_db = Database::::default(); + let mut invalid_relayed_tx = RelayedTransaction::default(); + let mut tx = script_tx_for_amount_of_base_asset(100); + tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) + let tx_bytes = tx.to_bytes(); + invalid_relayed_tx.set_serialized_transaction(tx_bytes); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[invalid_relayed_tx.clone().into()], + ); + relayer_db + } + #[test] fn execute_without_commit__validation__includes_status_of_failed_relayed_tx() { let genesis_da_height = 3u64; From ae4ffce404955dfcea7bc0624a373c004d2dac4c Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 5 Apr 2024 18:41:45 -0700 Subject: [PATCH 21/56] More cleanup of executor tests --- crates/fuel-core/src/executor.rs | 176 +++++++++++++++++-------------- 1 file changed, 95 insertions(+), 81 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index b6f0a4e9f40..de2d08f9422 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3070,17 +3070,12 @@ mod tests { } fn relayer_db_with_valid_relayed_txs(da_height: u64) -> Database { - let mut relayer_db = Database::::default(); let mut relayed_tx = RelayedTransaction::default(); let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[relayed_tx.clone().into()], - ); - relayer_db + + relayer_db_for_events(&[relayed_tx.clone().into()], da_height) } #[test] @@ -3120,18 +3115,13 @@ mod tests { fn relayer_db_with_valid_relayed_tx_with_no_base_assets( da_height: u64, ) -> Database { - let mut relayer_db = Database::::default(); let mut relayed_tx = RelayedTransaction::default(); let not_base_asset = AssetId::new([5; 32]); let tx = script_tx_for_amount(100, not_base_asset); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[relayed_tx.clone().into()], - ); - relayer_db + + relayer_db_for_events(&[relayed_tx.clone().into()], da_height) } #[test] @@ -3167,7 +3157,6 @@ mod tests { da_height: u64, duplicate_count: usize, ) -> Database { - let mut relayer_db = Database::::default(); let mut relayed_tx = RelayedTransaction::default(); let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); @@ -3175,8 +3164,8 @@ mod tests { let events = std::iter::repeat(relayed_tx.clone().into()) .take(duplicate_count + 1) .collect::>(); - add_events_to_relayer(&mut relayer_db, da_height.into(), &events); - relayer_db + + relayer_db_for_events(&events, da_height) } #[test] @@ -3216,49 +3205,31 @@ mod tests { } fn relayer_db_with_invalid_relayed_txs(da_height: u64) -> Database { - let mut relayer_db = Database::::default(); - let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount_of_base_asset(100); - tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) - let tx_bytes = tx.to_bytes(); - invalid_relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[invalid_relayed_tx.clone().into()], - ); - relayer_db + let event = arb_invalid_relayed_tx_event(); + relayer_db_for_events(&[event], da_height) } #[test] fn execute_without_commit__validation__includes_status_of_failed_relayed_tx() { let genesis_da_height = 3u64; - let producer_db = database_with_genesis_block(genesis_da_height); - let verifyer_db = database_with_genesis_block(genesis_da_height); - let mut producer_relayer_db = Database::::default(); - let mut verifier_relayer_db = Database::::default(); - // given let block_height = 1u32; let da_height = 10u64; - let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount_of_base_asset(100); - tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) - let tx_bytes = tx.to_bytes(); - invalid_relayed_tx.set_serialized_transaction(tx_bytes); - let events = vec![invalid_relayed_tx.clone().into()]; - add_events_to_relayer(&mut producer_relayer_db, da_height.into(), &events); - add_events_to_relayer(&mut verifier_relayer_db, da_height.into(), &events); - let producer = create_relayer_executor(producer_db, producer_relayer_db); - let verifier = create_relayer_executor(verifyer_db, verifier_relayer_db); - let block = test_block(block_height.into(), da_height.into(), 0); - let (produced_result, _) = producer - .execute_without_commit(ExecutionTypes::Production(block.into())) - .unwrap() - .into(); - let produced_block = produced_result.block; + // given + let event = arb_invalid_relayed_tx_event(); + let produced_block = produce_block_with_relayed_event( + event.clone(), + genesis_da_height, + block_height, + da_height, + ); // when + let verifyer_db = database_with_genesis_block(genesis_da_height); + let mut verifier_relayer_db = Database::::default(); + let events = vec![event]; + add_events_to_relayer(&mut verifier_relayer_db, da_height.into(), &events); + let verifier = create_relayer_executor(verifyer_db, verifier_relayer_db); let (result, _) = verifier .execute_without_commit(ExecutionTypes::Validation(produced_block.into())) .unwrap() @@ -3267,6 +3238,8 @@ mod tests { // then let txs = result.block.transactions(); assert_eq!(txs.len(), 1); + + // and let events = result.events; let fuel_core_types::services::executor::Event::ForcedTransactionFailed { failure: actual, @@ -3279,17 +3252,79 @@ mod tests { assert_eq!(expected, actual); } + fn produce_block_with_relayed_event( + event: Event, + genesis_da_height: u64, + block_height: u32, + da_height: u64, + ) -> Block { + let producer_db = database_with_genesis_block(genesis_da_height); + let events = vec![event]; + let producer_relayer_db = relayer_db_for_events(&events, da_height); + + let producer = create_relayer_executor(producer_db, producer_relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + let (produced_result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + produced_result.block + } + + fn arb_invalid_relayed_tx_event() -> Event { + let mut invalid_relayed_tx = RelayedTransaction::default(); + let mut tx = script_tx_for_amount_of_base_asset(100); + tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) + let tx_bytes = tx.to_bytes(); + invalid_relayed_tx.set_serialized_transaction(tx_bytes); + invalid_relayed_tx.into() + } + #[test] fn execute_without_commit__relayed_mint_tx_not_included_in_block() { let genesis_da_height = 3u64; - let on_chain_db = database_with_genesis_block(genesis_da_height); - let mut relayer_db = Database::::default(); - // given let block_height = 1u32; let da_height = 10u64; + let tx_count = 0; + + // given + let relayer_db = + relayer_db_with_mint_relayed_tx(da_height, block_height, tx_count); + + // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = + test_block(block_height.into(), da_height.into(), tx_count as usize); + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + + // and + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[0] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Transaction type is not accepted"; + assert_eq!(expected, actual); + } + + fn relayer_db_with_mint_relayed_tx( + da_height: u64, + block_height: u32, + tx_count: u16, + ) -> Database { let mut relayed_tx = RelayedTransaction::default(); let base_asset_id = AssetId::BASE; - let tx_count = 0; let mint = Transaction::mint( TxPointer::new(block_height.into(), tx_count), contract::Contract { @@ -3311,34 +3346,13 @@ mod tests { let tx = Transaction::Mint(mint.into()); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - add_events_to_relayer( - &mut relayer_db, - da_height.into(), - &[relayed_tx.clone().into()], - ); - let producer = create_relayer_executor(on_chain_db, relayer_db); - let block = - test_block(block_height.into(), da_height.into(), tx_count as usize); - - // when - let (result, _) = producer - .execute_without_commit(ExecutionTypes::Production(block.into())) - .unwrap() - .into(); + relayer_db_for_events(&[relayed_tx.into()], da_height) + } - // then - let txs = result.block.transactions(); - assert_eq!(txs.len(), 1); - let events = result.events; - let fuel_core_types::services::executor::Event::ForcedTransactionFailed { - failure: actual, - .. - } = &events[0] - else { - panic!("Expected `ForcedTransactionFailed` event") - }; - let expected = "Transaction type is not accepted"; - assert_eq!(expected, actual); + fn relayer_db_for_events(events: &[Event], da_height: u64) -> Database { + let mut relayer_db = Database::::default(); + add_events_to_relayer(&mut relayer_db, da_height.into(), events); + relayer_db } #[test] From b83e3373d15e71013c38ec8ccc6d813a4edd68f6 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 5 Apr 2024 18:51:50 -0700 Subject: [PATCH 22/56] Remove clones --- crates/fuel-core/src/executor.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index de2d08f9422..52056e64d3e 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3075,7 +3075,7 @@ mod tests { let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - relayer_db_for_events(&[relayed_tx.clone().into()], da_height) + relayer_db_for_events(&[relayed_tx.into()], da_height) } #[test] @@ -3121,7 +3121,7 @@ mod tests { let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - relayer_db_for_events(&[relayed_tx.clone().into()], da_height) + relayer_db_for_events(&[relayed_tx.into()], da_height) } #[test] @@ -3161,7 +3161,7 @@ mod tests { let tx = script_tx_for_amount_of_base_asset(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - let events = std::iter::repeat(relayed_tx.clone().into()) + let events = std::iter::repeat(relayed_tx.into()) .take(duplicate_count + 1) .collect::>(); @@ -3259,8 +3259,7 @@ mod tests { da_height: u64, ) -> Block { let producer_db = database_with_genesis_block(genesis_da_height); - let events = vec![event]; - let producer_relayer_db = relayer_db_for_events(&events, da_height); + let producer_relayer_db = relayer_db_for_events(&[event], da_height); let producer = create_relayer_executor(producer_db, producer_relayer_db); let block = test_block(block_height.into(), da_height.into(), 0); From 78a26c3995bee9aa8f5ba1cde30ef03cdbdf31da Mon Sep 17 00:00:00 2001 From: Turner Date: Sat, 6 Apr 2024 11:28:26 -0700 Subject: [PATCH 23/56] Cleanup tests --- .../src/graphql_api/worker_service/tests.rs | 62 +++++++++++-------- tests/tests/relayer.rs | 5 +- 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index 9f1618db1b7..f4b5c3298e9 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -36,15 +36,38 @@ impl ports::worker::TxPool for MockTxPool { async fn run__relayed_transaction_events_are_added_to_storage() { let tx_id: Bytes32 = [1; 32].into(); let block_height = 8.into(); - let block_time = Tai64::UNIX_EPOCH; + let block_time = Tai64(456); let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); + let database = Database::in_memory(); + let (_, receiver) = tokio::sync::watch::channel(State::Started); + + // given let event = Event::ForcedTransactionFailed { id: tx_id.into(), block_height, - block_time: Tai64::UNIX_EPOCH, + block_time, failure: failure.clone(), }; - let tx_pool = MockTxPool; + let block_importer = block_importer_for_event(event); + + // when + let mut task = + worker_task_with_block_importer_and_db(block_importer, database.clone()); + let mut state_watcher = receiver.into(); + task.run(&mut state_watcher).await.unwrap(); + + // then + let expected = RelayedTransactionStatus::Failed { + block_height, + block_time, + failure, + }; + let storage = database.storage_as_ref::(); + let actual = storage.get(&tx_id).unwrap().unwrap(); + assert_eq!(*actual, expected); +} + +fn block_importer_for_event(event: Event) -> BoxStream { let block = Arc::new(ImportResult { sealed_block: Default::default(), tx_status: vec![], @@ -52,30 +75,19 @@ async fn run__relayed_transaction_events_are_added_to_storage() { source: Default::default(), }); let blocks: Vec + Send + Sync>> = vec![block]; - let block_importer = tokio_stream::iter(blocks).into_boxed(); - let database = Database::in_memory(); + tokio_stream::iter(blocks).into_boxed() +} + +fn worker_task_with_block_importer_and_db( + block_importer: BoxStream, + database: D, +) -> Task { + let tx_pool = MockTxPool; let chain_id = Default::default(); - let mut task = Task { + Task { tx_pool, block_importer, - database: database.clone(), + database, chain_id, - }; - // given - - let expected = RelayedTransactionStatus::Failed { - block_height, - block_time, - failure, - }; - - // when - let (_sender, receiver) = tokio::sync::watch::channel(State::Started); - let mut state_watcher = receiver.into(); - task.run(&mut state_watcher).await.unwrap(); - - // then - let storage = database.storage_as_ref::(); - let actual = storage.get(&tx_id).unwrap().unwrap(); - assert_eq!(*actual, expected); + } } diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index b9c2ea8f626..bd21c70a254 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -270,6 +270,8 @@ async fn can_find_failed_relayed_tx() { let block_height: BlockHeight = 999.into(); let block_time = Tai64::UNIX_EPOCH; let failure = "lolz".to_string(); + + // given let status = FuelRelayedTransactionStatus::Failed { block_height: block_height.clone(), block_time: block_time.clone(), @@ -280,12 +282,13 @@ async fn can_find_failed_relayed_tx() { .insert(&id, &status) .unwrap(); + // when let srv = FuelService::from_combined_database(db.clone(), Config::local_node()) .await .unwrap(); - let client = FuelClient::from(srv.bound_address); + // then let expected = Some(ClientRelayedTransactionStatus::Failed { block_height, block_time, From 98c5a082ddc27f389372d97f9caddc8fc44abfa9 Mon Sep 17 00:00:00 2001 From: Turner Date: Sat, 6 Apr 2024 11:32:12 -0700 Subject: [PATCH 24/56] Appease Clippy-sama --- crates/fuel-core/src/executor.rs | 4 ++-- crates/fuel-core/src/graphql_api/worker_service/tests.rs | 1 - tests/tests/relayer.rs | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 52056e64d3e..551710b4780 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3231,7 +3231,7 @@ mod tests { add_events_to_relayer(&mut verifier_relayer_db, da_height.into(), &events); let verifier = create_relayer_executor(verifyer_db, verifier_relayer_db); let (result, _) = verifier - .execute_without_commit(ExecutionTypes::Validation(produced_block.into())) + .execute_without_commit(ExecutionTypes::Validation(produced_block)) .unwrap() .into(); @@ -3342,7 +3342,7 @@ mod tests { base_asset_id, 0, ); - let tx = Transaction::Mint(mint.into()); + let tx = Transaction::Mint(mint); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); relayer_db_for_events(&[relayed_tx.into()], da_height) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index f4b5c3298e9..58bbedcf64e 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -28,7 +28,6 @@ impl ports::worker::TxPool for MockTxPool { _status: TransactionStatus, ) { // Do nothing - () } } diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index bd21c70a254..36e7f1f40e3 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -273,8 +273,8 @@ async fn can_find_failed_relayed_tx() { // given let status = FuelRelayedTransactionStatus::Failed { - block_height: block_height.clone(), - block_time: block_time.clone(), + block_height, + block_time, failure: failure.clone(), }; db.off_chain_mut() From 18a78b38b88243093ff6aeae2118112f53873f71 Mon Sep 17 00:00:00 2001 From: Turner Date: Sat, 6 Apr 2024 11:56:29 -0700 Subject: [PATCH 25/56] Fix integ test --- crates/fuel-core/src/graphql_api/worker_service/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index 58bbedcf64e..12c5a9b3ea2 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -39,6 +39,7 @@ async fn run__relayed_transaction_events_are_added_to_storage() { let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); let database = Database::in_memory(); let (_, receiver) = tokio::sync::watch::channel(State::Started); + let mut state_watcher = receiver.into(); // given let event = Event::ForcedTransactionFailed { @@ -52,8 +53,8 @@ async fn run__relayed_transaction_events_are_added_to_storage() { // when let mut task = worker_task_with_block_importer_and_db(block_importer, database.clone()); - let mut state_watcher = receiver.into(); task.run(&mut state_watcher).await.unwrap(); + tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; // then let expected = RelayedTransactionStatus::Failed { From 43e7ef48972126e751dea0b9e332b0e402123862 Mon Sep 17 00:00:00 2001 From: Turner Date: Sat, 6 Apr 2024 12:35:11 -0700 Subject: [PATCH 26/56] WTF, why does this fix this? --- crates/fuel-core/src/graphql_api/worker_service/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index 12c5a9b3ea2..58efdab8759 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -36,9 +36,9 @@ async fn run__relayed_transaction_events_are_added_to_storage() { let tx_id: Bytes32 = [1; 32].into(); let block_height = 8.into(); let block_time = Tai64(456); - let failure = "peanut butter chocolate cake with Kool-Aid".to_string(); + let failure = "blah blah blah".to_string(); let database = Database::in_memory(); - let (_, receiver) = tokio::sync::watch::channel(State::Started); + let (_sender, receiver) = tokio::sync::watch::channel(State::Started); let mut state_watcher = receiver.into(); // given From 109271c4595b83fabdc82b6f05d2cc39d4af9321 Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 8 Apr 2024 11:28:28 -0700 Subject: [PATCH 27/56] Use `RelayedTransctionId` instead of `Bytes32` --- crates/client/assets/schema.sdl | 4 +++- crates/client/src/client/schema/primitives.rs | 1 + crates/client/src/client/schema/relayed_tx.rs | 4 ++-- crates/fuel-core/src/schema/relayed_tx.rs | 4 ++-- crates/fuel-core/src/schema/scalars.rs | 1 + 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 233895e373b..4acc9057e3e 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -850,7 +850,7 @@ type Query { messages(owner: Address, first: Int, after: String, last: Int, before: String): MessageConnection! messageProof(transactionId: TransactionId!, nonce: Nonce!, commitBlockId: BlockId, commitBlockHeight: U32): MessageProof messageStatus(nonce: Nonce!): MessageStatus! - status(id: Bytes32!): RelayedTransactionStatus + status(id: RelayedTransactionId!): RelayedTransactionStatus } type Receipt { @@ -909,6 +909,8 @@ type RelayedTransactionFailed { failure: String! } +scalar RelayedTransactionId + union RelayedTransactionStatus = RelayedTransactionFailed enum ReturnType { diff --git a/crates/client/src/client/schema/primitives.rs b/crates/client/src/client/schema/primitives.rs index db41e3ff0ba..c9a6911349e 100644 --- a/crates/client/src/client/schema/primitives.rs +++ b/crates/client/src/client/schema/primitives.rs @@ -112,6 +112,7 @@ fuel_type_scalar!(AssetId, AssetId); fuel_type_scalar!(ContractId, ContractId); fuel_type_scalar!(Salt, Salt); fuel_type_scalar!(TransactionId, Bytes32); +fuel_type_scalar!(RelayedTransactionId, Bytes32); fuel_type_scalar!(Signature, Bytes64); fuel_type_scalar!(Nonce, Nonce); diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index f719d06571f..ebe0bc731fe 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -1,6 +1,6 @@ use crate::client::schema::{ schema, - Bytes32, + RelayedTransactionId, Tai64Timestamp, U32, }; @@ -19,7 +19,7 @@ pub struct RelayedTransactionStatusQuery { #[derive(cynic::QueryVariables, Debug)] pub struct RelayedTransactionStatusArgs { /// Transaction id that contains the output message. - pub id: Bytes32, + pub id: RelayedTransactionId, } #[allow(clippy::enum_variant_names)] diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index a90926e9b5d..a379b1cc033 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -1,7 +1,7 @@ use crate::{ fuel_core_graphql_api::database::ReadView, schema::scalars::{ - Bytes32, + RelayedTransactionId, Tai64Timestamp, U32, }, @@ -25,7 +25,7 @@ impl RelayedTransactionQuery { async fn status( &self, ctx: &Context<'_>, - #[graphql(desc = "The id of the relayed tx")] id: Bytes32, + #[graphql(desc = "The id of the relayed tx")] id: RelayedTransactionId, ) -> async_graphql::Result> { let query: &ReadView = ctx.data_unchecked(); let status = diff --git a/crates/fuel-core/src/schema/scalars.rs b/crates/fuel-core/src/schema/scalars.rs index 2b95ee79622..4ead3d0b6e8 100644 --- a/crates/fuel-core/src/schema/scalars.rs +++ b/crates/fuel-core/src/schema/scalars.rs @@ -299,6 +299,7 @@ fuel_type_scalar!("AssetId", AssetId, AssetId, 32); fuel_type_scalar!("ContractId", ContractId, ContractId, 32); fuel_type_scalar!("Salt", Salt, Salt, 32); fuel_type_scalar!("TransactionId", TransactionId, Bytes32, 32); +fuel_type_scalar!("RelayedTransactionId", RelayedTransactionId, Bytes32, 32); fuel_type_scalar!("MessageId", MessageId, MessageId, 32); fuel_type_scalar!("Nonce", Nonce, Nonce, 32); fuel_type_scalar!("Signature", Signature, Bytes64, 64); From ab275523efa85b8d14c829948c4631d224f75b6a Mon Sep 17 00:00:00 2001 From: mitchell Date: Mon, 8 Apr 2024 11:34:01 -0700 Subject: [PATCH 28/56] Fix formatting for attributes macros --- crates/fuel-core/src/executor.rs | 72 ++++++++++++++++---------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 551710b4780..7b6596e4f8d 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2904,52 +2904,52 @@ mod tests { } #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(0), - } => matches Ok(()); "block producer takes all 10 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(0), + } => matches Ok(()); "block producer takes all 10 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 5, - genesis_da_height: Some(0), - } => matches Ok(()); "block producer takes first 5 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 5, + genesis_da_height: Some(0), + } => matches Ok(()); "block producer takes first 5 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(5), - } => matches Ok(()); "block producer takes last 5 messages from the relayer" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(5), + } => matches Ok(()); "block producer takes last 5 messages from the relayer" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: Some(u64::MAX), - } => matches Err(ExecutorError::DaHeightExceededItsLimit); "block producer fails when previous block exceeds `u64::MAX`" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: Some(u64::MAX), + } => matches Err(ExecutorError::DaHeightExceededItsLimit); "block producer fails when previous block exceeds `u64::MAX`" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 1, - block_da_height: 10, - genesis_da_height: None, - } => matches Err(ExecutorError::PreviousBlockIsNotFound); "block producer fails when previous block doesn't exist" + Input { + relayer_da_height: 10, + block_height: 1, + block_da_height: 10, + genesis_da_height: None, + } => matches Err(ExecutorError::PreviousBlockIsNotFound); "block producer fails when previous block doesn't exist" )] #[test_case::test_case( - Input { - relayer_da_height: 10, - block_height: 0, - block_da_height: 10, - genesis_da_height: Some(0), - } => matches Err(ExecutorError::ExecutingGenesisBlock); "block producer fails when block height is zero" + Input { + relayer_da_height: 10, + block_height: 0, + block_da_height: 10, + genesis_da_height: Some(0), + } => matches Err(ExecutorError::ExecutingGenesisBlock); "block producer fails when block height is zero" )] fn block_producer_takes_messages_from_the_relayer( input: Input, From 33354f183220d853ddef8cb0916b8447d21b1e61 Mon Sep 17 00:00:00 2001 From: Mitchell Turner Date: Mon, 8 Apr 2024 21:10:49 -0700 Subject: [PATCH 29/56] Update crates/client/src/client/types.rs Co-authored-by: Brandon Kite --- crates/client/src/client/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/client/src/client/types.rs b/crates/client/src/client/types.rs index e9543407268..73a5b9675ba 100644 --- a/crates/client/src/client/types.rs +++ b/crates/client/src/client/types.rs @@ -146,7 +146,7 @@ impl TryFrom for TransactionStatus { TransactionStatus::SqueezedOut { reason: s.reason } } SchemaTxStatus::Unknown => { - return Err(Self::Error::UnknownVariant("SchemaTxStatus")); + return Err(Self::Error::UnknownVariant("SchemaTxStatus")) } }) } From 1af20a0b1c0851a130d8e346cce9eaf1acae5bc1 Mon Sep 17 00:00:00 2001 From: Mitchell Turner Date: Mon, 8 Apr 2024 21:10:58 -0700 Subject: [PATCH 30/56] Update crates/client/src/client/schema/relayed_tx.rs Co-authored-by: Brandon Kite --- crates/client/src/client/schema/relayed_tx.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index ebe0bc731fe..c405d446de3 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -12,7 +12,7 @@ use crate::client::schema::{ variables = "RelayedTransactionStatusArgs" )] pub struct RelayedTransactionStatusQuery { - #[arguments(id: $ id)] + #[arguments(id: $id)] pub status: Option, } From c17faa8f97dba9f55eada215dd80e61961ab4bf0 Mon Sep 17 00:00:00 2001 From: Turner Date: Tue, 9 Apr 2024 11:00:15 -0700 Subject: [PATCH 31/56] Add log instead of comment for unknown executor events in worker service --- crates/fuel-core/src/graphql_api/worker_service.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index b26cfa97bb7..7603b56bdb6 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -189,7 +189,10 @@ where .insert(&Bytes32::from(id.to_owned()), &status)?; } _ => { - // unknown execution event (likely due to a runtime upgrade) + tracing::error!( + "Unknown executor event (possibly due to runtime upgrade): {:?}", + event.deref() + ); } } } From eb33a4fa14f489f5c644128d9c68f1fc6bcdd36e Mon Sep 17 00:00:00 2001 From: Mitchell Turner Date: Tue, 9 Apr 2024 15:51:00 -0700 Subject: [PATCH 32/56] Update crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs Co-authored-by: Brandon Kite --- crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs index d20ac6ec8f1..60c4aba5974 100644 --- a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs @@ -16,7 +16,6 @@ use fuel_core_storage::{ }; use fuel_core_types::{ entities::relayer::transaction::{ - // RelayedTransactionId, RelayedTransactionStatus, }, fuel_tx::Bytes32, From 752e17c829417c72504a52cec4152d0649293b1f Mon Sep 17 00:00:00 2001 From: Mitchell Turner Date: Tue, 9 Apr 2024 16:21:10 -0700 Subject: [PATCH 33/56] Update relayed_transactions.rs --- .../fuel-core/src/graphql_api/storage/relayed_transactions.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs index 60c4aba5974..7a378adab78 100644 --- a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs @@ -15,9 +15,7 @@ use fuel_core_storage::{ Mappable, }; use fuel_core_types::{ - entities::relayer::transaction::{ - RelayedTransactionStatus, - }, + entities::relayer::transaction::RelayedTransactionStatus, fuel_tx::Bytes32, }; From c7382df09cbe73a244ea940232ecbef14b7bf0b1 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 10 Apr 2024 09:28:07 -0700 Subject: [PATCH 34/56] Remove non-exhaustive for events type, move variable, remove comment --- crates/fuel-core/src/graphql_api/worker_service.rs | 6 ------ crates/services/executor/src/executor.rs | 5 +---- crates/types/src/services/executor.rs | 1 - 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index 7603b56bdb6..879c5ac2d31 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -188,12 +188,6 @@ where .storage_as_mut::() .insert(&Bytes32::from(id.to_owned()), &status)?; } - _ => { - tracing::error!( - "Unknown executor event (possibly due to runtime upgrade): {:?}", - event.deref() - ); - } } } Ok(()) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 4370ab07d13..ba14f1b64a0 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -496,10 +496,6 @@ where .with_policy(ConflictPolicy::Overwrite); debug_assert!(block.transactions.is_empty()); - // initiate transaction stream with relayed (forced) transactions first, - // and pull the rest from the TxSource (txpool) if there is remaining blockspace available. - // We use `block.transactions` to store executed transactions. - let relayed_tx_iter = forced_transactions.into_iter().peekable(); let mut execute_transaction = |execution_data: &mut ExecutionData, tx: MaybeCheckedTransaction, @@ -570,6 +566,7 @@ where Ok(()) }; + let relayed_tx_iter = forced_transactions.into_iter(); for transaction in relayed_tx_iter { const RELAYED_GAS_PRICE: Word = 0; execute_transaction(&mut *execution_data, transaction, RELAYED_GAS_PRICE)?; diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 37a9f31ad93..85d0929042e 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -60,7 +60,6 @@ pub struct ExecutionResult { /// The event represents some internal state changes caused by the block execution. #[derive(Debug, Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[non_exhaustive] pub enum Event { /// Imported a new spendable message from the relayer. MessageImported(Message), From 8ca67e268f44b7e47335bcb2df5d589f553663d1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 11 Apr 2024 10:51:12 -0700 Subject: [PATCH 35/56] Remove timestamp from failure type --- crates/client/assets/schema.sdl | 1 - crates/client/src/client/schema/relayed_tx.rs | 2 -- crates/client/src/client/types.rs | 2 -- crates/fuel-core/src/graphql_api/worker_service.rs | 2 -- crates/fuel-core/src/schema/relayed_tx.rs | 9 --------- crates/services/executor/src/executor.rs | 2 -- crates/types/src/entities/relayer/transaction.rs | 3 --- crates/types/src/services/executor.rs | 3 --- 8 files changed, 24 deletions(-) diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 4acc9057e3e..76f52040214 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -905,7 +905,6 @@ enum ReceiptType { type RelayedTransactionFailed { blockHeight: U32! - blockTime: Tai64Timestamp! failure: String! } diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index c405d446de3..ac8f7382fb7 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -1,7 +1,6 @@ use crate::client::schema::{ schema, RelayedTransactionId, - Tai64Timestamp, U32, }; @@ -36,6 +35,5 @@ pub enum RelayedTransactionStatus { #[cynic(schema_path = "./assets/schema.sdl")] pub struct RelayedTransactionFailed { pub block_height: U32, - pub block_time: Tai64Timestamp, pub failure: String, } diff --git a/crates/client/src/client/types.rs b/crates/client/src/client/types.rs index 73a5b9675ba..6b7af6e2bc8 100644 --- a/crates/client/src/client/types.rs +++ b/crates/client/src/client/types.rs @@ -175,7 +175,6 @@ impl TryFrom for TransactionResponse { pub enum RelayedTransactionStatus { Failed { block_height: BlockHeight, - block_time: Tai64, failure: String, }, } @@ -188,7 +187,6 @@ impl TryFrom for RelayedTransactionStatus { SchemaRelayedTransactionStatus::Failed(s) => { RelayedTransactionStatus::Failed { block_height: s.block_height.into(), - block_time: s.block_time.0, failure: s.failure, } } diff --git a/crates/fuel-core/src/graphql_api/worker_service.rs b/crates/fuel-core/src/graphql_api/worker_service.rs index 879c5ac2d31..f743d4fce91 100644 --- a/crates/fuel-core/src/graphql_api/worker_service.rs +++ b/crates/fuel-core/src/graphql_api/worker_service.rs @@ -175,12 +175,10 @@ where Event::ForcedTransactionFailed { id, block_height, - block_time, failure, } => { let status = RelayedTransactionStatus::Failed { block_height: *block_height, - block_time: *block_time, failure: failure.clone(), }; diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index a379b1cc033..4a6cec4712f 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -2,7 +2,6 @@ use crate::{ fuel_core_graphql_api::database::ReadView, schema::scalars::{ RelayedTransactionId, - Tai64Timestamp, U32, }, }; @@ -14,7 +13,6 @@ use async_graphql::{ use fuel_core_types::{ entities::relayer::transaction::RelayedTransactionStatus as FuelRelayedTransactionStatus, fuel_types::BlockHeight, - tai64::Tai64, }; #[derive(Default)] @@ -42,7 +40,6 @@ pub enum RelayedTransactionStatus { #[derive(Debug)] pub struct RelayedTransactionFailed { pub block_height: BlockHeight, - pub block_time: Tai64, pub failure: String, } @@ -53,10 +50,6 @@ impl RelayedTransactionFailed { as_u32.into() } - async fn block_time(&self) -> Tai64Timestamp { - self.block_time.into() - } - async fn failure(&self) -> String { self.failure.clone() } @@ -67,11 +60,9 @@ impl From for RelayedTransactionStatus { match status { FuelRelayedTransactionStatus::Failed { block_height, - block_time, failure, } => RelayedTransactionStatus::Failed(RelayedTransactionFailed { block_height, - block_time, failure, }), } diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index ba14f1b64a0..a0e229d5617 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -526,7 +526,6 @@ where ExecutorEvent::ForcedTransactionFailed { id, block_height, - block_time: *block.header.time(), failure: ForcedTransactionFailure::ExecutionError(*err) .to_string(), }, @@ -698,7 +697,6 @@ where ExecutorEvent::ForcedTransactionFailed { id, block_height, - block_time: header.consensus.time, failure: err.to_string(), }, ); diff --git a/crates/types/src/entities/relayer/transaction.rs b/crates/types/src/entities/relayer/transaction.rs index fb7f3723ba7..8a924132d2b 100644 --- a/crates/types/src/entities/relayer/transaction.rs +++ b/crates/types/src/entities/relayer/transaction.rs @@ -9,7 +9,6 @@ use crate::{ Nonce, }, }; -use tai64::Tai64; /// Transaction sent from the DA layer to fuel by the relayer #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -158,8 +157,6 @@ pub enum RelayedTransactionStatus { Failed { /// The height of the block that processed this transaction block_height: BlockHeight, - /// The time of the block that processed this transaction - block_time: Tai64, /// The actual failure reason for why the forced transaction was not included failure: String, }, diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 85d0929042e..b9894383818 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -34,7 +34,6 @@ use crate::{ }, services::Uncommitted, }; -use tai64::Tai64; /// The alias for executor result. pub type Result = core::result::Result; @@ -75,8 +74,6 @@ pub enum Event { id: RelayedTransactionId, /// The height of the block that processed this transaction block_height: BlockHeight, - /// The time of the block that processed this transaction - block_time: Tai64, /// The actual failure reason for why the forced transaction was not included failure: String, }, From e384ce552970f8ac3f8077e507e570d20cb0d647 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 11 Apr 2024 11:27:06 -0700 Subject: [PATCH 36/56] Remove relayed tx variant of `MaybeCheckedTransaction` and clean up event emitting logic --- .../storage/relayed_transactions.rs | 6 +-- .../src/graphql_api/worker_service/tests.rs | 4 -- crates/services/executor/src/executor.rs | 37 +++---------------- crates/services/executor/src/ports.rs | 14 ------- crates/types/src/services/executor.rs | 2 - 5 files changed, 6 insertions(+), 57 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs index 7a378adab78..6ba2b61f8c0 100644 --- a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs @@ -53,17 +53,13 @@ impl AddTable for StateConfigBuilder { #[cfg(test)] mod tests { use super::*; - use fuel_core_types::{ - fuel_tx::Bytes32, - tai64::Tai64, - }; + use fuel_core_types::fuel_tx::Bytes32; fuel_core_storage::basic_storage_tests!( RelayedTransactionStatuses, ::Key::from(Bytes32::default()), RelayedTransactionStatus::Failed { block_height: 0.into(), - block_time: Tai64::UNIX_EPOCH, failure: "Some reason".to_string(), } ); diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index 58efdab8759..c1876b3cb9f 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -14,7 +14,6 @@ use fuel_core_types::{ fuel_tx::Bytes32, fuel_types::BlockHeight, services::txpool::TransactionStatus, - tai64::Tai64, }; use std::sync::Arc; @@ -35,7 +34,6 @@ impl ports::worker::TxPool for MockTxPool { async fn run__relayed_transaction_events_are_added_to_storage() { let tx_id: Bytes32 = [1; 32].into(); let block_height = 8.into(); - let block_time = Tai64(456); let failure = "blah blah blah".to_string(); let database = Database::in_memory(); let (_sender, receiver) = tokio::sync::watch::channel(State::Started); @@ -45,7 +43,6 @@ async fn run__relayed_transaction_events_are_added_to_storage() { let event = Event::ForcedTransactionFailed { id: tx_id.into(), block_height, - block_time, failure: failure.clone(), }; let block_importer = block_importer_for_event(event); @@ -59,7 +56,6 @@ async fn run__relayed_transaction_events_are_added_to_storage() { // then let expected = RelayedTransactionStatus::Failed { block_height, - block_time, failure, }; let storage = database.storage_as_ref::(); diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index a0e229d5617..c0d61291907 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -48,7 +48,6 @@ use fuel_core_types::{ CompressedCoinV1, }, contract::ContractUtxoInfo, - relayer::transaction::RelayedTransactionId, RelayedTransaction, }, fuel_asm::{ @@ -519,19 +518,6 @@ where ); let tx = match result { - Err(ExecutorError::RelayedTransactionFailed(id, err)) => { - // if it was a relayed tx that failed, we need to record the reason - // and all nodes should process this event the same way - execution_data.events.push( - ExecutorEvent::ForcedTransactionFailed { - id, - block_height, - failure: ForcedTransactionFailure::ExecutionError(*err) - .to_string(), - }, - ); - return Ok(()); - } Err(err) => { return match execution_kind { ExecutionKind::Production => { @@ -568,6 +554,7 @@ where let relayed_tx_iter = forced_transactions.into_iter(); for transaction in relayed_tx_iter { const RELAYED_GAS_PRICE: Word = 0; + let transaction = MaybeCheckedTransaction::CheckedTransaction(transaction); execute_transaction(&mut *execution_data, transaction, RELAYED_GAS_PRICE)?; } @@ -639,7 +626,7 @@ where &mut self, header: &PartialBlockHeader, execution_data: &mut ExecutionData, - ) -> ExecutorResult> { + ) -> ExecutorResult> { let block_height = *header.height(); let prev_block_height = block_height .pred() @@ -717,7 +704,7 @@ where relayed_tx: RelayedTransaction, header: &PartialBlockHeader, consensus_params: &ConsensusParameters, - ) -> Result { + ) -> Result { let parsed_tx = Transaction::from_bytes(relayed_tx.serialized_transaction()) .map_err(|_| ForcedTransactionFailure::CodecError)?; @@ -740,10 +727,7 @@ where } } - Ok(MaybeCheckedTransaction::RelayedCheckedTransaction( - relayed_tx.id(), - CheckedTransaction::from(checked_tx), - )) + Ok(CheckedTransaction::from(checked_tx)) } #[allow(clippy::too_many_arguments)] @@ -774,19 +758,14 @@ where } let block_height = *header.height(); - let mut relayed_tx: Option = None; 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::RelayedCheckedTransaction(id, checked_tx) => { - relayed_tx = Some(id); - checked_tx - } }; - let mut res = match checked_tx { + let res = match checked_tx { CheckedTransaction::Script(script) => self.execute_create_or_script( script, header, @@ -816,12 +795,6 @@ where ), }; - // if it's a relayed tx, wrap the error for proper handling - if let Some(id) = &relayed_tx { - res = res.map_err(|err| { - ExecutorError::RelayedTransactionFailed(id.clone(), Box::new(err)) - }); - } res } diff --git a/crates/services/executor/src/ports.rs b/crates/services/executor/src/ports.rs index 49edfe44efb..40304a4d375 100644 --- a/crates/services/executor/src/ports.rs +++ b/crates/services/executor/src/ports.rs @@ -1,6 +1,5 @@ use fuel_core_types::{ blockchain::primitives::DaBlockHeight, - entities::relayer::transaction::RelayedTransactionId, fuel_tx, fuel_tx::{ TxId, @@ -15,7 +14,6 @@ use fuel_core_types::{ pub enum MaybeCheckedTransaction { CheckedTransaction(CheckedTransaction), Transaction(fuel_tx::Transaction), - RelayedCheckedTransaction(RelayedTransactionId, CheckedTransaction), } impl MaybeCheckedTransaction { @@ -31,18 +29,6 @@ impl MaybeCheckedTransaction { tx.id() } MaybeCheckedTransaction::Transaction(tx) => tx.id(chain_id), - MaybeCheckedTransaction::RelayedCheckedTransaction( - _, - CheckedTransaction::Script(tx), - ) => tx.id(), - MaybeCheckedTransaction::RelayedCheckedTransaction( - _, - CheckedTransaction::Create(tx), - ) => tx.id(), - MaybeCheckedTransaction::RelayedCheckedTransaction( - _, - CheckedTransaction::Mint(tx), - ) => tx.id(), } } } diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index b9894383818..c4f0397bff3 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -390,8 +390,6 @@ pub enum Error { RelayerGivesIncorrectMessages, #[display(fmt = "Consensus parameters not found for version {_0}")] ConsensusParametersNotFound(ConsensusParametersVersion), - #[display(fmt = "Failed to execute relayed transaction {_0}")] - RelayedTransactionFailed(RelayedTransactionId, Box), /// It is possible to occur untyped errors in the case of the upgrade. #[display(fmt = "Occurred untyped error: {_0}")] Other(String), From 8420a6ca655242b60f019dc870bb99b3306f67b7 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 11 Apr 2024 11:41:29 -0700 Subject: [PATCH 37/56] Use test helper state watcher --- crates/fuel-core/src/graphql_api/worker_service/tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index c1876b3cb9f..a83a1304a7f 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -36,8 +36,7 @@ async fn run__relayed_transaction_events_are_added_to_storage() { let block_height = 8.into(); let failure = "blah blah blah".to_string(); let database = Database::in_memory(); - let (_sender, receiver) = tokio::sync::watch::channel(State::Started); - let mut state_watcher = receiver.into(); + let state_watcher = StateWatcher::started(); // given let event = Event::ForcedTransactionFailed { From 135c9af441436d5390a17fd067cf5630822c202d Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 11 Apr 2024 12:54:47 -0700 Subject: [PATCH 38/56] Include test for relayed txs that pass checks but fail execution, fix code for test --- crates/fuel-core/src/executor.rs | 56 +++++++++++++++++++ .../storage/relayed_transactions.rs | 3 + .../src/graphql_api/worker_service/tests.rs | 7 +-- crates/services/executor/src/executor.rs | 37 ++++++++++-- crates/types/src/services/executor.rs | 4 +- 5 files changed, 95 insertions(+), 12 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 7b6596e4f8d..f10603609ca 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3209,6 +3209,62 @@ mod tests { relayer_db_for_events(&[event], da_height) } + #[test] + fn execute_without_commit__relayed_tx_that_passes_checks_but_fails_execution_is_reported( + ) { + let genesis_da_height = 3u64; + let block_height = 1u32; + let da_height = 10u64; + + // given + let relayer_db = + relayer_db_with_tx_that_passes_checks_but_fails_exectution(da_height); + + // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 2); + + // and + let skipped_txs = result.skipped_transactions; + assert_eq!(skipped_txs.len(), 1); + + // and + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[3] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Transaction id was already used: "; + assert!(actual.starts_with(expected)); + } + + fn relayer_db_with_tx_that_passes_checks_but_fails_exectution( + da_height: u64, + ) -> Database { + let mut relayed_tx = RelayedTransaction::default(); + let tx = script_tx_for_amount_of_base_asset(100); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + let mut bad_relayed_tx = relayed_tx.clone(); + let new_nonce = [9; 32].into(); + bad_relayed_tx.set_nonce(new_nonce); + // let mut bad_tx = tx.clone(); + // bad_relayed_tx.set_serialized_transaction(bad_tx_bytes); + relayer_db_for_events(&[relayed_tx.into(), bad_relayed_tx.into()], da_height) + } + #[test] fn execute_without_commit__validation__includes_status_of_failed_relayed_tx() { let genesis_da_height = 3u64; diff --git a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs index 6ba2b61f8c0..c704714313d 100644 --- a/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs +++ b/crates/fuel-core/src/graphql_api/storage/relayed_transactions.rs @@ -19,6 +19,9 @@ use fuel_core_types::{ fuel_tx::Bytes32, }; +/// Tracks the status of transactions from the L1. These are tracked separately from tx-pool +/// transactions because they might fail as part of the relay process, not just due +/// to execution. pub struct RelayedTransactionStatuses; impl Mappable for RelayedTransactionStatuses { diff --git a/crates/fuel-core/src/graphql_api/worker_service/tests.rs b/crates/fuel-core/src/graphql_api/worker_service/tests.rs index a83a1304a7f..e0b9821011b 100644 --- a/crates/fuel-core/src/graphql_api/worker_service/tests.rs +++ b/crates/fuel-core/src/graphql_api/worker_service/tests.rs @@ -5,10 +5,7 @@ use crate::{ database::Database, graphql_api::storage::relayed_transactions::RelayedTransactionStatuses, }; -use fuel_core_services::{ - stream::IntoBoxStream, - State, -}; +use fuel_core_services::stream::IntoBoxStream; use fuel_core_storage::StorageAsRef; use fuel_core_types::{ fuel_tx::Bytes32, @@ -36,7 +33,7 @@ async fn run__relayed_transaction_events_are_added_to_storage() { let block_height = 8.into(); let failure = "blah blah blah".to_string(); let database = Database::in_memory(); - let state_watcher = StateWatcher::started(); + let mut state_watcher = StateWatcher::started(); // given let event = Event::ForcedTransactionFailed { diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index c0d61291907..2c5047ea166 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -499,7 +499,7 @@ where let mut execute_transaction = |execution_data: &mut ExecutionData, tx: MaybeCheckedTransaction, gas_price: Word| - -> ExecutorResult<()> { + -> ExecutorResult> { let tx_count = execution_data.tx_count; let tx = { let mut tx_st_transaction = thread_block_transaction @@ -528,8 +528,10 @@ where // (or in another place that should validate it). Or we forgot to // clean up some dependent/conflict transactions. But it definitely // means that something went wrong, and we must fix it. - execution_data.skipped_transactions.push((tx_id, err)); - Ok(()) + execution_data + .skipped_transactions + .push((tx_id, err.clone())); + Ok(Some(err)) } ExecutionKind::DryRun | ExecutionKind::Validation => Err(err), }; @@ -548,14 +550,39 @@ where .checked_add(1) .ok_or(ExecutorError::TooManyTransactions)?; - Ok(()) + Ok(None) }; let relayed_tx_iter = forced_transactions.into_iter(); for transaction in relayed_tx_iter { const RELAYED_GAS_PRICE: Word = 0; let transaction = MaybeCheckedTransaction::CheckedTransaction(transaction); - execute_transaction(&mut *execution_data, transaction, RELAYED_GAS_PRICE)?; + let tx_id = transaction.id(&self.consensus_params.chain_id()); + match execute_transaction( + &mut *execution_data, + transaction, + RELAYED_GAS_PRICE, + ) { + Ok(None) => {} + Ok(Some(err)) => { + let id_bytes: Bytes32 = tx_id.into(); + let event = ExecutorEvent::ForcedTransactionFailed { + id: id_bytes.into(), + block_height, + failure: err.to_string(), + }; + execution_data.events.push(event); + } + Err(err) => { + let id_bytes: Bytes32 = tx_id.into(); + let event = ExecutorEvent::ForcedTransactionFailed { + id: id_bytes.into(), + block_height, + failure: err.to_string(), + }; + execution_data.events.push(event); + } + } } let remaining_gas_limit = block_gas_limit.saturating_sub(execution_data.used_gas); diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index c4f0397bff3..dc9be8d2a42 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -324,7 +324,7 @@ impl ExecutionKind { } #[allow(missing_docs)] -#[derive(Debug, PartialEq, derive_more::Display, derive_more::From)] +#[derive(Debug, Clone, PartialEq, derive_more::Display, derive_more::From)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[non_exhaustive] pub enum Error { @@ -414,7 +414,7 @@ impl From for Error { } #[allow(missing_docs)] -#[derive(thiserror::Error, Debug, PartialEq)] +#[derive(thiserror::Error, Debug, Clone, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[non_exhaustive] pub enum TransactionValidityError { From d7663838c4168c7b18148aab5d95c09d078c2e33 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 15:48:31 -0700 Subject: [PATCH 39/56] Add low max gas test, fix code to pass test --- crates/fuel-core/src/executor.rs | 113 ++++++++++++++++------- crates/services/executor/src/executor.rs | 21 ++++- crates/types/src/services/executor.rs | 10 ++ 3 files changed, 108 insertions(+), 36 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index f10603609ca..a2d65d7a050 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -284,9 +284,7 @@ mod tests { da_block_height: DaBlockHeight, num_txs: usize, ) -> Block { - let transactions = (1..num_txs + 1) - .map(script_tx_for_amount_of_base_asset) - .collect_vec(); + let transactions = (1..num_txs + 1).map(script_tx_for_amount).collect_vec(); let mut block = Block::default(); block.header_mut().set_block_height(block_height); @@ -295,12 +293,8 @@ mod tests { block } - fn script_tx_for_amount_of_base_asset(amount: usize) -> Transaction { + fn script_tx_for_amount(amount: usize) -> Transaction { let asset = AssetId::BASE; - script_tx_for_amount(amount, asset) - } - - fn script_tx_for_amount(amount: usize, asset: AssetId) -> Transaction { TxBuilder::new(2322u64) .script_gas_limit(10) .coin_input(asset, (amount as Word) * 100) @@ -3051,9 +3045,11 @@ mod tests { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; + let arb_large_max_gas = 10_000; // given - let relayer_db = relayer_db_with_valid_relayed_txs(da_height); + let relayer_db = + relayer_db_with_valid_relayed_txs(da_height, arb_large_max_gas); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3069,26 +3065,30 @@ mod tests { assert_eq!(txs.len(), 2); } - fn relayer_db_with_valid_relayed_txs(da_height: u64) -> Database { + fn relayer_db_with_valid_relayed_txs( + da_height: u64, + max_gas: u64, + ) -> Database { let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount_of_base_asset(100); + let tx = script_tx_for_amount(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); + relayed_tx.set_max_gas(max_gas); relayer_db_for_events(&[relayed_tx.into()], da_height) } #[test] - fn execute_without_commit_with_coinbase__relayed_tx_can_have_no_base_asset_and_mint_will_have_no_fees( + fn execute_without_commit_with_coinbase__relayed_tx_execute_and_mint_will_have_no_fees( ) { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; let gas_price = 1; + let arb_max_gas = 10_000; // given - let relayer_db = - relayer_db_with_valid_relayed_tx_with_no_base_assets(da_height); + let relayer_db = relayer_db_with_valid_relayed_txs(da_height, arb_max_gas); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3112,28 +3112,20 @@ mod tests { assert_eq!(*mint.mint_amount(), 0); } - fn relayer_db_with_valid_relayed_tx_with_no_base_assets( - da_height: u64, - ) -> Database { - let mut relayed_tx = RelayedTransaction::default(); - let not_base_asset = AssetId::new([5; 32]); - let tx = script_tx_for_amount(100, not_base_asset); - let tx_bytes = tx.to_bytes(); - relayed_tx.set_serialized_transaction(tx_bytes); - - relayer_db_for_events(&[relayed_tx.into()], da_height) - } - #[test] fn execute_without_commit__duplicated_relayed_tx_not_included_in_block() { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; let duplicate_count = 10; + let arb_large_max_gas = 10_000; // given - let relayer_db = - relayer_db_with_duplicate_valid_relayed_txs(da_height, duplicate_count); + let relayer_db = relayer_db_with_duplicate_valid_relayed_txs( + da_height, + duplicate_count, + arb_large_max_gas, + ); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3156,11 +3148,13 @@ mod tests { fn relayer_db_with_duplicate_valid_relayed_txs( da_height: u64, duplicate_count: usize, + max_gas: u64, ) -> Database { let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount_of_base_asset(100); + let tx = script_tx_for_amount(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); + relayed_tx.set_max_gas(max_gas); let events = std::iter::repeat(relayed_tx.into()) .take(duplicate_count + 1) .collect::>(); @@ -3209,16 +3203,65 @@ mod tests { relayer_db_for_events(&[event], da_height) } + #[test] + fn execute_without_commit__relayed_tx_with_low_max_gas_fails() { + let genesis_da_height = 3u64; + let block_height = 1u32; + let da_height = 10u64; + + // given + let relayer_db = relayer_db_with_relayed_tx_with_low_max_gas(da_height); + + // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + let (result, _) = producer + .execute_without_commit(ExecutionTypes::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 1); + + // and + let events = result.events; + let fuel_core_types::services::executor::Event::ForcedTransactionFailed { + failure: actual, + .. + } = &events[0] + else { + panic!("Expected `ForcedTransactionFailed` event") + }; + let expected = "Insufficient max gas."; + assert!(actual.starts_with(expected)); + } + + fn relayer_db_with_relayed_tx_with_low_max_gas( + da_height: u64, + ) -> Database { + let mut relayed_tx = RelayedTransaction::default(); + let tx = script_tx_for_amount(100); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + relayed_tx.set_max_gas(0); + relayer_db_for_events(&[relayed_tx.into()], da_height) + } + #[test] fn execute_without_commit__relayed_tx_that_passes_checks_but_fails_execution_is_reported( ) { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; + let arb_max_gas = 10_000; // given - let relayer_db = - relayer_db_with_tx_that_passes_checks_but_fails_exectution(da_height); + let relayer_db = relayer_db_with_tx_that_passes_checks_but_fails_execution( + da_height, + arb_max_gas, + ); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3250,13 +3293,15 @@ mod tests { assert!(actual.starts_with(expected)); } - fn relayer_db_with_tx_that_passes_checks_but_fails_exectution( + fn relayer_db_with_tx_that_passes_checks_but_fails_execution( da_height: u64, + max_gas: u64, ) -> Database { let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount_of_base_asset(100); + let tx = script_tx_for_amount(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); + relayed_tx.set_max_gas(max_gas); let mut bad_relayed_tx = relayed_tx.clone(); let new_nonce = [9; 32].into(); bad_relayed_tx.set_nonce(new_nonce); @@ -3328,7 +3373,7 @@ mod tests { fn arb_invalid_relayed_tx_event() -> Event { let mut invalid_relayed_tx = RelayedTransaction::default(); - let mut tx = script_tx_for_amount_of_base_asset(100); + let mut tx = script_tx_for_amount(100); tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) let tx_bytes = tx.to_bytes(); invalid_relayed_tx.set_serialized_transaction(tx_bytes); diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 2c5047ea166..1e2ba1fd7ae 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -107,12 +107,13 @@ use fuel_core_types::{ CheckPredicateParams, CheckPredicates, Checked, + CheckedMetadata, CheckedTransaction, Checks, IntoChecked, }, interpreter::{ - CheckedMetadata, + CheckedMetadata as CheckedMetadataTrait, ExecutableTransaction, InterpreterParams, }, @@ -739,6 +740,22 @@ where .into_checked(header.consensus.height, consensus_params) .map_err(ForcedTransactionFailure::CheckError)?; + let claimed_max_gas = relayed_tx.max_gas(); + let actual_max_gas = match check_tx_res.metadata() { + CheckedMetadata::Mint(_) => { + return Err(ForcedTransactionFailure::InvalidTransactionType); + } + + CheckedMetadata::Script(script_metadata) => script_metadata.max_gas, + CheckedMetadata::Create(create_metadata) => create_metadata.max_gas, + }; + if actual_max_gas > claimed_max_gas { + return Err(ForcedTransactionFailure::InsufficientMaxGas { + claimed_max_gas, + actual_max_gas, + }); + } + let checked_tx = check_tx_res .check_predicates(&CheckPredicateParams::from(consensus_params)) .map_err(ForcedTransactionFailure::CheckError)?; @@ -994,7 +1011,7 @@ where ) -> ExecutorResult where Tx: ExecutableTransaction + PartialEq + Cacheable + Send + Sync + 'static, - ::Metadata: CheckedMetadata + Clone + Send + Sync, + ::Metadata: CheckedMetadataTrait + Clone + Send + Sync, T: KeyValueInspect, { let tx_id = checked_tx.id(); diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index dc9be8d2a42..c882aa35952 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -96,6 +96,16 @@ pub enum ForcedTransactionFailure { /// Execution error which failed to include #[display(fmt = "Transaction inclusion failed {_0}")] ExecutionError(Error), + /// Relayed Transaction didn't specify high enough max gas + #[display( + fmt = "Insufficient max gas. Expected: {claimed_max_gas:?}, Actual: {actual_max_gas:?}" + )] + InsufficientMaxGas { + /// The max gas claimed by the L1 transaction submitter + claimed_max_gas: u64, + /// The actual max gas used by the transaction + actual_max_gas: u64, + }, } /// The status of a transaction after it is executed. From c63a7a4fb08a102e0a38e4030d78be7d962740b2 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 11 Apr 2024 15:52:20 -0700 Subject: [PATCH 40/56] Remove skips for relayed txs and only skip regular txs --- crates/fuel-core/src/executor.rs | 14 +++++++------ crates/services/executor/src/executor.rs | 26 ++++++++---------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index a2d65d7a050..436c89a2c52 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3141,8 +3141,14 @@ mod tests { assert_eq!(txs.len(), 2); // and - let skipped_txs = result.skipped_transactions; - assert_eq!(skipped_txs.len(), duplicate_count); + let events = result.events; + let count = events + .into_iter() + .filter(|event| { + matches!(event, ExecutorEvent::ForcedTransactionFailed { .. }) + }) + .count(); + assert_eq!(count, 10); } fn relayer_db_with_duplicate_valid_relayed_txs( @@ -3276,10 +3282,6 @@ mod tests { let txs = result.block.transactions(); assert_eq!(txs.len(), 2); - // and - let skipped_txs = result.skipped_transactions; - assert_eq!(skipped_txs.len(), 1); - // and let events = result.events; let fuel_core_types::services::executor::Event::ForcedTransactionFailed { diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 1e2ba1fd7ae..e65f73ae13b 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -520,22 +520,7 @@ where let tx = match result { Err(err) => { - return match execution_kind { - ExecutionKind::Production => { - // If, during block production, we get an invalid transaction, - // remove it from the block and continue block creation. An invalid - // transaction means that the caller didn't validate it first, so - // maybe something is wrong with validation rules in the `TxPool` - // (or in another place that should validate it). Or we forgot to - // clean up some dependent/conflict transactions. But it definitely - // means that something went wrong, and we must fix it. - execution_data - .skipped_transactions - .push((tx_id, err.clone())); - Ok(Some(err)) - } - ExecutionKind::DryRun | ExecutionKind::Validation => Err(err), - }; + return Err(err); } Ok(tx) => tx, }; @@ -593,7 +578,14 @@ where let mut regular_tx_iter = source.next(remaining_gas_limit).into_iter().peekable(); while regular_tx_iter.peek().is_some() { for transaction in regular_tx_iter { - execute_transaction(&mut *execution_data, transaction, gas_price)?; + let tx_id = transaction.id(&self.consensus_params.chain_id()); + if let Some(err) = + execute_transaction(&mut *execution_data, transaction, gas_price)? + { + execution_data + .skipped_transactions + .push((tx_id, err.clone())); + } } let new_remaining_gas_limit = From 593d2447d5159a0e7e6e4284373ac6676718be98 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 15:57:37 -0700 Subject: [PATCH 41/56] Simplify code more and fix some compilation stuff --- crates/services/executor/src/executor.rs | 44 +++++++----------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index e65f73ae13b..415909b1da5 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -500,14 +500,14 @@ where let mut execute_transaction = |execution_data: &mut ExecutionData, tx: MaybeCheckedTransaction, gas_price: Word| - -> ExecutorResult> { + -> ExecutorResult<()> { let tx_count = execution_data.tx_count; let tx = { let mut tx_st_transaction = thread_block_transaction .write_transaction() .with_policy(ConflictPolicy::Overwrite); let tx_id = tx.id(&self.consensus_params.chain_id()); - let result = self.execute_transaction( + let tx = self.execute_transaction( tx, &tx_id, &block.header, @@ -516,18 +516,8 @@ where execution_data, execution_kind, &mut tx_st_transaction, - ); - - let tx = match result { - Err(err) => { - return Err(err); - } - Ok(tx) => tx, - }; - - if let Err(err) = tx_st_transaction.commit() { - return Err(err.into()); - } + )?; + tx_st_transaction.commit()?; tx }; @@ -536,7 +526,7 @@ where .checked_add(1) .ok_or(ExecutorError::TooManyTransactions)?; - Ok(None) + Ok(()) }; let relayed_tx_iter = forced_transactions.into_iter(); @@ -549,16 +539,7 @@ where transaction, RELAYED_GAS_PRICE, ) { - Ok(None) => {} - Ok(Some(err)) => { - let id_bytes: Bytes32 = tx_id.into(); - let event = ExecutorEvent::ForcedTransactionFailed { - id: id_bytes.into(), - block_height, - failure: err.to_string(), - }; - execution_data.events.push(event); - } + Ok(_) => {} Err(err) => { let id_bytes: Bytes32 = tx_id.into(); let event = ExecutorEvent::ForcedTransactionFailed { @@ -579,12 +560,13 @@ where while regular_tx_iter.peek().is_some() { for transaction in regular_tx_iter { let tx_id = transaction.id(&self.consensus_params.chain_id()); - if let Some(err) = - execute_transaction(&mut *execution_data, transaction, gas_price)? - { - execution_data - .skipped_transactions - .push((tx_id, err.clone())); + match execute_transaction(&mut *execution_data, transaction, gas_price) { + Ok(_) => {} + Err(err) => { + execution_data + .skipped_transactions + .push((tx_id, err.clone())); + } } } From ce3bae9151a6b41626e189d75998257164b97e93 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 16:17:46 -0700 Subject: [PATCH 42/56] Refactor `validate_forced_tx` to be more declarative --- crates/services/executor/src/executor.rs | 71 +++++++++++++++++------- 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 415909b1da5..4da46604437 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -707,21 +707,56 @@ where header: &PartialBlockHeader, consensus_params: &ConsensusParameters, ) -> Result { - let parsed_tx = Transaction::from_bytes(relayed_tx.serialized_transaction()) + let parsed_tx = Self::parse_tx_bytes(&relayed_tx)?; + let checked_tx = + Self::get_checked_tx(parsed_tx, *header.height(), consensus_params)?; + Self::tx_is_valid_variant(&checked_tx)?; + Self::relayed_tx_claimed_enough_max_gas(&checked_tx, &relayed_tx)?; + let predicate_checked_tx = + Self::predicates_on_tx_are_valid(checked_tx, consensus_params)?; + Ok(CheckedTransaction::from(predicate_checked_tx)) + } + + fn parse_tx_bytes( + relayed_transaction: &RelayedTransaction, + ) -> Result { + let tx_bytes = relayed_transaction.serialized_transaction(); + let tx = Transaction::from_bytes(tx_bytes) .map_err(|_| ForcedTransactionFailure::CodecError)?; + Ok(tx) + } - let check_tx_res = parsed_tx - .into_checked(header.consensus.height, consensus_params) + fn get_checked_tx( + tx: Transaction, + height: BlockHeight, + consensus_params: &ConsensusParameters, + ) -> Result, ForcedTransactionFailure> { + let checked_tx = tx + .into_checked(height, consensus_params) .map_err(ForcedTransactionFailure::CheckError)?; + Ok(checked_tx) + } + fn tx_is_valid_variant( + tx: &Checked, + ) -> Result<(), ForcedTransactionFailure> { + match tx.transaction() { + Transaction::Mint(_) => Err(ForcedTransactionFailure::InvalidTransactionType), + Transaction::Script(_) | Transaction::Create(_) => Ok(()), + } + } + + fn relayed_tx_claimed_enough_max_gas( + tx: &Checked, + relayed_tx: &RelayedTransaction, + ) -> Result<(), ForcedTransactionFailure> { let claimed_max_gas = relayed_tx.max_gas(); - let actual_max_gas = match check_tx_res.metadata() { + let actual_max_gas = match tx.metadata() { + CheckedMetadata::Script(script_metadata) => script_metadata.max_gas, + CheckedMetadata::Create(create_metadata) => create_metadata.max_gas, CheckedMetadata::Mint(_) => { return Err(ForcedTransactionFailure::InvalidTransactionType); } - - CheckedMetadata::Script(script_metadata) => script_metadata.max_gas, - CheckedMetadata::Create(create_metadata) => create_metadata.max_gas, }; if actual_max_gas > claimed_max_gas { return Err(ForcedTransactionFailure::InsufficientMaxGas { @@ -729,23 +764,17 @@ where actual_max_gas, }); } + Ok(()) + } - let checked_tx = check_tx_res + fn predicates_on_tx_are_valid( + tx: Checked, + consensus_params: &ConsensusParameters, + ) -> Result, ForcedTransactionFailure> { + let checked_tx = tx .check_predicates(&CheckPredicateParams::from(consensus_params)) .map_err(ForcedTransactionFailure::CheckError)?; - - // Fail if transaction is not a script or create - // todo: Ideally we'd have a helper such as `.is_chargeable()` to be more future proof in - // case new tx types are added in the future. - match checked_tx.transaction() { - Transaction::Script(_) => {} - Transaction::Create(_) => {} - Transaction::Mint(_) => { - return Err(ForcedTransactionFailure::InvalidTransactionType); - } - } - - Ok(CheckedTransaction::from(checked_tx)) + Ok(checked_tx) } #[allow(clippy::too_many_arguments)] From 4c4333c7d7d6fbcd5c771d5d7863cbf5fca64a31 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 17:00:00 -0700 Subject: [PATCH 43/56] Add test showing relayed tx can spend message from same da block --- crates/fuel-core/src/database.rs | 1 - crates/fuel-core/src/executor.rs | 62 +++++++++++++++++++ crates/fuel-core/src/query.rs | 1 - crates/fuel-core/src/query/relayed_tx.rs | 12 ---- crates/fuel-core/src/schema/relayed_tx.rs | 8 ++- .../upgradable-executor/src/instance.rs | 5 -- 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/crates/fuel-core/src/database.rs b/crates/fuel-core/src/database.rs index 7f6c3c823d5..707fd00222b 100644 --- a/crates/fuel-core/src/database.rs +++ b/crates/fuel-core/src/database.rs @@ -593,7 +593,6 @@ mod tests { .insert(&prev_height, &CompressedBlock::default()); // Then - assert!(result.is_err()); assert_eq!( result.unwrap_err().to_string(), StorageError::from(DatabaseError::HeightsAreNotLinked { diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 436c89a2c52..789fe18e908 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3457,6 +3457,68 @@ mod tests { relayer_db } + #[test] + fn execute_without_commit__relayed_tx_can_spend_message_from_same_da_block() { + let genesis_da_height = 3u64; + let block_height = 1u32; + let da_height = 10u64; + let arb_max_gas = 10_000; + + // given + let relayer_db = + relayer_db_with_relayed_tx_spending_message_from_same_da_block( + da_height, + arb_max_gas, + ); + + // when + let on_chain_db = database_with_genesis_block(genesis_da_height); + let producer = create_relayer_executor(on_chain_db, relayer_db); + let block = test_block(block_height.into(), da_height.into(), 0); + let (result, _) = producer + .execute_without_commit(ExecutionBlock::Production(block.into())) + .unwrap() + .into(); + + // then + let txs = result.block.transactions(); + assert_eq!(txs.len(), 2); + } + + fn relayer_db_with_relayed_tx_spending_message_from_same_da_block( + da_height: u64, + max_gas: u64, + ) -> Database { + let mut relayer_db = Database::::default(); + let mut message = Message::default(); + let nonce = 1.into(); + message.set_da_height(da_height.into()); + message.set_nonce(nonce); + let message_event = Event::Message(message); + + let mut relayed_tx = RelayedTransaction::default(); + let tx = TransactionBuilder::script(vec![], vec![]) + .script_gas_limit(10) + .add_unsigned_message_input( + SecretKey::random(&mut StdRng::seed_from_u64(2322)), + Default::default(), + nonce, + Default::default(), + vec![], + ) + .finalize_as_transaction(); + let tx_bytes = tx.to_bytes(); + relayed_tx.set_serialized_transaction(tx_bytes); + relayed_tx.set_max_gas(max_gas); + let tx_event = Event::Transaction(relayed_tx); + add_events_to_relayer( + &mut relayer_db, + da_height.into(), + &[message_event, tx_event], + ); + relayer_db + } + #[test] fn block_producer_does_not_take_messages_for_the_same_height() { let genesis_da_height = 1u64; diff --git a/crates/fuel-core/src/query.rs b/crates/fuel-core/src/query.rs index 82f3212759c..844866fc0c4 100644 --- a/crates/fuel-core/src/query.rs +++ b/crates/fuel-core/src/query.rs @@ -16,6 +16,5 @@ pub use chain::*; pub use coin::*; pub use contract::*; pub use message::*; -pub use relayed_tx::*; pub(crate) use subscriptions::*; pub use tx::*; diff --git a/crates/fuel-core/src/query/relayed_tx.rs b/crates/fuel-core/src/query/relayed_tx.rs index a272458c0ea..8b137891791 100644 --- a/crates/fuel-core/src/query/relayed_tx.rs +++ b/crates/fuel-core/src/query/relayed_tx.rs @@ -1,13 +1 @@ -use crate::fuel_core_graphql_api::ports::DatabaseRelayedTransactions; -use fuel_core_storage::Result as StorageResult; -use fuel_core_types::{ - entities::relayer::transaction::RelayedTransactionStatus, - fuel_tx::Bytes32, -}; -pub fn relayed_tx_status( - db: &DB, - id: Bytes32, -) -> StorageResult> { - db.transaction_status(id) -} diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index 4a6cec4712f..c89b43ed788 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -1,5 +1,8 @@ use crate::{ - fuel_core_graphql_api::database::ReadView, + fuel_core_graphql_api::{ + database::ReadView, + ports::DatabaseRelayedTransactions, + }, schema::scalars::{ RelayedTransactionId, U32, @@ -26,8 +29,7 @@ impl RelayedTransactionQuery { #[graphql(desc = "The id of the relayed tx")] id: RelayedTransactionId, ) -> async_graphql::Result> { let query: &ReadView = ctx.data_unchecked(); - let status = - crate::query::relayed_tx_status(query, id.0)?.map(|status| status.into()); + let status = query.transaction_status(id.0)?.map(|status| status.into()); Ok(status) } } diff --git a/crates/services/upgradable-executor/src/instance.rs b/crates/services/upgradable-executor/src/instance.rs index 27c0a3492ea..7f26dcecdf0 100644 --- a/crates/services/upgradable-executor/src/instance.rs +++ b/crates/services/upgradable-executor/src/instance.rs @@ -166,11 +166,6 @@ impl Instance { tx } MaybeCheckedTransaction::Transaction(tx) => tx, - MaybeCheckedTransaction::RelayedCheckedTransaction(_, checked) => { - let checked: Checked = checked.into(); - let (tx, _) = checked.into(); - tx - } }) .collect(); From 1ef34642a4f003cc360cbb8dcc4486f5783d59d1 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 17:01:37 -0700 Subject: [PATCH 44/56] Appease Clippy-sama --- crates/services/executor/src/executor.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 4da46604437..fd7236d1a8f 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -541,9 +541,8 @@ where ) { Ok(_) => {} Err(err) => { - let id_bytes: Bytes32 = tx_id.into(); let event = ExecutorEvent::ForcedTransactionFailed { - id: id_bytes.into(), + id: tx_id.into(), block_height, failure: err.to_string(), }; @@ -812,7 +811,7 @@ where MaybeCheckedTransaction::CheckedTransaction(checked_tx) => checked_tx, }; - let res = match checked_tx { + match checked_tx { CheckedTransaction::Script(script) => self.execute_create_or_script( script, header, @@ -840,9 +839,7 @@ where tx_st_transaction, execution_kind, ), - }; - - res + } } #[allow(clippy::too_many_arguments)] From 8e28483525a44a586e9e7a6e65992d04b5613482 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 11 Apr 2024 17:08:04 -0700 Subject: [PATCH 45/56] Fix more CI checks --- tests/tests/relayer.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/tests/relayer.rs b/tests/tests/relayer.rs index 36e7f1f40e3..8a5434dfc47 100644 --- a/tests/tests/relayer.rs +++ b/tests/tests/relayer.rs @@ -51,7 +51,6 @@ use fuel_core_types::{ BlockHeight, Nonce, }, - tai64::Tai64, }; use hyper::{ service::{ @@ -268,13 +267,11 @@ async fn can_find_failed_relayed_tx() { let mut db = CombinedDatabase::in_memory(); let id = [1; 32].into(); let block_height: BlockHeight = 999.into(); - let block_time = Tai64::UNIX_EPOCH; let failure = "lolz".to_string(); // given let status = FuelRelayedTransactionStatus::Failed { block_height, - block_time, failure: failure.clone(), }; db.off_chain_mut() @@ -291,7 +288,6 @@ async fn can_find_failed_relayed_tx() { // then let expected = Some(ClientRelayedTransactionStatus::Failed { block_height, - block_time, failure, }); let actual = client.relayed_transaction_status(&id).await.unwrap(); From fa58407b112b59ec80577755d17f19bcc5e62fdd Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 01:42:57 -0700 Subject: [PATCH 46/56] Fix silly bug in code --- crates/services/executor/src/executor.rs | 38 ++++++++++++++++-------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index fd7236d1a8f..b51855107ad 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -507,7 +507,7 @@ where .write_transaction() .with_policy(ConflictPolicy::Overwrite); let tx_id = tx.id(&self.consensus_params.chain_id()); - let tx = self.execute_transaction( + let result = self.execute_transaction( tx, &tx_id, &block.header, @@ -516,8 +516,30 @@ where execution_data, execution_kind, &mut tx_st_transaction, - )?; - tx_st_transaction.commit()?; + ); + let tx = match result { + Err(err) => { + return match execution_kind { + ExecutionKind::Production => { + // If, during block production, we get an invalid transaction, + // remove it from the block and continue block creation. An invalid + // transaction means that the caller didn't validate it first, so + // maybe something is wrong with validation rules in the `TxPool` + // (or in another place that should validate it). Or we forgot to + // clean up some dependent/conflict transactions. But it definitely + // means that something went wrong, and we must fix it. + execution_data.skipped_transactions.push((tx_id, err)); + Ok(()) + } + ExecutionKind::DryRun | ExecutionKind::Validation => Err(err), + } + } + Ok(tx) => tx, + }; + + if let Err(err) = tx_st_transaction.commit() { + return Err(err.into()) + } tx }; @@ -558,15 +580,7 @@ where let mut regular_tx_iter = source.next(remaining_gas_limit).into_iter().peekable(); while regular_tx_iter.peek().is_some() { for transaction in regular_tx_iter { - let tx_id = transaction.id(&self.consensus_params.chain_id()); - match execute_transaction(&mut *execution_data, transaction, gas_price) { - Ok(_) => {} - Err(err) => { - execution_data - .skipped_transactions - .push((tx_id, err.clone())); - } - } + execute_transaction(&mut *execution_data, transaction, gas_price)?; } let new_remaining_gas_limit = From 9a7b027bdd586bdaea47e0da0e7bb4b4c678aabf Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 02:53:34 -0700 Subject: [PATCH 47/56] Fix that closure more and do other cleanup --- crates/client/assets/schema.sdl | 2 +- crates/client/src/client.rs | 2 +- crates/client/src/client/schema/relayed_tx.rs | 2 +- crates/fuel-core/src/executor.rs | 30 ++++---- crates/fuel-core/src/query.rs | 2 - crates/fuel-core/src/query/relayed_tx.rs | 1 - crates/fuel-core/src/schema/relayed_tx.rs | 2 +- crates/services/executor/src/executor.rs | 71 +++++++++---------- crates/types/src/services/executor.rs | 2 +- 9 files changed, 55 insertions(+), 59 deletions(-) delete mode 100644 crates/fuel-core/src/query/relayed_tx.rs diff --git a/crates/client/assets/schema.sdl b/crates/client/assets/schema.sdl index 76f52040214..23f8a066cc9 100644 --- a/crates/client/assets/schema.sdl +++ b/crates/client/assets/schema.sdl @@ -850,7 +850,7 @@ type Query { messages(owner: Address, first: Int, after: String, last: Int, before: String): MessageConnection! messageProof(transactionId: TransactionId!, nonce: Nonce!, commitBlockId: BlockId, commitBlockHeight: U32): MessageProof messageStatus(nonce: Nonce!): MessageStatus! - status(id: RelayedTransactionId!): RelayedTransactionStatus + relayedTransactionStatus(id: RelayedTransactionId!): RelayedTransactionStatus } type Receipt { diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 4c9933cec29..2ff901afcea 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -984,7 +984,7 @@ impl FuelClient { let status = self .query(query) .await? - .status + .relayed_transaction_status .map(|status| status.try_into()) .transpose()?; Ok(status) diff --git a/crates/client/src/client/schema/relayed_tx.rs b/crates/client/src/client/schema/relayed_tx.rs index ac8f7382fb7..44c6e130aa8 100644 --- a/crates/client/src/client/schema/relayed_tx.rs +++ b/crates/client/src/client/schema/relayed_tx.rs @@ -12,7 +12,7 @@ use crate::client::schema::{ )] pub struct RelayedTransactionStatusQuery { #[arguments(id: $id)] - pub status: Option, + pub relayed_transaction_status: Option, } #[derive(cynic::QueryVariables, Debug)] diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 789fe18e908..43a32cb2a3c 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2839,6 +2839,7 @@ mod tests { entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, fuel_tx::output, + services::executor::ForcedTransactionFailure, }; fn database_with_genesis_block(da_block_height: u64) -> Database { @@ -3174,9 +3175,11 @@ mod tests { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; + let arb_large_max_gas = 10_000; // given - let relayer_db = relayer_db_with_invalid_relayed_txs(da_height); + let relayer_db = + relayer_db_with_invalid_relayed_txs(da_height, arb_large_max_gas); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3204,8 +3207,11 @@ mod tests { assert_eq!(expected, actual); } - fn relayer_db_with_invalid_relayed_txs(da_height: u64) -> Database { - let event = arb_invalid_relayed_tx_event(); + fn relayer_db_with_invalid_relayed_txs( + da_height: u64, + max_gas: u64, + ) -> Database { + let event = arb_invalid_relayed_tx_event(max_gas); relayer_db_for_events(&[event], da_height) } @@ -3240,8 +3246,8 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected = "Insufficient max gas."; - assert!(actual.starts_with(expected)); + let expected_start_of_message = "Insufficient max gas:"; + assert!(actual.starts_with(expected_start_of_message)); } fn relayer_db_with_relayed_tx_with_low_max_gas( @@ -3291,8 +3297,8 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected = "Transaction id was already used: "; - assert!(actual.starts_with(expected)); + let expected_start_of_message = "Transaction id was already used: "; + assert!(actual.starts_with(expected_start_of_message)); } fn relayer_db_with_tx_that_passes_checks_but_fails_execution( @@ -3307,8 +3313,6 @@ mod tests { let mut bad_relayed_tx = relayed_tx.clone(); let new_nonce = [9; 32].into(); bad_relayed_tx.set_nonce(new_nonce); - // let mut bad_tx = tx.clone(); - // bad_relayed_tx.set_serialized_transaction(bad_tx_bytes); relayer_db_for_events(&[relayed_tx.into(), bad_relayed_tx.into()], da_height) } @@ -3317,9 +3321,10 @@ mod tests { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; + let arb_large_max_gas = 10_000; // given - let event = arb_invalid_relayed_tx_event(); + let event = arb_invalid_relayed_tx_event(arb_large_max_gas); let produced_block = produce_block_with_relayed_event( event.clone(), genesis_da_height, @@ -3373,12 +3378,13 @@ mod tests { produced_result.block } - fn arb_invalid_relayed_tx_event() -> Event { + fn arb_invalid_relayed_tx_event(max_gas: u64) -> Event { let mut invalid_relayed_tx = RelayedTransaction::default(); let mut tx = script_tx_for_amount(100); tx.as_script_mut().unwrap().inputs_mut().drain(..); // Remove all the inputs :) let tx_bytes = tx.to_bytes(); invalid_relayed_tx.set_serialized_transaction(tx_bytes); + invalid_relayed_tx.set_max_gas(max_gas); invalid_relayed_tx.into() } @@ -3416,7 +3422,7 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected = "Transaction type is not accepted"; + let expected = &ForcedTransactionFailure::InvalidTransactionType.to_string(); assert_eq!(expected, actual); } diff --git a/crates/fuel-core/src/query.rs b/crates/fuel-core/src/query.rs index 844866fc0c4..50283d1a275 100644 --- a/crates/fuel-core/src/query.rs +++ b/crates/fuel-core/src/query.rs @@ -7,8 +7,6 @@ mod message; mod subscriptions; mod tx; -mod relayed_tx; - // TODO: Remove reexporting of everything pub use balance::*; pub use block::*; diff --git a/crates/fuel-core/src/query/relayed_tx.rs b/crates/fuel-core/src/query/relayed_tx.rs deleted file mode 100644 index 8b137891791..00000000000 --- a/crates/fuel-core/src/query/relayed_tx.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/crates/fuel-core/src/schema/relayed_tx.rs b/crates/fuel-core/src/schema/relayed_tx.rs index c89b43ed788..b219280ebbb 100644 --- a/crates/fuel-core/src/schema/relayed_tx.rs +++ b/crates/fuel-core/src/schema/relayed_tx.rs @@ -23,7 +23,7 @@ pub struct RelayedTransactionQuery {} #[Object] impl RelayedTransactionQuery { - async fn status( + async fn relayed_transaction_status( &self, ctx: &Context<'_>, #[graphql(desc = "The id of the relayed tx")] id: RelayedTransactionId, diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index b51855107ad..4f29230a841 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -107,7 +107,6 @@ use fuel_core_types::{ CheckPredicateParams, CheckPredicates, Checked, - CheckedMetadata, CheckedTransaction, Checks, IntoChecked, @@ -507,7 +506,7 @@ where .write_transaction() .with_policy(ConflictPolicy::Overwrite); let tx_id = tx.id(&self.consensus_params.chain_id()); - let result = self.execute_transaction( + let tx = self.execute_transaction( tx, &tx_id, &block.header, @@ -516,30 +515,8 @@ where execution_data, execution_kind, &mut tx_st_transaction, - ); - let tx = match result { - Err(err) => { - return match execution_kind { - ExecutionKind::Production => { - // If, during block production, we get an invalid transaction, - // remove it from the block and continue block creation. An invalid - // transaction means that the caller didn't validate it first, so - // maybe something is wrong with validation rules in the `TxPool` - // (or in another place that should validate it). Or we forgot to - // clean up some dependent/conflict transactions. But it definitely - // means that something went wrong, and we must fix it. - execution_data.skipped_transactions.push((tx_id, err)); - Ok(()) - } - ExecutionKind::DryRun | ExecutionKind::Validation => Err(err), - } - } - Ok(tx) => tx, - }; - - if let Err(err) = tx_st_transaction.commit() { - return Err(err.into()) - } + )?; + tx_st_transaction.commit()?; tx }; @@ -580,7 +557,18 @@ where let mut regular_tx_iter = source.next(remaining_gas_limit).into_iter().peekable(); while regular_tx_iter.peek().is_some() { for transaction in regular_tx_iter { - execute_transaction(&mut *execution_data, transaction, gas_price)?; + let tx_id = transaction.id(&self.consensus_params.chain_id()); + match execute_transaction(&mut *execution_data, transaction, gas_price) { + Ok(_) => {} + Err(err) => match execution_kind { + ExecutionKind::Production => { + execution_data.skipped_transactions.push((tx_id, err)); + } + ExecutionKind::DryRun | ExecutionKind::Validation => { + return Err(err); + } + }, + } } let new_remaining_gas_limit = @@ -721,10 +709,14 @@ where consensus_params: &ConsensusParameters, ) -> Result { let parsed_tx = Self::parse_tx_bytes(&relayed_tx)?; + Self::tx_is_valid_variant(&parsed_tx)?; + Self::relayed_tx_claimed_enough_max_gas( + &parsed_tx, + &relayed_tx, + &consensus_params, + )?; let checked_tx = Self::get_checked_tx(parsed_tx, *header.height(), consensus_params)?; - Self::tx_is_valid_variant(&checked_tx)?; - Self::relayed_tx_claimed_enough_max_gas(&checked_tx, &relayed_tx)?; let predicate_checked_tx = Self::predicates_on_tx_are_valid(checked_tx, consensus_params)?; Ok(CheckedTransaction::from(predicate_checked_tx)) @@ -750,25 +742,26 @@ where Ok(checked_tx) } - fn tx_is_valid_variant( - tx: &Checked, - ) -> Result<(), ForcedTransactionFailure> { - match tx.transaction() { + fn tx_is_valid_variant(tx: &Transaction) -> Result<(), ForcedTransactionFailure> { + match tx { Transaction::Mint(_) => Err(ForcedTransactionFailure::InvalidTransactionType), Transaction::Script(_) | Transaction::Create(_) => Ok(()), } } fn relayed_tx_claimed_enough_max_gas( - tx: &Checked, + tx: &Transaction, relayed_tx: &RelayedTransaction, + consensus_params: &ConsensusParameters, ) -> Result<(), ForcedTransactionFailure> { let claimed_max_gas = relayed_tx.max_gas(); - let actual_max_gas = match tx.metadata() { - CheckedMetadata::Script(script_metadata) => script_metadata.max_gas, - CheckedMetadata::Create(create_metadata) => create_metadata.max_gas, - CheckedMetadata::Mint(_) => { - return Err(ForcedTransactionFailure::InvalidTransactionType); + let gas_costs = consensus_params.gas_costs(); + let fee_params = consensus_params.fee_params(); + let actual_max_gas = match tx { + Transaction::Script(script) => script.max_gas(gas_costs, fee_params), + Transaction::Create(create) => create.max_gas(gas_costs, fee_params), + Transaction::Mint(_) => { + return Err(ForcedTransactionFailure::InvalidTransactionType) } }; if actual_max_gas > claimed_max_gas { diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index c882aa35952..58f9ab13b56 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -98,7 +98,7 @@ pub enum ForcedTransactionFailure { ExecutionError(Error), /// Relayed Transaction didn't specify high enough max gas #[display( - fmt = "Insufficient max gas. Expected: {claimed_max_gas:?}, Actual: {actual_max_gas:?}" + fmt = "Insufficient max gas: Expected: {claimed_max_gas:?}, Actual: {actual_max_gas:?}" )] InsufficientMaxGas { /// The max gas claimed by the L1 transaction submitter From 3fc20851dc2bbcf40add446fa7b462075c6327a1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 02:55:03 -0700 Subject: [PATCH 48/56] Appease Clippy-san --- crates/services/executor/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 4f29230a841..d5232e62cf0 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -713,7 +713,7 @@ where Self::relayed_tx_claimed_enough_max_gas( &parsed_tx, &relayed_tx, - &consensus_params, + consensus_params, )?; let checked_tx = Self::get_checked_tx(parsed_tx, *header.height(), consensus_params)?; From c6c1b2d49ae0546e85f4225d1c432db4a7c1374a Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 03:09:58 -0700 Subject: [PATCH 49/56] Use display impls of errors instead of hardcoding --- crates/fuel-core/src/executor.rs | 10 ++++++++-- crates/types/src/services/executor.rs | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 43a32cb2a3c..efe99c5bb7e 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3203,7 +3203,10 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected = "Failed validity checks"; + let expected = &ForcedTransactionFailure::CheckError(CheckError::Validity( + ValidityError::NoSpendableInput, + )) + .to_string(); assert_eq!(expected, actual); } @@ -3356,7 +3359,10 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected = "Failed validity checks"; + let expected = &ForcedTransactionFailure::CheckError(CheckError::Validity( + ValidityError::NoSpendableInput, + )) + .to_string(); assert_eq!(expected, actual); } diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 58f9ab13b56..dd456a706ff 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -88,7 +88,7 @@ pub enum ForcedTransactionFailure { #[display(fmt = "Failed to decode transaction")] CodecError, /// Transaction failed basic checks - #[display(fmt = "Failed validity checks")] + #[display(fmt = "Failed validity checks: {_0:?}")] CheckError(CheckError), /// Invalid transaction type #[display(fmt = "Transaction type is not accepted")] From dab539c7e2daf37b76d93c91d938b63daad1ae7e Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 15:59:02 -0700 Subject: [PATCH 50/56] Remove unnecessary check --- crates/services/executor/src/executor.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index d5232e62cf0..19a37d500d9 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -717,9 +717,7 @@ where )?; let checked_tx = Self::get_checked_tx(parsed_tx, *header.height(), consensus_params)?; - let predicate_checked_tx = - Self::predicates_on_tx_are_valid(checked_tx, consensus_params)?; - Ok(CheckedTransaction::from(predicate_checked_tx)) + Ok(CheckedTransaction::from(checked_tx)) } fn parse_tx_bytes( @@ -773,16 +771,6 @@ where Ok(()) } - fn predicates_on_tx_are_valid( - tx: Checked, - consensus_params: &ConsensusParameters, - ) -> Result, ForcedTransactionFailure> { - let checked_tx = tx - .check_predicates(&CheckPredicateParams::from(consensus_params)) - .map_err(ForcedTransactionFailure::CheckError)?; - Ok(checked_tx) - } - #[allow(clippy::too_many_arguments)] fn execute_transaction( &self, From a8e714fa0f2b71f51866c52320f96e0549e42f87 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 16:15:07 -0700 Subject: [PATCH 51/56] Remove extra semi-colons --- crates/fuel-core/src/schema.rs | 12 ++--- crates/services/executor/src/executor.rs | 62 ++++++++++++------------ 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/crates/fuel-core/src/schema.rs b/crates/fuel-core/src/schema.rs index ca122781738..e4185d5bcf3 100644 --- a/crates/fuel-core/src/schema.rs +++ b/crates/fuel-core/src/schema.rs @@ -94,22 +94,22 @@ where return Err(anyhow!( "Either first `{first}` or latest `{last}` elements, not both" ) - .into()); + .into()) } (Some(after), _, _, Some(last)) => { return Err(anyhow!( "After `{after:?}` with last `{last}` elements is not supported" ) - .into()); + .into()) } (_, Some(before), Some(first), _) => { return Err(anyhow!( "Before `{before:?}` with first `{first}` elements is not supported" ) - .into()); + .into()) } (None, None, None, None) => { - return Err(anyhow!("The queries for the whole range is not supported").into()); + return Err(anyhow!("The queries for the whole range is not supported").into()) } (_, _, _, _) => { /* Other combinations are allowed */ } }; @@ -154,7 +154,7 @@ where // Skip until start + 1 if key == start { has_previous_page = true; - return true; + return true } } } @@ -168,7 +168,7 @@ where // take until we've reached the end if key == end { has_next_page = true; - return false; + return false } } count = count.saturating_sub(1); diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 19a37d500d9..d2ad053089e 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -382,7 +382,7 @@ where if let Some(pre_exec_block_id) = pre_exec_block_id { // The block id comparison compares the whole blocks including all fields. if pre_exec_block_id != finalized_block_id { - return Err(ExecutorError::InvalidBlockId); + return Err(ExecutorError::InvalidBlockId) } } @@ -617,7 +617,7 @@ where .commit_changes(block_with_relayer_data_transaction.into_changes())?; if execution_kind != ExecutionKind::DryRun && !data.found_mint { - return Err(ExecutorError::MintMissing); + return Err(ExecutorError::MintMissing) } data.changes = self.block_st_transaction.into_changes(); @@ -642,7 +642,7 @@ where .ok_or(ExecutorError::PreviousBlockIsNotFound)?; let previous_da_height = prev_block_header.header().da_height; let Some(next_unprocessed_da_height) = previous_da_height.0.checked_add(1) else { - return Err(ExecutorError::DaHeightExceededItsLimit); + return Err(ExecutorError::DaHeightExceededItsLimit) }; let mut root_calculator = MerkleRootCalculator::new(); @@ -660,7 +660,7 @@ where match event { Event::Message(message) => { if message.da_height() != da_height { - return Err(ExecutorError::RelayerGivesIncorrectMessages); + return Err(ExecutorError::RelayerGivesIncorrectMessages) } self.block_st_transaction .storage_as_mut::() @@ -787,7 +787,7 @@ where T: KeyValueInspect, { if execution_data.found_mint { - return Err(ExecutorError::MintIsNotLastTransaction); + return Err(ExecutorError::MintIsNotLastTransaction) } // Throw a clear error if the transaction id is a duplicate @@ -795,7 +795,7 @@ where .storage::() .contains_key(tx_id)? { - return Err(ExecutorError::TransactionIdCollision(*tx_id)); + return Err(ExecutorError::TransactionIdCollision(*tx_id)) } let block_height = *header.height(); @@ -854,7 +854,7 @@ where execution_data.found_mint = true; if checked_mint.transaction().tx_pointer().tx_index() != execution_data.tx_count { - return Err(ExecutorError::MintHasUnexpectedIndex); + return Err(ExecutorError::MintHasUnexpectedIndex) } let coinbase_id = checked_mint.id(); @@ -862,7 +862,7 @@ where fn verify_mint_for_empty_contract(mint: &Mint) -> ExecutorResult<()> { if *mint.mint_amount() != 0 { - return Err(ExecutorError::CoinbaseAmountMismatch); + return Err(ExecutorError::CoinbaseAmountMismatch) } let input = input::contract::Contract { @@ -878,7 +878,7 @@ where state_root: Bytes32::zeroed(), }; if mint.input_contract() != &input || mint.output_contract() != &output { - return Err(ExecutorError::MintMismatch); + return Err(ExecutorError::MintMismatch) } Ok(()) } @@ -887,10 +887,10 @@ where verify_mint_for_empty_contract(&mint)?; } else { if *mint.mint_amount() != execution_data.coinbase { - return Err(ExecutorError::CoinbaseAmountMismatch); + return Err(ExecutorError::CoinbaseAmountMismatch) } if *mint.gas_price() != gas_price { - return Err(ExecutorError::CoinbaseGasPriceMismatch); + return Err(ExecutorError::CoinbaseGasPriceMismatch) } let block_height = *header.height(); @@ -965,7 +965,7 @@ where if execution_kind == ExecutionKind::Validation { if mint.input_contract() != &input || mint.output_contract() != &output { - return Err(ExecutorError::MintMismatch); + return Err(ExecutorError::MintMismatch) } } else { *mint.input_contract_mut() = input; @@ -988,7 +988,7 @@ where .insert(&coinbase_id, &())? .is_some() { - return Err(ExecutorError::TransactionIdCollision(coinbase_id)); + return Err(ExecutorError::TransactionIdCollision(coinbase_id)) } Ok(tx) } @@ -1112,7 +1112,7 @@ where or we added a new predicate inputs."); return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } } } @@ -1161,7 +1161,7 @@ where { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } let final_tx = tx.into(); @@ -1223,12 +1223,12 @@ where { return Err( TransactionValidityError::CoinMismatch(*utxo_id).into() - ); + ) } } else { return Err( TransactionValidityError::CoinDoesNotExist(*utxo_id).into() - ); + ) } } Input::Contract(contract) => { @@ -1239,7 +1239,7 @@ where return Err(TransactionValidityError::ContractDoesNotExist( contract.contract_id, ) - .into()); + .into()) } } Input::MessageCoinSigned(MessageCoinSigned { nonce, .. }) @@ -1257,7 +1257,7 @@ where return Err(TransactionValidityError::MessageSpendTooEarly( *nonce, ) - .into()); + .into()) } if !message @@ -1266,12 +1266,12 @@ where { return Err( TransactionValidityError::MessageMismatch(*nonce).into() - ); + ) } } else { return Err( TransactionValidityError::MessageDoesNotExist(*nonce).into() - ); + ) } } } @@ -1329,7 +1329,7 @@ where if reverted => { // Don't spend the retryable messages if transaction is reverted - continue; + continue } Input::MessageCoinSigned(MessageCoinSigned { nonce, .. }) | Input::MessageCoinPredicate(MessageCoinPredicate { nonce, .. }) @@ -1341,7 +1341,7 @@ where db.storage::().insert(nonce, &())?; // ensure message wasn't already marked as spent if was_already_spent.is_some() { - return Err(ExecutorError::MessageAlreadySpent(*nonce)); + return Err(ExecutorError::MessageAlreadySpent(*nonce)) } // cleanup message contents let message = db @@ -1369,7 +1369,7 @@ where for r in receipts { if let Receipt::ScriptResult { gas_used, .. } = r { used_gas = *gas_used; - break; + break } } @@ -1478,7 +1478,7 @@ where if tx_pointer != coin.tx_pointer() { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } } Input::Contract(Contract { @@ -1498,17 +1498,17 @@ where { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } if balance_root != &contract.balance_root()? { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } if state_root != &contract.state_root()? { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) } } _ => {} @@ -1542,7 +1542,7 @@ where } else { return Err(ExecutorError::InvalidTransactionOutcome { transaction_id: tx_id, - }); + }) }; let contract = ContractRef::new(StructuredStorage::new(db), *contract_id); @@ -1658,7 +1658,7 @@ where } else { return Err(ExecutorError::TransactionValidity( TransactionValidityError::InvalidContractInputIndex(utxo_id), - )); + )) } } Output::Change { @@ -1724,7 +1724,7 @@ where .into(); if db.storage::().insert(&utxo_id, &coin)?.is_some() { - return Err(ExecutorError::OutputAlreadyExists); + return Err(ExecutorError::OutputAlreadyExists) } execution_data .events From c83eb18dc67b7bcc35c33099638350cf2ca3039e Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 16:57:40 -0700 Subject: [PATCH 52/56] Add specific values to low gas test --- crates/fuel-core/src/executor.rs | 38 +++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index efe99c5bb7e..95c1d9f4ecc 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2818,6 +2818,9 @@ mod tests { #[cfg(feature = "relayer")] mod relayer { + + use fuel_core_types::fuel_vm::checked_transaction::CheckedMetadata; + use super::*; use crate::{ database::database_description::{ @@ -2839,6 +2842,7 @@ mod tests { entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, fuel_tx::output, + fuel_vm::checked_transaction::IntoChecked, services::executor::ForcedTransactionFailure, }; @@ -3223,9 +3227,16 @@ mod tests { let genesis_da_height = 3u64; let block_height = 1u32; let da_height = 10u64; + let zero_max_gas = 0; // given - let relayer_db = relayer_db_with_relayed_tx_with_low_max_gas(da_height); + let tx = script_tx_for_amount(100); + + let relayer_db = relayer_db_with_specific_tx_for_relayed_tx( + da_height, + tx.clone(), + zero_max_gas, + ); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3241,6 +3252,16 @@ mod tests { assert_eq!(txs.len(), 1); // and + let consensus_params = ConsensusParameters::default(); + let actual_max_gas = if let CheckedMetadata::Script(metadata) = tx + .into_checked(block_height.into(), &consensus_params) + .unwrap() + .metadata() + { + metadata.max_gas + } else { + panic!("Expected `CheckedMetadata::Script`") + }; let events = result.events; let fuel_core_types::services::executor::Event::ForcedTransactionFailed { failure: actual, @@ -3249,18 +3270,23 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected_start_of_message = "Insufficient max gas:"; - assert!(actual.starts_with(expected_start_of_message)); + let expected = &ForcedTransactionFailure::InsufficientMaxGas { + claimed_max_gas: zero_max_gas, + actual_max_gas, + } + .to_string(); + assert_eq!(expected, actual); } - fn relayer_db_with_relayed_tx_with_low_max_gas( + fn relayer_db_with_specific_tx_for_relayed_tx( da_height: u64, + tx: Transaction, + max_gas: u64, ) -> Database { let mut relayed_tx = RelayedTransaction::default(); - let tx = script_tx_for_amount(100); let tx_bytes = tx.to_bytes(); relayed_tx.set_serialized_transaction(tx_bytes); - relayed_tx.set_max_gas(0); + relayed_tx.set_max_gas(max_gas); relayer_db_for_events(&[relayed_tx.into()], da_height) } From d344888b51033b14f96c1479f3588c42f3f0af6e Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 17:10:27 -0700 Subject: [PATCH 53/56] Use actual tx_id in error for failed execution --- crates/fuel-core/src/executor.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 95c1d9f4ecc..199fa3a2c6d 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3299,10 +3299,11 @@ mod tests { let arb_max_gas = 10_000; // given - let relayer_db = relayer_db_with_tx_that_passes_checks_but_fails_execution( - da_height, - arb_max_gas, - ); + let (tx_id, relayer_db) = + relayer_db_with_tx_that_passes_checks_but_fails_execution( + da_height, + arb_max_gas, + ); // when let on_chain_db = database_with_genesis_block(genesis_da_height); @@ -3326,14 +3327,19 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - let expected_start_of_message = "Transaction id was already used: "; - assert!(actual.starts_with(expected_start_of_message)); + // let expected = "Transaction id was already used: "; + let expected = + &fuel_core_types::services::executor::Error::TransactionIdCollision( + tx_id, + ) + .to_string(); + assert_eq!(expected, actual); } fn relayer_db_with_tx_that_passes_checks_but_fails_execution( da_height: u64, max_gas: u64, - ) -> Database { + ) -> (Bytes32, Database) { let mut relayed_tx = RelayedTransaction::default(); let tx = script_tx_for_amount(100); let tx_bytes = tx.to_bytes(); @@ -3342,7 +3348,11 @@ mod tests { let mut bad_relayed_tx = relayed_tx.clone(); let new_nonce = [9; 32].into(); bad_relayed_tx.set_nonce(new_nonce); - relayer_db_for_events(&[relayed_tx.into(), bad_relayed_tx.into()], da_height) + let relayer_db = relayer_db_for_events( + &[relayed_tx.into(), bad_relayed_tx.into()], + da_height, + ); + (tx.id(&Default::default()), relayer_db) } #[test] From 99d21e68e422c1e296ecec0d872ca45385854406 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 17:14:43 -0700 Subject: [PATCH 54/56] Cleanup --- crates/fuel-core/src/executor.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 199fa3a2c6d..4ab379d1e21 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3300,7 +3300,7 @@ mod tests { // given let (tx_id, relayer_db) = - relayer_db_with_tx_that_passes_checks_but_fails_execution( + tx_id_and_relayer_db_with_tx_that_passes_checks_but_fails_execution( da_height, arb_max_gas, ); @@ -3327,7 +3327,6 @@ mod tests { else { panic!("Expected `ForcedTransactionFailed` event") }; - // let expected = "Transaction id was already used: "; let expected = &fuel_core_types::services::executor::Error::TransactionIdCollision( tx_id, @@ -3336,7 +3335,7 @@ mod tests { assert_eq!(expected, actual); } - fn relayer_db_with_tx_that_passes_checks_but_fails_execution( + fn tx_id_and_relayer_db_with_tx_that_passes_checks_but_fails_execution( da_height: u64, max_gas: u64, ) -> (Bytes32, Database) { From 9c703f35e068f10167eed6714f41b5dcc7e2f1e1 Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 17:26:14 -0700 Subject: [PATCH 55/56] Cleanup max gas calc --- crates/fuel-core/src/executor.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 4ab379d1e21..94567227d98 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2818,9 +2818,6 @@ mod tests { #[cfg(feature = "relayer")] mod relayer { - - use fuel_core_types::fuel_vm::checked_transaction::CheckedMetadata; - use super::*; use crate::{ database::database_description::{ @@ -2841,8 +2838,10 @@ mod tests { use fuel_core_types::{ entities::RelayedTransaction, fuel_merkle::binary::root_calculator::MerkleRootCalculator, - fuel_tx::output, - fuel_vm::checked_transaction::IntoChecked, + fuel_tx::{ + output, + Chargeable, + }, services::executor::ForcedTransactionFailure, }; @@ -3253,15 +3252,19 @@ mod tests { // and let consensus_params = ConsensusParameters::default(); - let actual_max_gas = if let CheckedMetadata::Script(metadata) = tx - .into_checked(block_height.into(), &consensus_params) + // let actual_max_gas = if let CheckedMetadata::Script(metadata) = tx + // .into_checked(block_height.into(), &consensus_params) + // .unwrap() + // .metadata() + // { + // metadata.max_gas + // } else { + // panic!("Expected `CheckedMetadata::Script`") + // }; + let actual_max_gas = tx + .as_script() .unwrap() - .metadata() - { - metadata.max_gas - } else { - panic!("Expected `CheckedMetadata::Script`") - }; + .max_gas(consensus_params.gas_costs(), consensus_params.fee_params()); let events = result.events; let fuel_core_types::services::executor::Event::ForcedTransactionFailed { failure: actual, From abae5ab602821ea21f5f1a52aa9e28c94d387e7a Mon Sep 17 00:00:00 2001 From: mitchell Date: Fri, 12 Apr 2024 18:22:35 -0700 Subject: [PATCH 56/56] Remove comments --- crates/fuel-core/src/executor.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 94567227d98..d8c60da6c8b 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -3252,15 +3252,6 @@ mod tests { // and let consensus_params = ConsensusParameters::default(); - // let actual_max_gas = if let CheckedMetadata::Script(metadata) = tx - // .into_checked(block_height.into(), &consensus_params) - // .unwrap() - // .metadata() - // { - // metadata.max_gas - // } else { - // panic!("Expected `CheckedMetadata::Script`") - // }; let actual_max_gas = tx .as_script() .unwrap()