-
Notifications
You must be signed in to change notification settings - Fork 54
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
Bandwidth estimate as a parameter #941
Conversation
b8c8466
to
0abb75c
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #941 +/- ##
=========================================
Coverage 83.51% 83.52%
=========================================
Files 91 91
Lines 15145 15149 +4
=========================================
+ Hits 12649 12653 +4
Misses 2496 2496
|
@@ -74,7 +74,8 @@ proc init*(_: type[GossipSubParams]): GossipSubParams = | |||
behaviourPenaltyWeight: -1.0, | |||
behaviourPenaltyDecay: 0.999, | |||
disconnectBadPeers: false, | |||
enablePX: false | |||
enablePX: false, | |||
bandwidthEstimateMbps: 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 should disable flood-limiting completely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, I think it would make more sense for the parameter to be expressed in bps - this way, users can implement a parser that understands G, M, k, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, I think it would make more sense for the parameter to be expressed in bps - this way, users can implement a parser that understands G, M, k, etc
Is it common to specify bandwidth in a unit different than Mb? Maybe in Gb sometimes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it more flexible and easier when API:s use basic units rather than pre-baked multiples - there's more flexibility / future-proofing this way for API users (ie imagine working on an embedded system with more aggressive bandwidth constraints).
This is somewhat different from when working with "normal users" or end-user UX.
What's blocking this? |
nothing, just wanted @arnetheduck to take a look again |
This is to replace the hard-coded value in #911