-
Notifications
You must be signed in to change notification settings - Fork 376
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
Update Channel Monitor without broadcasting transactions #3428
base: main
Are you sure you want to change the base?
Update Channel Monitor without broadcasting transactions #3428
Conversation
aa8a8f8
to
9ae5067
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3428 +/- ##
==========================================
- Coverage 89.24% 89.23% -0.01%
==========================================
Files 130 130
Lines 106912 107132 +220
Branches 106912 107132 +220
==========================================
+ Hits 95410 95602 +192
- Misses 8706 8728 +22
- Partials 2796 2802 +6 ☔ View full report in Codecov by Sentry. |
Allowing Channel Monitor to be updated without executing any commands outside ChannelMonitor. This allows reading Channel Monitor in MonitorUpdatingPersister without using broadcaster and fee estimator and broadcast claims when the node is ready for it.
9ae5067
to
d767353
Compare
|
||
/// Same as [provide_payment_preimage], but does not immediately broadcasts transactions, | ||
/// returning them back to the caller instead. | ||
fn provide_payment_preimage_without_broadcasting<L: Deref>( |
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.
@TheBlueMatt Not sure about this func. On one hand I don't want to change provide_payment_preimage
, on the other -- there is clear duplicated logic. Do you think it would be better to modify provide_payment_preimage
to support returning claims?
I'm a bit confused how this differs from #3396, but IMO we should ship this by building a |
This implementation does not require a broadcaster and uses your suggestion to store packages internally in CMon and automatically broadcast when there is a new block connected, which is potentially easier for the user. I have the version with storing broadcaster in some other project, but note that it will not automatically broadcast transactions, a user have to call a function for that. Here is the PR for a Broadcaster that stores transactions #3448 |
Allowing Channel Monitor to be updated without executing any commands outside ChannelMonitor. This allows reading Channel Monitor in MonitorUpdatingPersister without using broadcaster and fee estimator and broadcast claims when the node is ready for it.