[RFC] event: store the outpoint when is_manual_broadcast - #3259
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3259 +/- ##
==========================================
+ Coverage 89.66% 90.57% +0.90%
==========================================
Files 125 126 +1
Lines 102797 110724 +7927
Branches 102797 110724 +7927
==========================================
+ Hits 92176 100288 +8112
+ Misses 7908 7863 -45
+ Partials 2713 2573 -140 ☔ View full report in Codecov by Sentry. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Much better, one more note on serialization.
aa94d3b to
d0db6d3
Compare
d0db6d3 to
24b2401
Compare
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
LGTM, one comment about docs still but I can do it in a followup.
With [1], it's possible to specify `manual_broadcast` for the channel funding transaction. When `is_manual_broadcast` is set to true, the transaction in the `DiscardFunding` event is replaced with a dummy empty transaction. This commit checks if `is_manual_broadcast` is true and stores the funding OutPoint in the DiscardFunding event instead. [1] lightningdevkit#3024 Link: lightningdevkit#3164 Suggested-by: TheBlueMatt Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
24b2401 to
5e874c3
Compare
| (0, channel_id, required), | ||
| (2, transaction, required) | ||
| (2, transaction, option), | ||
| (4, funding_info, required), |
There was a problem hiding this comment.
Tried to look into ways to not break downgrades for this (i.e. writing a dummy or partially-dummy tx if unsafe_manual_funding_transaction_generated was used) but it doesn't seem worth it. So just noting that we'll need a release note for this since I don't see one in pending_changelog (can happen in follow-up).
There was a problem hiding this comment.
I can ask how the pending_changelog works? I could make the followup, sorry if I missed it
There was a problem hiding this comment.
We just put things in there to remember to include them in the changelog. I realized there was another issue so i went ahead and did the followup at #3275
3 participants
This should be trivial enough, but I would like to have some discussion on some of the
code that I implemented (I left some FIXME)
Commit ceab25c should have more context on the PR
Fixes: #3164