From 31ec3ecb034cf990a5c3409ca730ae73384f86bf Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Tue, 16 Apr 2024 18:54:07 +0300 Subject: [PATCH 1/2] Remove SpentMessages table; populate ProcessedTransactions on regenesis --- crates/fuel-core/src/database/message.rs | 16 +------- crates/fuel-core/src/executor.rs | 29 ++++++-------- crates/fuel-core/src/graphql_api/database.rs | 4 -- crates/fuel-core/src/graphql_api/ports.rs | 2 - crates/fuel-core/src/query/message.rs | 8 ++-- crates/fuel-core/src/schema/message.rs | 12 +++--- .../service/adapters/graphql_api/on_chain.rs | 4 -- .../fuel-core/src/service/adapters/txpool.rs | 5 --- .../src/service/genesis/importer/on_chain.rs | 3 ++ crates/services/executor/src/executor.rs | 20 ++-------- .../txpool/src/containers/dependency.rs | 7 ---- crates/services/txpool/src/mock_db.rs | 4 -- crates/services/txpool/src/ports.rs | 2 - .../src/structured_storage/messages.rs | 20 +--------- crates/storage/src/tables.rs | 10 ----- crates/types/src/entities/relayer/message.rs | 39 ++----------------- crates/types/src/services/executor.rs | 6 +-- 17 files changed, 36 insertions(+), 155 deletions(-) diff --git a/crates/fuel-core/src/database/message.rs b/crates/fuel-core/src/database/message.rs index 270ce465382..b66e40bb17f 100644 --- a/crates/fuel-core/src/database/message.rs +++ b/crates/fuel-core/src/database/message.rs @@ -14,10 +14,7 @@ use fuel_core_storage::{ IterDirection, IteratorOverTable, }, - tables::{ - Messages, - SpentMessages, - }, + tables::Messages, Error as StorageError, Result as StorageResult, }; @@ -63,13 +60,8 @@ impl Database { ) -> impl Iterator>> + '_ { self.iter_all_by_start::(None, None) .filter_map(|msg| { - // Return only unspent messages if let Ok(msg) = msg { - match self.message_is_spent(msg.1.id()) { - Ok(false) => Some(Ok(msg)), - Ok(true) => None, - Err(e) => Some(Err(e)), - } + Some(Ok(msg)) } else { Some(msg.map_err(StorageError::from)) } @@ -77,10 +69,6 @@ impl Database { .map_ok(|(key, value)| TableEntry { key, value }) } - pub fn message_is_spent(&self, id: &Nonce) -> StorageResult { - fuel_core_storage::StorageAsRef::storage::(&self).contains_key(id) - } - pub fn message_exists(&self, id: &Nonce) -> StorageResult { fuel_core_storage::StorageAsRef::storage::(&self).contains_key(id) } diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index d8c60da6c8b..94d890433b0 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -2372,8 +2372,8 @@ mod tests { let mut exec = make_executor(&messages); let view = exec.storage_view_provider.latest_view(); - assert!(!view.message_is_spent(message_coin.nonce()).unwrap()); - assert!(!view.message_is_spent(message_data.nonce()).unwrap()); + assert!(view.message_exists(message_coin.nonce()).unwrap()); + assert!(view.message_exists(message_data.nonce()).unwrap()); let ExecutionResult { skipped_transactions, @@ -2385,8 +2385,8 @@ mod tests { // Successful execution consumes `message_coin` and `message_data`. let view = exec.storage_view_provider.latest_view(); - assert!(view.message_is_spent(message_coin.nonce()).unwrap()); - assert!(view.message_is_spent(message_data.nonce()).unwrap()); + assert!(!view.message_exists(message_coin.nonce()).unwrap()); + assert!(!view.message_exists(message_data.nonce()).unwrap()); assert_eq!( *view.coin(&UtxoId::new(tx_id, 0)).unwrap().amount(), amount + amount @@ -2421,8 +2421,8 @@ mod tests { let mut exec = make_executor(&messages); let view = exec.storage_view_provider.latest_view(); - assert!(!view.message_is_spent(message_coin.nonce()).unwrap()); - assert!(!view.message_is_spent(message_data.nonce()).unwrap()); + assert!(view.message_exists(message_coin.nonce()).unwrap()); + assert!(view.message_exists(message_data.nonce()).unwrap()); let ExecutionResult { skipped_transactions, @@ -2434,8 +2434,8 @@ mod tests { // We should spend only `message_coin`. The `message_data` should be unspent. let view = exec.storage_view_provider.latest_view(); - assert!(view.message_is_spent(message_coin.nonce()).unwrap()); - assert!(!view.message_is_spent(message_data.nonce()).unwrap()); + assert!(!view.message_exists(message_coin.nonce()).unwrap()); + assert!(view.message_exists(message_data.nonce()).unwrap()); assert_eq!(*view.coin(&UtxoId::new(tx_id, 0)).unwrap().amount(), amount); } @@ -2579,10 +2579,11 @@ mod tests { // One of two transactions is skipped. assert_eq!(skipped_transactions.len(), 1); let err = &skipped_transactions[0].1; + dbg!(err); assert!(matches!( err, &ExecutorError::TransactionValidity( - TransactionValidityError::MessageAlreadySpent(_) + TransactionValidityError::MessageDoesNotExist(_) ) )); @@ -2601,7 +2602,7 @@ mod tests { assert!(matches!( res, Err(ExecutorError::TransactionValidity( - TransactionValidityError::MessageAlreadySpent(_) + TransactionValidityError::MessageDoesNotExist(_) )) )); } @@ -2829,10 +2830,7 @@ mod tests { use fuel_core_relayer::storage::EventsHistory; use fuel_core_storage::{ iter::IteratorOverTable, - tables::{ - FuelBlocks, - SpentMessages, - }, + tables::FuelBlocks, StorageAsMut, }; use fuel_core_types::{ @@ -3603,7 +3601,6 @@ mod tests { // Given assert_eq!(on_chain_db.iter_all::(None).count(), 0); - assert_eq!(on_chain_db.iter_all::(None).count(), 0); let tx = TransactionBuilder::script(vec![], vec![]) .script_gas_limit(10) .add_unsigned_message_input( @@ -3628,8 +3625,6 @@ mod tests { let view = ChangesIterator::::new(&changes); assert!(result.skipped_transactions.is_empty()); assert_eq!(view.iter_all::(None).count() as u64, 0); - // Message added during this block immediately became spent. - assert_eq!(view.iter_all::(None).count(), 1); assert_eq!(result.events.len(), 2); assert!(matches!( result.events[0], diff --git a/crates/fuel-core/src/graphql_api/database.rs b/crates/fuel-core/src/graphql_api/database.rs index cefccb93dad..26af35802c0 100644 --- a/crates/fuel-core/src/graphql_api/database.rs +++ b/crates/fuel-core/src/graphql_api/database.rs @@ -196,10 +196,6 @@ impl DatabaseMessages for ReadView { self.on_chain.all_messages(start_message_id, direction) } - fn message_is_spent(&self, nonce: &Nonce) -> StorageResult { - self.on_chain.message_is_spent(nonce) - } - fn message_exists(&self, nonce: &Nonce) -> StorageResult { self.on_chain.message_exists(nonce) } diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 809629e1022..cbc8f11d875 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -147,8 +147,6 @@ pub trait DatabaseMessages: StorageInspect { direction: IterDirection, ) -> BoxedIter<'_, StorageResult>; - fn message_is_spent(&self, nonce: &Nonce) -> StorageResult; - fn message_exists(&self, nonce: &Nonce) -> StorageResult; } diff --git a/crates/fuel-core/src/query/message.rs b/crates/fuel-core/src/query/message.rs index b9e527272b0..95f8bdef392 100644 --- a/crates/fuel-core/src/query/message.rs +++ b/crates/fuel-core/src/query/message.rs @@ -285,11 +285,9 @@ pub fn message_status( database: &T, message_nonce: Nonce, ) -> StorageResult { - if database.message_is_spent(&message_nonce)? { - Ok(MessageStatus::spent()) - } else if database.message_exists(&message_nonce)? { - Ok(MessageStatus::unspent()) + if database.message_exists(&message_nonce)? { + Ok(MessageStatus::Unspent) } else { - Ok(MessageStatus::not_found()) + Ok(MessageStatus::SpentOrNonExistent) } } diff --git a/crates/fuel-core/src/schema/message.rs b/crates/fuel-core/src/schema/message.rs index f2dfc16639e..8a1fc491a06 100644 --- a/crates/fuel-core/src/schema/message.rs +++ b/crates/fuel-core/src/schema/message.rs @@ -234,17 +234,17 @@ pub struct MessageStatus(pub(crate) entities::relayer::message::MessageStatus); #[derive(Enum, Copy, Clone, Eq, PartialEq)] enum MessageState { Unspent, - Spent, - NotFound, + SpentOrNonExistent, } #[Object] impl MessageStatus { async fn state(&self) -> MessageState { - match self.0.state { - entities::relayer::message::MessageState::Unspent => MessageState::Unspent, - entities::relayer::message::MessageState::Spent => MessageState::Spent, - entities::relayer::message::MessageState::NotFound => MessageState::NotFound, + match self.0 { + entities::relayer::message::MessageStatus::Unspent => MessageState::Unspent, + entities::relayer::message::MessageStatus::SpentOrNonExistent => { + MessageState::SpentOrNonExistent + } } } } diff --git a/crates/fuel-core/src/service/adapters/graphql_api/on_chain.rs b/crates/fuel-core/src/service/adapters/graphql_api/on_chain.rs index 1d47947c0fd..61dd7c998af 100644 --- a/crates/fuel-core/src/service/adapters/graphql_api/on_chain.rs +++ b/crates/fuel-core/src/service/adapters/graphql_api/on_chain.rs @@ -70,10 +70,6 @@ impl DatabaseMessages for Database { .into_boxed() } - fn message_is_spent(&self, nonce: &Nonce) -> StorageResult { - self.message_is_spent(nonce) - } - fn message_exists(&self, nonce: &Nonce) -> StorageResult { self.message_exists(nonce) } diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs index ee241fafbfd..2caebd15619 100644 --- a/crates/fuel-core/src/service/adapters/txpool.rs +++ b/crates/fuel-core/src/service/adapters/txpool.rs @@ -13,7 +13,6 @@ use fuel_core_storage::{ Coins, ContractsRawCode, Messages, - SpentMessages, }, Result as StorageResult, StorageAsRef, @@ -135,10 +134,6 @@ impl fuel_core_txpool::ports::TxPoolDb for Database { .get(id) .map(|t| t.map(|t| t.as_ref().clone())) } - - fn is_message_spent(&self, id: &Nonce) -> StorageResult { - self.storage::().contains_key(id) - } } impl GasPriceProvider for StaticGasPrice { diff --git a/crates/fuel-core/src/service/genesis/importer/on_chain.rs b/crates/fuel-core/src/service/genesis/importer/on_chain.rs index a6486fa6f20..15aa6a11bb4 100644 --- a/crates/fuel-core/src/service/genesis/importer/on_chain.rs +++ b/crates/fuel-core/src/service/genesis/importer/on_chain.rs @@ -18,6 +18,7 @@ use fuel_core_storage::{ ContractsRawCode, ContractsState, Messages, + ProcessedTransactions, Transactions, }, transactional::StorageTransaction, @@ -143,6 +144,8 @@ impl ImportTable for Handler { for transaction in &group { tx.storage::() .insert(&transaction.key, &transaction.value)?; + tx.storage::() + .insert(&transaction.key, &())?; } Ok(()) } diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 5feb1ebbac2..a828c5cb385 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -18,7 +18,6 @@ use fuel_core_storage::{ FuelBlocks, Messages, ProcessedTransactions, - SpentMessages, }, transactional::{ Changes, @@ -1271,12 +1270,6 @@ where | Input::MessageCoinPredicate(MessageCoinPredicate { nonce, .. }) | Input::MessageDataSigned(MessageDataSigned { nonce, .. }) | Input::MessageDataPredicate(MessageDataPredicate { nonce, .. }) => { - // Eagerly return already spent if status is known. - 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( @@ -1360,19 +1353,12 @@ where | Input::MessageCoinPredicate(MessageCoinPredicate { nonce, .. }) | Input::MessageDataSigned(MessageDataSigned { nonce, .. }) | Input::MessageDataPredicate(MessageDataPredicate { nonce, .. }) => { - // `MessageDataSigned` and `MessageDataPredicate` are spent only if tx is not reverted - // mark message id as spent - let was_already_spent = - db.storage::().insert(nonce, &())?; - // ensure message wasn't already marked as spent - if was_already_spent.is_some() { - return Err(ExecutorError::MessageAlreadySpent(*nonce)) - } - // cleanup message contents + // ensure message wasn't already marked as spent, + // and cleanup message contents let message = db .storage::() .remove(nonce)? - .ok_or(ExecutorError::MessageAlreadySpent(*nonce))?; + .ok_or(ExecutorError::MessageDoesNotExist(*nonce))?; execution_data .events .push(ExecutorEvent::MessageConsumed(message)); diff --git a/crates/services/txpool/src/containers/dependency.rs b/crates/services/txpool/src/containers/dependency.rs index 84d53cec3fc..9801a16bcf7 100644 --- a/crates/services/txpool/src/containers/dependency.rs +++ b/crates/services/txpool/src/containers/dependency.rs @@ -372,13 +372,6 @@ impl Dependency { { return Err(Error::NotInsertedIoMessageMismatch) } - // return an error if spent block is set - if db - .is_message_spent(nonce) - .map_err(|e| Error::Database(format!("{:?}", e)))? - { - return Err(Error::NotInsertedInputMessageSpent(*nonce)) - } } else { return Err(Error::NotInsertedInputMessageUnknown(*nonce)) } diff --git a/crates/services/txpool/src/mock_db.rs b/crates/services/txpool/src/mock_db.rs index 962a59ed9c0..a7b8406b485 100644 --- a/crates/services/txpool/src/mock_db.rs +++ b/crates/services/txpool/src/mock_db.rs @@ -84,10 +84,6 @@ impl TxPoolDb for MockDb { fn message(&self, id: &Nonce) -> StorageResult> { Ok(self.data.lock().unwrap().messages.get(id).cloned()) } - - fn is_message_spent(&self, id: &Nonce) -> StorageResult { - Ok(self.data.lock().unwrap().spent_messages.contains(id)) - } } pub struct MockDBProvider(pub MockDb); diff --git a/crates/services/txpool/src/ports.rs b/crates/services/txpool/src/ports.rs index c24e4c9eb71..3a2a5f4ff24 100644 --- a/crates/services/txpool/src/ports.rs +++ b/crates/services/txpool/src/ports.rs @@ -58,8 +58,6 @@ pub trait TxPoolDb: Send + Sync { fn contract_exist(&self, contract_id: &ContractId) -> StorageResult; fn message(&self, message_id: &Nonce) -> StorageResult>; - - fn is_message_spent(&self, message_id: &Nonce) -> StorageResult; } /// Trait for getting gas price for the Tx Pool code to look up the gas price for a given block height diff --git a/crates/storage/src/structured_storage/messages.rs b/crates/storage/src/structured_storage/messages.rs index 78d92c66a17..04f4b67647d 100644 --- a/crates/storage/src/structured_storage/messages.rs +++ b/crates/storage/src/structured_storage/messages.rs @@ -8,10 +8,7 @@ use crate::{ }, column::Column, structured_storage::TableWithBlueprint, - tables::{ - Messages, - SpentMessages, - }, + tables::Messages, }; impl TableWithBlueprint for Messages { @@ -23,15 +20,6 @@ impl TableWithBlueprint for Messages { } } -impl TableWithBlueprint for SpentMessages { - type Blueprint = Plain; - type Column = Column; - - fn column() -> Column { - Column::SpentMessages - } -} - #[cfg(test)] mod test { use super::*; @@ -41,10 +29,4 @@ mod test { ::Key::default(), ::Value::default() ); - - crate::basic_storage_tests!( - SpentMessages, - ::Key::default(), - ::Value::default() - ); } diff --git a/crates/storage/src/tables.rs b/crates/storage/src/tables.rs index a3b95a79266..9daef59ff6e 100644 --- a/crates/storage/src/tables.rs +++ b/crates/storage/src/tables.rs @@ -92,16 +92,6 @@ impl Mappable for Messages { type OwnedValue = Message; } -/// The storage table that indicates if the message is spent or not. -pub struct SpentMessages; - -impl Mappable for SpentMessages { - type Key = Self::OwnedKey; - type OwnedKey = Nonce; - type Value = Self::OwnedValue; - type OwnedValue = (); -} - /// The storage table of confirmed transactions. pub struct Transactions; diff --git a/crates/types/src/entities/relayer/message.rs b/crates/types/src/entities/relayer/message.rs index fee7440136d..0d74a7fef99 100644 --- a/crates/types/src/entities/relayer/message.rs +++ b/crates/types/src/entities/relayer/message.rs @@ -273,40 +273,9 @@ impl MessageProof { } /// Represents the status of a message -pub struct MessageStatus { - /// The message state - pub state: MessageState, -} - -impl MessageStatus { - /// Constructor for `MessageStatus` that fills with `Unspent` state - pub fn unspent() -> Self { - Self { - state: MessageState::Unspent, - } - } - - /// Constructor for `MessageStatus` that fills with `Spent` state - pub fn spent() -> Self { - Self { - state: MessageState::Spent, - } - } - - /// Constructor for `MessageStatus` that fills with `Unknown` state - pub fn not_found() -> Self { - Self { - state: MessageState::NotFound, - } - } -} - -/// The possible states a Message can be in -pub enum MessageState { - /// Message is still unspent +pub enum MessageStatus { + /// The message is unspent Unspent, - /// Message has already been spent - Spent, - /// There is no record of this Message - NotFound, + /// The message is either already spent, or does not exist + SpentOrNonExistent, } diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index dd456a706ff..040b7ff7765 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -387,7 +387,7 @@ pub enum Error { #[display(fmt = "No matching utxo for contract id ${_0:#x}")] ContractUtxoMissing(ContractId), #[display(fmt = "message already spent {_0:#x}")] - MessageAlreadySpent(Nonce), + MessageDoesNotExist(Nonce), #[display(fmt = "Expected input of type {_0}")] InputTypeMismatch(String), #[display(fmt = "Executing of the genesis block is not allowed")] @@ -434,13 +434,11 @@ pub enum TransactionValidityError { CoinMismatch(UtxoId), #[error("The specified coin({0:#x}) doesn't exist")] CoinDoesNotExist(UtxoId), - #[error("The specified message({0:#x}) was already spent")] - MessageAlreadySpent(Nonce), #[error( "Message({0:#x}) is not yet spendable, as it's DA height is newer than this block allows" )] MessageSpendTooEarly(Nonce), - #[error("The specified message({0:#x}) doesn't exist")] + #[error("The specified message({0:#x}) doesn't exist, possibly because it was already spent")] MessageDoesNotExist(Nonce), #[error("The input message({0:#x}) doesn't match the relayer message")] MessageMismatch(Nonce), From 435d1c66c0f8e3598de68835dd9da802ff440f9c Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Wed, 17 Apr 2024 03:40:10 +0300 Subject: [PATCH 2/2] Remove ProcessedTransactions migration from this PR --- crates/fuel-core/src/service/genesis/importer/on_chain.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/fuel-core/src/service/genesis/importer/on_chain.rs b/crates/fuel-core/src/service/genesis/importer/on_chain.rs index 15aa6a11bb4..13d68356e46 100644 --- a/crates/fuel-core/src/service/genesis/importer/on_chain.rs +++ b/crates/fuel-core/src/service/genesis/importer/on_chain.rs @@ -144,8 +144,6 @@ impl ImportTable for Handler { for transaction in &group { tx.storage::() .insert(&transaction.key, &transaction.value)?; - tx.storage::() - .insert(&transaction.key, &())?; } Ok(()) }