Followup to #31829:
OrphanTxBase to OrphanInfoSize() method by replacing all calls with CountUniqueOrphans
Since this field holds a total number of bytes, overflow is within the
realm of possibility. Use int64 to be safe.
This is move-only.
Expiry is going away in a later commit.
This is only an RPC change. Behavior of the orphanage does not change.
Note that getorphantxs is marked experimental.
These internals should and will be private.
The orphanage will no longer have a maximum number of unique orphans.
This is move only.
Move towards a model where TxOrphanage is initialized with limits that
it remembers throughout its lifetime.
Remove the param. Limiting by number of unique orphans will be removed
in a later commit.
Now that -maxorphantx is gone, this does not change the node behavior.
The parameter is only used in tests.
This function only ever returns 0 or 1 (number of unique orphans
erased).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32941.
See the guideline for information on the review process. A summary of reviews will appear here.
Reviewers, this pull request conflicts with the following ones:
std::variant by marcofleon)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Possible typos and grammar issues:
drahtbot_id_4_m
This is largely a reimplementation using boost::multi_index_container.
All the same public methods are available. It has an index by outpoint,
per-peer tracking, peer worksets, etc.
A few differences:
- Limits have changed: instead of a global limit of 100 unique orphans,
we have a maximum number of announcements (which can include duplicate
orphans) and a global memory limit which scales with the number of
peers.
- The maximum announcements limit is 100 to match the original limit,
but this is actually a stricter limit because the announcement count
is not de-duplicated.
- Eviction strategy: when global limits are reached, a per-peer limit
comes into play. While limits are exceeded, we choose the peer whose
“DoS score” (max usage / limit ratio for announcements and memory
limits) is highest and evict announcements by entry time, sorting
non-reconsiderable ones before reconsiderable ones. Since announcements
are unique by (wtxid, peer), as long as 1 announcement remains for a
transaction, it remains in the orphanage.
- This eviction strategy means no peer can influence the eviction of
another peer’s orphans.
- Also, since global limits are a multiple of per-peer limits, as long
as a peer does not exceed its limits, its orphans are protected from
eviction.
- Orphans no longer expire, since older announcements are generally
removed before newer ones.
- GetChildrenFromSamePeer returns the transactions from newest to
oldest.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
For the default number of peers (125), allows each to relay a default
descendant package (up to 25-1=24 can be missing inputs) of small (9
inputs or fewer) transactions out of order.
This limit also gives acceptable bounds for worst case LimitOrphans iterations.
Functional tests aren't changed to check for larger cap because it would
make the runtime too long.
Also deletes the now-unused DEFAULT_MAX_ORPHAN_TRANSACTIONS.
This is preparation for the simulation fuzz test added in a later commit. Since
AddChildrenToWorkSet consumes randomness, there is no way for the simulator to
exactly predict its behavior. By returning the set of made-reconsiderable announcements
instead, the simulator can instead test that it is *a* valid choice, and then
apply it to its own data structures.
This adds a large simulation fuzz test for all TxOrphanage public interface
functions, using a mix of comparison with expected behavior (in case it is
fully specified), and testing of properties exhibited otherwise.
If we want to restart at all during the tests, we can't have future timestamps.
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
This introduces an invariant that TxOrphanageImpl never holds more than one
announcement with m_reconsider=true for a given wtxid. This avoids duplicate
work, both in the caller might otherwise reconsider the same transaction multiple
times before it is ready, and internally in AddChildrenToWorkSet, which might
otherwise iterate over all announcements multiple times.
This struct was previously used by the underlying data strcuture. Now,
it is only used to return information to external callers, so "Base"
does not make sense anymore.
🚧 At least one of the CI tasks failed.
Task fuzzer,address,undefined,integer, no depends: https://github.com/bitcoin/bitcoin/runs/45743710188
LLM reason (✨ experimental): Fuzz test failed due to an assertion failure causing a deadly signal crash.
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.