-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Allow insertion of transactions in the TxPool v2 #2186
Labels
Comments
Open
Yeah, the order it right=) |
This was referenced Sep 13, 2024
Merged
AurelienFT
added a commit
that referenced
this issue
Sep 27, 2024
REVIEWER NOTE : Review #2162 first for context ## Linked Issues/PRs Closes #2186 ## Description This PR allows you to insert `Transaction` in the `TxPool v2` service and so perform all the necessary verifications and the insertion. ### 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` ### Changes from the base branch `txpool-v2` All the tests has been refactored to be way more clearer to understand and easier to read. They also now all follow the naming and the GWT convention. ## 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: Green Baneling <XgreenX9999@gmail.com>
AurelienFT
added a commit
that referenced
this issue
Oct 3, 2024
## 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, in the TxPool V2 you can only insert
PoolTransaction
. We need to instantiate the workflow to pass fromTransaction
toPoolTransaction
. In fact it already exists in thetransaction_conversion.rs
file but it's a copy paste from the previous TxPool and used in tests only for now.We want to change the behavior to verify in this order :
utxo_validation
is asked)@xgreenx can you confirm me the order ?
It will also be a great time to instantiate more tests and standardize them all.
Better type for the insert result in the pool
The text was updated successfully, but these errors were encountered: