[release-v2.0] multi: Use backported mixing updates. #3317
Merged
Conversation
This file contains 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
This value needs to be lower than the confirmations wallet requires (2) before mixing any output, otherwise it is very easy to race on rejecting these messages as new blocks were just created. This is a backport candidate for 2.0.1.
reconsiderOrphans was accessing pool state without the pool mutex held, which
results in data races.
To resolve this, some refactoring is necessary. reconsiderOrphans calls
acceptKE, which did acquire the mutex, so that needs to be hoisted out.
In this commit, the pre-mutex-acquire sanity checks for PR and KE messages in
acceptPR and acceptKE are moved to separate checkAccept{PR,KE} functions for
the caller, who then becomes responsible for acquiring the mutex. After
acceptPR/acceptKE return, reconsiderOrphans is then called with the mutex
still held.
This is a backport candidate for 2.0.1.
The Hash160 method in the stdaddr package is preferred over dcrutil since ideally dcrutil will be removed one day.
The Hash method on wire messages unfortunately involves allocations, so it's generally best to keep the result as a local and use that local through to avoid more allocs. Also, since fmt.Errorf involves interfaces, taking the address of the local session id forces it to the heap. It's faster to copy it into the interface instead and avoid the GC hit.
To reduce spurious error logs when invalid messages are received off the network, the RuleError type is expanded to cover submission errors that should not necessarily result in an instant permanent ban. A new function IsBannable is added to check whether the error condition is still such that it should be treated as bannable.
This adds a new RPC named getmixmessage to the JSON-RPC server which queries the current mixpool for a mixing message by its hash. This RPC will be used in later wallet releases to fetch unknown pair request messages when a run-0 key exchange is rejected from the wallet's mixpool for referencing an unknown PR by the same identity. This is also how initial PR sync occurs during the warmup period, replacing the earlier use of getmixpairrequests.
Pair request messages were being unintentionally discarded from the mixpool even on sessions that did not run to completion. This was occurring under two separate situations: The first situation occured if the context passed to the mixpool's Receive method while receiving KEs was cancelled. A last-minute change in semantics of the error return result of the Receive method during review of the mixpool package resulted in this method no longer returning the non-nil context error. Most of the client was updated for this change in semantics, but the KE receiving is special and was not handling the change properly. This resulted in the run method returning with a nil error, and the PRs from the incompleted session were removed like it was actually completed. The second situation is a longstanding bug unrelated to the recent mixpool review. If a run is aborted due to the next epoch about to start, this was also causing the run method to return with a nil error, and the PRs were likewise incorrectly removed. In both of these situations, peers were still trying to mix using these PRs, but other peers on the network would not accept their later KEs after the PRs had been removed. Debug logging has been improved substantially as part of debugging these issues, and in particular, the session ID and the coinjoin transaction hash are now both logged after a mix is successful and the PRs are finally (and correctly) removed from the mixpool.
When creating the alternate session failed during a run, the wrong error was being returned to the caller. To simplify this logic, don't check the alternate session's error field here, and just return the alternate session (itself implementing the error type) to the caller (pairSessions, who calls run). While here, some additional details are included in the wrapped error returned when receiving KEs, which will aid in future debugging.
jrick
approved these changes
May 24, 2024
davecgh
force-pushed
the
rel20_mixing_backports
branch
2 times, most recently
from
18f9b2d to
54656c7
Compare
This updates the 2.0 release branch to use the latest versions of the dcrjson/v4, rpc/jsonrpc/types/v4, and mixing modules which include some minor improvements to the pairing process and the addition of a new RPC method to obtain mixing messages from the mixpool. In particular, the following updated module versions are used: - github.com/decred/dcrd/dcrjson/[email protected] - github.com/decred/dcrd/[email protected] - github.com/decred/dcrd/rpc/jsonrpc/types/[email protected]
davecgh
force-pushed
the
rel20_mixing_backports
branch
from
54656c7 to
422aef9
Compare
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 updates the 2.0 release branch to use the latest versions of the
dcrjson/v4,rpc/jsonrpc/types/v4, andmixingmodules which include some minor improvements to the pairing process and the addition of a new RPC method namedgetmixmessageto obtain mixing messages from the mixpool. It also backports the relevant RPC server code to implement the new method.In particular, the following updated module version is used:
Note that it also cherry picks all of the commits included in updates to the
dcrjson/v4,rpc/jsonrpc/types/v4, andmixingmodules to ensure they are also included in the release branch even though it is not strictly necessary sincego.modhas been updated to require the new releases and thus will pull in the new code. However, from past experience, not having code backported to modules available in the release branch too leads to headaches for devs building from source in their local workspace with overrides such as those ingo.work.