fix: make detector report output deterministic - #30
Merged
Conversation
Detectors iterated HashSet/HashMap directly, so two scans of the same wallet produced findings in different order and, in three spots, with different content (sets dumped into descriptions and details). Sort txids and addresses before iterating, sort set-derived arrays before formatting, and stable-sort findings and warnings in detect_all.
Both corrections only pointed at coin control and CoinJoin. When the user is making a payment anyway, a Payjoin (BIP-78) merges inputs without revealing common ownership, so mention it as the preferred option for that case.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Two identical scans could produce reports with findings in different orders, and in two spots with different wording, because detectors iterated HashSet/HashMap directly: the our_txids collect pattern repeated across 13 detectors, our_addrs iteration in address reuse, and two places that formatted a HashSet straight into description/details text.
Changes:
Test: a new node-free test target (engine/tests/determinism.rs) builds a synthetic WalletHistory rich enough to fire all 16 detectors (asserted one by one), then runs the full pipeline five times and requires byte-identical serialized reports. Each in-process run uses fresh HashMap seeds, so this fails reliably against the old code (recorded red run) and passed in three extra fresh processes after the fix. Also verified live on mainnet: two identical scans through the HTTP API returned byte-identical bodies. Suite 41/41, clippy -D warnings clean.