-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
htlcswitch/link: correct bias in fee update backoff #1482
htlcswitch/link: correct bias in fee update backoff #1482
Conversation
472d8d3
to
0bc37f7
Compare
This commit corrects the distribution used to schedule a link's randomized backoff for fee updates. Currently, our algorithm biases the lowest value in the range, with probability equal to lower/upper, or the ratio of the lower bound to the upper. This distribution is skewed more heavily as lower approaches upper. The solution is to sample a random value in the range upper-lower, then add this to our lower bound. The effect is a uniformly distributed timeout in [lower, upper).
0bc37f7
to
5fa8124
Compare
f9be9f3
to
12f74f7
Compare
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.
LGTM ⚡️
@@ -1497,7 +1497,7 @@ func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) ( | |||
// to not trigger commit updates automatically during tests. | |||
BatchSize: 10000, | |||
MinFeeUpdateTimeout: 30 * time.Minute, | |||
MaxFeeUpdateTimeout: 30 * time.Minute, | |||
MaxFeeUpdateTimeout: 40 * time.Minute, |
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.
Why is 40 more realistic than 30?
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.
Since we now do upper-lower
, this would panic because we're passing in 0 to math.Rand
.
This commit corrects the distribution used to
schedule a link's randomized backoff for fee
updates. Currently, our algorithm biases the
lowest value in the range, with probability
equal to lower/upper, or the ratio of the lower
bound to the upper. This distribution is skewed
more heavily as lower approaches upper.
The solution is to sample a random value in the
range upper-lower, then add this to our lower
bound. The effect is a uniformly distributed
timeout in [lower, upper).
Just realizing that I noted this in a pending review of #1229, but forgot to publish before PR was merged.