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

opts: Be more lenient about locktime_max and fee range by default #1563

Merged
merged 4 commits into from
Jun 14, 2018

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Jun 11, 2018

This has been brought up a few times, and I think we should at least ensure that the default parameters of the three implementations match. Together with #1551 this obviates all the workarounds I had in the integration testing suite. I still think our old defaults were better, but maybe we can go back to saner defaults over time.

I added the max_fee_multiplier as an option since it was the only non-configurable parameter that needed to be changed for this.

Closes #1110
Closes #1560

@cdecker cdecker added this to the v0.6 milestone Jun 11, 2018
@cdecker cdecker self-assigned this Jun 11, 2018
@cdecker cdecker requested a review from rustyrussell June 11, 2018 12:44
@cdecker cdecker force-pushed the stupid-default-params branch from 51837f5 to f9ede7b Compare June 11, 2018 12:46
I still believe that 2 weeks is way too much, but we were promised that these
defaults would be slowly reduced to saner values as the stability increases.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
@cdecker cdecker force-pushed the stupid-default-params branch from f9ede7b to fbe3a92 Compare June 11, 2018 16:03
cdecker added 3 commits June 11, 2018 19:50
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The fee range can sometimes cause channels to be closed when the estimator
jumps. This has been the case a few times in the last months, and causes a
number of channels to be closed, and issue reports to be filed.

Increasing this from 5x to 10x should get rid of 84%+ of these
closures (measured based on 1h windows over the last 6 months and assuming
worst case situations).

Signed-off-by: Christian Decker <decker.christian@gmail.com>
@cdecker cdecker force-pushed the stupid-default-params branch from fbe3a92 to 619575b Compare June 11, 2018 17:52
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 619575b

@rustyrussell rustyrussell merged commit 2848103 into ElementsProject:master Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants