Skip to content

Commit

Permalink
TxPool v2 General architecture (#2162)
Browse files Browse the repository at this point in the history
## Linked Issues/PRs
Closes #2160

## Description

This PR contains a new way to organize and store transactions. This new
architecture have multiple benefits :
- Transactions are now organized as a graph and so can easily remove a
branch fixing this kind of problems
#2020
- Storage, collision detection and selections are well split and can be
viewed as independent mechanisms that are plugged to the pool. This
allow to fix this : #1961
and reduce potential related issues in the future.
- Transactions selection is still basic (only ratio gas/tip) but already
address : #868 (for the
selection part) and is highly customizable for next usages.
- Usage of `dependencies` and `dependents` transactions everywhere (not
a mix with parents etc). `dependencies` is for transaction that need to
be included **before** us and `dependents` is for transaction that need
to be included **after** us.
- Reduced the number of caches to maintain to avoid desync between
caches and storages
- Better separation of concerns to have a clearer code in the `Pool`
structure
- Changed the priority over collision by computing the average ratio of
all collision instead of having a better ratio than all collision (idea
from @xgreenx )
- Unwrap is forbidden
- Avoid Arc and have a storage place that can give references on-demand.
- Highly documented
- All the previous tests of the pool passes

### Changes in code logic from TxPool v1
- The verifications are performed in a new order specified in
#2186. The goal is to avoid
making the computation heavy work if the simple checks aren't valid. In
this new version we also ensure that verifications are done in order by
having wrapper type around each step to allow only one verification
path.
- The insertion is performed in a separate thread pool, the goal is to
not block the pool on any verifications/insertions and to manage the
ressources we allocate to these works
- The insertion rules and conditions has change to the following :
- A transaction with dependencies can collide only with one other
transaction
- A transaction without dependencies can collide with multiple
transaction
- Rules to free up space for new transaction
- If a transaction is colliding with another verify if deleting the
colliding transaction and dependents subtree is enough otherwise refuses
the tx
- If a transaction is dependent and not enough space, don't accept
transaction
- If a transaction is executable, try to free has much space used by
less profitable transactions as possible in the pool to include it

- New limits on the size of the pool : max_pool_bytes_size and
max_pool_gas


Some parts of this refactoring can still be improved : 
- Keep not includable transactions in a side buffer
- Benchmarks
- Metrics
- High level documentation (drawings, general explanations) 

Ideas that could be added but maybe overkill : 
- Generic cache structure that is linked to the graph pool somehow, and
automatically update caches when a transaction is included/removed to
avoid cache desync.

## 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
- [ ] I have created follow-up issues caused by this PR and linked them
here

---------

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
  • Loading branch information
AurelienFT and xgreenx authored Oct 3, 2024
1 parent c039c28 commit 6111cef
Show file tree
Hide file tree
Showing 29 changed files with 5,391 additions and 30 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- [2131](https://github.com/FuelLabs/fuel-core/pull/2131): Add flow in TxPool in order to ask to newly connected peers to share their transaction pool
- [2182](https://github.com/FuelLabs/fuel-core/pull/2151): Limit number of transactions that can be fetched via TxSource::next
- [2189](https://github.com/FuelLabs/fuel-core/pull/2151): Select next DA height to never include more than u16::MAX -1 transactions from L1.
- [2162](https://github.com/FuelLabs/fuel-core/pull/2162): Pool structure with dependencies, etc.. for the next transaction pool module. Also adds insertion/verification process in PoolV2 and tests refactoring
- [2265](https://github.com/FuelLabs/fuel-core/pull/2265): Integrate Block Committer API for DA Block Costs.

### Changed
Expand Down
22 changes: 22 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ members = [
"crates/services/relayer",
"crates/services/sync",
"crates/services/txpool",
"crates/services/txpool_v2",
"crates/services/upgradable-executor",
"crates/services/upgradable-executor/wasm-executor",
"crates/storage",
Expand Down Expand Up @@ -75,6 +76,7 @@ fuel-core-producer = { version = "0.36.0", path = "./crates/services/producer" }
fuel-core-relayer = { version = "0.36.0", path = "./crates/services/relayer" }
fuel-core-sync = { version = "0.36.0", path = "./crates/services/sync" }
fuel-core-txpool = { version = "0.36.0", path = "./crates/services/txpool" }
fuel-core-txpool-v2 = { version = "0.36.0", path = "./crates/services/txpool_v2" }
fuel-core-storage = { version = "0.36.0", path = "./crates/storage", default-features = false }
fuel-core-trace = { version = "0.36.0", path = "./crates/trace" }
fuel-core-types = { version = "0.36.0", path = "./crates/types", default-features = false }
Expand Down
15 changes: 9 additions & 6 deletions crates/services/txpool/src/containers/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,15 @@ impl Dependency {
}
} else {
// tx output is in pool
let output_tx = txs.get(utxo_id.tx_id()).unwrap();
let output =
&output_tx.outputs()[utxo_id.output_index() as usize];
Self::check_if_coin_input_can_spend_output(
output, input, false,
)?;
let output_tx = txs.get(utxo_id.tx_id());
let output = output_tx.and_then(|tx| {
tx.outputs().get(utxo_id.output_index() as usize)
});
if let Some(output) = output {
Self::check_if_coin_input_can_spend_output(
output, input, false,
)?;
}
}

collided.push(*spend_by);
Expand Down
7 changes: 5 additions & 2 deletions crates/services/txpool/src/transaction_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ mod tests {
checked_transaction::builder::TransactionBuilderExt,
SecretKey,
},
services::txpool::PoolTransaction,
services::txpool::{
Metadata,
PoolTransaction,
},
};
use itertools::Itertools;
use std::sync::Arc;
Expand Down Expand Up @@ -157,7 +160,7 @@ mod tests {
// so it doesn't need to compute valid sigs for tests
PoolTransaction::Script(
script.finalize_checked_basic(Default::default()),
ConsensusParametersVersion::MIN,
Metadata::new(ConsensusParametersVersion::MIN),
)
})
.map(Arc::new)
Expand Down
22 changes: 12 additions & 10 deletions crates/services/txpool/src/txpool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ use fuel_core_types::{
checked_transaction::CheckPredicateParams,
interpreter::Memory,
},
services::executor::TransactionExecutionStatus,
services::{
executor::TransactionExecutionStatus,
txpool::Metadata,
},
};
use std::{
collections::HashMap,
Expand Down Expand Up @@ -343,26 +346,25 @@ where
tx: Checked<Transaction>,
) -> Result<InsertionResult, Error> {
let view = self.database.latest_view().unwrap();
self.insert_inner(tx, ConsensusParametersVersion::MIN, &view)
self.insert_inner(tx, Metadata::new(ConsensusParametersVersion::MIN), &view)
}

#[tracing::instrument(level = "debug", skip_all, fields(tx_id = %tx.id()), ret, err)]
// this is atomic operation. Return removed(pushed out/replaced) transactions
fn insert_inner(
&mut self,
tx: Checked<Transaction>,
version: ConsensusParametersVersion,
metadata: Metadata,
view: &View,
) -> Result<InsertionResult, Error> {
let tx: CheckedTransaction = tx.into();

let tx = Arc::new(match tx {
CheckedTransaction::Script(tx) => PoolTransaction::Script(tx, version),
CheckedTransaction::Create(tx) => PoolTransaction::Create(tx, version),
CheckedTransaction::Script(tx) => PoolTransaction::Script(tx, metadata),
CheckedTransaction::Create(tx) => PoolTransaction::Create(tx, metadata),
CheckedTransaction::Mint(_) => return Err(Error::MintIsDisallowed),
CheckedTransaction::Upgrade(tx) => PoolTransaction::Upgrade(tx, version),
CheckedTransaction::Upload(tx) => PoolTransaction::Upload(tx, version),
CheckedTransaction::Blob(tx) => PoolTransaction::Blob(tx, version),
CheckedTransaction::Upgrade(tx) => PoolTransaction::Upgrade(tx, metadata),
CheckedTransaction::Upload(tx) => PoolTransaction::Upload(tx, metadata),
CheckedTransaction::Blob(tx) => PoolTransaction::Blob(tx, metadata),
});

self.check_blacklisting(tx.as_ref())?;
Expand Down Expand Up @@ -450,7 +452,7 @@ where
};

for tx in txs.into_iter() {
res.push(self.insert_inner(tx, version, &view));
res.push(self.insert_inner(tx, Metadata::new(version), &view));
}

// announce to subscribers
Expand Down
39 changes: 39 additions & 0 deletions crates/services/txpool_v2/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
[package]
name = "fuel-core-txpool-v2"
version = { workspace = true }
authors = { workspace = true }
categories = ["cryptography::cryptocurrencies"]
edition = { workspace = true }
homepage = { workspace = true }
keywords = ["blockchain", "cryptocurrencies", "fuel-vm", "vm"]
license = { workspace = true }
repository = { workspace = true }
description = "Transaction pool v2"
publish = false

[dependencies]
anyhow = { workspace = true }
async-trait = { workspace = true }
derive_more = { workspace = true }
fuel-core-services = { workspace = true }
fuel-core-storage = { workspace = true, features = ["std"] }
fuel-core-types = { workspace = true, features = ["test-helpers"] }
futures = { workspace = true }
mockall = { workspace = true, optional = true }
num-rational = { workspace = true }
parking_lot = { workspace = true }
petgraph = "0.6.5"
rayon = { workspace = true }
tokio = { workspace = true, default-features = false, features = ["sync"] }
tracing = { workspace = true }

[dev-dependencies]
fuel-core-trace = { workspace = true }
rand = { workspace = true }

[features]
test-helpers = [
"dep:mockall",
"fuel-core-types/test-helpers",
"fuel-core-storage/test-helpers",
]
Loading

0 comments on commit 6111cef

Please sign in to comment.