Skip to content

Commit e1989ad

Browse files
jkczyzjkczyz
committed
Pass InitFeatures by reference to Channel
1 parent f1c07b5 commit e1989ad

File tree

3 files changed

+37
-33
lines changed

3 files changed

+37
-33
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ impl<Signer: Sign> Channel<Signer> {
574574
}
575575

576576
// Constructors:
577-
pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
577+
pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
578578
where K::Target: KeysInterface<Signer = Signer>,
579579
F::Target: FeeEstimator,
580580
{
@@ -607,7 +607,7 @@ impl<Signer: Sign> Channel<Signer> {
607607
} else { None };
608608

609609
if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
610-
if !shutdown_scriptpubkey.is_compatible(&their_features) {
610+
if !shutdown_scriptpubkey.is_compatible(their_features) {
611611
return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) });
612612
}
613613
}
@@ -720,7 +720,7 @@ impl<Signer: Sign> Channel<Signer> {
720720

721721
/// Creates a new channel from a remote sides' request for one.
722722
/// Assumes chain_hash has already been checked and corresponds with what we expect!
723-
pub fn new_from_req<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, ChannelError>
723+
pub fn new_from_req<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, ChannelError>
724724
where K::Target: KeysInterface<Signer = Signer>,
725725
F::Target: FeeEstimator
726726
{
@@ -841,7 +841,7 @@ impl<Signer: Sign> Channel<Signer> {
841841
if script.len() == 0 {
842842
None
843843
} else {
844-
match ShutdownScript::try_from((script.clone(), &their_features)) {
844+
match ShutdownScript::try_from((script.clone(), their_features)) {
845845
Ok(shutdown_script) => Some(shutdown_script.into_inner()),
846846
Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format. script: ({})", script.to_bytes().to_hex()))),
847847
}
@@ -1513,7 +1513,7 @@ impl<Signer: Sign> Channel<Signer> {
15131513

15141514
// Message handlers:
15151515

1516-
pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> {
1516+
pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: &InitFeatures) -> Result<(), ChannelError> {
15171517
// Check sanity of message fields:
15181518
if !self.is_outbound() {
15191519
return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned()));
@@ -1585,7 +1585,7 @@ impl<Signer: Sign> Channel<Signer> {
15851585
if script.len() == 0 {
15861586
None
15871587
} else {
1588-
match ShutdownScript::try_from((script.clone(), &their_features)) {
1588+
match ShutdownScript::try_from((script.clone(), their_features)) {
15891589
Ok(shutdown_script) => Some(shutdown_script.into_inner()),
15901590
Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format. script: ({})", script.to_bytes().to_hex()))),
15911591
}
@@ -5259,7 +5259,7 @@ mod tests {
52595259
let secp_ctx = Secp256k1::new();
52605260
let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
52615261
let config = UserConfig::default();
5262-
match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, features, 10000000, 100000, 42, &config) {
5262+
match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config) {
52635263
Err(APIError::APIMisuseError { err }) => {
52645264
assert_eq!(err, "Provided a scriptpubkey format not accepted by peer. script: (60020028)");
52655265
},
@@ -5281,7 +5281,7 @@ mod tests {
52815281

52825282
let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
52835283
let config = UserConfig::default();
5284-
let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
5284+
let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
52855285

52865286
// Now change the fee so we can check that the fee in the open_channel message is the
52875287
// same as the old fee.
@@ -5306,18 +5306,18 @@ mod tests {
53065306
// Create Node A's channel pointing to Node B's pubkey
53075307
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
53085308
let config = UserConfig::default();
5309-
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
5309+
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
53105310

53115311
// Create Node B's channel by receiving Node A's open_channel message
53125312
// Make sure A's dust limit is as we expect.
53135313
let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
53145314
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
5315-
let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
5315+
let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
53165316

53175317
// Node B --> Node A: accept channel, explicitly setting B's dust limit.
53185318
let mut accept_channel_msg = node_b_chan.get_accept_channel();
53195319
accept_channel_msg.dust_limit_satoshis = 546;
5320-
node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap();
5320+
node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap();
53215321
node_a_chan.holder_dust_limit_satoshis = 1560;
53225322

53235323
// Put some inbound and outbound HTLCs in A's channel.
@@ -5373,7 +5373,7 @@ mod tests {
53735373

53745374
let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
53755375
let config = UserConfig::default();
5376-
let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
5376+
let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
53775377

53785378
let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0);
53795379
let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1);
@@ -5422,16 +5422,16 @@ mod tests {
54225422
// Create Node A's channel pointing to Node B's pubkey
54235423
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
54245424
let config = UserConfig::default();
5425-
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
5425+
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
54265426

54275427
// Create Node B's channel by receiving Node A's open_channel message
54285428
let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
54295429
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
5430-
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
5430+
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
54315431

54325432
// Node B --> Node A: accept channel
54335433
let accept_channel_msg = node_b_chan.get_accept_channel();
5434-
node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap();
5434+
node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap();
54355435

54365436
// Node A --> Node B: funding created
54375437
let output_script = node_a_chan.get_funding_redeemscript();
@@ -5484,7 +5484,7 @@ mod tests {
54845484
// Create a channel.
54855485
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
54865486
let config = UserConfig::default();
5487-
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
5487+
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
54885488
assert!(node_a_chan.counterparty_forwarding_info.is_none());
54895489
assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default
54905490
assert!(node_a_chan.counterparty_forwarding_info().is_none());
@@ -5548,7 +5548,7 @@ mod tests {
55485548
let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
55495549
let mut config = UserConfig::default();
55505550
config.channel_options.announced_channel = false;
5551-
let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
5551+
let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
55525552
chan.holder_dust_limit_satoshis = 546;
55535553
chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel
55545554

‎lightning/src/ln/channelmanager.rs

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,15 +1179,18 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
11791179
return Err(APIError::APIMisuseError { err: format!("Channel value must be at least 1000 satoshis. It was {}", channel_value_satoshis) });
11801180
}
11811181

1182-
let their_features = {
1182+
let channel = {
11831183
let per_peer_state = self.per_peer_state.read().unwrap();
11841184
match per_peer_state.get(&their_network_key) {
1185-
Some(peer_state) => peer_state.lock().unwrap().latest_features.clone(),
1185+
Some(peer_state) => {
1186+
let peer_state = peer_state.lock().unwrap();
1187+
let their_features = &peer_state.latest_features;
1188+
let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
1189+
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?
1190+
},
11861191
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
11871192
}
11881193
};
1189-
let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
1190-
let channel = Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_id, config)?;
11911194
let res = channel.get_open_channel(self.genesis_hash.clone());
11921195

11931196
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
@@ -1289,14 +1292,15 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
12891292
match channel_state.by_id.entry(channel_id.clone()) {
12901293
hash_map::Entry::Occupied(mut chan_entry) => {
12911294
counterparty_node_id = chan_entry.get().get_counterparty_node_id();
1292-
let their_features = {
1293-
let per_peer_state = self.per_peer_state.read().unwrap();
1294-
match per_peer_state.get(&counterparty_node_id) {
1295-
Some(peer_state) => peer_state.lock().unwrap().latest_features.clone(),
1296-
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", counterparty_node_id) }),
1297-
}
1295+
let per_peer_state = self.per_peer_state.read().unwrap();
1296+
let (shutdown_msg, monitor_update, htlcs) = match per_peer_state.get(&counterparty_node_id) {
1297+
Some(peer_state) => {
1298+
let peer_state = peer_state.lock().unwrap();
1299+
let their_features = &peer_state.latest_features;
1300+
chan_entry.get_mut().get_shutdown(&self.keys_manager, their_features)?
1301+
},
1302+
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", counterparty_node_id) }),
12981303
};
1299-
let (shutdown_msg, monitor_update, htlcs) = chan_entry.get_mut().get_shutdown(&self.keys_manager, &their_features)?;
13001304
failed_htlcs = htlcs;
13011305

13021306
// Update the monitor with the shutdown script if necessary.
@@ -3068,7 +3072,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30683072
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
30693073
}
30703074

3071-
let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), their_features, msg, 0, &self.default_configuration)
3075+
let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration)
30723076
.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
30733077
let mut channel_state_lock = self.channel_state.lock().unwrap();
30743078
let channel_state = &mut *channel_state_lock;
@@ -3094,7 +3098,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
30943098
if chan.get().get_counterparty_node_id() != *counterparty_node_id {
30953099
return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!".to_owned(), msg.temporary_channel_id));
30963100
}
3097-
try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, their_features), channel_state, chan);
3101+
try_chan_entry!(self, chan.get_mut().accept_channel(&msg, &self.default_configuration, &their_features), channel_state, chan);
30983102
(chan.get().get_value_satoshis(), chan.get().get_funding_redeemscript().to_v0_p2wsh(), chan.get().get_user_id())
30993103
},
31003104
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.temporary_channel_id))

‎lightning/src/ln/functional_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7767,7 +7767,7 @@ fn test_user_configurable_csv_delay() {
77677767
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
77687768

77697769
// We test config.our_to_self > BREAKDOWN_TIMEOUT is enforced in Channel::new_outbound()
7770-
if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config) {
7770+
if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config) {
77717771
match error {
77727772
APIError::APIMisuseError { err } => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
77737773
_ => panic!("Unexpected event"),
@@ -7778,7 +7778,7 @@ fn test_user_configurable_csv_delay() {
77787778
nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
77797779
let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
77807780
open_channel.to_self_delay = 200;
7781-
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &low_our_to_self_config) {
7781+
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &low_our_to_self_config) {
77827782
match error {
77837783
ChannelError::Close(err) => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
77847784
_ => panic!("Unexpected event"),
@@ -7804,7 +7804,7 @@ fn test_user_configurable_csv_delay() {
78047804
nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
78057805
let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
78067806
open_channel.to_self_delay = 200;
7807-
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), InitFeatures::known(), &open_channel, 0, &high_their_to_self_config) {
7807+
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &high_their_to_self_config) {
78087808
match error {
78097809
ChannelError::Close(err) => { assert!(regex::Regex::new(r"They wanted our payments to be delayed by a needlessly long period\. Upper limit: \d+\. Actual: \d+").unwrap().is_match(err.as_str())); },
78107810
_ => panic!("Unexpected event"),

0 commit comments

Comments
 (0)