One relatively common pattern of 1P1C relay is receiving a just-below-minfee parent, dropping it and marking it as reconsiderable, then fetching it again from the peer once the child is added to the orphanage.
A cache of dropped parents would be useful, and it turns out we’re already opportunistically storing transactions like this for compact blocks as “extra transactions”. Use this size-limited cache to potentially fetch a reconsiderable parent from memory, and submit for validation.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
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:
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.
One relatively common pattern of 1P1C relay is receiving
a just-below-minfee parent, dropping it and marking it
as reconsiderable, then fetching it again from the peer
once the child is added to the orphanage.
A cache of dropped parents would be useful, and it turns
out we're already opportunistically storing transactions
like this for compact blocks as "extra transactions".
Use this size-limited cache to potentially fetch a
reconsiderable parent, and submit for validation.
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.
Possibly this is 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.
Leave a comment here, if you need help tracking down a confusing failure.
1085
1086 /** Orphan/conflicted/etc transactions that are kept for compact block reconstruction.
1087 * The last -blockreconstructionextratxn/DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN of
1088 * these are kept in a ring buffer */
1089 std::vector<CTransactionRef> vExtraTxnForCompact GUARDED_BY(g_msgproc_mutex);
1090+ std::vector<NodeId> m_extra_txn_senders GUARDED_BY(g_msgproc_mutex);
1907+ for (size_t i = 0; i < vExtraTxnForCompact.size(); i++) {
1908+ const auto& maybe_parent = vExtraTxnForCompact[i];
1909+ // We stopped checking it earlier due to fee reasons, try again?
1910+ if (maybe_parent && m_recent_rejects_reconsiderable.contains(maybe_parent->GetWitnessHash().ToUint256())) {
1911+ Package maybe_cpfp_package{maybe_parent, orphan_child};
1912+ if (IsChildWithParents(maybe_cpfp_package) && !m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) {
62- ]]
63+ if self.options.noextratxn:
64+ self.extra_args = [[
65+ "-datacarriersize=100000",
66+ "-maxmempool=5",
67+ "-blockreconstructionextratxn=0", # require fetching parent even if cached
Nice. Have you tried running this to see how often it hits?
I’ll do some logging :+1: I suspect it’ll be quite above non-0 because from my log-reading just-below-minfee is quite commonly a reason for missing the parent.
edit: this isn’t ready for prime-time, have to more carefully think about handling the results. This new case is triggered even if we would consider continuing on failure in various cases, so I likely have bugs here.
Labels
P2P