Skip to content

rustfmt non-test ln module's not-currently-being-edited files #3763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 5, 2025
Merged

rustfmt non-test ln module's not-currently-being-edited files #3763

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1e91b4b
Drop deprecated methods in `invoice_utils`
TheBlueMatt May 1, 2025
b6bf505
Run rustfmt on `msgs.rs`
TheBlueMatt May 1, 2025
facdc72
Cleanup `msgs.rs` rather trivially after rustfmt'ing it
TheBlueMatt May 2, 2025
ed28eee
f additional cleanup
TheBlueMatt May 2, 2025
5d606e8
Pull hex constants out in `peer_channel_encryptor` tests
TheBlueMatt May 1, 2025
06cff13
Run rustfmt on `peer_channel_encryptor`
TheBlueMatt May 1, 2025
6cecdeb
Clean up some nits in `peer_channel_encryptor`
TheBlueMatt May 1, 2025
0daf48a
f pce split rather than slice
TheBlueMatt May 2, 2025
1883f79
Run `rustfmt` on `invoice_utils.rs`
TheBlueMatt May 1, 2025
aadff79
Clean up `invoice_utils` functional tests somewhat
TheBlueMatt May 1, 2025
01f50bc
Run `rustfmt` on `inbound_payment.rs`
TheBlueMatt May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
229 changes: 71 additions & 158 deletions lightning/src/ln/invoice_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ mod test {
};
use crate::ln::functional_test_utils::*;
use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, MessageSendEvent};
use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig};
use crate::routing::router::{PaymentParameters, RouteParameters};
use crate::sign::PhantomKeysManager;
use crate::types::payment::{PaymentHash, PaymentPreimage};
use crate::util::config::UserConfig;
Expand Down Expand Up @@ -680,6 +680,8 @@ mod test {
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001);

let node_a_id = nodes[0].node.get_our_node_id();

let description =
Bolt11InvoiceDescription::Direct(Description::new("test".to_string()).unwrap());
let non_default_invoice_expiry_secs = 4200;
Expand Down Expand Up @@ -715,30 +717,18 @@ mod test {
assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat);
assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat);

let payment_event = {
nodes[0]
.node
.pay_for_bolt11_invoice(
&invoice,
PaymentId([42; 32]),
None,
RouteParametersConfig::default(),
Retry::Attempts(0),
)
.unwrap();
check_added_monitors(&nodes[0], 1);

let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
SendEvent::from_event(events.remove(0))
};
nodes[1]
let retry = Retry::Attempts(0);
nodes[0]
.node
.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
nodes[1].node.handle_commitment_signed_batch_test(
nodes[0].node.get_our_node_id(),
&payment_event.commitment_msg,
);
.pay_for_bolt11_invoice(&invoice, PaymentId([42; 32]), None, Default::default(), retry)
.unwrap();
check_added_monitors(&nodes[0], 1);

let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
let payment_event = SendEvent::from_event(events.remove(0));
nodes[1].node.handle_update_add_htlc(node_a_id, &payment_event.msgs[0]);
nodes[1].node.handle_commitment_signed_batch_test(node_a_id, &payment_event.commitment_msg);
check_added_monitors(&nodes[1], 1);
let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
Expand Down Expand Up @@ -892,6 +882,9 @@ mod test {
create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_a_id = nodes[0].node.get_our_node_id();
let node_b_id = nodes[1].node.get_our_node_id();

// Create a private channel with lots of capacity and a lower value public channel (without
// confirming the funding tx yet).
let unannounced_scid =
Expand All @@ -907,40 +900,29 @@ mod test {
// channel we'll immediately switch to including it as a route hint, even though it isn't
// yet announced.
let pub_channel_scid = mine_transaction(&nodes[0], &conf_tx);
let node_a_pub_channel_ready = get_event_msg!(
nodes[0],
MessageSendEvent::SendChannelReady,
nodes[1].node.get_our_node_id()
);
nodes[1]
.node
.handle_channel_ready(nodes[0].node.get_our_node_id(), &node_a_pub_channel_ready);
let node_a_pub_channel_ready =
get_event_msg!(nodes[0], MessageSendEvent::SendChannelReady, node_b_id);
nodes[1].node.handle_channel_ready(node_a_id, &node_a_pub_channel_ready);

assert_eq!(mine_transaction(&nodes[1], &conf_tx), pub_channel_scid);
let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
if let MessageSendEvent::SendChannelReady { msg, .. } = &events[0] {
nodes[0].node.handle_channel_ready(nodes[1].node.get_our_node_id(), msg);
nodes[0].node.handle_channel_ready(node_b_id, msg);
} else {
panic!();
}
if let MessageSendEvent::SendChannelUpdate { msg, .. } = &events[1] {
nodes[0].node.handle_channel_update(nodes[1].node.get_our_node_id(), msg);
nodes[0].node.handle_channel_update(node_b_id, msg);
} else {
panic!();
}

nodes[1].node.handle_channel_update(
nodes[0].node.get_our_node_id(),
&get_event_msg!(
nodes[0],
MessageSendEvent::SendChannelUpdate,
nodes[1].node.get_our_node_id()
),
);
let as_update = get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, node_b_id);
nodes[1].node.handle_channel_update(node_a_id, &as_update);

expect_channel_ready_event(&nodes[0], &nodes[1].node.get_our_node_id());
expect_channel_ready_event(&nodes[1], &nodes[0].node.get_our_node_id());
expect_channel_ready_event(&nodes[0], &node_b_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also after reviewing this PR, my opinion remains unchanged that changes like this aren't worth it. This PR is compact, so not a problem, but I'd be hesitant with future formatting changes that are anything more than what's the absolute undisputed minimum required for readability.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really agree that we should have a culture of only cleaning up code that is "undisputed minimum for readability", that seems like a weird bar to have for cleanup PRs? Unrelated to if we can do this in a separate PR or not, of course.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If some people think the clean up makes it less readable (making it 'disputed'), I don't think we should do it and stick to the rustfmt output.

expect_channel_ready_event(&nodes[1], &node_a_id);

scid_aliases.clear();
scid_aliases.insert(node_a_pub_channel_ready.short_channel_id_alias.unwrap());
Expand All @@ -954,11 +936,7 @@ mod test {
connect_blocks(&nodes[1], 5);
match_invoice_routes(Some(5000), &nodes[1], scid_aliases.clone());
connect_blocks(&nodes[1], 1);
get_event_msg!(
nodes[1],
MessageSendEvent::SendAnnouncementSignatures,
nodes[0].node.get_our_node_id()
);
get_event_msg!(nodes[1], MessageSendEvent::SendAnnouncementSignatures, node_a_id);
match_invoice_routes(Some(5000), &nodes[1], HashSet::new());
}

Expand Down Expand Up @@ -1080,34 +1058,23 @@ mod test {
let chan_1_0 =
create_unannounced_chan_between_nodes_with_value(&nodes, 1, 0, 100000, 10001);

let node_a_id = nodes[0].node.get_our_node_id();
let node_c_id = nodes[2].node.get_our_node_id();

// Create an unannonced channel between `nodes[2]` and `nodes[0]`, for which the
// `msgs::ChannelUpdate` is never handled for the node(s). As the `msgs::ChannelUpdate`
// is never handled, the `channel.counterparty.forwarding_info` is never assigned.
let mut private_chan_cfg = UserConfig::default();
private_chan_cfg.channel_handshake_config.announce_for_forwarding = false;
let temporary_channel_id = nodes[2]
.node
.create_channel(
nodes[0].node.get_our_node_id(),
1_000_000,
500_000_000,
42,
None,
Some(private_chan_cfg),
)
.create_channel(node_a_id, 1_000_000, 500_000_000, 42, None, Some(private_chan_cfg))
.unwrap();
let open_channel = get_event_msg!(
nodes[2],
MessageSendEvent::SendOpenChannel,
nodes[0].node.get_our_node_id()
);
nodes[0].node.handle_open_channel(nodes[2].node.get_our_node_id(), &open_channel);
let accept_channel = get_event_msg!(
nodes[0],
MessageSendEvent::SendAcceptChannel,
nodes[2].node.get_our_node_id()
);
nodes[2].node.handle_accept_channel(nodes[0].node.get_our_node_id(), &accept_channel);
let open_channel = get_event_msg!(nodes[2], MessageSendEvent::SendOpenChannel, node_a_id);
nodes[0].node.handle_open_channel(node_c_id, &open_channel);
let accept_channel =
get_event_msg!(nodes[0], MessageSendEvent::SendAcceptChannel, node_c_id);
nodes[2].node.handle_accept_channel(node_a_id, &accept_channel);

let tx = sign_funding_transaction(&nodes[2], &nodes[0], 1_000_000, temporary_channel_id);

Expand All @@ -1117,32 +1084,16 @@ mod test {
connect_blocks(&nodes[2], CHAN_CONFIRM_DEPTH - 1);
confirm_transaction_at(&nodes[0], &tx, conf_height);
connect_blocks(&nodes[0], CHAN_CONFIRM_DEPTH - 1);
let as_channel_ready = get_event_msg!(
nodes[2],
MessageSendEvent::SendChannelReady,
nodes[0].node.get_our_node_id()
);
nodes[2].node.handle_channel_ready(
nodes[0].node.get_our_node_id(),
&get_event_msg!(
nodes[0],
MessageSendEvent::SendChannelReady,
nodes[2].node.get_our_node_id()
),
);
get_event_msg!(
nodes[2],
MessageSendEvent::SendChannelUpdate,
nodes[0].node.get_our_node_id()
);
nodes[0].node.handle_channel_ready(nodes[2].node.get_our_node_id(), &as_channel_ready);
get_event_msg!(
nodes[0],
MessageSendEvent::SendChannelUpdate,
nodes[2].node.get_our_node_id()
);
expect_channel_ready_event(&nodes[0], &nodes[2].node.get_our_node_id());
expect_channel_ready_event(&nodes[2], &nodes[0].node.get_our_node_id());
let as_channel_ready =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing as_channel_ready from the message that C omits to send to A to the message that A omits to send C seems consistent with naming elsewhere in the codebase (at least the majority of cases) 👌

get_event_msg!(nodes[0], MessageSendEvent::SendChannelReady, node_c_id);
let cs_channel_ready =
get_event_msg!(nodes[2], MessageSendEvent::SendChannelReady, node_a_id);
nodes[2].node.handle_channel_ready(node_a_id, &as_channel_ready);
get_event_msg!(nodes[2], MessageSendEvent::SendChannelUpdate, node_a_id);
nodes[0].node.handle_channel_ready(node_c_id, &cs_channel_ready);
get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, node_c_id);
expect_channel_ready_event(&nodes[0], &node_c_id);
expect_channel_ready_event(&nodes[2], &node_a_id);

// As `msgs::ChannelUpdate` was never handled for the participating node(s) of the second
// channel, the channel will never be assigned any `counterparty.forwarding_info`.
Expand Down Expand Up @@ -1348,20 +1299,14 @@ mod test {
payment_params,
invoice.amount_milli_satoshis().unwrap(),
);
let (payment_event, fwd_idx) = {
let payment_hash = PaymentHash(invoice.payment_hash().to_byte_array());
nodes[0]
.node
.send_payment(
payment_hash,
RecipientOnionFields::secret_only(*invoice.payment_secret()),
PaymentId(payment_hash.0),
params,
Retry::Attempts(0),
)
.unwrap();
check_added_monitors(&nodes[0], 1);

let payment_hash = PaymentHash(invoice.payment_hash().to_byte_array());
let id = PaymentId(payment_hash.0);
let onion = RecipientOnionFields::secret_only(*invoice.payment_secret());
nodes[0].node.send_payment(payment_hash, onion, id, params, Retry::Attempts(0)).unwrap();
check_added_monitors(&nodes[0], 1);

let (send_event, fwd_idx) = {
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
let fwd_idx = match events[0] {
Expand All @@ -1378,14 +1323,8 @@ mod test {
};
nodes[fwd_idx]
.node
.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
commitment_signed_dance!(
nodes[fwd_idx],
nodes[0],
&payment_event.commitment_msg,
false,
true
);
.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &send_event.msgs[0]);
commitment_signed_dance!(nodes[fwd_idx], nodes[0], &send_event.commitment_msg, false, true);

// Note that we have to "forward pending HTLCs" twice before we see the PaymentClaimable as
// this "emulates" the payment taking two hops, providing some privacy to make phantom node
Expand Down Expand Up @@ -1644,6 +1583,9 @@ mod test {
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
let nodes = create_network(4, &node_cfgs, &node_chanmgrs);

let node_b_id = nodes[1].node.get_our_node_id();
let node_d_id = nodes[3].node.get_our_node_id();

let chan_0_2 =
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 2, 100000, 10001);
let chan_0_3 =
Expand All @@ -1656,26 +1598,12 @@ mod test {
private_chan_cfg.channel_handshake_config.announce_for_forwarding = false;
let temporary_channel_id = nodes[1]
.node
.create_channel(
nodes[3].node.get_our_node_id(),
1_000_000,
500_000_000,
42,
None,
Some(private_chan_cfg),
)
.create_channel(node_d_id, 1_000_000, 500_000_000, 42, None, Some(private_chan_cfg))
.unwrap();
let open_channel = get_event_msg!(
nodes[1],
MessageSendEvent::SendOpenChannel,
nodes[3].node.get_our_node_id()
);
let open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, node_d_id);
nodes[3].node.handle_open_channel(nodes[1].node.get_our_node_id(), &open_channel);
let accept_channel = get_event_msg!(
nodes[3],
MessageSendEvent::SendAcceptChannel,
nodes[1].node.get_our_node_id()
);
let accept_channel =
get_event_msg!(nodes[3], MessageSendEvent::SendAcceptChannel, node_b_id);
nodes[1].node.handle_accept_channel(nodes[3].node.get_our_node_id(), &accept_channel);

let tx = sign_funding_transaction(&nodes[1], &nodes[3], 1_000_000, temporary_channel_id);
Expand All @@ -1686,30 +1614,15 @@ mod test {
connect_blocks(&nodes[1], CHAN_CONFIRM_DEPTH - 1);
confirm_transaction_at(&nodes[3], &tx, conf_height);
connect_blocks(&nodes[3], CHAN_CONFIRM_DEPTH - 1);
let as_channel_ready = get_event_msg!(
nodes[1],
MessageSendEvent::SendChannelReady,
nodes[3].node.get_our_node_id()
);
nodes[1].node.handle_channel_ready(
nodes[3].node.get_our_node_id(),
&get_event_msg!(
nodes[3],
MessageSendEvent::SendChannelReady,
nodes[1].node.get_our_node_id()
),
);
get_event_msg!(
nodes[1],
MessageSendEvent::SendChannelUpdate,
nodes[3].node.get_our_node_id()
);
nodes[3].node.handle_channel_ready(nodes[1].node.get_our_node_id(), &as_channel_ready);
get_event_msg!(
nodes[3],
MessageSendEvent::SendChannelUpdate,
nodes[1].node.get_our_node_id()
);

let bs_channel_ready =
get_event_msg!(nodes[1], MessageSendEvent::SendChannelReady, node_d_id);
let ds_channel_ready =
get_event_msg!(nodes[3], MessageSendEvent::SendChannelReady, node_b_id);
nodes[1].node.handle_channel_ready(node_d_id, &ds_channel_ready);
get_event_msg!(nodes[1], MessageSendEvent::SendChannelUpdate, node_d_id);
nodes[3].node.handle_channel_ready(nodes[1].node.get_our_node_id(), &bs_channel_ready);
get_event_msg!(nodes[3], MessageSendEvent::SendChannelUpdate, node_b_id);
expect_channel_ready_event(&nodes[1], &nodes[3].node.get_our_node_id());
expect_channel_ready_event(&nodes[3], &nodes[1].node.get_our_node_id());

Expand Down