Canonicalization should handle transactions that spend from two conflicting transactions #1917
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
evanlinjin
force-pushed
the
canonicalization_improvements
branch
from
ac28f70 to
9f73527
Compare
evanlinjin
force-pushed
the
canonicalization_improvements
branch
3 times, most recently
from
3e7c702 to
133f09c
Compare
Detect self-double-spends in `CanonicalIter::mark_canonical`. Disregard the tx that self-double-spends and all it's attached meta data (such as timestamps and anchors). Add new test cases on `test_tx_conflict_handling` for transactions that self-double-spend.
evanlinjin
force-pushed
the
canonicalization_improvements
branch
from
133f09c to
370497c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1898
Description
When we initially created the canonicalization algorithm, we didn't expect callers to insert invalid transactions into the graph. However, user error happens and we should handle it correctly.
Before this PR, when inserting transactions that double-spend themselves (where two or more different inputs of the transactions conflict), the canonicalization result will have inconsistencies.
Notes to the reviewers
Logic changes are all contained in
CanonicalIter::mark_canonical.mark_canonicalwill detect whether thetxbeing passed in double spends itself. If so, we abort and undo all changes made so far.There is a slight <2% degradation in performance with this change (except in two cases where there is a performance improvement of ~10%).
bench.txt
Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingBugfixes:
* [ ] This pull request breaks the existing API