Ask for locked TXs after removing conflicting TXs #2898
Merged
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
When we removed a conflicting TX from the mempool, the correct/locked TX is not available locally as the first-seen rule would have filtered before. We need to re-request this TX if any other node announced it before.
UdjinM6
requested changes
May 3, 2019
Co-Authored-By: codablock <[email protected]>
5 tasks
PastaPastaPasta
added a commit
that referenced
this pull request
Jul 26, 2025
…ons, avoid redundant transaction queries f488c43 fix: remove unnecessary tx fetching in `RemoveMempoolConflictsForLock` (Kittywhiskers Van Gogh) dd54011 refactor: move inexpensive checks earlier in `ProcessInstantSendLock()` (Kittywhiskers Van Gogh) c86d886 refactor: abstract away InstantSend parent implementation from signer (Kittywhiskers Van Gogh) cbfd325 refactor: avoid shared_ptr copies in `{WriteNew,Remove}InstantSendLock()` (Kittywhiskers Van Gogh) 234a16a refactor: use unordered set for faster duplicates checking (Kittywhiskers Van Gogh) 0459848 refactor: `constexpr` static string definitions (Kittywhiskers Van Gogh) 710c504 refactor: s/cs_inputReqests/cs_input_requests/g (Kittywhiskers Van Gogh) 31065d1 chore: mark `hashBlock` as unused in `HandleNewInputLockRecoveredSig()` (Kittywhiskers Van Gogh) cf47f14 chore: apply most `clang-format` suggestions (Kittywhiskers Van Gogh) 63371e0 trivial: move forward declaration above `using` declarations (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6742 * This pull request aims to address reviewer concerns raised during [dash#6742](#6742), the following changes were made with that in mind * Marking unused variable `hashBlock` by prefixing with underscore, [comment](#6742 (comment)) * Correct typo in variable name `cs_inputReqests` (now `cs_input_requests`, [comment](#6770 (comment))), [comment](#6742 (comment)) * Using `constexpr` for `std::string_view`s, [comment](#6742 (comment)) * Utilizing a faster way to validate uniqueness of entries (`std::unordered_set`), [comment](#6742 (comment)). The construction of the unordered set is inspired by similar usage in `policy/package.cpp` ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/policy/packages.cpp#L51)). * Avoiding unnecessary `std::shared_ptr` copying (`InstantSendLockPtr` is a `std::shared_ptr<InstantSendLock>`), [comment](#6742 (comment)) * Resolving circular dependency by cherry-picking 1e2e854, [comment](#6742 (comment)) but has been renamed to `InstantSendSignerParent`. For rationale, see [comment](#6761 (comment)). * Additionally, optimizations have been made to `ProcessInstantSendLock()` * The _cheaper_ database checks have been moved _earlier_ in the function to allow for faster bail-out. This is alongside consolidating network requests to the tail of the function and general cleanup for better clarity. * Dropping the peer request for a transaction we know about in `RemoveMempoolConflictsForLock()` (introduced in [dash#2898](#2898)) as `ProcessInstantSendLock()` calls it when we _know_ about the transaction and seek to remove conflicts ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/instantsend/instantsend.cpp#L419-L420)) The only other caller, `TransactionAddedToMempool()` calls it when we have knowledge of the transaction and want to purge conflicts from the mempool ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/src/instantsend/instantsend.cpp#L455-L463)). In both cases, we already know the _good_ transaction. Additionally, tests introduced in [dash#2898](#2898) still pass ([source](https://github.com/dashpay/dash/blob/f648039258d2192ec5e505721e34edaa0bda568c/test/functional/p2p_instantsend.py#L95-L142)), indicating a lack of regression but this change may require additional scrutiny. ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK f488c43 knst: utACK f488c43 UdjinM6: utACK f488c43 Tree-SHA512: dcd153e14747639f8dbbbd5c3bea1123e4f84f82884787992e20f494227913763bf19a73b485e345dddb63ad551c856bc56dcad72e67304f84df60cb76deaa74
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
No description provided.