From 183b33789fe83162ff42dde99e3df8c9d21243e0 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 30 Jan 2025 17:27:23 -0500 Subject: [PATCH 1/6] Remove Copy implementation from UserConfig We need to add a Vec to the config as part of async receive support, see upcoming commits. The vec will contain blinded paths to reach an always-online node that will serve static invoices on our behalf. None of those things implement Copy so remove support for the trait. --- lightning/src/ln/async_signer_tests.rs | 2 +- lightning/src/ln/chanmon_update_fail_tests.rs | 6 ++--- lightning/src/ln/channelmanager.rs | 6 ++--- lightning/src/ln/functional_test_utils.rs | 4 +-- lightning/src/ln/functional_tests.rs | 14 +++++----- lightning/src/ln/invoice_utils.rs | 2 +- lightning/src/ln/monitor_tests.rs | 26 +++++++++---------- lightning/src/ln/onion_route_tests.rs | 10 +++---- lightning/src/ln/payment_tests.rs | 10 +++---- lightning/src/ln/priv_short_conf_tests.rs | 14 +++++----- lightning/src/ln/reorg_tests.rs | 4 +-- lightning/src/ln/shutdown_tests.rs | 2 +- lightning/src/util/config.rs | 2 +- 13 files changed, 51 insertions(+), 51 deletions(-) diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index 76c253c988d..9c0802bd921 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -816,7 +816,7 @@ fn do_test_async_holder_signatures(anchors: bool, remote_commitment: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let closing_node = if remote_commitment { &nodes[1] } else { &nodes[0] }; diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index e99cf017b66..f63988eed57 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -2591,7 +2591,7 @@ fn test_temporary_error_during_shutdown() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, channel_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1); @@ -2762,7 +2762,7 @@ fn do_test_outbound_reload_without_init_mon(use_0conf: bool) { chan_config.manually_accept_inbound_channels = true; chan_config.channel_handshake_limits.trust_own_funding_0conf = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config), Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config.clone()), Some(chan_config)]); let nodes_0_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -2853,7 +2853,7 @@ fn do_test_inbound_reload_without_init_mon(use_0conf: bool, lock_commitment: boo chan_config.manually_accept_inbound_channels = true; chan_config.channel_handshake_limits.trust_own_funding_0conf = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config), Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(chan_config.clone()), Some(chan_config)]); let nodes_1_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index fab15bfea28..520d6a3bae0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1728,7 +1728,7 @@ where /// let default_config = UserConfig::default(); /// let channel_manager = ChannelManager::new( /// fee_estimator, chain_monitor, tx_broadcaster, router, message_router, logger, -/// entropy_source, node_signer, signer_provider, default_config, params, current_timestamp, +/// entropy_source, node_signer, signer_provider, default_config.clone(), params, current_timestamp, /// ); /// /// // Restart from deserialized data @@ -16120,7 +16120,7 @@ mod tests { let chanmon_cfg = create_chanmon_cfgs(2); let node_cfg = create_node_cfgs(2, &chanmon_cfg); let mut user_config = test_default_channel_config(); - let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config.clone()), Some(user_config.clone())]); let nodes = create_network(2, &node_cfg, &node_chanmgr); let _ = create_announced_chan_between_nodes(&nodes, 0, 1); let channel = &nodes[0].node.list_channels()[0]; @@ -16207,7 +16207,7 @@ mod tests { let chanmon_cfg = create_chanmon_cfgs(2); let node_cfg = create_node_cfgs(2, &chanmon_cfg); let user_config = test_default_channel_config(); - let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfg, &node_chanmgr); let error_message = "Channel force-closed"; diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 611fcd2b1ad..56cd64a5567 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -716,7 +716,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { let mut w = test_utils::TestVecWriter(Vec::new()); self.node.write(&mut w).unwrap(); <(BlockHash, ChannelManager<&test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestRouter, &test_utils::TestMessageRouter, &test_utils::TestLogger>)>::read(&mut io::Cursor::new(w.0), ChannelManagerReadArgs { - default_config: *self.node.get_current_default_configuration(), + default_config: self.node.get_current_default_configuration().clone(), entropy_source: self.keys_manager, node_signer: self.keys_manager, signer_provider: self.keys_manager, @@ -3979,7 +3979,7 @@ pub fn create_batch_channel_funding<'a, 'b, 'c>( let temp_chan_id = funding_node.node.create_channel( other_node.node.get_our_node_id(), *channel_value_satoshis, *push_msat, *user_channel_id, None, - *override_config, + override_config.clone(), ).unwrap(); let open_channel_msg = get_event_msg!(funding_node, MessageSendEvent::SendOpenChannel, other_node.node.get_our_node_id()); other_node.node.handle_open_channel(funding_node.node.get_our_node_id(), &open_channel_msg); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4cfd63ab2d6..bf7bdc4917f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -105,7 +105,7 @@ pub fn test_insane_channel_opens() { cfg.channel_handshake_limits.max_funding_satoshis = TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(cfg)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(cfg.clone())]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // Instantiate channel parameters where we push the maximum msats given our @@ -800,7 +800,7 @@ pub fn test_update_fee_that_saturates_subs() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(default_config), Some(default_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(default_config.clone()), Some(default_config)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan_id = create_chan_between_nodes_with_value(&nodes[0], &nodes[1], 10_000, 8_500_000).3; @@ -2966,7 +2966,7 @@ pub fn test_multiple_package_conflicts() { user_cfg.manually_accept_inbound_channels = true; let node_chanmgrs = - create_node_chanmgrs(3, &node_cfgs, &[Some(user_cfg), Some(user_cfg), Some(user_cfg)]); + create_node_chanmgrs(3, &node_cfgs, &[Some(user_cfg.clone()), Some(user_cfg.clone()), Some(user_cfg)]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); // Since we're using anchor channels, make sure each node has a UTXO for paying fees. @@ -10669,7 +10669,7 @@ pub fn test_nondust_htlc_excess_fees_are_dust() { config.channel_handshake_config.our_htlc_minimum_msat = 1; config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100; - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config)]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); // Leave enough on the funder side to let it pay the mining fees for a commit tx with tons of htlcs @@ -11857,7 +11857,7 @@ fn do_test_funding_and_commitment_tx_confirm_same_block(confirm_remote_commitmen let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let mut min_depth_1_block_cfg = test_default_channel_config(); min_depth_1_block_cfg.channel_handshake_config.minimum_depth = 1; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(min_depth_1_block_cfg), Some(min_depth_1_block_cfg)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(min_depth_1_block_cfg.clone()), Some(min_depth_1_block_cfg)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let funding_tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 1_000_000, 0); @@ -11946,7 +11946,7 @@ pub fn test_manual_funding_abandon() { cfg.channel_handshake_config.minimum_depth = 1; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg), Some(cfg)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg.clone()), Some(cfg)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); assert!(nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).is_ok()); @@ -11988,7 +11988,7 @@ pub fn test_funding_signed_event() { cfg.channel_handshake_config.minimum_depth = 1; let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg), Some(cfg)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(cfg.clone()), Some(cfg)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); assert!(nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).is_ok()); diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs index 98d27a82868..79220456bbd 100644 --- a/lightning/src/ln/invoice_utils.rs +++ b/lightning/src/ln/invoice_utils.rs @@ -934,7 +934,7 @@ mod test { let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let mut config = test_default_channel_config(); config.channel_handshake_config.minimum_depth = 1; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // Create a private channel with lots of capacity and a lower value public channel (without diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index adb79b94356..9e5d026855c 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -171,7 +171,7 @@ fn archive_fully_resolved_monitors() { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let mut user_config = test_default_channel_config(); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, chan_id, funding_tx) = @@ -315,7 +315,7 @@ fn do_chanmon_claim_value_coop_close(anchors: bool) { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, chan_id, funding_tx) = @@ -459,7 +459,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { @@ -862,7 +862,7 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { @@ -1359,7 +1359,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, chan_id, funding_tx) = @@ -1645,7 +1645,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { @@ -1946,7 +1946,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { @@ -2236,7 +2236,7 @@ fn do_test_claimable_balance_correct_while_payment_pending(outbound_payment: boo user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(user_config), Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(user_config.clone()), Some(user_config.clone()), Some(user_config)]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { @@ -2401,7 +2401,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) { config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, _, chan_id, funding_tx) = create_chan_between_nodes_with_value( @@ -2533,7 +2533,7 @@ fn do_test_yield_anchors_events(have_htlcs: bool) { anchors_config.channel_handshake_config.announce_for_forwarding = true; anchors_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; anchors_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config), Some(anchors_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config.clone()), Some(anchors_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value( @@ -2731,7 +2731,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() { anchors_config.channel_handshake_config.announce_for_forwarding = true; anchors_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; anchors_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config), Some(anchors_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config.clone()), Some(anchors_config.clone())]); let bob_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -3032,7 +3032,7 @@ fn do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(confirm_c let mut user_config = test_default_channel_config(); user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config.clone())]); let node_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -3120,7 +3120,7 @@ fn do_test_monitor_claims_with_random_signatures(anchors: bool, confirm_counterp user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; user_config.manually_accept_inbound_channels = true; } - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config.clone()), Some(user_config)]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let coinbase_tx = Transaction { diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 127d4a45588..684e1f244e1 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -295,7 +295,7 @@ fn test_fee_failures() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config)]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); let channels = [create_announced_chan_between_nodes(&nodes, 0, 1), create_announced_chan_between_nodes(&nodes, 1, 2)]; @@ -351,7 +351,7 @@ fn test_onion_failure() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(node_2_cfg)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config), Some(node_2_cfg)]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); let channels = [create_announced_chan_between_nodes(&nodes, 0, 1), create_announced_chan_between_nodes(&nodes, 1, 2)]; for node in nodes.iter() { @@ -756,7 +756,7 @@ fn test_onion_failure() { fn test_overshoot_final_cltv() { let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None; 3]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1); create_announced_chan_between_nodes(&nodes, 1, 2); @@ -920,7 +920,7 @@ fn do_test_onion_failure_stale_channel_update(announce_for_forwarding: bool) { .find(|channel| channel.channel_id == channel_to_update.0).unwrap() .config.unwrap(); config.forwarding_fee_base_msat = u32::max_value(); - let msg = update_and_get_channel_update(&config, true, None, false).unwrap(); + let msg = update_and_get_channel_update(&config.clone(), true, None, false).unwrap(); // The old policy should still be in effect until a new block is connected. send_along_route_with_secret(&nodes[0], route.clone(), &[&[&nodes[1], &nodes[2]]], PAYMENT_AMT, @@ -957,7 +957,7 @@ fn do_test_onion_failure_stale_channel_update(announce_for_forwarding: bool) { let config_after_restart = { let chan_1_monitor_serialized = get_monitor!(nodes[1], other_channel.3).encode(); let chan_2_monitor_serialized = get_monitor!(nodes[1], channel_to_update.0).encode(); - reload_node!(nodes[1], *nodes[1].node.get_current_default_configuration(), &nodes[1].node.encode(), + reload_node!(nodes[1], nodes[1].node.get_current_default_configuration().clone(), &nodes[1].node.encode(), &[&chan_1_monitor_serialized, &chan_2_monitor_serialized], persister, chain_monitor, channel_manager_1_deserialized); nodes[1].node.list_channels().iter() .find(|channel| channel.channel_id == channel_to_update.0).unwrap() diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 5adc2d66b11..84a90c50ecd 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -182,7 +182,7 @@ fn mpp_retry_overpay() { let mut limited_config_2 = user_config.clone(); limited_config_2.channel_handshake_config.our_htlc_minimum_msat = 34_500_000; let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, - &[Some(user_config), Some(limited_config_1), Some(limited_config_2), Some(user_config)]); + &[Some(user_config.clone()), Some(limited_config_1), Some(limited_config_2), Some(user_config)]); let nodes = create_network(4, &node_cfgs, &node_chanmgrs); let (chan_1_update, _, _, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 40_000, 0); @@ -3706,7 +3706,7 @@ fn test_custom_tlvs_explicit_claim() { fn do_test_custom_tlvs(spontaneous: bool, even_tlvs: bool, known_tlvs: bool) { let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None; 2]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes(&nodes, 0, 1); @@ -4032,7 +4032,7 @@ fn do_test_payment_metadata_consistency(do_reload: bool, do_modify: bool) { let mut config = test_default_channel_config(); config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 50; - let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, Some(config), Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, Some(config.clone()), Some(config.clone()), Some(config.clone())]); let nodes_0_deserialized; let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs); @@ -4193,7 +4193,7 @@ fn test_htlc_forward_considers_anchor_outputs_value() { // discovery of this bug. let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config.clone())]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); const CHAN_AMT: u64 = 1_000_000; @@ -4328,7 +4328,7 @@ fn test_non_strict_forwarding() { let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut config = test_default_channel_config(); config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100; - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config.clone()), Some(config.clone()), Some(config)]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); // Create a routing node with two outbound channels, each of which can forward 2 payments of diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index c482d97ea8b..045f4793343 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -38,7 +38,7 @@ fn test_priv_forwarding_rejection() { no_announce_cfg.accept_forwards_to_priv_channels = false; let persister; let new_chain_monitor; - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg), None]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(no_announce_cfg.clone()), None]); let nodes_1_deserialized; let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); @@ -577,7 +577,7 @@ fn test_0conf_channel_with_async_monitor() { let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut chan_config = test_default_channel_config(); chan_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(chan_config), None]); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, Some(chan_config.clone()), None]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); @@ -744,7 +744,7 @@ fn test_0conf_close_no_early_chan_update() { let mut chan_config = test_default_channel_config(); chan_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config.clone())]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let error_message = "Channel force-closed"; @@ -769,7 +769,7 @@ fn test_public_0conf_channel() { let mut chan_config = test_default_channel_config(); chan_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config.clone())]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway @@ -823,7 +823,7 @@ fn test_0conf_channel_reorg() { let mut chan_config = test_default_channel_config(); chan_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config.clone())]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway @@ -902,7 +902,7 @@ fn test_zero_conf_accept_reject() { // 2.1 First try the non-0conf method to manually accept nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, - None, Some(manually_accept_conf)).unwrap(); + None, Some(manually_accept_conf.clone())).unwrap(); let mut open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); @@ -1014,7 +1014,7 @@ fn test_0conf_ann_sigs_racing_conf() { let mut chan_config = test_default_channel_config(); chan_config.manually_accept_inbound_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config.clone())]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // This is the default but we force it on anyway diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 934ca0d5fdc..c83f06026e5 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -317,7 +317,7 @@ fn do_test_unconf_chan(reload_node: bool, reorg_after_reload: bool, use_funding_ let nodes_0_serialized = nodes[0].node.encode(); let chan_0_monitor_serialized = get_monitor!(nodes[0], chan.2).encode(); - reload_node!(nodes[0], *nodes[0].node.get_current_default_configuration(), &nodes_0_serialized, &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); + reload_node!(nodes[0], nodes[0].node.get_current_default_configuration().clone(), &nodes_0_serialized, &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized); } if reorg_after_reload { @@ -793,7 +793,7 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c } let persister; let new_chain_monitor; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config.clone())]); let nodes_1_deserialized; let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index e49bc4e83be..1fb26a270d4 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -987,7 +987,7 @@ fn test_unsupported_anysegwit_shutdown_script() { config.channel_handshake_config.announce_for_forwarding = true; config.channel_handshake_limits.force_announced_channel_preference = false; config.channel_handshake_config.commit_upfront_shutdown_pubkey = false; - let user_cfgs = [None, Some(config), None]; + let user_cfgs = [None, Some(config.clone()), None]; let chanmon_cfgs = create_chanmon_cfgs(3); let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut features = channelmanager::provided_init_features(&config); diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 37ba74a0226..152a6a62eb6 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -807,7 +807,7 @@ impl crate::util::ser::Readable for LegacyChannelConfig { /// /// `Default::default()` provides sane defaults for most configurations /// (but currently with zero relay fees!) -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub struct UserConfig { /// Channel handshake config that we propose to our counterparty. pub channel_handshake_config: ChannelHandshakeConfig, From ffa537e95e6579d6253683fd679691c3725354a1 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 6 Mar 2025 15:30:56 -0500 Subject: [PATCH 2/6] Don't override DecodeError on BOLT 12 message read Previously when reading a BOLT 12 message from bytes, if it resulted in a DecodeError then that error would be overridden by DecodeError::InvalidValue specifically. Better to return to underlying specific error. --- lightning/src/offers/invoice.rs | 5 ++++- lightning/src/offers/invoice_request.rs | 5 ++++- lightning/src/offers/offer.rs | 5 ++++- lightning/src/offers/refund.rs | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index cc7e8a8ab27..3615850a22e 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -1403,7 +1403,10 @@ impl LengthReadable for Bolt12Invoice { reader: &mut R, ) -> Result { let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; - Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) + Self::try_from(bytes.0).map_err(|e| match e { + Bolt12ParseError::Decode(e) => e, + _ => DecodeError::InvalidValue, + }) } } diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index ff64f85f46a..eecb68e0826 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -1123,7 +1123,10 @@ impl Writeable for InvoiceRequestContents { impl LengthReadable for InvoiceRequest { fn read_from_fixed_length_buffer(r: &mut R) -> Result { let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(r)?; - Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) + Self::try_from(bytes.0).map_err(|e| match e { + Bolt12ParseError::Decode(e) => e, + _ => DecodeError::InvalidValue, + }) } } diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index fee7b6fe486..d4330df3223 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -1039,7 +1039,10 @@ impl LengthReadable for Offer { reader: &mut R, ) -> Result { let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; - Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) + Self::try_from(bytes.0).map_err(|e| match e { + Bolt12ParseError::Decode(e) => e, + _ => DecodeError::InvalidValue, + }) } } diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index ec7477e24a6..e38a3522ec8 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -829,7 +829,10 @@ impl LengthReadable for Refund { reader: &mut R, ) -> Result { let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; - Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) + Self::try_from(bytes.0).map_err(|e| match e { + Bolt12ParseError::Decode(e) => e, + _ => DecodeError::InvalidValue, + }) } } From 21822ca14bc24459a8568e42721f4c6221ccbb29 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 6 Mar 2025 15:18:11 -0500 Subject: [PATCH 3/6] ser_macros: encoding wrapper for required_vec For context, many of our higher level ser macros such as impl_writeable_msg support the syntax: impl_writeable_msg!(StructName, { .. }, { (42, tlv_field, (option, encoding: (Vec, WithoutLength>), }) i.e. specifying an encoding wrapper around an optional field being serialized. Here we add add similar support for an encoding wrapper around required_vecs that are being serialized, eg (42, tlv_field, (required_vec: (encoding: (..)))). One difficulty here is that some ser macros such as impl_writeable_msg and tlv_stream will call the underlying macro _encode_tlv_stream with each struct field passed in as a reference, whereas others such as impl_writeable_tlv_based will call _encode_tlv_stream with each struct field passed in as a value. This difference led to compilation issues because the WithoutLength ser wrapper expects to wrap a reference to a Vec, not a value. As a result, here we also modify the macros that were previously passing in struct values as values to pass in references instead. --- lightning/src/ln/msgs.rs | 10 ++--- lightning/src/onion_message/packet.rs | 4 +- lightning/src/util/ser_macros.rs | 65 ++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 1887cb18a88..85ccd99d44d 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -3144,7 +3144,7 @@ impl<'a> Writeable for OutboundOnionPayload<'a> { }, Self::BlindedForward { encrypted_tlvs, intro_node_blinding_point } => { _encode_varint_length_prefixed_tlv!(w, { - (10, **encrypted_tlvs, required_vec), + (10, *encrypted_tlvs, required_vec), (12, intro_node_blinding_point, option) }); }, @@ -3165,7 +3165,7 @@ impl<'a> Writeable for OutboundOnionPayload<'a> { _encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required), (4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required), - (10, **encrypted_tlvs, required_vec), + (10, *encrypted_tlvs, required_vec), (12, intro_node_blinding_point, option), (18, HighZeroBytesDroppedBigSize(*total_msat), required) }, custom_tlvs.iter()); @@ -3206,7 +3206,7 @@ impl<'a> Writeable for OutboundTrampolinePayload<'a> { }, Self::BlindedForward { encrypted_tlvs, intro_node_blinding_point} => { _encode_varint_length_prefixed_tlv!(w, { - (10, **encrypted_tlvs, required_vec), + (10, *encrypted_tlvs, required_vec), (12, intro_node_blinding_point, option) }); }, @@ -3214,7 +3214,7 @@ impl<'a> Writeable for OutboundTrampolinePayload<'a> { _encode_varint_length_prefixed_tlv!(w, { (2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required), (4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required), - (10, **encrypted_tlvs, required_vec), + (10, *encrypted_tlvs, required_vec), (12, intro_node_blinding_point, option), (18, HighZeroBytesDroppedBigSize(*total_msat), required), (20, keysend_preimage, option) @@ -5592,7 +5592,7 @@ mod tests { let test_bytes = vec![42u8; 1000]; if let msgs::OutboundOnionPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } = payload { _encode_varint_length_prefixed_tlv!(&mut encoded_payload, { - (1, test_bytes, required_vec), + (1, &test_bytes, required_vec), (2, HighZeroBytesDroppedBigSize(amt_to_forward), required), (4, HighZeroBytesDroppedBigSize(outgoing_cltv_value), required), (6, short_channel_id, required) diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 33061f344fa..632cbc9c8a3 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -217,7 +217,7 @@ impl Writeable for (Payload, [u8; 32]) { fn write(&self, w: &mut W) -> Result<(), io::Error> { match &self.0 { Payload::Forward(ForwardControlTlvs::Blinded(encrypted_bytes)) => { - _encode_varint_length_prefixed_tlv!(w, { (4, *encrypted_bytes, required_vec) }) + _encode_varint_length_prefixed_tlv!(w, { (4, encrypted_bytes, required_vec) }) }, Payload::Receive { control_tlvs: ReceiveControlTlvs::Blinded(encrypted_bytes), @@ -226,7 +226,7 @@ impl Writeable for (Payload, [u8; 32]) { } => { _encode_varint_length_prefixed_tlv!(w, { (2, reply_path, option), - (4, *encrypted_bytes, required_vec), + (4, encrypted_bytes, required_vec), (message.tlv_type(), message, required) }) }, diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 56779e24a05..8c0e4bd3d10 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -33,7 +33,10 @@ macro_rules! _encode_tlv { $crate::_encode_tlv!($stream, $type, $field, required); }; ($stream: expr, $type: expr, $field: expr, required_vec $(, $self: ident)?) => { - $crate::_encode_tlv!($stream, $type, $crate::util::ser::WithoutLength(&$field), required); + $crate::_encode_tlv!($stream, $type, $crate::util::ser::WithoutLength($field), required); + }; + ($stream: expr, $type: expr, $field: expr, (required_vec, encoding: ($fieldty: ty, $encoding: ident)) $(, $self: ident)?) => { + $crate::_encode_tlv!($stream, $type, $encoding($field), required); }; ($stream: expr, $optional_type: expr, $optional_field: expr, option $(, $self: ident)?) => { if let Some(ref field) = $optional_field { @@ -166,7 +169,7 @@ macro_rules! _encode_tlv_stream { )* for tlv in $extra_tlvs { let (typ, value): &(u64, Vec) = tlv; - $crate::_encode_tlv!($stream, *typ, *value, required_vec); + $crate::_encode_tlv!($stream, *typ, value, required_vec); } #[allow(unused_mut, unused_variables, unused_assignments)] @@ -207,11 +210,15 @@ macro_rules! _get_varint_length_prefixed_tlv_length { $crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, required); }; ($len: expr, $type: expr, $field: expr, required_vec $(, $self: ident)?) => { - let field = $crate::util::ser::WithoutLength(&$field); + let field = $crate::util::ser::WithoutLength($field); + $crate::_get_varint_length_prefixed_tlv_length!($len, $type, field, required); + }; + ($len: expr, $type: expr, $field: expr, (required_vec, encoding: ($fieldty: ty, $encoding: ident)) $(, $self: ident)?) => { + let field = $encoding($field); $crate::_get_varint_length_prefixed_tlv_length!($len, $type, field, required); }; ($len: expr, $optional_type: expr, $optional_field: expr, option $(, $self: ident)?) => { - if let Some(ref field) = $optional_field { + if let Some(ref field) = $optional_field.as_ref() { BigSize($optional_type) .write(&mut $len) .expect("No in-memory data may fail to serialize"); @@ -265,7 +272,7 @@ macro_rules! _encode_varint_length_prefixed_tlv { )* for tlv in $extra_tlvs { let (typ, value): &(u64, Vec) = tlv; - $crate::_get_varint_length_prefixed_tlv_length!(len, *typ, *value, required_vec); + $crate::_get_varint_length_prefixed_tlv_length!(len, *typ, value, required_vec); } len.0 }; @@ -316,6 +323,9 @@ macro_rules! _check_decoded_tlv_order { ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, required_vec) => {{ $crate::_check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, required); }}; + ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (required_vec, encoding: $encoding: tt)) => {{ + $crate::_check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, required); + }}; ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, optional_vec) => {{ // no-op }}; @@ -363,6 +373,9 @@ macro_rules! _check_missing_tlv { ($last_seen_type: expr, $type: expr, $field: ident, required_vec) => {{ $crate::_check_missing_tlv!($last_seen_type, $type, $field, required); }}; + ($last_seen_type: expr, $type: expr, $field: ident, (required_vec, encoding: $encoding: tt)) => {{ + $crate::_check_missing_tlv!($last_seen_type, $type, $field, required); + }}; ($last_seen_type: expr, $type: expr, $field: ident, option) => {{ // no-op }}; @@ -412,6 +425,12 @@ macro_rules! _decode_tlv { let f: $crate::util::ser::WithoutLength> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?; $field = f.0; }}; + ($outer_reader: expr, $reader: expr, $field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => {{ + $field = { + let field: $encoding<$fieldty> = ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?; + $crate::util::ser::RequiredWrapper(Some(field.0)) + }; + }}; ($outer_reader: expr, $reader: expr, $field: ident, option) => {{ $field = Some($crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?); }}; @@ -762,7 +781,7 @@ macro_rules! write_ver_prefix { #[macro_export] macro_rules! write_tlv_fields { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { - $crate::_encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}) + $crate::_encode_varint_length_prefixed_tlv!($stream, {$(($type, &$field, $fieldty)),*}) } } @@ -830,6 +849,9 @@ macro_rules! _init_tlv_based_struct_field { ($field: ident, required_vec) => { $field }; + ($field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => { + $crate::_init_tlv_based_struct_field!($field, required) + }; ($field: ident, optional_vec) => { $field.unwrap() }; @@ -856,6 +878,9 @@ macro_rules! _init_tlv_field_var { ($field: ident, required_vec) => { let mut $field = Vec::new(); }; + ($field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => { + $crate::_init_tlv_field_var!($field, required); + }; ($field: ident, option) => { let mut $field = None; }; @@ -983,7 +1008,7 @@ macro_rules! impl_writeable_tlv_based { impl $crate::util::ser::Writeable for $st { fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { $crate::_encode_varint_length_prefixed_tlv!(writer, { - $(($type, self.$field, $fieldty, self)),* + $(($type, &self.$field, $fieldty, self)),* }); Ok(()) } @@ -995,7 +1020,7 @@ macro_rules! impl_writeable_tlv_based { #[allow(unused_mut)] let mut len = $crate::util::ser::LengthCalculatingWriter(0); $( - $crate::_get_varint_length_prefixed_tlv_length!(len, $type, self.$field, $fieldty, self); + $crate::_get_varint_length_prefixed_tlv_length!(len, $type, &self.$field, $fieldty, self); )* len.0 }; @@ -1112,7 +1137,7 @@ macro_rules! _impl_writeable_tlv_based_enum_common { let id: u8 = $variant_id; id.write(writer)?; $crate::_encode_varint_length_prefixed_tlv!(writer, { - $(($type, *$field, $fieldty, self)),* + $(($type, $field, $fieldty, self)),* }); }),* $($st::$tuple_variant_name (ref field) => { @@ -1370,7 +1395,8 @@ mod tests { use crate::io::{self, Cursor}; use crate::ln::msgs::DecodeError; use crate::util::ser::{ - HighZeroBytesDroppedBigSize, LengthReadable, MaybeReadable, Readable, VecWriter, Writeable, + HighZeroBytesDroppedBigSize, LengthReadable, MaybeReadable, Readable, VecWriter, + WithoutLength, Writeable, }; use bitcoin::hex::FromHex; use bitcoin::secp256k1::PublicKey; @@ -1879,4 +1905,23 @@ mod tests { let read = ::read(&mut &encoded[..]).unwrap(); assert_eq!(read, ExpandedField { new_field: (42, 0) }); } + + #[test] + fn required_vec_with_encoding() { + // Ensure that serializing a required vec with a specified encoding will survive a ser round + // trip. + #[derive(PartialEq, Eq, Debug)] + struct MyCustomStruct { + tlv_field: Vec, + } + impl_writeable_tlv_based!(MyCustomStruct, { + (0, tlv_field, (required_vec, encoding: (Vec, WithoutLength))), + }); + + let instance = MyCustomStruct { tlv_field: vec![42; 32] }; + let encoded = instance.encode(); + let decoded: MyCustomStruct = + LengthReadable::read_from_fixed_length_buffer(&mut &encoded[..]).unwrap(); + assert_eq!(decoded, instance); + } } From 9d05f4cbc5bad9ee73c05b4255bbe93d75aab055 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 18 Feb 2025 16:19:18 -0500 Subject: [PATCH 4/6] ChannelManager: DRY queueing onion messages The new utility will be used in upcoming commits as part of supporting static invoice server. --- lightning/src/ln/channelmanager.rs | 71 ++++++++++++++---------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 520d6a3bae0..396ec9a14c3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -77,6 +77,7 @@ use crate::onion_message::async_payments::{AsyncPaymentsMessage, HeldHtlcAvailab use crate::onion_message::dns_resolution::HumanReadableName; use crate::onion_message::messenger::{Destination, MessageRouter, Responder, ResponseInstruction, MessageSendInstructions}; use crate::onion_message::offers::{OffersMessage, OffersMessageHandler}; +use crate::onion_message::packet::OnionMessageContents; use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider}; use crate::sign::ecdsa::EcdsaChannelSigner; use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelConfigOverrides, UserConfig}; @@ -4967,19 +4968,10 @@ where }; let mut pending_async_payments_messages = self.pending_async_payments_messages.lock().unwrap(); - const HTLC_AVAILABLE_LIMIT: usize = 10; - reply_paths - .iter() - .flat_map(|reply_path| invoice.message_paths().iter().map(move |invoice_path| (invoice_path, reply_path))) - .take(HTLC_AVAILABLE_LIMIT) - .for_each(|(invoice_path, reply_path)| { - let instructions = MessageSendInstructions::WithSpecifiedReplyPath { - destination: Destination::BlindedPath(invoice_path.clone()), - reply_path: reply_path.clone(), - }; - let message = AsyncPaymentsMessage::HeldHtlcAvailable(HeldHtlcAvailable {}); - pending_async_payments_messages.push((message, instructions)); - }); + let message = AsyncPaymentsMessage::HeldHtlcAvailable(HeldHtlcAvailable {}); + enqueue_onion_message_with_reply_paths( + message, invoice.message_paths(), reply_paths, &mut pending_async_payments_messages + ); NotifyOption::DoPersist }); @@ -10544,18 +10536,10 @@ where ) -> Result<(), Bolt12SemanticError> { let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if !invoice_request.paths().is_empty() { - reply_paths - .iter() - .flat_map(|reply_path| invoice_request.paths().iter().map(move |path| (path, reply_path))) - .take(OFFERS_MESSAGE_REQUEST_LIMIT) - .for_each(|(path, reply_path)| { - let instructions = MessageSendInstructions::WithSpecifiedReplyPath { - destination: Destination::BlindedPath(path.clone()), - reply_path: reply_path.clone(), - }; - let message = OffersMessage::InvoiceRequest(invoice_request.clone()); - pending_offers_messages.push((message, instructions)); - }); + let message = OffersMessage::InvoiceRequest(invoice_request.clone()); + enqueue_onion_message_with_reply_paths( + message, invoice_request.paths(), reply_paths, &mut pending_offers_messages + ); } else if let Some(node_id) = invoice_request.issuer_signing_pubkey() { for reply_path in reply_paths { let instructions = MessageSendInstructions::WithSpecifiedReplyPath { @@ -10653,18 +10637,10 @@ where pending_offers_messages.push((message, instructions)); } } else { - reply_paths - .iter() - .flat_map(|reply_path| refund.paths().iter().map(move |path| (path, reply_path))) - .take(OFFERS_MESSAGE_REQUEST_LIMIT) - .for_each(|(path, reply_path)| { - let instructions = MessageSendInstructions::WithSpecifiedReplyPath { - destination: Destination::BlindedPath(path.clone()), - reply_path: reply_path.clone(), - }; - let message = OffersMessage::Invoice(invoice.clone()); - pending_offers_messages.push((message, instructions)); - }); + let message = OffersMessage::Invoice(invoice.clone()); + enqueue_onion_message_with_reply_paths( + message, refund.paths(), reply_paths, &mut pending_offers_messages + ); } Ok(invoice) @@ -12806,6 +12782,27 @@ where } } +fn enqueue_onion_message_with_reply_paths( + message: T, message_paths: &[BlindedMessagePath], reply_paths: Vec, + queue: &mut Vec<(T, MessageSendInstructions)> +) { + reply_paths + .iter() + .flat_map(|reply_path| + message_paths + .iter() + .map(move |path| (path.clone(), reply_path)) + ) + .take(OFFERS_MESSAGE_REQUEST_LIMIT) + .for_each(|(path, reply_path)| { + let instructions = MessageSendInstructions::WithSpecifiedReplyPath { + destination: Destination::BlindedPath(path.clone()), + reply_path: reply_path.clone(), + }; + queue.push((message.clone(), instructions)); + }); +} + /// Fetches the set of [`NodeFeatures`] flags that are provided by or required by /// [`ChannelManager`]. pub(crate) fn provided_node_features(config: &UserConfig) -> NodeFeatures { From e050044c28e03e91250a49cef3cfd67140255b65 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 6 Mar 2025 16:58:26 -0500 Subject: [PATCH 5/6] Enforce onion message has correct context when peeling Previously the compiler would not enforce that a peeled onion message had a corresponding context that matched, e.g. an offers message needs an offers blinded path context. Refactor PeeledOnion to have multiple Receive message types such that this is now enforced. --- fuzz/src/onion_message.rs | 3 +- lightning/src/ln/offers_tests.rs | 56 ++++---- lightning/src/onion_message/messenger.rs | 158 +++++++++++------------ 3 files changed, 101 insertions(+), 116 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index abdd588f1c1..a5782dacd42 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -343,8 +343,7 @@ mod tests { assert_eq!( log_entries.get(&( "lightning::onion_message::messenger".to_string(), - "Received an onion message with a reply_path: Custom(TestCustomMessage)" - .to_string() + "Received an onion message with a reply_path: TestCustomMessage".to_string() )), Some(&1) ); diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index e01f8d847b6..c8419a14239 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -46,7 +46,7 @@ use core::time::Duration; use crate::blinded_path::IntroductionNode; use crate::blinded_path::message::BlindedMessagePath; use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext}; -use crate::blinded_path::message::{MessageContext, OffersContext}; +use crate::blinded_path::message::OffersContext; use crate::events::{ClosureReason, Event, HTLCDestination, PaymentFailureReason, PaymentPurpose}; use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, RecipientOnionFields, Retry, self}; use crate::types::features::Bolt12InvoiceFeatures; @@ -60,7 +60,6 @@ use crate::offers::nonce::Nonce; use crate::offers::parse::Bolt12SemanticError; use crate::onion_message::messenger::{Destination, PeeledOnion, MessageSendInstructions}; use crate::onion_message::offers::OffersMessage; -use crate::onion_message::packet::ParsedOnionMessageContents; use crate::routing::gossip::{NodeAlias, NodeId}; use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig}; use crate::sign::{NodeSigner, Recipient}; @@ -193,9 +192,10 @@ fn claim_bolt12_payment<'a, 'b, 'c>( fn extract_offer_nonce<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> Nonce { match node.onion_messenger.peel_onion_message(message) { - Ok(PeeledOnion::Receive(_, Some(MessageContext::Offers(OffersContext::InvoiceRequest { nonce })), _)) => nonce, - Ok(PeeledOnion::Receive(_, context, _)) => panic!("Unexpected onion message context: {:?}", context), + Ok(PeeledOnion::Offers(_, Some(OffersContext::InvoiceRequest { nonce }), _)) => nonce, + Ok(PeeledOnion::Offers(_, context, _)) => panic!("Unexpected onion message context: {:?}", context), Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"), + Ok(_) => panic!("Unexpected onion message"), Err(e) => panic!("Failed to process onion message {:?}", e), } } @@ -204,34 +204,30 @@ pub(super) fn extract_invoice_request<'a, 'b, 'c>( node: &Node<'a, 'b, 'c>, message: &OnionMessage ) -> (InvoiceRequest, BlindedMessagePath) { match node.onion_messenger.peel_onion_message(message) { - Ok(PeeledOnion::Receive(message, _, reply_path)) => match message { - ParsedOnionMessageContents::Offers(offers_message) => match offers_message { - OffersMessage::InvoiceRequest(invoice_request) => (invoice_request, reply_path.unwrap()), - OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), - #[cfg(async_payments)] - OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice), - OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error), - }, - _ => panic!("Unexpected message {:?}", message), + Ok(PeeledOnion::Offers(message, _, reply_path)) => match message { + OffersMessage::InvoiceRequest(invoice_request) => (invoice_request, reply_path.unwrap()), + OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), + #[cfg(async_payments)] + OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice), + OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error), }, Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"), + Ok(_) => panic!("Unexpected onion message"), Err(e) => panic!("Failed to process onion message {:?}", e), } } fn extract_invoice<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> (Bolt12Invoice, BlindedMessagePath) { match node.onion_messenger.peel_onion_message(message) { - Ok(PeeledOnion::Receive(message, _, reply_path)) => match message { - ParsedOnionMessageContents::Offers(offers_message) => match offers_message { - OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request), - OffersMessage::Invoice(invoice) => (invoice, reply_path.unwrap()), - #[cfg(async_payments)] - OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice), - OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error), - }, - _ => panic!("Unexpected message {:?}", message), + Ok(PeeledOnion::Offers(message, _, reply_path)) => match message { + OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request), + OffersMessage::Invoice(invoice) => (invoice, reply_path.unwrap()), + #[cfg(async_payments)] + OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice), + OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error), }, Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"), + Ok(_) => panic!("Unexpected onion message"), Err(e) => panic!("Failed to process onion message {:?}", e), } } @@ -240,17 +236,15 @@ fn extract_invoice_error<'a, 'b, 'c>( node: &Node<'a, 'b, 'c>, message: &OnionMessage ) -> InvoiceError { match node.onion_messenger.peel_onion_message(message) { - Ok(PeeledOnion::Receive(message, _, _)) => match message { - ParsedOnionMessageContents::Offers(offers_message) => match offers_message { - OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request), - OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), - #[cfg(async_payments)] - OffersMessage::StaticInvoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), - OffersMessage::InvoiceError(error) => error, - }, - _ => panic!("Unexpected message: {:?}", message), + Ok(PeeledOnion::Offers(message, _, _)) => match message { + OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request), + OffersMessage::Invoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), + #[cfg(async_payments)] + OffersMessage::StaticInvoice(invoice) => panic!("Unexpected invoice: {:?}", invoice), + OffersMessage::InvoiceError(error) => error, }, Ok(PeeledOnion::Forward(_, _)) => panic!("Unexpected onion message forward"), + Ok(_) => panic!("Unexpected onion message"), Err(e) => panic!("Failed to process onion message {:?}", e), } } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index df317d45df2..cd11a12cef1 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -19,16 +19,18 @@ use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1, SecretKey}; use super::async_payments::AsyncPaymentsMessage; use super::async_payments::AsyncPaymentsMessageHandler; use super::dns_resolution::{DNSResolverMessage, DNSResolverMessageHandler}; -use super::offers::OffersMessageHandler; +use super::offers::{OffersMessage, OffersMessageHandler}; use super::packet::OnionMessageContents; use super::packet::ParsedOnionMessageContents; use super::packet::{ ForwardControlTlvs, Packet, Payload, ReceiveControlTlvs, BIG_PACKET_HOP_DATA_LEN, SMALL_PACKET_HOP_DATA_LEN, }; +#[cfg(async_payments)] +use crate::blinded_path::message::AsyncPaymentsContext; use crate::blinded_path::message::{ - BlindedMessagePath, ForwardTlvs, MessageContext, MessageForwardNode, NextMessageHop, - ReceiveTlvs, + BlindedMessagePath, DNSResolverContext, ForwardTlvs, MessageContext, MessageForwardNode, + NextMessageHop, OffersContext, ReceiveTlvs, }; use crate::blinded_path::utils; use crate::blinded_path::{IntroductionNode, NodeIdLookUp}; @@ -897,8 +899,15 @@ pub trait CustomOnionMessageHandler { pub enum PeeledOnion { /// Forwarded onion, with the next node id and a new onion Forward(NextMessageHop, OnionMessage), - /// Received onion message, with decrypted contents, context, and reply path - Receive(ParsedOnionMessageContents, Option, Option), + /// Received offers onion message, with decrypted contents, context, and reply path + Offers(OffersMessage, Option, Option), + /// Received async payments onion message, with decrypted contents, context, and reply path + #[cfg(async_payments)] + AsyncPayments(AsyncPaymentsMessage, AsyncPaymentsContext, Option), + /// Received DNS resolver onion message, with decrypted contents, context, and reply path + DNSResolver(DNSResolverMessage, Option, Option), + /// Received custom onion message, with decrypted contents, context, and reply path + Custom(T, Option>, Option), } /// Creates an [`OnionMessage`] with the given `contents` for sending to the destination of @@ -1073,26 +1082,35 @@ where reply_path, }, None, - )) => match (&message, &context) { - (_, None) => Ok(PeeledOnion::Receive(message, None, reply_path)), - (ParsedOnionMessageContents::Offers(_), Some(MessageContext::Offers(_))) => { - Ok(PeeledOnion::Receive(message, context, reply_path)) + )) => match (message, context) { + (ParsedOnionMessageContents::Offers(msg), Some(MessageContext::Offers(ctx))) => { + Ok(PeeledOnion::Offers(msg, Some(ctx), reply_path)) + }, + (ParsedOnionMessageContents::Offers(msg), None) => { + Ok(PeeledOnion::Offers(msg, None, reply_path)) }, #[cfg(async_payments)] ( - ParsedOnionMessageContents::AsyncPayments(_), - Some(MessageContext::AsyncPayments(_)), - ) => Ok(PeeledOnion::Receive(message, context, reply_path)), - (ParsedOnionMessageContents::Custom(_), Some(MessageContext::Custom(_))) => { - Ok(PeeledOnion::Receive(message, context, reply_path)) + ParsedOnionMessageContents::AsyncPayments(msg), + Some(MessageContext::AsyncPayments(ctx)), + ) => Ok(PeeledOnion::AsyncPayments(msg, ctx, reply_path)), + (ParsedOnionMessageContents::Custom(msg), Some(MessageContext::Custom(ctx))) => { + Ok(PeeledOnion::Custom(msg, Some(ctx), reply_path)) + }, + (ParsedOnionMessageContents::Custom(msg), None) => { + Ok(PeeledOnion::Custom(msg, None, reply_path)) }, - (ParsedOnionMessageContents::DNSResolver(_), Some(MessageContext::DNSResolver(_))) => { - Ok(PeeledOnion::Receive(message, context, reply_path)) + ( + ParsedOnionMessageContents::DNSResolver(msg), + Some(MessageContext::DNSResolver(ctx)), + ) => Ok(PeeledOnion::DNSResolver(msg, Some(ctx), reply_path)), + (ParsedOnionMessageContents::DNSResolver(msg), None) => { + Ok(PeeledOnion::DNSResolver(msg, None, reply_path)) }, _ => { log_trace!( logger, - "Received message was sent on a blinded path with the wrong context." + "Received message was sent on a blinded path with wrong or missing context." ); Err(()) }, @@ -1894,44 +1912,33 @@ where { fn handle_onion_message(&self, peer_node_id: PublicKey, msg: &OnionMessage) { let logger = WithContext::from(&self.logger, Some(peer_node_id), None, None); - match self.peel_onion_message(msg) { - Ok(PeeledOnion::Receive(message, context, reply_path)) => { + macro_rules! log_receive { + ($message: expr, $with_reply_path: expr) => { log_trace!( logger, "Received an onion message with {} reply_path: {:?}", - if reply_path.is_some() { "a" } else { "no" }, - message + if $with_reply_path { "a" } else { "no" }, + $message ); + }; + } + match self.peel_onion_message(msg) { + Ok(PeeledOnion::Offers(message, context, reply_path)) => { + log_receive!(message, reply_path.is_some()); + let responder = reply_path.map(Responder::new); + let response_instructions = + self.offers_handler.handle_message(message, context, responder); + if let Some((msg, instructions)) = response_instructions { + let _ = self.handle_onion_message_response(msg, instructions); + } + }, + #[cfg(async_payments)] + Ok(PeeledOnion::AsyncPayments(message, context, reply_path)) => { + log_receive!(message, reply_path.is_some()); let responder = reply_path.map(Responder::new); match message { - ParsedOnionMessageContents::Offers(msg) => { - let context = match context { - None => None, - Some(MessageContext::Offers(context)) => Some(context), - _ => { - debug_assert!(false, "Checked in peel_onion_message"); - return; - }, - }; - let response_instructions = - self.offers_handler.handle_message(msg, context, responder); - if let Some((msg, instructions)) = response_instructions { - let _ = self.handle_onion_message_response(msg, instructions); - } - }, - #[cfg(async_payments)] - ParsedOnionMessageContents::AsyncPayments( - AsyncPaymentsMessage::HeldHtlcAvailable(msg), - ) => { - let context = match context { - Some(MessageContext::AsyncPayments(context)) => context, - Some(_) => { - debug_assert!(false, "Checked in peel_onion_message"); - return; - }, - None => return, - }; + AsyncPaymentsMessage::HeldHtlcAvailable(msg) => { let response_instructions = self .async_payments_handler .handle_held_htlc_available(msg, context, responder); @@ -1939,53 +1946,38 @@ where let _ = self.handle_onion_message_response(msg, instructions); } }, - #[cfg(async_payments)] - ParsedOnionMessageContents::AsyncPayments( - AsyncPaymentsMessage::ReleaseHeldHtlc(msg), - ) => { - let context = match context { - Some(MessageContext::AsyncPayments(context)) => context, - Some(_) => { - debug_assert!(false, "Checked in peel_onion_message"); - return; - }, - None => return, - }; + AsyncPaymentsMessage::ReleaseHeldHtlc(msg) => { self.async_payments_handler.handle_release_held_htlc(msg, context); }, - ParsedOnionMessageContents::DNSResolver(DNSResolverMessage::DNSSECQuery( - msg, - )) => { + } + }, + Ok(PeeledOnion::DNSResolver(message, context, reply_path)) => { + log_receive!(message, reply_path.is_some()); + let responder = reply_path.map(Responder::new); + match message { + DNSResolverMessage::DNSSECQuery(msg) => { let response_instructions = self.dns_resolver_handler.handle_dnssec_query(msg, responder); if let Some((msg, instructions)) = response_instructions { let _ = self.handle_onion_message_response(msg, instructions); } }, - ParsedOnionMessageContents::DNSResolver(DNSResolverMessage::DNSSECProof( - msg, - )) => { + DNSResolverMessage::DNSSECProof(msg) => { let context = match context { - Some(MessageContext::DNSResolver(context)) => context, - _ => return, + Some(ctx) => ctx, + None => return, }; self.dns_resolver_handler.handle_dnssec_proof(msg, context); }, - ParsedOnionMessageContents::Custom(msg) => { - let context = match context { - None => None, - Some(MessageContext::Custom(data)) => Some(data), - _ => { - debug_assert!(false, "Checked in peel_onion_message"); - return; - }, - }; - let response_instructions = - self.custom_handler.handle_custom_message(msg, context, responder); - if let Some((msg, instructions)) = response_instructions { - let _ = self.handle_onion_message_response(msg, instructions); - } - }, + } + }, + Ok(PeeledOnion::Custom(message, context, reply_path)) => { + log_receive!(message, reply_path.is_some()); + let responder = reply_path.map(Responder::new); + let response_instructions = + self.custom_handler.handle_custom_message(message, context, responder); + if let Some((msg, instructions)) = response_instructions { + let _ = self.handle_onion_message_response(msg, instructions); } }, Ok(PeeledOnion::Forward(next_hop, onion_message)) => { From 92f0718f6a3346c61f3f991eca3d0b3004ed8c36 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 25 Mar 2025 17:41:08 -0400 Subject: [PATCH 6/6] Ignore DNSSECQuery OMs with unexpected context Prior to this commit we would allow DNSSEDQuery onion messages that were sent over blinded paths that were created for other purposes. This could be used to correlate identities and unblind a path, so disallow this. For consistency we also add a log on receipt of a DNSSECProof message with a missing context. --- lightning/src/onion_message/messenger.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index cd11a12cef1..6009a276976 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -1956,6 +1956,14 @@ where let responder = reply_path.map(Responder::new); match message { DNSResolverMessage::DNSSECQuery(msg) => { + if context.is_some() { + log_trace!( + logger, + "Ignoring DNSSECQuery onion message with unexpected context: {:?}", + context.unwrap() + ); + return; + } let response_instructions = self.dns_resolver_handler.handle_dnssec_query(msg, responder); if let Some((msg, instructions)) = response_instructions { @@ -1965,7 +1973,13 @@ where DNSResolverMessage::DNSSECProof(msg) => { let context = match context { Some(ctx) => ctx, - None => return, + None => { + log_trace!( + logger, + "Ignoring DNSSECProof onion message due to missing context" + ); + return; + }, }; self.dns_resolver_handler.handle_dnssec_proof(msg, context); },