policy: don't CheckEphemeralSpends on reorg #33616
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33616. ReviewsSee the guideline for information on the review process. ConflictsReviewers, 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. |
glozow
left a comment
There was a problem hiding this comment.
This seems to make sense. I'd feel more confident if we add some more test cases for topologies and reorg scenarios that this does (not) apply to?
| @@ -1608,7 +1608,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: | |||
| } | |||
|
|
|||
| // Now that we've bounded the resulting possible ancestry count, check package for dust spends | |||
| if (m_pool.m_opts.require_standard) { | |||
| if (!args.m_bypass_limits && m_pool.m_opts.require_standard) { | |||
There was a problem hiding this comment.
I think it's impossible for m_bypass_limits to be true in this function, so undoing this diff doesn't make a difference. Could we omit it or Assume(!args.m_bypass_limits)?
There was a problem hiding this comment.
Took feedback in other PR ahead of time #33504 (comment)
I can definitely put an Assume() on it.
04832d0 to
0ae5db4
Compare
|
marking as draft for now until it becomes dependency-less and I've looked over the tests some more |
0ae5db4 to
9ddde71
Compare
9ddde71 to
a5299c3
Compare
|
Added some more coverage and now has no dependencies. |
Similar reasoning to #33504
During a deeper reorg it's possible that a long sequence of dust-having transactions that are connected in a linear fashion. On reorg, this could cause each subsequent "generation" to be rejected. These rejected transactions may contain a large amount of competitive fees via normal means.
PreCheck based
PreCheckEphemeralSpendsis left in place because we wouldn't have relayed them prior to the reorg.