Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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