From 010c34f351f4ae37c419fc5d1c55c60b637bf241 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 16 Feb 2022 05:36:54 +0000 Subject: [PATCH 1/2] Fix what `bolt2_open_channel_sending_node_checks_part1` tests There are currently two issues with `bolt2_open_channel_sending_node_checks_part1` which counteract each other and hide that the test isn't testing what it should be. First of all, the final `create_channel` call actually fails because we try to open a channel with ourselves, instead of panicing as the test is supposed to check for. However, when we fix the create_channel call to panic, when we drop `nodes[1]` after `create_channel` panics, we fail the no-pending-messages test as it as an expeted `accept_channel` in its outbound buffer. This causes a double-panic. Previously, these two offset each other - instead of panicing in `create_channel` we'd panic in the Node drop checks. This fixes both by fetching the `accept_channel` before we go into the panic'ing `create_channel` call (who's arguments were corrected). --- lightning/src/ln/functional_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 3c7174a9d71..7816db2b178 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -6010,9 +6010,10 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42, None).unwrap(); let node0_to_1_send_open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), InitFeatures::known(), &node0_to_1_send_open_channel); + get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id()); //Create a second channel with a channel_id collision - assert!(nodes[0].node.create_channel(nodes[0].node.get_our_node_id(), channel_value_satoshis, push_msat, 42, None).is_err()); + assert!(nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), channel_value_satoshis, push_msat, 42, None).is_err()); } #[test] From 6e776d9fb99b90c60ce88d951b9e9fbc2cb35a1a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 4 Mar 2022 21:31:55 +0000 Subject: [PATCH 2/2] Clean up `TestKeysInterface` random bytes override interface Its very confusing to have multiple fields that do the same thing, one of which isn't even used for its stated purpose anymore after the previous few commits. --- lightning/src/ln/functional_tests.rs | 2 +- lightning/src/ln/onion_route_tests.rs | 4 ++-- lightning/src/util/test_utils.rs | 19 +++++-------------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 7816db2b178..00db1f5ca55 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -6001,7 +6001,7 @@ fn bolt2_open_channel_sending_node_checks_part1() { //This test needs to be on i let nodes = create_network(2, &node_cfgs, &node_chanmgrs); //Force duplicate channel ids for node in nodes.iter() { - *node.keys_manager.override_channel_id_priv.lock().unwrap() = Some([0; 32]); + *node.keys_manager.override_random_bytes.lock().unwrap() = Some([0; 32]); } // BOLT #2 spec: Sending node must ensure temporary_channel_id is unique from any other channel ID with the same peer. diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 414c98d4e85..d67abeefdad 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -320,7 +320,7 @@ fn test_onion_failure() { let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(node_2_cfg)]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); for node in nodes.iter() { - *node.keys_manager.override_session_priv.lock().unwrap() = Some([3; 32]); + *node.keys_manager.override_random_bytes.lock().unwrap() = Some([3; 32]); } let channels = [create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()), create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known())]; let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 40000); @@ -693,7 +693,7 @@ fn test_phantom_invalid_onion_payload() { // We'll use the session priv later when constructing an invalid onion packet. let session_priv = [3; 32]; - *nodes[0].keys_manager.override_session_priv.lock().unwrap() = Some(session_priv); + *nodes[0].keys_manager.override_random_bytes.lock().unwrap() = Some(session_priv); nodes[0].node.send_payment(&route, payment_hash.clone(), &Some(payment_secret)).unwrap(); check_added_monitors!(nodes[0], 1); let update_0 = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 9cd2f9ec13e..afdc10dda3c 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -471,8 +471,7 @@ impl Logger for TestLogger { pub struct TestKeysInterface { pub backing: keysinterface::PhantomKeysManager, - pub override_session_priv: Mutex>, - pub override_channel_id_priv: Mutex>, + pub override_random_bytes: Mutex>, pub disable_revocation_policy_check: bool, enforcement_states: Mutex>>>, expectations: Mutex>>, @@ -506,16 +505,9 @@ impl keysinterface::KeysInterface for TestKeysInterface { } fn get_secure_random_bytes(&self) -> [u8; 32] { - let override_channel_id = self.override_channel_id_priv.lock().unwrap(); - let override_session_key = self.override_session_priv.lock().unwrap(); - if override_channel_id.is_some() && override_session_key.is_some() { - panic!("We don't know which override key to use!"); - } - if let Some(key) = &*override_channel_id { - return *key; - } - if let Some(key) = &*override_session_key { - return *key; + let override_random_bytes = self.override_random_bytes.lock().unwrap(); + if let Some(bytes) = &*override_random_bytes { + return *bytes; } self.backing.get_secure_random_bytes() } @@ -543,8 +535,7 @@ impl TestKeysInterface { let now = Duration::from_secs(genesis_block(network).header.time as u64); Self { backing: keysinterface::PhantomKeysManager::new(seed, now.as_secs(), now.subsec_nanos(), seed), - override_session_priv: Mutex::new(None), - override_channel_id_priv: Mutex::new(None), + override_random_bytes: Mutex::new(None), disable_revocation_policy_check: false, enforcement_states: Mutex::new(HashMap::new()), expectations: Mutex::new(None),