forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 4
arbitrary package validation (bonus linearization with conflict discount) #12
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
Draft
glozow
wants to merge
81
commits into
master
Choose a base branch
from
2025-09-2-linearizations
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e22d8a5 to
3dfa654
Compare
This takes the same amount of space as CTransaction pointers, and saves a map lookup in many common uses.
Effectively this is treating all transactions in txgraph as being in a cluster of size 1.
Preparatory commit to the rbf functional test, before changes are made to the rbf rules as part of cluster mempool.
Include an adjustment to mempool_tests.cpp due to the additional memory used by txgraph. Includes a temporary change to the mempool_ephemeral_dust.py functional test, due to validation checks being reordered. This change will revert once the RBF rules are changed in a later commit.
Rather than evicting the transactions with the lowest descendant feerate, instead evict transactions that have the lowest chunk feerate. Once mining is implemented based on choosing transactions with highest chunk feerate (see next commit), mining and eviction will be opposites, so that we will evict the transactions that would be mined last.
Co-Authored-By: Gregory Sanders <gsanders87@gmail.com>
The addition of a cluster size limit makes the CPFP carveout rule useless, because carveout cannot be used to bypass the cluster size limit. Remove this policy rule and update tests to no longer rely on the behavior.
With a total ordering on mempool transactions, we are now able to calculate a transaction's mining score at all times. Use this to improve the RBF logic: - we no longer enforce a "no new unconfirmed parents" rule - we now require that the mempool's feerate diagram must improve in order to accept a replacement - the topology restrictions for conflicts in the package rbf setting have been eliminated Revert the temporary change to mempool_ephemeral_dust.py that were previously made due to RBF validation checks being reordered. Co-authored-by: Gregory Sanders <gsanders87@gmail.com>, glozow <gloriajzhao@gmail.com>
Previously, transaction batches were first sorted by ancestor count and then feerate, to ensure transactions are announced in a topologically valid order, while prioritizing higher feerate transactions. Ancestor count is a crude topological sort criteria, so replace this with linearization order so that the highest feerate transactions (as would be observed by the mining algorithm) are relayed before lower feerate ones, in a topologically valid way. This also fixes a test that only worked due to the ancestor-count-based sort order.
The mempool clusters and linearization permit sorting the mempool topologically without making use of ancestor counts (as long as the graph is not oversized). Co-authored-by: Pieter Wuille <pieter@wuille.net>
In preparation for removing ancestor data from CTxMemPoolEntry, recalculate the ancestor statistics on demand wherever needed.
Changes AddToMempool() helper to only apply changes if the mempool limits are respected. Fix package_rbf fuzz target to handle mempool policy violations
Add benchmarks for: - mempool update time when blocks are found - adding a transaction - performing the mempool's RBF calculation - calculating mempool ancestors/descendants
Including test coverage for mempool eviction and expiry
Co-authored-by: glozow <gloriajzhao@gmail.com>
Co-authored-by: glozow <gloriajzhao@gmail.com>
This is in preparation for eliminating the block template building happening in mini_miner, in favor of directly using the linearizations done in the mempool.
…ScoreWithTopology() We use CompareMiningScoreWithTopology() for sorting transaction announcements during tx relay, and we use GetSortedScoreWithTopology() in CTxMemPool::check().
Calculating mempool ancestors for a new transaction should not be done until after cluster size limits have been enforced, to limit CPU DoS potential. Achieve this by reworking TRUC and RBF validation logic: - TRUC policy enforcement is now done using only mempool parents of new transactions, not all mempool ancestors (note that it's fine to calculate ancestors of in-mempool transactions, if the number of such calls is reasonably bounded). - RBF replacement checks are performed earlier (which allows for checking cluster size limits earlier, because cluster size checks cannot happen until after all conflicts are staged for removal). - Verifying that a new transaction doesn't conflict with an ancestor now happens later, in AcceptSingleTransaction() rather than in PreChecks(). This means that the test is not performed at all in AcceptMultipleTransactions(), but in package acceptance we already disallow RBF in situations where a package transaction has in-mempool parents. Also to ensure that all RBF validation logic is applied in both the single transaction and multiple transaction cases, remove the optimization that skips the PackageMempoolChecks() in the case of a single transaction being validated in AcceptMultipleTransactions().
-BEGIN VERIFY SCRIPT- sed -i 's/m_wtxids_fee_calculations/m_subpackage_wtxids/g' $(git grep -l 'm_wtxids_fee_calculations') sed -i 's/wtxids_fee_calculations/subpackage_wtxids/g' $(git grep -l 'wtxids_fee_calculations') sed -i 's/FeeFailure/Failure/g' src/validation.cpp -END VERIFY SCRIPT-
When a transaction fails, it's helpful to know which transactions it was packaged with. This can help with debugging and we can return it to clients. Adding tests now can also help with demonstrating inner behavior changes if/when the subpackaging algorithm is improved.
- package_ppfp shows benefit of assessing subpackages - package_ppfc shows that this doesn't let us do "parent pays for child;" we only do this when the individual and ancestor feerates meet mempool minimum feerate - package_confirmed_parent shows benefit of treating txns-already-known as "in mempool" tx - package_with_rbf tests existing behavior that shouldn't change. Even though package RBF is not enabled, it's important that submitting a transaction as part of a package does not block it from doing a normal replacement. Otherwise we may blind ourselves to a replacement simply because it has a child and we happened to download it as a package. - package_needs_reorder shows necessity of linearizing using cluster information. If we used the original order, we would reject everything, even if we submit subpackages. - package_cluster_limits shows that linearization can help us pick the most incentive-compatible tx to accept, and that we can take a subchunk when it is sufficient to meet the minimum feerate.
We can safely allow any package since MiniGraph can linearize and group things into ancestor subsets.
3dfa654 to
a94d643
Compare
Owner
Author
|
Just a rebase |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.