Skip to content

Commit c93b59e

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2871 from dunxen/2024-02-msgcommonfields
Combine common fields for OpenChannel and AcceptChannel V1 & V2
2 parents e0323ec + 0a7a5f9 commit c93b59e

File tree

9 files changed

+752
-552
lines changed

9 files changed

+752
-552
lines changed

‎lightning/src/ln/async_signer_tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ fn test_async_commitment_signature_for_funding_signed_0conf() {
197197

198198
// nodes[0] <-- accept_channel --- nodes[1]
199199
let accept_channel = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
200-
assert_eq!(accept_channel.minimum_depth, 0, "Expected minimum depth of 0");
200+
assert_eq!(accept_channel.common_fields.minimum_depth, 0, "Expected minimum depth of 0");
201201
nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_channel);
202202

203203
// nodes[0] --- funding_created --> nodes[1]

‎lightning/src/ln/channel.rs‎

Lines changed: 147 additions & 143 deletions
Large diffs are not rendered by default.

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6194,12 +6194,14 @@ where
61946194
fn internal_open_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
61956195
// Note that the ChannelManager is NOT re-persisted on disk after this, so any changes are
61966196
// likely to be lost on restart!
6197-
if msg.chain_hash != self.chain_hash {
6198-
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
6197+
if msg.common_fields.chain_hash != self.chain_hash {
6198+
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(),
6199+
msg.common_fields.temporary_channel_id.clone()));
61996200
}
62006201

62016202
if !self.default_configuration.accept_inbound_channels {
6202-
return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone()));
6203+
return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(),
6204+
msg.common_fields.temporary_channel_id.clone()));
62036205
}
62046206

62056207
// Get the number of peers with channels, but without funded ones. We don't care too much
@@ -6212,7 +6214,9 @@ where
62126214
let peer_state_mutex = per_peer_state.get(counterparty_node_id)
62136215
.ok_or_else(|| {
62146216
debug_assert!(false);
6215-
MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id.clone())
6217+
MsgHandleErrInternal::send_err_msg_no_close(
6218+
format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id),
6219+
msg.common_fields.temporary_channel_id.clone())
62166220
})?;
62176221
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
62186222
let peer_state = &mut *peer_state_lock;
@@ -6226,34 +6230,36 @@ where
62266230
{
62276231
return Err(MsgHandleErrInternal::send_err_msg_no_close(
62286232
"Have too many peers with unfunded channels, not accepting new ones".to_owned(),
6229-
msg.temporary_channel_id.clone()));
6233+
msg.common_fields.temporary_channel_id.clone()));
62306234
}
62316235

62326236
let best_block_height = self.best_block.read().unwrap().height();
62336237
if Self::unfunded_channel_count(peer_state, best_block_height) >= MAX_UNFUNDED_CHANS_PER_PEER {
62346238
return Err(MsgHandleErrInternal::send_err_msg_no_close(
62356239
format!("Refusing more than {} unfunded channels.", MAX_UNFUNDED_CHANS_PER_PEER),
6236-
msg.temporary_channel_id.clone()));
6240+
msg.common_fields.temporary_channel_id.clone()));
62376241
}
62386242

6239-
let channel_id = msg.temporary_channel_id;
6243+
let channel_id = msg.common_fields.temporary_channel_id;
62406244
let channel_exists = peer_state.has_channel(&channel_id);
62416245
if channel_exists {
6242-
return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision for the same peer!".to_owned(), msg.temporary_channel_id.clone()));
6246+
return Err(MsgHandleErrInternal::send_err_msg_no_close(
6247+
"temporary_channel_id collision for the same peer!".to_owned(),
6248+
msg.common_fields.temporary_channel_id.clone()));
62436249
}
62446250

62456251
// If we're doing manual acceptance checks on the channel, then defer creation until we're sure we want to accept.
62466252
if self.default_configuration.manually_accept_inbound_channels {
62476253
let channel_type = channel::channel_type_from_open_channel(
62486254
&msg, &peer_state.latest_features, &self.channel_type_features()
62496255
).map_err(|e|
6250-
MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id)
6256+
MsgHandleErrInternal::from_chan_no_close(e, msg.common_fields.temporary_channel_id)
62516257
)?;
62526258
let mut pending_events = self.pending_events.lock().unwrap();
62536259
pending_events.push_back((events::Event::OpenChannelRequest {
6254-
temporary_channel_id: msg.temporary_channel_id.clone(),
6260+
temporary_channel_id: msg.common_fields.temporary_channel_id.clone(),
62556261
counterparty_node_id: counterparty_node_id.clone(),
6256-
funding_satoshis: msg.funding_satoshis,
6262+
funding_satoshis: msg.common_fields.funding_satoshis,
62576263
push_msat: msg.push_msat,
62586264
channel_type,
62596265
}, None));
@@ -6273,17 +6279,21 @@ where
62736279
&self.default_configuration, best_block_height, &self.logger, /*is_0conf=*/false)
62746280
{
62756281
Err(e) => {
6276-
return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id));
6282+
return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.common_fields.temporary_channel_id));
62776283
},
62786284
Ok(res) => res
62796285
};
62806286

62816287
let channel_type = channel.context.get_channel_type();
62826288
if channel_type.requires_zero_conf() {
6283-
return Err(MsgHandleErrInternal::send_err_msg_no_close("No zero confirmation channels accepted".to_owned(), msg.temporary_channel_id.clone()));
6289+
return Err(MsgHandleErrInternal::send_err_msg_no_close(
6290+
"No zero confirmation channels accepted".to_owned(),
6291+
msg.common_fields.temporary_channel_id.clone()));
62846292
}
62856293
if channel_type.requires_anchors_zero_fee_htlc_tx() {
6286-
return Err(MsgHandleErrInternal::send_err_msg_no_close("No channels with anchor outputs accepted".to_owned(), msg.temporary_channel_id.clone()));
6294+
return Err(MsgHandleErrInternal::send_err_msg_no_close(
6295+
"No channels with anchor outputs accepted".to_owned(),
6296+
msg.common_fields.temporary_channel_id.clone()));
62876297
}
62886298

62896299
let outbound_scid_alias = self.create_and_insert_outbound_scid_alias();
@@ -6305,28 +6315,28 @@ where
63056315
let peer_state_mutex = per_peer_state.get(counterparty_node_id)
63066316
.ok_or_else(|| {
63076317
debug_assert!(false);
6308-
MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.temporary_channel_id)
6318+
MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.common_fields.temporary_channel_id)
63096319
})?;
63106320
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
63116321
let peer_state = &mut *peer_state_lock;
6312-
match peer_state.channel_by_id.entry(msg.temporary_channel_id) {
6322+
match peer_state.channel_by_id.entry(msg.common_fields.temporary_channel_id) {
63136323
hash_map::Entry::Occupied(mut phase) => {
63146324
match phase.get_mut() {
63156325
ChannelPhase::UnfundedOutboundV1(chan) => {
63166326
try_chan_phase_entry!(self, chan.accept_channel(&msg, &self.default_configuration.channel_handshake_limits, &peer_state.latest_features), phase);
63176327
(chan.context.get_value_satoshis(), chan.context.get_funding_redeemscript().to_v0_p2wsh(), chan.context.get_user_id())
63186328
},
63196329
_ => {
6320-
return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got an unexpected accept_channel message from peer with counterparty_node_id {}", counterparty_node_id), msg.temporary_channel_id));
6330+
return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got an unexpected accept_channel message from peer with counterparty_node_id {}", counterparty_node_id), msg.common_fields.temporary_channel_id));
63216331
}
63226332
}
63236333
},
6324-
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.temporary_channel_id))
6334+
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.common_fields.temporary_channel_id))
63256335
}
63266336
};
63276337
let mut pending_events = self.pending_events.lock().unwrap();
63286338
pending_events.push_back((events::Event::FundingGenerationReady {
6329-
temporary_channel_id: msg.temporary_channel_id,
6339+
temporary_channel_id: msg.common_fields.temporary_channel_id,
63306340
counterparty_node_id: *counterparty_node_id,
63316341
channel_value_satoshis: value,
63326342
output_script,
@@ -8713,7 +8723,7 @@ where
87138723
fn handle_open_channel_v2(&self, counterparty_node_id: &PublicKey, msg: &msgs::OpenChannelV2) {
87148724
let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close(
87158725
"Dual-funded channels not supported".to_owned(),
8716-
msg.temporary_channel_id.clone())), *counterparty_node_id);
8726+
msg.common_fields.temporary_channel_id.clone())), *counterparty_node_id);
87178727
}
87188728

87198729
fn handle_accept_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::AcceptChannel) {
@@ -8729,7 +8739,7 @@ where
87298739
fn handle_accept_channel_v2(&self, counterparty_node_id: &PublicKey, msg: &msgs::AcceptChannelV2) {
87308740
let _: Result<(), _> = handle_error!(self, Err(MsgHandleErrInternal::send_err_msg_no_close(
87318741
"Dual-funded channels not supported".to_owned(),
8732-
msg.temporary_channel_id.clone())), *counterparty_node_id);
8742+
msg.common_fields.temporary_channel_id.clone())), *counterparty_node_id);
87338743
}
87348744

87358745
fn handle_funding_created(&self, counterparty_node_id: &PublicKey, msg: &msgs::FundingCreated) {
@@ -12033,14 +12043,15 @@ mod tests {
1203312043
check_added_monitors!(nodes[0], 1);
1203412044
expect_channel_pending_event(&nodes[0], &nodes[1].node.get_our_node_id());
1203512045
}
12036-
open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
12046+
open_channel_msg.common_fields.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
1203712047
}
1203812048

1203912049
// A MAX_UNFUNDED_CHANS_PER_PEER + 1 channel will be summarily rejected
12040-
open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
12050+
open_channel_msg.common_fields.temporary_channel_id = ChannelId::temporary_from_entropy_source(
12051+
&nodes[0].keys_manager);
1204112052
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
1204212053
assert_eq!(get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()).channel_id,
12043-
open_channel_msg.temporary_channel_id);
12054+
open_channel_msg.common_fields.temporary_channel_id);
1204412055

1204512056
// Further, because all of our channels with nodes[0] are inbound, and none of them funded,
1204612057
// it doesn't count as a "protected" peer, i.e. it counts towards the MAX_NO_CHANNEL_PEERS
@@ -12088,11 +12099,11 @@ mod tests {
1208812099
for i in 0..super::MAX_UNFUNDED_CHANNEL_PEERS - 1 {
1208912100
nodes[1].node.handle_open_channel(&peer_pks[i], &open_channel_msg);
1209012101
get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, peer_pks[i]);
12091-
open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
12102+
open_channel_msg.common_fields.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
1209212103
}
1209312104
nodes[1].node.handle_open_channel(&last_random_pk, &open_channel_msg);
1209412105
assert_eq!(get_err_msg(&nodes[1], &last_random_pk).channel_id,
12095-
open_channel_msg.temporary_channel_id);
12106+
open_channel_msg.common_fields.temporary_channel_id);
1209612107

1209712108
// Of course, however, outbound channels are always allowed
1209812109
nodes[1].node.create_channel(last_random_pk, 100_000, 0, 42, None, None).unwrap();
@@ -12128,14 +12139,14 @@ mod tests {
1212812139
for _ in 0..super::MAX_UNFUNDED_CHANS_PER_PEER {
1212912140
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
1213012141
get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id());
12131-
open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
12142+
open_channel_msg.common_fields.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
1213212143
}
1213312144

1213412145
// Once we have MAX_UNFUNDED_CHANS_PER_PEER unfunded channels, new inbound channels will be
1213512146
// rejected.
1213612147
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
1213712148
assert_eq!(get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()).channel_id,
12138-
open_channel_msg.temporary_channel_id);
12149+
open_channel_msg.common_fields.temporary_channel_id);
1213912150

1214012151
// but we can still open an outbound channel.
1214112152
nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 100_000, 0, 42, None, None).unwrap();
@@ -12144,7 +12155,7 @@ mod tests {
1214412155
// but even with such an outbound channel, additional inbound channels will still fail.
1214512156
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
1214612157
assert_eq!(get_err_msg(&nodes[1], &nodes[0].node.get_our_node_id()).channel_id,
12147-
open_channel_msg.temporary_channel_id);
12158+
open_channel_msg.common_fields.temporary_channel_id);
1214812159
}
1214912160

1215012161
#[test]
@@ -12180,7 +12191,7 @@ mod tests {
1218012191
_ => panic!("Unexpected event"),
1218112192
}
1218212193
get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, random_pk);
12183-
open_channel_msg.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
12194+
open_channel_msg.common_fields.temporary_channel_id = ChannelId::temporary_from_entropy_source(&nodes[0].keys_manager);
1218412195
}
1218512196

1218612197
// If we try to accept a channel from another peer non-0conf it will fail.
@@ -12202,7 +12213,7 @@ mod tests {
1220212213
_ => panic!("Unexpected event"),
1220312214
}
1220412215
assert_eq!(get_err_msg(&nodes[1], &last_random_pk).channel_id,
12205-
open_channel_msg.temporary_channel_id);
12216+
open_channel_msg.common_fields.temporary_channel_id);
1220612217

1220712218
// ...however if we accept the same channel 0conf it should work just fine.
1220812219
nodes[1].node.handle_open_channel(&last_random_pk, &open_channel_msg);
@@ -12347,7 +12358,7 @@ mod tests {
1234712358

1234812359
nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 0, None, None).unwrap();
1234912360
let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
12350-
assert!(open_channel_msg.channel_type.as_ref().unwrap().supports_anchors_zero_fee_htlc_tx());
12361+
assert!(open_channel_msg.common_fields.channel_type.as_ref().unwrap().supports_anchors_zero_fee_htlc_tx());
1235112362

1235212363
nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel_msg);
1235312364
let events = nodes[1].node.get_and_clear_pending_events();
@@ -12362,7 +12373,7 @@ mod tests {
1236212373
nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &error_msg);
1236312374

1236412375
let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
12365-
assert!(!open_channel_msg.channel_type.unwrap().supports_anchors_zero_fee_htlc_tx());
12376+
assert!(!open_channel_msg.common_fields.channel_type.unwrap().supports_anchors_zero_fee_htlc_tx());
1236612377

1236712378
// Since nodes[1] should not have accepted the channel, it should
1236812379
// not have generated any events.

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
12031203
};
12041204

12051205
let accept_channel = get_event_msg!(receiver, MessageSendEvent::SendAcceptChannel, initiator.node.get_our_node_id());
1206-
assert_eq!(accept_channel.minimum_depth, 0);
1206+
assert_eq!(accept_channel.common_fields.minimum_depth, 0);
12071207
initiator.node.handle_accept_channel(&receiver.node.get_our_node_id(), &accept_channel);
12081208

12091209
let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver.node.get_our_node_id(), 100_000, 42);
@@ -1257,7 +1257,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
12571257
pub fn exchange_open_accept_chan<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> ChannelId {
12581258
let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None, None).unwrap();
12591259
let open_channel_msg = get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id());
1260-
assert_eq!(open_channel_msg.temporary_channel_id, create_chan_id);
1260+
assert_eq!(open_channel_msg.common_fields.temporary_channel_id, create_chan_id);
12611261
assert_eq!(node_a.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 42);
12621262
node_b.node.handle_open_channel(&node_a.node.get_our_node_id(), &open_channel_msg);
12631263
if node_b.node.get_current_default_configuration().manually_accept_inbound_channels {
@@ -1270,7 +1270,7 @@ pub fn exchange_open_accept_chan<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b:
12701270
};
12711271
}
12721272
let accept_channel_msg = get_event_msg!(node_b, MessageSendEvent::SendAcceptChannel, node_a.node.get_our_node_id());
1273-
assert_eq!(accept_channel_msg.temporary_channel_id, create_chan_id);
1273+
assert_eq!(accept_channel_msg.common_fields.temporary_channel_id, create_chan_id);
12741274
node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &accept_channel_msg);
12751275
assert_ne!(node_b.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 0);
12761276

0 commit comments

Comments
 (0)