-
Notifications
You must be signed in to change notification settings - Fork 313
mixing: Improve orphan handling. #3606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
+333
−91
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
d0fd026 to
2df0bde
Compare
jholdstock
reviewed
Jan 30, 2026
jholdstock
reviewed
Jan 30, 2026
jholdstock
reviewed
Jan 30, 2026
jholdstock
reviewed
Jan 30, 2026
Member
jholdstock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
davecgh
commented
Jan 30, 2026
2df0bde to
8499e50
Compare
Member
Author
Fixed. |
8499e50 to
a31b0f4
Compare
davecgh
commented
Jan 30, 2026
a31b0f4 to
38faeb9
Compare
jrick
reviewed
Jan 30, 2026
38faeb9 to
6a09310
Compare
jrick
approved these changes
Jan 30, 2026
This renames the internal rpcserver SubmitMixMessage interface method to AcceptMixMessage to more accurately reflect what it actually does and updates the comments accordingly. The comments also previously incorrectly claimed that the method submits the message to the network after it processes it, but that is handled by RelayMixMessages instead.
This makes use of the slices.Contains method to determine when a key exchange orphan references a pair request that has been added during orphan reconsideration.
This renames the type used for orphan messages to orphanMsg to avoid shadowing the name with variables. The name "orphan" is better for variables.
The current behavior when adding orphans is slightly different for KEs and other types of mixing messages. Specifically, orphan KEs overwrite any existing entries while other types do not. The preferred behavior is to keep existing orphans so that the timestamps reflect the time they were first added, and, in the future, any other data associated with the orphan also reflects when it was first seen. Instead of just adding an additional check for KEs to remedy the aforementioned, this consolidates the logic for adding mixpool orphans by introducing a new function named addOrphan and updates the places that add orphans to use it. The new function will only add orphans when they are not already in the pool. This approach simplifies future modifications to the logic related to adding orphans, ensures the same logic is consistently applied regardless of the path leading up to the addition of an orphan, and is more consistent with the how the long standing and battle tested existing code in the mempool handles adding orphans.
This consolidates the logic for removing mixpool orphans by introducing a new function named removeOrphan and updates the places that remove orphans to use it. This approach simplifies future modifications to the logic related to removing orphans, ensures the same logic is consistently applied regardless of the path leading up to the removal of an orphan, and is more consistent with the how the long standing and battle tested existing code in the mempool handles removing orphans. It is perhaps worth noting that there is a very slight tradeoff in terms of a few extra map length checks and removals that the existing code was able to avoid in some circumstances due to having specialized knowledge, but the difference is entirely negligible and does not warrant the downsides to splitting the removal logic all over the code.
This modifies the map that tracks orphans by their associated identity to point to the entire orphan struct instead of only the mixing message associated with it. This will allow any additional data associated with the orphan message to be immediately available without needing another orphans map lookup.
This allows a caller-provided source to be associated with mix messages. This is useful for things such as keeping track of which peers messages were first seen from and grouping messages on per-peer source. This currently only keeps track of the source with orphans to pave the way for some upcoming changes related to orphans. It might also be useful in the future to store it with messages in the main pool too, but that is not done for now since there are no immediate plans to make use of it.
6a09310 to
974dfbd
Compare
davecgh
commented
Jan 31, 2026
In addition to the existing logic which evicts orphans once an epoch has expired and when the peer that sent the orphans disconnects, this implements proactive eviction of mixing message orphans once the orphan pool grows to a maximum limit. The eviction algorithm works as follows: - Determine the source peer that sent the most remaining active orphans - Remove as many orphans sent by that peer as possible until the orphan pool size reaches 75% of the maximum allowed amount - Repeat the previous two steps as many times as necessary to reach the target pruned orphan pool size In short, it prioritizes removing the orphans sent by the peer that sent the most. This approach was chosen because it is fairly efficient and, in practice, orphan messages are quite rare after initial startup when ongoing mixing sessions are discovered, so any peer sending a lot of orphans is likely experiencing severe connectivity issues or otherwise misbehaving. It also has the added benefit of handling a variety of orphan flooding misbehavior well. A comprehensive set of tests is included to ensure the behavior works as expected.
974dfbd to
52fe7dc
Compare
jholdstock
approved these changes
Jan 31, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This improves the handling of mixing message orphans destined for the the mixing pool and includes some other miscellaneous improvements related to readability and maintainability.
A high level overview of the changes follows:
SubmitMixMessagetoAcceptMixMessageThe changes have been split into of a series of commits to help ease the review process. Each commit is intended to be a self-contained and logically easy to follow change such that the code continues to compile and the tests continue to pass at each step.
See the description of each commit for further details.