From ccae2d50c4d694a8c77500d2843a3322f694e80b Mon Sep 17 00:00:00 2001 From: AurelienFT <32803821+AurelienFT@users.noreply.github.com> Date: Wed, 13 Nov 2024 21:40:50 +0100 Subject: [PATCH] Improve TxPool tests and documentation (#2327) ## Linked Issues/PRs Closes https://github.com/FuelLabs/fuel-core/issues/762 ## Description This PR improves the tests to make sure all the structures that stores transactions/caches have the expected state after the execution of a test. We also added an high level documentation for users and contributors at the root of the pool ## Checklist - [x] Breaking changes are clearly marked as such in the PR description and changelog - [x] New behavior is reflected in tests - [x] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [x] I have reviewed the code myself - [x] I have created follow-up issues caused by this PR and linked them here --------- Co-authored-by: Andrea Cerone Co-authored-by: Green Baneling --- CHANGELOG.md | 2 +- .../txpool_v2/src/collision_manager/basic.rs | 78 ++++++++++++++++ crates/services/txpool_v2/src/lib.rs | 40 ++++++++- crates/services/txpool_v2/src/pool.rs | 19 ++++ .../src/selection_algorithms/ratio_tip_gas.rs | 27 ++++++ .../services/txpool_v2/src/storage/graph.rs | 88 ++++++++++++++++++ crates/services/txpool_v2/src/tests/mod.rs | 2 + .../txpool_v2/src/tests/stability_test.rs | 1 - .../txpool_v2/src/tests/tests_pool.rs | 89 +++++++++++++------ .../services/txpool_v2/src/tests/universe.rs | 33 ++++++- 10 files changed, 346 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 053d3258a87..4385b4daae4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2362](https://github.com/FuelLabs/fuel-core/pull/2362): Added a new request_response protocol version `/fuel/req_res/0.0.2`. In comparison with `/fuel/req/0.0.1`, which returns an empty response when a request cannot be fulfilled, this version returns more meaningful error codes. Nodes still support the version `0.0.1` of the protocol to guarantee backward compatibility with fuel-core nodes. Empty responses received from nodes using the old protocol `/fuel/req/0.0.1` are automatically converted into an error `ProtocolV1EmptyResponse` with error code 0, which is also the only error code implemented. More specific error codes will be added in the future. - [2386](https://github.com/FuelLabs/fuel-core/pull/2386): Add a flag to define the maximum number of file descriptors that RocksDB can use. By default it's half of the OS limit. - [2376](https://github.com/FuelLabs/fuel-core/pull/2376): Add a way to fetch transactions in P2P without specifying a peer. +- [2327](https://github.com/FuelLabs/fuel-core/pull/2327): Add more services tests and more checks of the pool. Also add an high level documentation for users of the pool and contributors. ### Fixed - [2366](https://github.com/FuelLabs/fuel-core/pull/2366): The `importer_gas_price_for_block` metric is properly collected. @@ -56,7 +57,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2324](https://github.com/FuelLabs/fuel-core/pull/2324): Added metrics for sync, async processor and for all GraphQL queries. - [2320](https://github.com/FuelLabs/fuel-core/pull/2320): Added new CLI flag `graphql-max-resolver-recursive-depth` to limit recursion within resolver. The default value it "1". - ## Fixed - [2320](https://github.com/FuelLabs/fuel-core/issues/2320): Prevent `/health` and `/v1/health` from being throttled by the concurrency limiter. - [2322](https://github.com/FuelLabs/fuel-core/issues/2322): Set the salt of genesis contracts to zero on execution. diff --git a/crates/services/txpool_v2/src/collision_manager/basic.rs b/crates/services/txpool_v2/src/collision_manager/basic.rs index 0143c6687f2..bf971d34756 100644 --- a/crates/services/txpool_v2/src/collision_manager/basic.rs +++ b/crates/services/txpool_v2/src/collision_manager/basic.rs @@ -47,6 +47,9 @@ use super::{ Collisions, }; +#[cfg(test)] +use fuel_core_types::services::txpool::ArcPoolTx; + pub struct BasicCollisionManager { /// Message -> Transaction that currently use the Message messages_spenders: HashMap, @@ -75,6 +78,81 @@ impl BasicCollisionManager { && self.contracts_creators.is_empty() && self.blobs_users.is_empty() } + + #[cfg(test)] + /// Expected transactions are the transactions that must be populate all elements present in the collision manager. + /// This function will check if all elements present in the collision manager are present in the expected transactions and vice versa. + pub(crate) fn assert_integrity(&self, expected_txs: &[ArcPoolTx]) { + use std::ops::Deref; + + let mut message_spenders = HashMap::new(); + let mut coins_spenders = BTreeMap::new(); + let mut contracts_creators = HashMap::new(); + let mut blobs_users = HashMap::new(); + for tx in expected_txs { + if let PoolTransaction::Blob(checked_tx, _) = tx.deref() { + let blob_id = checked_tx.transaction().blob_id(); + blobs_users.insert(*blob_id, tx.id()); + } + for input in tx.inputs() { + match input { + Input::CoinSigned(CoinSigned { utxo_id, .. }) + | Input::CoinPredicate(CoinPredicate { utxo_id, .. }) => { + coins_spenders.insert(*utxo_id, tx.id()); + } + Input::MessageCoinSigned(MessageCoinSigned { nonce, .. }) + | Input::MessageCoinPredicate(MessageCoinPredicate { + nonce, .. + }) + | Input::MessageDataSigned(MessageDataSigned { nonce, .. }) + | Input::MessageDataPredicate(MessageDataPredicate { + nonce, .. + }) => { + message_spenders.insert(*nonce, tx.id()); + } + Input::Contract { .. } => {} + } + } + for output in tx.outputs() { + if let Output::ContractCreated { contract_id, .. } = output { + contracts_creators.insert(*contract_id, tx.id()); + } + } + } + for nonce in self.messages_spenders.keys() { + message_spenders.remove(nonce).unwrap_or_else(|| panic!( + "A message ({}) spender is present on the collision manager that shouldn't be there.", + nonce + )); + } + assert!( + message_spenders.is_empty(), + "Some message spenders are missing from the collision manager: {:?}", + message_spenders + ); + for utxo_id in self.coins_spenders.keys() { + coins_spenders.remove(utxo_id).unwrap_or_else(|| panic!( + "A coin ({}) spender is present on the collision manager that shouldn't be there.", + utxo_id + )); + } + assert!( + coins_spenders.is_empty(), + "Some coin senders are missing from the collision manager: {:?}", + coins_spenders + ); + for contract_id in self.contracts_creators.keys() { + contracts_creators.remove(contract_id).unwrap_or_else(|| panic!( + "A contract ({}) creator is present on the collision manager that shouldn't be there.", + contract_id + )); + } + assert!( + contracts_creators.is_empty(), + "Some contract creators are missing from the collision manager: {:?}", + contracts_creators + ); + } } impl Default for BasicCollisionManager { diff --git a/crates/services/txpool_v2/src/lib.rs b/crates/services/txpool_v2/src/lib.rs index d2530e00038..c462d43b760 100644 --- a/crates/services/txpool_v2/src/lib.rs +++ b/crates/services/txpool_v2/src/lib.rs @@ -1,10 +1,46 @@ +//! This crate manage the verification, storage, organisation and selection of the transactions for the network. +//! A transaction in Fuel has inputs and outputs. Inputs are outputs of previous transactions. +//! In a case where one of the input is an output of a transaction that has not been executed in a committed block (transaction still in the pool), +//! then the new transaction is considered dependent on that transaction. +//! +//! If a transaction has a dependency, it cannot be selected in a block until the dependent transaction has been selected. +//! A transaction can have a dependency per input and this dependency transaction can also have its own dependencies. +//! This creates a dependency tree between transactions inside the pool which can be very costly to compute for insertion/deletion etc... +//! In order to avoid too much cost, the transaction pool only allow a maximum number of transaction inside a dependency chain. +//! There is others limits on the pool that prevent its size to grow too much: maximum gas in the pool, maximum bytes in the pool, maximum number of transactions in the pool. +//! The pool also implements a TTL for the transactions, if a transaction is not selected in a block after a certain time, it is removed from the pool. +//! +//! All the transactions ordered by their ratio of gas/tip to be selected in a block. +//! It's possible that a transaction is not profitable enough to be selected for now and so either it will be selected later or it will be removed from the pool. +//! In order to make a transaction more likely to be selected, it's needed to submit a new colliding transaction (see below) with a higher tip/gas ratio. +//! +//! When a transaction is inserted it's possible that it use same inputs as one or multiple transactions already in the pool: this is what we call a collision. +//! The pool has to choose which transaction to keep and which to remove. +//! The pool will always try to maximize the number of transactions that can be selected in the next block and so +//! during collision resolution it will prioritize transactions without dependencies. +//! In a collision case, the transaction is considered a conflict and can be inserted under certain conditions : +//! - The transaction has dependencies: +//! - Can collide only with one other transaction. So, the user can submit +//! the same transaction with a higher tip but not merge one or more +//! transactions into one. +//! - A new transaction can be accepted if its profitability is higher than +//! the cumulative profitability of the colliding transactions, and all +//! the transactions that depend on it. +//! - A transaction doesn't have dependencies: +//! - A new transaction can be accepted if its profitability is higher +//! than the collided subtrees'. +//! +//! The pool provides a way to subscribe for updates on a transaction status. +//! It usually stream one or two messages: +//! - If the insertion of the transaction fails, you can expect only one message with the error. +//! - If the transaction is inserted, you can expect two messages: one with the validation of the insertion and one when the transaction is selected in a block. + +// TODO: Rename the folder from `txpool_v2` to `txpool` after the migration is complete. #![deny(clippy::arithmetic_side_effects)] #![deny(clippy::cast_possible_truncation)] #![deny(unused_crate_dependencies)] #![deny(warnings)] -// TODO: Rename the folder from `txpool_v2` to `txpool` after the migration is complete. - mod collision_manager; pub mod config; pub mod error; diff --git a/crates/services/txpool_v2/src/pool.rs b/crates/services/txpool_v2/src/pool.rs index be417dce5f4..07c7774f95a 100644 --- a/crates/services/txpool_v2/src/pool.rs +++ b/crates/services/txpool_v2/src/pool.rs @@ -46,6 +46,9 @@ use crate::{ }, }; +#[cfg(test)] +use std::collections::HashSet; + /// The pool is the main component of the txpool service. It is responsible for storing transactions /// and allowing the selection of transactions for inclusion in a block. pub struct Pool { @@ -561,6 +564,22 @@ where } self.register_transaction_counts(); } + + #[cfg(test)] + pub fn assert_integrity(&self, mut expected_txs: HashSet) { + for tx in &self.tx_id_to_storage_id { + if !expected_txs.remove(tx.0) { + panic!( + "Transaction with id {:?} is not in the expected transactions", + tx.0 + ); + } + } + assert!( + expected_txs.is_empty(), + "Some transactions are not found in the pool" + ); + } } pub struct NotEnoughSpace { diff --git a/crates/services/txpool_v2/src/selection_algorithms/ratio_tip_gas.rs b/crates/services/txpool_v2/src/selection_algorithms/ratio_tip_gas.rs index 7236940ef14..61fc4141736 100644 --- a/crates/services/txpool_v2/src/selection_algorithms/ratio_tip_gas.rs +++ b/crates/services/txpool_v2/src/selection_algorithms/ratio_tip_gas.rs @@ -21,6 +21,12 @@ use super::{ SelectionAlgorithm, }; +#[cfg(test)] +use fuel_core_types::services::txpool::ArcPoolTx; + +#[cfg(test)] +use std::collections::HashMap; + pub trait RatioTipGasSelectionAlgorithmStorage { type StorageIndex: Debug; @@ -116,6 +122,27 @@ where self.executable_transactions_sorted_tip_gas_ratio .remove(&Reverse(key)); } + + #[cfg(test)] + pub(crate) fn assert_integrity(&self, expected_txs: &[ArcPoolTx]) { + let mut expected_txs: HashMap = expected_txs + .iter() + .map(|tx| (tx.id(), tx.clone())) + .collect(); + for key in self.executable_transactions_sorted_tip_gas_ratio.keys() { + expected_txs.remove(&key.0.tx_id).unwrap_or_else(|| { + panic!( + "Transaction with id {:?} is not in the expected transactions.", + key.0.tx_id + ) + }); + } + assert!( + expected_txs.is_empty(), + "Some transactions are missing from the selection algorithm: {:?}", + expected_txs.keys().collect::>() + ); + } } impl SelectionAlgorithm for RatioTipGasSelection diff --git a/crates/services/txpool_v2/src/storage/graph.rs b/crates/services/txpool_v2/src/storage/graph.rs index b3b40575331..8f690eb17fd 100644 --- a/crates/services/txpool_v2/src/storage/graph.rs +++ b/crates/services/txpool_v2/src/storage/graph.rs @@ -368,6 +368,94 @@ impl GraphStorage { fn has_dependent(&self, index: NodeIndex) -> bool { self.get_direct_dependents(index).next().is_some() } + + #[cfg(test)] + pub(crate) fn assert_integrity( + &self, + expected_txs: &[ArcPoolTx], + ) -> Vec<(NodeIndex, bool)> { + use std::ops::Deref; + + let mut txs_map: HashMap = expected_txs + .iter() + .map(|tx| (tx.id(), tx.clone())) + .collect(); + let mut tx_id_node_id = HashMap::new(); + let mut txs_info = Vec::new(); + + for node_id in self.graph.node_indices() { + let node = self + .graph + .node_weight(node_id) + .expect("A node not expected exists in storage"); + let has_dependencies = Storage::has_dependencies(self, &node_id); + let tx_id = node.transaction.id(); + let tx = txs_map + .remove(&tx_id) + .expect("A transaction not expected exists in storage"); + assert_eq!(tx.deref(), node.transaction.deref()); + tx_id_node_id.insert(tx_id, node_id); + txs_info.push((node_id, has_dependencies)); + } + assert!( + txs_map.is_empty(), + "Some transactions are missing in storage {:?}", + txs_map.keys() + ); + + let mut coins_creators = HashMap::new(); + let mut contracts_creators = HashMap::new(); + for expected_tx in expected_txs { + for (i, output) in expected_tx.outputs().iter().enumerate() { + match output { + Output::Coin { .. } => { + let utxo_id = + UtxoId::new(expected_tx.id(), i.try_into().unwrap()); + coins_creators.insert(utxo_id, expected_tx.id()); + } + Output::ContractCreated { contract_id, .. } => { + contracts_creators.insert(*contract_id, expected_tx.id()); + } + Output::Contract(_) + | Output::Change { .. } + | Output::Variable { .. } => {} + } + } + } + for (utxo_id, node_id) in &self.coins_creators { + let tx_id = coins_creators.remove(utxo_id).unwrap_or_else(|| panic!("A coin creator (coin: {}) is present in the storage that shouldn't be there", utxo_id)); + let expected_node_id = tx_id_node_id.get(&tx_id).unwrap_or_else(|| { + panic!("A node id is missing for a transaction (tx_id: {})", tx_id) + }); + assert_eq!( + expected_node_id, node_id, + "The node id is different from the expected one" + ); + } + assert!( + coins_creators.is_empty(), + "Some contract creators are missing in storage: {:?}", + coins_creators + ); + + for (contract_id, node_id) in &self.contracts_creators { + let tx_id = contracts_creators.remove(contract_id).unwrap_or_else(|| panic!("A contract creator (contract: {}) is present in the storage that shouldn't be there", contract_id)); + let expected_node_id = tx_id_node_id.get(&tx_id).unwrap_or_else(|| { + panic!("A node id is missing for a transaction (tx_id: {})", tx_id) + }); + assert_eq!( + expected_node_id, node_id, + "The node id is different from the expected one" + ); + } + assert!( + contracts_creators.is_empty(), + "Some contract creators are missing in storage: {:?}", + contracts_creators + ); + + txs_info + } } impl Storage for GraphStorage { diff --git a/crates/services/txpool_v2/src/tests/mod.rs b/crates/services/txpool_v2/src/tests/mod.rs index 982a9034dbb..ed8c3f0b3c2 100644 --- a/crates/services/txpool_v2/src/tests/mod.rs +++ b/crates/services/txpool_v2/src/tests/mod.rs @@ -1,3 +1,5 @@ +#![allow(non_snake_case)] + mod mocks; mod stability_test; mod tests_e2e; diff --git a/crates/services/txpool_v2/src/tests/stability_test.rs b/crates/services/txpool_v2/src/tests/stability_test.rs index ea7dccdcec3..e7d2dbdf0d2 100644 --- a/crates/services/txpool_v2/src/tests/stability_test.rs +++ b/crates/services/txpool_v2/src/tests/stability_test.rs @@ -3,7 +3,6 @@ //! correct(not in the unexpected state). //! It relies on the `debug_assert` which are present in the code. -#![allow(non_snake_case)] #![allow(clippy::cast_possible_truncation)] #![allow(clippy::arithmetic_side_effects)] diff --git a/crates/services/txpool_v2/src/tests/tests_pool.rs b/crates/services/txpool_v2/src/tests/tests_pool.rs index 358143cf09a..d601150f4db 100644 --- a/crates/services/txpool_v2/src/tests/tests_pool.rs +++ b/crates/services/txpool_v2/src/tests/tests_pool.rs @@ -1,5 +1,3 @@ -#![allow(non_snake_case)] - use crate::{ config::{ Config, @@ -74,10 +72,12 @@ fn insert_one_tx_succeeds() { let tx = universe.build_script_transaction(None, None, 0); // When - let result = universe.verify_and_insert(tx); + let result = universe.verify_and_insert(tx.clone()); // Then assert!(result.is_ok()); + let tx = result.unwrap().0; + universe.assert_pool_integrity(&[tx]); } #[test] @@ -98,6 +98,7 @@ fn insert__tx_with_blacklisted_utxo_id() { assert!( matches!(err, Error::Blacklisted(BlacklistedError::BlacklistedUTXO(id)) if id == utxo_id) ); + universe.assert_pool_integrity(&[]); } #[test] @@ -118,6 +119,7 @@ fn insert__tx_with_blacklisted_owner() { assert!( matches!(err, Error::Blacklisted(BlacklistedError::BlacklistedOwner(id)) if id == owner_addr) ); + universe.assert_pool_integrity(&[]); } #[test] @@ -149,6 +151,7 @@ fn insert__tx_with_blacklisted_contract() { assert!( matches!(err, Error::Blacklisted(BlacklistedError::BlacklistedContract(id)) if id == contract_id) ); + universe.assert_pool_integrity(&[]); } #[test] @@ -169,6 +172,7 @@ fn insert__tx_with_blacklisted_message() { assert!( matches!(err, Error::Blacklisted(BlacklistedError::BlacklistedMessage(id)) if id == nonce) ); + universe.assert_pool_integrity(&[]); } #[test] @@ -190,6 +194,7 @@ fn insert__tx2_succeeds_after_dependent_tx1() { // Then assert!(result1.is_ok()); assert!(result2.is_ok()); + universe.assert_pool_integrity(&[result1.unwrap().0, result2.unwrap().0]); } #[test] @@ -227,7 +232,7 @@ fn insert__tx2_collided_on_contract_id() { .add_input(gas_coin) .add_output(create_contract_output(contract_id)) .finalize_as_transaction(); - universe.verify_and_insert(tx).unwrap(); + let tx = universe.verify_and_insert(tx).unwrap().0; // When let result2 = universe.verify_and_insert(tx_faulty); @@ -237,6 +242,7 @@ fn insert__tx2_collided_on_contract_id() { assert!( matches!(err, Error::Collided(CollisionReason::ContractCreation(id)) if id == contract_id) ); + universe.assert_pool_integrity(&[tx]); } #[test] @@ -263,7 +269,7 @@ fn insert__tx_with_dependency_on_invalid_utxo_type() { universe.random_predicate(AssetId::BASE, TEST_COIN_AMOUNT, Some(utxo_id)); let tx_faulty = universe.build_script_transaction(Some(vec![random_predicate]), None, 0); - universe.verify_and_insert(tx).unwrap(); + let tx = universe.verify_and_insert(tx).unwrap().0; // When let result2 = universe.verify_and_insert(tx_faulty); @@ -274,6 +280,7 @@ fn insert__tx_with_dependency_on_invalid_utxo_type() { assert!( matches!(err, Error::InputValidation(InputValidationError::UtxoNotFound(id)) if id == utxo_id) ); + universe.assert_pool_integrity(&[tx]); } #[test] @@ -286,7 +293,7 @@ fn insert__already_known_tx_returns_error() { // Given let tx = universe.build_script_transaction(None, None, 0); - universe.verify_and_insert(tx.clone()).unwrap(); + let pool_tx = universe.verify_and_insert(tx.clone()).unwrap().0; // When let result2 = universe.verify_and_insert(tx.clone()); @@ -296,6 +303,7 @@ fn insert__already_known_tx_returns_error() { assert!( matches!(err, Error::InputValidation(InputValidationError::DuplicateTxId(id)) if id == tx.id(&ChainId::default())) ); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -316,6 +324,7 @@ fn insert__unknown_utxo_returns_error() { assert!( matches!(err, Error::InputValidation(InputValidationError::UtxoNotFound(id)) if id == utxo_id) ); + universe.assert_pool_integrity(&[]); } #[test] @@ -335,7 +344,8 @@ fn insert__higher_priced_tx_removes_lower_priced_tx() { let result = universe.verify_and_insert(tx2).unwrap(); // Then - assert_eq!(result[0].id(), tx_id); + assert_eq!(result.1[0].id(), tx_id); + universe.assert_pool_integrity(&[result.0]); } #[test] @@ -351,8 +361,8 @@ fn insert__colliding_dependent_and_underpriced_returns_error() { // Given let tx2 = universe.build_script_transaction(Some(vec![input.clone()]), None, 20); let tx3 = universe.build_script_transaction(Some(vec![input]), None, 10); - universe.verify_and_insert(tx1).unwrap(); - universe.verify_and_insert(tx2).unwrap(); + let tx1 = universe.verify_and_insert(tx1).unwrap().0; + let tx2 = universe.verify_and_insert(tx2).unwrap().0; // When let result3 = universe.verify_and_insert(tx3); @@ -360,6 +370,7 @@ fn insert__colliding_dependent_and_underpriced_returns_error() { // Then let err = result3.unwrap_err(); assert!(matches!(err, Error::Collided(CollisionReason::Utxo(id)) if id == utxo_id)); + universe.assert_pool_integrity(&[tx1, tx2]); } #[test] @@ -402,6 +413,7 @@ fn insert_dependent_contract_creation() { // Then assert!(result1.is_ok()); assert!(result2.is_ok()); + universe.assert_pool_integrity(&[result1.unwrap().0, result2.unwrap().0]); } #[test] @@ -432,10 +444,11 @@ fn insert_more_priced_tx3_removes_tx1_and_dependent_tx2() { let result3 = universe.verify_and_insert(tx3); // Then - let removed_txs = result3.unwrap(); + let (pool_tx, removed_txs) = result3.unwrap(); assert_eq!(removed_txs.len(), 2); assert_eq!(removed_txs[0].id(), tx1_id); assert_eq!(removed_txs[1].id(), tx2_id); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -464,13 +477,14 @@ fn insert_more_priced_tx2_removes_tx1_and_more_priced_tx3_removes_tx2() { // Then assert!(result2.is_ok()); - let removed_txs = result2.unwrap(); + let removed_txs = result2.unwrap().1; assert_eq!(removed_txs.len(), 1); assert_eq!(removed_txs[0].id(), tx1_id); assert!(result3.is_ok()); - let removed_txs = result3.unwrap(); + let (pool_tx, removed_txs) = result3.unwrap(); assert_eq!(removed_txs.len(), 1); assert_eq!(removed_txs[0].id(), tx2_id); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -488,7 +502,7 @@ fn insert__tx_limit_hit() { // Given let tx1 = universe.build_script_transaction(None, None, 10); let tx2 = universe.build_script_transaction(None, None, 0); - universe.verify_and_insert(tx1).unwrap(); + let pool_tx = universe.verify_and_insert(tx1).unwrap().0; // When let result2 = universe.verify_and_insert(tx2); @@ -496,6 +510,7 @@ fn insert__tx_limit_hit() { // Then let err = result2.unwrap_err(); assert!(matches!(err, Error::NotInsertedLimitHit)); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -522,7 +537,7 @@ fn insert__tx_gas_limit() { ..Default::default() }); universe.build_pool(); - universe.verify_and_insert(tx1).unwrap(); + let pool_tx = universe.verify_and_insert(tx1).unwrap().0; // When let result2 = universe.verify_and_insert(tx2); @@ -530,6 +545,7 @@ fn insert__tx_gas_limit() { // Then let err = result2.unwrap_err(); assert!(matches!(err, Error::NotInsertedLimitHit)); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -556,7 +572,7 @@ fn insert__tx_bytes_limit() { ..Default::default() }); universe.build_pool(); - universe.verify_and_insert(tx1).unwrap(); + let pool_tx = universe.verify_and_insert(tx1).unwrap().0; // When let result2 = universe.verify_and_insert(tx2); @@ -564,6 +580,7 @@ fn insert__tx_bytes_limit() { // Then let err = result2.unwrap_err(); assert!(matches!(err, Error::NotInsertedLimitHit)); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -584,8 +601,8 @@ fn insert__dependency_chain_length_hit() { let input = unset_input.into_input(UtxoId::new(tx2.id(&Default::default()), 0)); let tx3 = universe.build_script_transaction(Some(vec![input]), None, 0); - universe.verify_and_insert(tx1).unwrap(); - universe.verify_and_insert(tx2).unwrap(); + let tx1 = universe.verify_and_insert(tx1).unwrap().0; + let tx2 = universe.verify_and_insert(tx2).unwrap().0; // When let result3 = universe.verify_and_insert(tx3); @@ -596,6 +613,7 @@ fn insert__dependency_chain_length_hit() { err, Error::Dependency(DependencyError::NotInsertedChainDependencyTooBig) )); + universe.assert_pool_integrity(&[tx1, tx2]); } #[test] @@ -632,6 +650,7 @@ fn get_sorted_out_tx1_2_3() { assert_eq!(txs[0].id(), tx3_id, "First should be tx3"); assert_eq!(txs[1].id(), tx1_id, "Second should be tx1"); assert_eq!(txs[2].id(), tx2_id, "Third should be tx2"); + universe.assert_pool_integrity(&[]); } #[test] @@ -688,6 +707,7 @@ fn get_sorted_out_tx_same_tips() { assert_eq!(txs[0].id(), tx3_id, "First should be tx3"); assert_eq!(txs[1].id(), tx2_id, "Second should be tx2"); assert_eq!(txs[2].id(), tx1_id, "Third should be tx1"); + universe.assert_pool_integrity(&[]); } #[test] @@ -744,6 +764,7 @@ fn get_sorted_out_tx_profitable_ratios() { assert_eq!(txs[0].id(), tx3_id, "First should be tx3"); assert_eq!(txs[1].id(), tx2_id, "Second should be tx2"); assert_eq!(txs[2].id(), tx1_id, "Third should be tx1"); + universe.assert_pool_integrity(&[]); } #[test] @@ -786,6 +807,7 @@ fn get_sorted_out_tx_by_creation_instant() { assert_eq!(txs[1].id(), tx2_id, "Second should be tx2"); assert_eq!(txs[2].id(), tx3_id, "Third should be tx3"); assert_eq!(txs[3].id(), tx4_id, "Fourth should be tx4"); + universe.assert_pool_integrity(&[]); } #[test] @@ -826,6 +848,7 @@ fn insert__tx_below_min_gas_price() { // Then assert!(matches!(err, Error::InsufficientMaxFee { .. })); + universe.assert_pool_integrity(&[]); } #[test] @@ -839,9 +862,10 @@ fn insert_tx_when_input_message_id_exists_in_db() { let tx = universe.build_script_transaction(Some(vec![input]), None, 0); // When - universe.verify_and_insert(tx) + let pool_tx = universe.verify_and_insert(tx) // Then - .unwrap(); + .unwrap().0; + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -861,6 +885,7 @@ fn insert__tx_when_input_message_id_do_not_exists_in_db() { err, Error::InputValidation(InputValidationError::NotInsertedInputMessageUnknown(msg_id)) if msg_id == *message.id() )); + universe.assert_pool_integrity(&[]); } #[test] @@ -887,13 +912,14 @@ fn insert__tx_tip_lower_than_another_tx_with_same_message_id() { ); // When - universe.verify_and_insert(tx_high).unwrap(); + let pool_tx = universe.verify_and_insert(tx_high).unwrap().0; let err = universe.verify_and_insert(tx_low).unwrap_err(); // Then assert!( matches!(err, Error::Collided(CollisionReason::Message(msg_id)) if msg_id == *message.id()) ); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -927,9 +953,10 @@ fn insert_tx_tip_higher_than_another_tx_with_same_message_id() { // Then assert!(result1.is_ok()); assert!(result2.is_ok()); - let removed_txs = result2.unwrap(); + let (pool_tx, removed_txs) = result2.unwrap(); assert_eq!(removed_txs.len(), 1); assert_eq!(removed_txs[0].id(), tx_high_id); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -965,6 +992,7 @@ fn insert_again_message_after_squeeze_with_even_lower_tip() { assert!(result1.is_ok()); assert!(result2.is_ok()); assert!(result3.is_ok()); + universe.assert_pool_integrity(&[result2.unwrap().0, result3.unwrap().0]); } #[test] @@ -993,6 +1021,7 @@ fn insert__tx_with_predicates_incorrect_owner() { ValidityError::InputPredicateOwner { index: 0 } )) )); + universe.assert_pool_integrity(&[]); } #[test] @@ -1035,6 +1064,7 @@ fn insert__tx_with_predicate_without_enough_gas() { PredicateVerificationFailed::OutOfGas )) )); + universe.assert_pool_integrity(&[]); } #[test] @@ -1068,6 +1098,7 @@ fn insert__tx_with_predicate_that_returns_false() { PredicateVerificationFailed::Panic(PanicReason::PredicateReturnedNonOne) )) )); + universe.assert_pool_integrity(&[]); } #[test] @@ -1089,9 +1120,10 @@ fn insert_tx_with_blob() { .finalize_as_transaction(); // When - universe.verify_and_insert(tx) + let pool_tx = universe.verify_and_insert(tx) // Then - .unwrap(); + .unwrap().0; + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -1113,7 +1145,7 @@ fn insert__tx_with_blob_already_inserted_at_higher_tip() { .add_fee_input() .finalize_as_transaction(); - universe.verify_and_insert(tx).unwrap(); + let pool_tx = universe.verify_and_insert(tx).unwrap().0; let same_blob_tx = TransactionBuilder::blob(BlobBody { id: blob_id, @@ -1128,6 +1160,7 @@ fn insert__tx_with_blob_already_inserted_at_higher_tip() { // Then assert!(matches!(err, Error::Collided(CollisionReason::Blob(b)) if b == blob_id)); + universe.assert_pool_integrity(&[pool_tx]); } #[test] @@ -1166,6 +1199,7 @@ fn insert_tx_with_blob_already_insert_at_lower_tip() { // Then assert!(result.is_ok()); + universe.assert_pool_integrity(&[result.unwrap().0]); } #[test] @@ -1196,6 +1230,7 @@ fn insert__tx_blob_already_in_db() { err, Error::InputValidation(InputValidationError::NotInsertedBlobIdAlreadyTaken(b)) if b == blob_id )); + universe.assert_pool_integrity(&[]); } #[test] @@ -1217,8 +1252,8 @@ fn insert__if_tx3_depends_and_collides_with_tx2() { // Given // tx3 {inputs: {coinA, coinB}, outputs:{}, tip: 20} let input_b = unset_input.into_input(UtxoId::new(tx2.id(&Default::default()), 0)); - universe.verify_and_insert(tx1).unwrap(); - universe.verify_and_insert(tx2).unwrap(); + let tx1 = universe.verify_and_insert(tx1).unwrap().0; + let tx2 = universe.verify_and_insert(tx2).unwrap().0; let tx3 = universe.build_script_transaction(Some(vec![input_a, input_b]), None, 20); @@ -1230,6 +1265,7 @@ fn insert__if_tx3_depends_and_collides_with_tx2() { err, Error::Dependency(DependencyError::DependentTransactionIsADiamondDeath) )); + universe.assert_pool_integrity(&[tx1, tx2]); } #[test] @@ -1266,4 +1302,5 @@ fn insert__tx_upgrade_with_invalid_wasm() { result, Error::WasmValidity(WasmValidityError::NotEnabled) )); + universe.assert_pool_integrity(&[]); } diff --git a/crates/services/txpool_v2/src/tests/universe.rs b/crates/services/txpool_v2/src/tests/universe.rs index 0703aaebe48..2813421d8da 100644 --- a/crates/services/txpool_v2/src/tests/universe.rs +++ b/crates/services/txpool_v2/src/tests/universe.rs @@ -1,4 +1,7 @@ -use std::sync::Arc; +use std::{ + collections::HashSet, + sync::Arc, +}; use fuel_core_types::{ entities::{ @@ -197,10 +200,11 @@ impl TestPoolUniverse { tx_builder.finalize().into() } + // Returns the added transaction and the list of transactions that were removed from the pool pub fn verify_and_insert( &mut self, tx: Transaction, - ) -> Result, Error> { + ) -> Result<(ArcPoolTx, Vec), Error> { if let Some(pool) = &self.pool { let mut mock_consensus_params_provider = MockConsensusParametersProvider::default(); @@ -222,7 +226,8 @@ impl TestPoolUniverse { Default::default(), true, )?; - pool.write().insert(Arc::new(tx), &self.mock_db) + let tx = Arc::new(tx); + Ok((tx.clone(), pool.write().insert(tx, &self.mock_db)?)) } else { panic!("Pool needs to be built first"); } @@ -293,6 +298,28 @@ impl TestPoolUniverse { } } + pub fn assert_pool_integrity(&self, expected_txs: &[ArcPoolTx]) { + let pool = self.pool.as_ref().unwrap(); + let pool = pool.read(); + let storage_ids_dependencies = pool.storage.assert_integrity(expected_txs); + let txs_without_dependencies = expected_txs + .iter() + .zip(storage_ids_dependencies) + .filter_map(|(tx, (_, has_dependencies))| { + if !has_dependencies { + Some(tx.clone()) + } else { + None + } + }) + .collect::>(); + pool.selection_algorithm + .assert_integrity(&txs_without_dependencies); + pool.collision_manager.assert_integrity(expected_txs); + let txs: HashSet = expected_txs.iter().map(|tx| tx.id()).collect(); + pool.assert_integrity(txs); + } + pub fn get_pool(&self) -> Shared { self.pool.clone().unwrap() }