Uh oh!
There was an error while loading. Please reload this page.
Use UserConfig to determine advertised InitFeatures by ChannelManager - #1946
Conversation
Uh oh!
There was an error while loading. Please reload this page.
07be198 to
a6c69b0Compare| let open_channel_msg = get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id()); | ||
| assert_eq!(open_channel_msg.temporary_channel_id, create_chan_id); | ||
| assert_eq!(node_a.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 42); | ||
| let a_flags = node_a.override_init_features.borrow().clone().unwrap_or_else(|| node_a.node.init_features()); |
There was a problem hiding this comment.
wouldn't unwrap_or do the trick?
There was a problem hiding this comment.
With unwrap_or, its argument will always be evaluated, whereas with unwrap_or_else, it's only evaluated if None. Doesn't really matter since it's just a test.
There was a problem hiding this comment.
Good point. I'd lean towards legibility > (imo premature) optimization in this case, but am fine either way.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
a6c69b0 to
69fd9d0CompareCodecov ReportBase: 90.73% // Head: 90.70% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@## main #1946 +/- ##
==========================================
- Coverage 90.73% 90.70% -0.04%
==========================================
Files 97 97 Lines 50548 50568 +20 Branches 50548 50568 +20 ==========================================
+ Hits 45864 45866 +2 - Misses 4684 4702 +18
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
TheBlueMatt
commented
Jan 14, 2023
Needs rebase, sorry. |
This is purely a refactor that does not change the InitFeatures advertised by a ChannelManager. This allows users to configure which features should be advertised based on the values of `UserConfig`. While there aren't any existing features currently leveraging this behavior, it will be used by the upcoming anchors_zero_fee_htlc_tx feature. The UserConfig dependency on provided_init_features caused most callsites of the main test methods responsible for opening channels to be updated. This commit foregos that completely by no longer requiring the InitFeatures of each side to be provided to these methods. The methods already require a reference to each node's ChannelManager to open the channel, so we use that same reference to obtain their InitFeatures. A way to override such features was required for some tests, so a new `override_init_features` config option now exists on the test harness.
arik-so
commented
Jan 14, 2023
Sorry about the labels. Looks good to me, but I'll rereview after rebase. |
69fd9d0 to
abf4e79Compare
TheBlueMatt
left a comment
There was a problem hiding this comment.
We should also do #1954.
| let msg = msgs::UnsignedChannelAnnouncement { | ||
| features: channelmanager::provided_channel_features(), | ||
| features: channelmanager::provided_channel_features(&user_config), |
There was a problem hiding this comment.
This is fine for now, but we'll need to tweak this further once we get to things like PTLC as we'll need the provided-channel-flags to include some info about the channel_type as well.
| let mut accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); | ||
| accept_channel.to_self_delay = 200; | ||
| nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), channelmanager::provided_init_features(), &accept_channel); | ||
| nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), nodes[0].node.init_features(), &accept_channel); |
There was a problem hiding this comment.
Heh, finally caught one with the wrong node passed :)
This is purely a refactor that does not change the InitFeatures advertised by a ChannelManager. This allows users to configure which features should be advertised based on the values of UserConfig. While there aren't any existing features currently leveraging this behavior, it will be used by the upcoming anchors_zero_fee_htlc_tx feature.
The UserConfig dependency on provided_init_features caused most callsites of the main test methods responsible for opening channels to be updated. This commit foregos that completely by no longer requiring the InitFeatures of each side to be provided to these methods. The methods already require a reference to each node's ChannelManager to open the channel, so we use that same reference to obtain their InitFeatures. A way to override such features was required for some tests, so a new override_init_features config option now exists on the test harness.
Motivated by the discussion in #1860 (comment).