Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
1 change: 1 addition & 0 deletions .changes/fixed/3264.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rollback stale preconfirmations in the mempool when the canonical block at that height omits the preconfirmed transactions, restoring spent inputs and removing dependent transactions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/services/txpool_v2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ tracing = { workspace = true }
[dev-dependencies]
fuel-core-storage = { workspace = true, features = ["std", "test-helpers"] }
fuel-core-trace = { path = "../../trace" }
fuel-core-txpool = { path = ".", features = ["test-helpers"] }
mockall = { workspace = true }
proptest = { workspace = true }
rand = { workspace = true }
Expand Down
63 changes: 60 additions & 3 deletions crates/services/txpool_v2/src/collision_manager/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use fuel_core_types::{
CoinPredicate,
CoinSigned,
},
contract::Contract as ContractInput,
message::{
MessageCoinPredicate,
MessageCoinSigned,
Expand Down Expand Up @@ -57,6 +58,9 @@ pub struct BasicCollisionManager<StorageIndex> {
coins_spenders: BTreeMap<UtxoId, StorageIndex>,
/// Contract -> Transaction that currently create the contract
contracts_creators: HashMap<ContractId, StorageIndex>,
/// Contract -> Transactions (by TxId) that currently use the contract as an input.
/// Symmetric to `contracts_creators`; used to evict dependents during rollback.
contract_users: HashMap<ContractId, Vec<TxId>>,
/// Blob -> Transaction that currently create the blob
blobs_users: HashMap<BlobId, StorageIndex>,
}
Expand All @@ -67,6 +71,7 @@ impl<StorageIndex> BasicCollisionManager<StorageIndex> {
messages_spenders: HashMap::new(),
coins_spenders: BTreeMap::new(),
contracts_creators: HashMap::new(),
contract_users: HashMap::new(),
Comment thread
cursor[bot] marked this conversation as resolved.
blobs_users: HashMap::new(),
}
}
Expand All @@ -76,6 +81,7 @@ impl<StorageIndex> BasicCollisionManager<StorageIndex> {
self.messages_spenders.is_empty()
&& self.coins_spenders.is_empty()
&& self.contracts_creators.is_empty()
&& self.contract_users.is_empty()
&& self.blobs_users.is_empty()
}

Expand All @@ -88,6 +94,7 @@ impl<StorageIndex> BasicCollisionManager<StorageIndex> {
let mut message_spenders = HashMap::new();
let mut coins_spenders = BTreeMap::new();
let mut contracts_creators = HashMap::new();
let mut contract_users: HashMap<ContractId, Vec<TxId>> = HashMap::new();
let mut blobs_users = HashMap::new();
for tx in expected_txs {
if let PoolTransaction::Blob(checked_tx, _) = tx.deref() {
Expand All @@ -110,7 +117,12 @@ impl<StorageIndex> BasicCollisionManager<StorageIndex> {
}) => {
message_spenders.insert(*nonce, tx.id());
}
Input::Contract { .. } => {}
Input::Contract(ContractInput { contract_id, .. }) => {
contract_users
.entry(*contract_id)
.or_default()
.push(tx.id());
}
}
}
for output in tx.outputs() {
Expand Down Expand Up @@ -152,6 +164,26 @@ impl<StorageIndex> BasicCollisionManager<StorageIndex> {
"Some contract creators are missing from the collision manager: {:?}",
contracts_creators
);
for (contract_id, users) in &self.contract_users {
let expected = contract_users.remove(contract_id).unwrap_or_else(|| panic!(
"A contract ({}) user list is present on the collision manager that shouldn't be there.",
contract_id
));
let mut actual_sorted = users.clone();
actual_sorted.sort();
let mut expected_sorted = expected;
expected_sorted.sort();
assert_eq!(
actual_sorted, expected_sorted,
"contract_users mismatch for contract {}",
contract_id
);
}
assert!(
contract_users.is_empty(),
"Some contract users are missing from the collision manager: {:?}",
contract_users
);
}
}

Expand All @@ -174,6 +206,17 @@ where
.collect()
}

fn get_contract_users(&self, contract_id: &ContractId) -> Vec<TxId> {
self.contract_users
.get(contract_id)
.cloned()
.unwrap_or_default()
}

fn contract_created_in_pool(&self, contract_id: &ContractId) -> bool {
self.contracts_creators.contains_key(contract_id)
}

fn find_collisions(
&self,
transaction: &PoolTransaction,
Expand Down Expand Up @@ -248,6 +291,7 @@ where
let blob_id = checked_tx.transaction().blob_id();
self.blobs_users.insert(*blob_id, storage_id);
}
let tx_id = store_entry.transaction.id();
for input in store_entry.transaction.inputs() {
match input {
Input::CoinSigned(CoinSigned { utxo_id, .. })
Expand All @@ -262,7 +306,12 @@ where
// insert message
self.messages_spenders.insert(*nonce, storage_id);
}
_ => {}
Input::Contract(ContractInput { contract_id, .. }) => {
self.contract_users
.entry(*contract_id)
.or_default()
.push(tx_id);
}
}
}
for output in store_entry.transaction.outputs().iter() {
Expand All @@ -284,6 +333,7 @@ where
let blob_id = checked_tx.transaction().blob_id();
self.blobs_users.remove(blob_id);
}
let tx_id = transaction.id();
for input in transaction.inputs() {
match input {
Input::CoinSigned(CoinSigned { utxo_id, .. })
Expand All @@ -298,7 +348,14 @@ where
// remove message
self.messages_spenders.remove(nonce);
}
_ => {}
Input::Contract(ContractInput { contract_id, .. }) => {
if let Some(users) = self.contract_users.get_mut(contract_id) {
users.retain(|id| id != &tx_id);
if users.is_empty() {
self.contract_users.remove(contract_id);
}
}
}
Comment on lines +351 to +358
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think removing transaction IDs here, you will not have access to them during rollback_preconfirmed_transaction. Because we remove it when a transaction is included in the block during production, while you call rollback_preconfirmed_transaction after block is done, so you will not have access to them. It only should affect authority, I guess, so maybe it is fine. I think sentries should still be able to work with it. But maybe we want to move this clean up int oprocess_block

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Follow-up: #3268

}
}
for output in transaction.outputs().iter() {
Expand Down
12 changes: 12 additions & 0 deletions crates/services/txpool_v2/src/collision_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use fuel_core_types::{
};
use std::collections::HashMap;

use fuel_core_types::fuel_tx::ContractId;

use crate::storage::StorageData;

pub mod basic;
Expand All @@ -27,6 +29,16 @@ pub trait CollisionManager {
/// Get spenders of coins UTXO created by a transaction ID.
fn get_coins_spenders(&self, tx_creator_id: &TxId) -> Vec<Self::StorageIndex>;

/// Get the IDs of pool transactions that have `Input::Contract(contract_id)`.
/// Used during preconfirmation rollback to evict pool txs that were admitted
/// only because a preconfirmed tx temporarily created the contract.
fn get_contract_users(&self, contract_id: &ContractId) -> Vec<TxId>;

/// Returns true if a currently in-pool transaction creates `contract_id`.
/// Used during rollback to skip eviction when the contract is still available
/// via a pool tx (independent of the rolled-back preconfirmation).
fn contract_created_in_pool(&self, contract_id: &ContractId) -> bool;

/// Inform the collision manager that a transaction was stored.
fn on_stored_transaction(
&mut self,
Expand Down
18 changes: 17 additions & 1 deletion crates/services/txpool_v2/src/extracted_outputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ impl ExtractedOutputs {
for (utxo_id, output) in outputs {
match output {
Output::ContractCreated { contract_id, .. } => {
self.contract_created.insert(*contract_id, *utxo_id.tx_id());
let tx_id = *utxo_id.tx_id();
self.contract_created.insert(*contract_id, tx_id);
// Track the reverse mapping so cleanup via new_executed_transaction works.
self.contract_created_by_tx
.entry(tx_id)
.or_default()
.push(*contract_id);
}
Output::Coin {
to,
Expand Down Expand Up @@ -131,6 +137,16 @@ impl ExtractedOutputs {
self.new_executed_transaction(tx_id);
}

/// Returns the contract IDs created by `tx_id`, if any.
/// Call this **before** [`new_skipped_transaction`] / [`new_executed_transaction`]
/// if the caller needs the list for cleanup.
pub fn contracts_created_by(&self, tx_id: &TxId) -> &[ContractId] {
self.contract_created_by_tx
.get(tx_id)
.map(Vec::as_slice)
.unwrap_or(&[])
}

pub fn new_executed_transaction(&mut self, tx_id: &TxId) {
let contract_ids = self.contract_created_by_tx.remove(tx_id);
if let Some(contract_ids) = contract_ids {
Expand Down
3 changes: 3 additions & 0 deletions crates/services/txpool_v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ mod spent_inputs;
mod tests;
#[cfg(test)]
fuel_core_trace::enable_tracing!();
// Needed to activate the `test-helpers` feature flag for integration tests.
#[cfg(test)]
use fuel_core_txpool as _;

use fuel_core_types::fuel_asm::Word;
pub use pool::TxPoolStats;
Expand Down
Loading
Loading