Improve startup resource usage #2733
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t-bast
force-pushed
the
improve-startup-resource-usage
branch
from
00a16b3
to
0eb0524
Compare
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #2733 +/- ##
==========================================
+ Coverage 85.78% 85.82% +0.03%
==========================================
Files 216 216
Lines 18013 18061 +48
Branches 759 743 -16
==========================================
+ Hits 15453 15500 +47
- Misses 2560 2561 +1
|
This should reduce the pressure on the file system and RAM without impacting our ability to troubleshoot common issues.
When we restart, we put watches on every public channel in the network. That creates a lot of RPC calls to bitcoind, which aren't time-sensitive. It's ok if we don't see immediately that an external channel was closed, the spec even recommends waiting for 12 blocks to distinguish a channel close from a splice. By default, we now smooth that over a 1 hour period. This means we should also allow our peers to be late at discovering that a channel was closed. We thus stop sending a `warning` in that case and increase our tolerance to that kind of behavior.
When we restart, we set watches on our funding transactions. But we don't actually need to watch them immediately, we just need enough time to react to our peer broadcasting their commitment. We use long `cltv_delta` delays to guarantee funds safety, so we can spread out the watches across several blocks to reduce the start-up load. It essentially is the same thing as receiving mempool transactions or blocks after a delay, which is something that our threat model already takes into account.
t-bast
force-pushed
the
improve-startup-resource-usage
branch
from
0eb0524
to
3800e6c
Compare
pm47
reviewed
Sep 26, 2023
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/CommonFundingHandlers.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/CommonFundingHandlers.scala
Show resolved
Hide resolved
sstone
reviewed
Sep 26, 2023
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala
Outdated
Show resolved
Hide resolved
pm47
approved these changes
Sep 26, 2023
sstone
approved these changes
Sep 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains independent commits to reduce the load and performance hit on node restart.
@pm47 the ACINQ node should set a higher value than the default for
eclair.channel.max-restart-watch-delay, since this depends on the number of channels the node has. I think this value can safely go up to 30 minutes (3 blocks) since ourcltv_expiry_deltais144blocks and ourto_self_delayis720blocks.