state: dedupe Reaction events by author#137
Merged
Merged
Conversation
Each peer can react with a given emoji at most once. Switch ChatMessage::reactions from BTreeMap<String, Vec<EndpointId>> to BTreeMap<String, BTreeSet<EndpointId>> and use insert() in the materializer so duplicate Reaction events from the same author collapse to a single entry. Without this change, an author replaying or retransmitting a Reaction would inflate the reactor list, causing UI miscounts and state-hash divergence between peers. Closes #111 Progresses #108
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
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.
Summary
ChatMessage::reactionsfromBTreeMap<String, Vec<EndpointId>>toBTreeMap<String, BTreeSet<EndpointId>>so each peer can only be registered once per (message, emoji).EventKind::Reactionhandler inmaterialize.rsto useBTreeSet::insertinstead ofVec::push, eliminating duplicate reactor entries when the same author emits (or replays) the same Reaction event twice.same_author_duplicate_reaction_is_idempotentand tighten the existingduplicate_reaction_from_same_peerassertion from "may dedupe or not" to "must dedupe exactly".Why
The previous
Vec-backed shape meant a retransmitted or replayed Reaction would inflate the reactor list, causing UI reaction miscounts andStateHashdivergence between peers that received different delivery counts of the same event. Deduping by author in the materialized state is both the simplest fix and the most compatible — the wire format ofEventKind::Reactionis unchanged.Scope notes
DisplayMessage.reactionsinwillow-clientis a separateHashMap<String, Vec<String>>of display names and is unaffected..reactions(e.g.willow-client::views::messages_view) work unchanged because they only use.iter().Test plan
cargo test -p willow-state— 156 tests pass including the newsame_author_duplicate_reaction_is_idempotentregressioncargo test -p willow-client— 66 tests passcargo clippy --workspace -- -D warnings— cleancargo fmt --check— cleancargo check --target wasm32-unknown-unknownfor the state/client/channel/messaging/crypto/identity/transport/common/network/actor crates — cleanCloses #111
Progresses #108