rust-lightning icon indicating copy to clipboard operation
rust-lightning copied to clipboard

Update Channel Monitor without broadcasting transactions

Open yellowred opened this issue 1 year ago • 4 comments

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.

yellowred avatar Nov 27 '24 00:11 yellowred

Codecov Report

Attention: Patch coverage is 81.85185% with 49 lines in your changes missing coverage. Please review.

Project coverage is 89.23%. Comparing base (12920d8) to head (9ae5067).

Files with missing lines Patch % Lines
lightning/src/chain/channelmonitor.rs 81.85% 36 Missing and 13 partials :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 27 '24 02:11 codecov[bot]

I'm a bit confused how this differs from #3396, but IMO we should ship this by building a Broadcaster that stores the transaction packages and then stores the transactions as they were to be broadcasted and broadcasts them later.

TheBlueMatt avatar Dec 03 '24 18:12 TheBlueMatt

I'm a bit confused how this differs from #3396, but IMO we should ship this by building a Broadcaster that stores the transaction packages and then stores the transactions as they were to be broadcasted and broadcasts them later.

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 https://github.com/lightningdevkit/rust-lightning/pull/3448

yellowred avatar Dec 05 '24 23:12 yellowred

Right, sorry for the delay here (release plus vacation ate a lot of time). I think this patch size can be reduced by about 4x without impacting functionality. If we use the broadcaster from #3448 as an internal/private utility, we can implement this without touching ChannelMonitorImpl's methods at all, I believe.

TheBlueMatt avatar Jan 16 '25 16:01 TheBlueMatt