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

Vec Broadcaster

Open yellowred opened this issue 1 year ago • 4 comments

Implementation of BroacasterInterface that does not broadcast transactions immediately, but stores them internally to broadcast later with a call to release_transactions.

yellowred avatar Dec 06 '24 01:12 yellowred

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 88.39%. Comparing base (ad462bd) to head (22554ee).

Files with missing lines Patch % Lines
lightning/src/chain/chaininterface.rs 0.00% 16 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3448      +/-   ##
==========================================
- Coverage   88.41%   88.39%   -0.02%     
==========================================
  Files         149      149              
  Lines      112959   112975      +16     
  Branches   112959   112975      +16     
==========================================
- Hits        99871    99863       -8     
- Misses      10616    10634      +18     
- Partials     2472     2478       +6     

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

codecov[bot] avatar Jan 14 '25 18:01 codecov[bot]

I'm not sure if there's a need for this to be upstreamed, it seems like a somewhat niche use case?

wpaulino avatar Jan 14 '25 18:01 wpaulino

I'm not sure if there's a need for this to be upstreamed, it seems like a somewhat niche use case?

The idea is to have better separation between deserializer and claim broadcaster. Originally I was thinking to have a separate function that deserializes a monitor and claims separately, which allows the user to choose what to do with this data, but eventually we decided to not change anything existing and just solve this by injecting vec broadcaster https://github.com/lightningdevkit/rust-lightning/pull/3396#issuecomment-2471551838

yellowred avatar Jan 14 '25 22:01 yellowred

I think I agree - I don't think we need to expose this publicly, but I think it may be a useful internal utility to build #3428. Doing it as an internal utility in #3428 also means that we can make the implementation more robust - we can automatically broadcast for the user once we're up and running (e.g. during block connection) so that a bug on the user's end failing to fetch the to-broadcast set won't cause them to accidentally not broadcast at all.

TheBlueMatt avatar Jan 16 '25 16:01 TheBlueMatt