Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SpentMessages table #1829

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 2 additions & 14 deletions crates/fuel-core/src/database/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ use fuel_core_storage::{
IterDirection,
IteratorOverTable,
},
tables::{
Messages,
SpentMessages,
},
tables::Messages,
Error as StorageError,
Result as StorageResult,
};
Expand Down Expand Up @@ -63,24 +60,15 @@ impl Database {
) -> impl Iterator<Item = StorageResult<TableEntry<Messages>>> + '_ {
self.iter_all_by_start::<Messages>(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))
}
})
.map_ok(|(key, value)| TableEntry { key, value })
}

pub fn message_is_spent(&self, id: &Nonce) -> StorageResult<bool> {
fuel_core_storage::StorageAsRef::storage::<SpentMessages>(&self).contains_key(id)
}

pub fn message_exists(&self, id: &Nonce) -> StorageResult<bool> {
fuel_core_storage::StorageAsRef::storage::<Messages>(&self).contains_key(id)
}
Expand Down
29 changes: 12 additions & 17 deletions crates/fuel-core/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}

Expand Down Expand Up @@ -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(_)
)
));

Expand All @@ -2601,7 +2602,7 @@ mod tests {
assert!(matches!(
res,
Err(ExecutorError::TransactionValidity(
TransactionValidityError::MessageAlreadySpent(_)
TransactionValidityError::MessageDoesNotExist(_)
))
));
}
Expand Down Expand Up @@ -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::{
Expand Down Expand Up @@ -3603,7 +3601,6 @@ mod tests {

// Given
assert_eq!(on_chain_db.iter_all::<Messages>(None).count(), 0);
assert_eq!(on_chain_db.iter_all::<SpentMessages>(None).count(), 0);
let tx = TransactionBuilder::script(vec![], vec![])
.script_gas_limit(10)
.add_unsigned_message_input(
Expand All @@ -3628,8 +3625,6 @@ mod tests {
let view = ChangesIterator::<OnChain>::new(&changes);
assert!(result.skipped_transactions.is_empty());
assert_eq!(view.iter_all::<Messages>(None).count() as u64, 0);
// Message added during this block immediately became spent.
assert_eq!(view.iter_all::<SpentMessages>(None).count(), 1);
assert_eq!(result.events.len(), 2);
assert!(matches!(
result.events[0],
Expand Down
4 changes: 0 additions & 4 deletions crates/fuel-core/src/graphql_api/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
self.on_chain.message_is_spent(nonce)
}

fn message_exists(&self, nonce: &Nonce) -> StorageResult<bool> {
self.on_chain.message_exists(nonce)
}
Expand Down
2 changes: 0 additions & 2 deletions crates/fuel-core/src/graphql_api/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,6 @@ pub trait DatabaseMessages: StorageInspect<Messages, Error = StorageError> {
direction: IterDirection,
) -> BoxedIter<'_, StorageResult<Message>>;

fn message_is_spent(&self, nonce: &Nonce) -> StorageResult<bool>;

fn message_exists(&self, nonce: &Nonce) -> StorageResult<bool>;
}

Expand Down
8 changes: 3 additions & 5 deletions crates/fuel-core/src/query/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,11 +285,9 @@ pub fn message_status<T: DatabaseMessages + ?Sized>(
database: &T,
message_nonce: Nonce,
) -> StorageResult<MessageStatus> {
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)
}
}
12 changes: 6 additions & 6 deletions crates/fuel-core/src/schema/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions crates/fuel-core/src/service/adapters/graphql_api/on_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ impl DatabaseMessages for Database {
.into_boxed()
}

fn message_is_spent(&self, nonce: &Nonce) -> StorageResult<bool> {
self.message_is_spent(nonce)
}

fn message_exists(&self, nonce: &Nonce) -> StorageResult<bool> {
self.message_exists(nonce)
}
Expand Down
5 changes: 0 additions & 5 deletions crates/fuel-core/src/service/adapters/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use fuel_core_storage::{
Coins,
ContractsRawCode,
Messages,
SpentMessages,
},
Result as StorageResult,
StorageAsRef,
Expand Down Expand Up @@ -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<bool> {
self.storage::<SpentMessages>().contains_key(id)
}
}

impl GasPriceProvider for StaticGasPrice {
Expand Down
3 changes: 3 additions & 0 deletions crates/fuel-core/src/service/genesis/importer/on_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use fuel_core_storage::{
ContractsRawCode,
ContractsState,
Messages,
ProcessedTransactions,
Transactions,
},
transactional::StorageTransaction,
Expand Down Expand Up @@ -143,6 +144,8 @@ impl ImportTable for Handler<Transactions> {
for transaction in &group {
tx.storage::<Transactions>()
.insert(&transaction.key, &transaction.value)?;
tx.storage::<ProcessedTransactions>()
.insert(&transaction.key, &())?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this migration even shouldn't exist because we have OldTransactions for that. I think we forgot to remove it during #1811.

Hmm, it seems we need to migrate the ProcessedTransactions table instead of fetching it from OldTransactions because we can't rely on off-chain data during migration. Off-chain data migration is optional and only for nodes that want to support GraphQL API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1811 is not merged yet, so we can still include that there. I'm going to remove this for now, we can do that in a separate PR if not in #1811. Removed in 435d1c6.

}
Ok(())
}
Expand Down
20 changes: 3 additions & 17 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use fuel_core_storage::{
FuelBlocks,
Messages,
ProcessedTransactions,
SpentMessages,
},
transactional::{
Changes,
Expand Down Expand Up @@ -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::<SpentMessages>().contains_key(nonce)? {
return Err(
TransactionValidityError::MessageAlreadySpent(*nonce).into()
);
}
if let Some(message) = db.storage::<Messages>().get(nonce)? {
if message.da_height() > block_da_height {
return Err(TransactionValidityError::MessageSpendTooEarly(
Expand Down Expand Up @@ -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::<SpentMessages>().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::<Messages>()
.remove(nonce)?
.ok_or(ExecutorError::MessageAlreadySpent(*nonce))?;
.ok_or(ExecutorError::MessageDoesNotExist(*nonce))?;
execution_data
.events
.push(ExecutorEvent::MessageConsumed(message));
Expand Down
7 changes: 0 additions & 7 deletions crates/services/txpool/src/containers/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
4 changes: 0 additions & 4 deletions crates/services/txpool/src/mock_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ impl TxPoolDb for MockDb {
fn message(&self, id: &Nonce) -> StorageResult<Option<Message>> {
Ok(self.data.lock().unwrap().messages.get(id).cloned())
}

fn is_message_spent(&self, id: &Nonce) -> StorageResult<bool> {
Ok(self.data.lock().unwrap().spent_messages.contains(id))
}
}

pub struct MockDBProvider(pub MockDb);
Expand Down
2 changes: 0 additions & 2 deletions crates/services/txpool/src/ports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ pub trait TxPoolDb: Send + Sync {
fn contract_exist(&self, contract_id: &ContractId) -> StorageResult<bool>;

fn message(&self, message_id: &Nonce) -> StorageResult<Option<Message>>;

fn is_message_spent(&self, message_id: &Nonce) -> StorageResult<bool>;
}

/// Trait for getting gas price for the Tx Pool code to look up the gas price for a given block height
Expand Down
20 changes: 1 addition & 19 deletions crates/storage/src/structured_storage/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ use crate::{
},
column::Column,
structured_storage::TableWithBlueprint,
tables::{
Messages,
SpentMessages,
},
tables::Messages,
};

impl TableWithBlueprint for Messages {
Expand All @@ -23,15 +20,6 @@ impl TableWithBlueprint for Messages {
}
}

impl TableWithBlueprint for SpentMessages {
type Blueprint = Plain<Raw, Postcard>;
type Column = Column;

fn column() -> Column {
Column::SpentMessages
}
}

#[cfg(test)]
mod test {
use super::*;
Expand All @@ -41,10 +29,4 @@ mod test {
<Messages as crate::Mappable>::Key::default(),
<Messages as crate::Mappable>::Value::default()
);

crate::basic_storage_tests!(
SpentMessages,
<SpentMessages as crate::Mappable>::Key::default(),
<SpentMessages as crate::Mappable>::Value::default()
);
}
10 changes: 0 additions & 10 deletions crates/storage/src/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading
Loading