Skip to content
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

Add topic scoring & reduce pubsub spam #573

Merged
merged 9 commits into from
Aug 14, 2024

Conversation

Stebalien
Copy link
Member

  1. Add topic scoring to both the GPBFT topic and the manifest topic.
  2. Use hashing for message IDs in the manifest network and increase the rebroadcast time.
  3. In the manifest validator, drop messages with old sequence numbers.

@Stebalien Stebalien force-pushed the steb/pubsub-validation-and-ids branch 2 times, most recently from 9fd30d7 to c38cfb2 Compare August 14, 2024 18:16
@Stebalien
Copy link
Member Author

Bump the pubsub topic versions at the same time?

@Stebalien Stebalien force-pushed the steb/pubsub-validation-and-ids branch from c38cfb2 to 544544d Compare August 14, 2024 18:35
1. Add topic scoring to both the GPBFT topic and the manifest topic.
2. Hash the data + topic for GPBFT messages.
3. Hash the data + sender + topic for manifest sever messages. We need
to include the sender because we reject messages from invalid senders.
4. In the manifest validator, drop messages with old sequence numbers.
@Stebalien Stebalien force-pushed the steb/pubsub-validation-and-ids branch from d5b83ea to a2201eb Compare August 14, 2024 18:48
Copy link

Fuzz test failed on commit 9bdf53b. To troubleshoot locally, download the seed corpus using GitHub CLI by running:

gh run download 10393399687 -n testdata

Alternatively, download directly from here.

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 46.51163% with 23 lines in your changes missing coverage. Please review.

Project coverage is 79.75%. Comparing base (4db84c3) to head (bfc8af6).
Report is 2 commits behind head on main.

Files Patch % Lines
internal/psutil/psutil.go 19.23% 16 Missing and 5 partials ⚠️
manifest/dynamic_manifest_provider.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #573      +/-   ##
==========================================
- Coverage   80.03%   79.75%   -0.28%     
==========================================
  Files          49       50       +1     
  Lines        4492     4525      +33     
==========================================
+ Hits         3595     3609      +14     
- Misses        547      562      +15     
- Partials      350      354       +4     
Files Coverage Δ
host.go 70.76% <100.00%> (-0.11%) ⬇️
manifest/dynamic_manifest_sender.go 82.25% <100.00%> (ø)
manifest/manifest.go 58.46% <100.00%> (ø)
manifest/dynamic_manifest_provider.go 60.48% <80.00%> (ø)
internal/psutil/psutil.go 19.23% <19.23%> (ø)

... and 2 files with indirect coverage changes

Copy link

Fuzz test failed on commit bfc8af6. To troubleshoot locally, download the seed corpus using GitHub CLI by running:

gh run download 10393581939 -n testdata

Alternatively, download directly from here.

@Stebalien Stebalien enabled auto-merge August 14, 2024 19:20
@Stebalien Stebalien added this pull request to the merge queue Aug 14, 2024
Merged via the queue into main with commit f14cefe Aug 14, 2024
9 of 12 checks passed
@Stebalien Stebalien deleted the steb/pubsub-validation-and-ids branch August 14, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants