-
Notifications
You must be signed in to change notification settings - Fork 942
Store onchaind state in the database and reduce rescan on startup #1398
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
Changes from 14 commits
fb8af75
3b3f62f
961e55b
2bdbcc4
57f9129
e147967
84376f8
ea97404
155335e
01cb08a
17b3286
47c4e74
8ef2e8a
ddd53f1
fc2b6c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -309,6 +309,10 @@ static void config_register_opts(struct lightningd *ld) | |
opt_register_arg("--fee-base", opt_set_u32, opt_show_u32, | ||
&ld->config.fee_base, | ||
"Millisatoshi minimum to charge for HTLC"); | ||
opt_register_arg("--rescan", opt_set_s32, opt_show_s32, | ||
&ld->config.rescan, | ||
"Number of blocks to rescan from the current head, or " | ||
"absolute blockheight if negative"); | ||
opt_register_arg("--fee-per-satoshi", opt_set_s32, opt_show_s32, | ||
&ld->config.fee_per_satoshi, | ||
"Microsatoshi fee for every satoshi in HTLC"); | ||
|
@@ -420,6 +424,9 @@ static const struct config testnet_config = { | |
|
||
/* Testnet sucks */ | ||
.ignore_fee_limits = true, | ||
|
||
/* Rescan 5 hours of blocks on testnet, it's reorg happy */ | ||
.rescan = 30, | ||
}; | ||
|
||
/* aka. "Dude, where's my coins?" */ | ||
|
@@ -481,6 +488,9 @@ static const struct config mainnet_config = { | |
|
||
/* Mainnet should have more stable fees */ | ||
.ignore_fee_limits = false, | ||
|
||
/* Rescan 2.5 hours of blocks on startup, it's not so reorg happy */ | ||
.rescan = 15, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will rescan 15 blocks before the current block height? The comment seems not matching? (should it not be 0 if we just continue scanning at where we left off?) Also, I remember we had an issue before that our block height would keep getting pushed back if we kept getting restarted, would that behavior occur again? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, I had 0 in there initially and then I remembered that we don't handle reorgs below our in-memory chain all too well, so I made it 15 blocks. If we process less than 15 blocks before crashing, then yes, this could end up walking backwards, we could add the lower bound at the segwit activation back in, but that's also painful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An alternative could be to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is still |
||
}; | ||
|
||
static void check_config(struct lightningd *ld) | ||
|
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.
Seems more sensible, to use negative to mean "current blockheight minus absolute value" and to use positive for "absolute blockheight". And to use 0 to mean "current blockheight, no rescan". Similar to Python arrays/strings, with the "end" being the current known blockheight and a negative index being relative from the end.
But this comment is not a blocking one and I think the current standard is reasonable.
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 preferred making the more complex case, i.e., choosing a fixed offset, the case with the added sign. Everybody should just use relative offsets from their current head, in particular if they run from a startup script, and only use the absolute offset (with negation) in a recovery scenario.
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 understand.
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.
Yeah, nobody should be using this anyway, so I think -ve for relative is pretty common. Otherwise you want --rescan-absolute or something...