Skip to content

Conversation

@StephenButtolph
Copy link
Contributor

Why this should be merged

gossip.BloomFilter is fairly tough to use as it is only partially thread safe (and requires external synchronization primitives with the underlying mempool.

This provides a single implementation that abstracts the usage of a bloom filter away from the VM (unless the VM is doing something weird, like the X-chain).

How this works

Implements a gossip.SetWithBloomFilter implementation that hides the bloom filter from the VM and uses it rather than using gossip.BloomFilter.

How this was tested

Unit tests + existing e2e tests

Need to be documented in RELEASES.md?

No

@blacksmith-sh

This comment has been minimized.

if err := s.set.Add(v); err != nil {
return err
}
return s.AddToBloom(v.GossipID())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Should this return the error? Or should this just log a warning. The value was added to the inner set and to the bloom filter... So it might make more sense to just log and move on.

@StephenButtolph StephenButtolph force-pushed the cleanup-gossip-package branch 2 times, most recently from fff1ca2 to 4e80be5 Compare November 25, 2025 20:48
@StephenButtolph StephenButtolph changed the base branch from master to bloom-count-noop November 26, 2025 17:54
Base automatically changed from bloom-count-noop to master December 1, 2025 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants