Add more information to OpenChannelRequest Event #3019
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3019 +/- ##
==========================================
- Coverage 89.09% 89.09% -0.01%
==========================================
Files 118 118
Lines 97678 97681 +3
Branches 97678 97681 +3
==========================================
Hits 87029 87029
- Misses 8394 8402 +8
+ Partials 2255 2250 -5 ☔ View full report in Codecov by Sentry. |
|
Msrv strikes again 😭 |
| /// True if this channel is (or will be) publicly-announced. | ||
| is_public: bool, | ||
| /// Opening fields for the channel given by the counterparty. | ||
| open_fields: msgs::CommonOpenChannelFields, |
There was a problem hiding this comment.
Mh, if we want to do this, I wonder if we should drop the channel_type field as it's redundant.
There was a problem hiding this comment.
I don't think so, channel type is calculated through a function above, eaiser to give that to the user so they don't need to calculate themselves
There was a problem hiding this comment.
I mean if we want to do that we should just (finally) require channel type features and then its never negotiated but rather we always use what is in the message :).
That said, there's a lot of extra crap here...I'm fairly confident users don't care about the basepoints of the channel, the temporary_channel_id is redundant, and the chain hash is always matching (cause we check it in LDK). ISTM this is being included primarily for the numerical fields (dust limit, max in-flight value, htlc min, initial feerate, to-self delay and max HTLCs), which makes sense, but its a bit annoying to shove a lot of additional data in here.
Can we instead add like a ChannelParameters struct which contains just these? Can be a part of the wire message struct or a standalone one for the event.
|
Btw, we should also look into allowing acting on any of these fields soon, see #2914 |
Noticed it was missing
is_publicand figured could just add all the fields since this isn't serialized.