Fuzz: Check individual and package transaction invariants #28764
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
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. 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. |
This was referenced Oct 31, 2023
dergoegge
reviewed
Nov 1, 2023
instagibbs
force-pushed
the
fuzz_atmp_invariants
branch
from
76bbd9e to
b847d48
Compare
glozow
reviewed
Nov 2, 2023
Member
glozow
left a comment
glozow
left a comment
There was a problem hiding this comment.
ACK b847d48f70b5a67263c362f9d28ee6d092068e27, thanks for shaving!
instagibbs
force-pushed
the
fuzz_atmp_invariants
branch
from
b847d48 to
fcb3069
Compare
Member
|
reACK fcb3069, only whitespace changes |
dergoegge
approved these changes
Nov 3, 2023
instagibbs
commented
Nov 3, 2023
| } | ||
| } else { | ||
| if (result.m_state.IsValid()) { | ||
| strprintf("Package validation unexpectedly succeeded. %s", result.m_state.ToString()); |
Member
Author
There was a problem hiding this comment.
missing some return statements in this function
Fabcien
added a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jul 10, 2024
…es and skipped transactions
Summary:
```
- Add 2 new TxValidationResults
- TX_RECONSIDERABLE helps us encode transactions who have failed fee checks that can be bypassed using package validation. This is distinguished from TX_MEMPOOL_POLICY so that we re-validate a transaction if and only if it is eligible for package CPFP. In the future, we will have a separate cache for reconsiderable rejects so these transactions don't go in m_recent_rejects.
- TX_UNKNOWN helps us communicate that we aborted package validation and didn't finish looking at this transaction: it's not valid but it's also not invalid (i.e. don't cache it as a rejected tx)
- Return effective feerate and the wtxids of transactions used to calculate that effective feerate when the error is TX_SINGLE_FAILURE. Previously, we would only provide this information if the transaction passed. Now that we have package validation, it's much more helpful to the caller to know how the failing feerate was calculated. This can also be used to improve our submitpackage RPC result (which is currently a bit unhelpful when things fail).
- Use the newly added CheckPackageMempoolAcceptResult for existing package validation tests. This increases test coverage and helps test the changes made in this PR.
```
Backport of [[bitcoin/bitcoin#28785 | core#28785]], [[bitcoin/bitcoin#28764 | core#28764]] and [[bitcoin/bitcoin#28788 | core#28788]] (last too are only the test function).
Test Plan:
ninja all check-all
Reviewers: #bitcoin_abc, roqqit
Reviewed By: roqqit
Differential Revision: https://reviews.bitcoinabc.org/D16439
roqqit
pushed a commit
to doged-io/doged
that referenced
this pull request
Aug 1, 2024
…es and skipped transactions
Summary:
```
- Add 2 new TxValidationResults
- TX_RECONSIDERABLE helps us encode transactions who have failed fee checks that can be bypassed using package validation. This is distinguished from TX_MEMPOOL_POLICY so that we re-validate a transaction if and only if it is eligible for package CPFP. In the future, we will have a separate cache for reconsiderable rejects so these transactions don't go in m_recent_rejects.
- TX_UNKNOWN helps us communicate that we aborted package validation and didn't finish looking at this transaction: it's not valid but it's also not invalid (i.e. don't cache it as a rejected tx)
- Return effective feerate and the wtxids of transactions used to calculate that effective feerate when the error is TX_SINGLE_FAILURE. Previously, we would only provide this information if the transaction passed. Now that we have package validation, it's much more helpful to the caller to know how the failing feerate was calculated. This can also be used to improve our submitpackage RPC result (which is currently a bit unhelpful when things fail).
- Use the newly added CheckPackageMempoolAcceptResult for existing package validation tests. This increases test coverage and helps test the changes made in this PR.
```
Backport of [[bitcoin/bitcoin#28785 | core#28785]], [[bitcoin/bitcoin#28764 | core#28764]] and [[bitcoin/bitcoin#28788 | core#28788]] (last too are only the test function).
Test Plan:
ninja all check-all
Reviewers: #bitcoin_abc, roqqit
Reviewed By: roqqit
Differential Revision: https://reviews.bitcoinabc.org/D16439
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Poached from #26711 since that PR is being split apart, and modified to match current behavior.