Skip to content

Commit e9432be

Browse files
tnulltnull
committed
Addressed nits.
1 parent fe6dd2b commit e9432be

File tree

4 files changed

+133
-156
lines changed

4 files changed

+133
-156
lines changed

‎fuzz/src/full_stack.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
393393
// Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the
394394
// keys subsequently generated in this test. Rather than regenerating all the messages manually,
395395
// it's easier to just increment the counter here so the keys don't change.
396-
keys_manager.counter.fetch_sub(1, Ordering::AcqRel);
396+
keys_manager.counter.fetch_sub(2, Ordering::AcqRel);
397397
let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
398398
let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger)));
399399
let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));

‎lightning/src/ln/channelmanager.rs

+2-154
Original file line numberDiff line numberDiff line change
@@ -2756,7 +2756,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
27562756
}
27572757

27582758
let route = Route { paths: vec![hops], payment_params: None };
2759-
2759+
27602760
match self.send_payment_internal(&route, payment_hash, &None, None, Some(payment_id), None) {
27612761
Ok(payment_id) => Ok((payment_hash, payment_id)),
27622762
Err(e) => Err(e)
@@ -2765,7 +2765,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
27652765

27662766
/// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a
27672767
/// payment probe.
2768-
fn payment_is_probe(&self, payment_hash: &PaymentHash, payment_id: &PaymentId) -> bool {
2768+
pub(crate) fn payment_is_probe(&self, payment_hash: &PaymentHash, payment_id: &PaymentId) -> bool {
27692769
let target_payment_hash = self.probing_cookie_from_id(payment_id);
27702770
target_payment_hash == *payment_hash
27712771
}
@@ -7752,158 +7752,6 @@ mod tests {
77527752
// Check that using the original payment hash succeeds.
77537753
assert!(inbound_payment::verify(payment_hash, &payment_data, nodes[0].node.highest_seen_timestamp.load(Ordering::Acquire) as u64, &nodes[0].node.inbound_payment_key, &nodes[0].logger).is_ok());
77547754
}
7755-
7756-
#[test]
7757-
fn sent_probe_is_probe_of_sending_node() {
7758-
let chanmon_cfgs = create_chanmon_cfgs(3);
7759-
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
7760-
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
7761-
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
7762-
7763-
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
7764-
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
7765-
7766-
// First check we refuse to build a single-hop probe
7767-
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
7768-
assert!(nodes[0].node.send_probe(route.paths[0].clone()).is_err());
7769-
7770-
// Then build an actual two-hop probing path
7771-
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);
7772-
7773-
match nodes[0].node.send_probe(route.paths[0].clone()) {
7774-
Ok((payment_hash, payment_id)) => {
7775-
assert!(nodes[0].node.payment_is_probe(&payment_hash, &payment_id));
7776-
assert!(!nodes[1].node.payment_is_probe(&payment_hash, &payment_id));
7777-
assert!(!nodes[2].node.payment_is_probe(&payment_hash, &payment_id));
7778-
},
7779-
_ => panic!(),
7780-
}
7781-
7782-
let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
7783-
assert_eq!(msg_events.len(), 1);
7784-
match msg_events.pop().unwrap() {
7785-
MessageSendEvent::UpdateHTLCs { node_id, .. } => {
7786-
assert_eq!(node_id, nodes[1].node.get_our_node_id());
7787-
},
7788-
_ => panic!(),
7789-
}
7790-
check_added_monitors!(nodes[0], 1);
7791-
}
7792-
7793-
#[test]
7794-
fn successful_probe_yields_event() {
7795-
let chanmon_cfgs = create_chanmon_cfgs(3);
7796-
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
7797-
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
7798-
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
7799-
7800-
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
7801-
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
7802-
7803-
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);
7804-
7805-
let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
7806-
7807-
// node[0] -- update_add_htlcs -> node[1]
7808-
check_added_monitors!(nodes[0], 1);
7809-
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
7810-
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
7811-
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
7812-
check_added_monitors!(nodes[1], 0);
7813-
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
7814-
expect_pending_htlcs_forwardable!(nodes[1]);
7815-
7816-
// node[1] -- update_add_htlcs -> node[2]
7817-
check_added_monitors!(nodes[1], 1);
7818-
let updates = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
7819-
assert_eq!(updates.update_add_htlcs.len(), 1);
7820-
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
7821-
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &probe_event.msgs[0]);
7822-
check_added_monitors!(nodes[2], 0);
7823-
commitment_signed_dance!(nodes[2], nodes[1], probe_event.commitment_msg, true, true);
7824-
7825-
// node[1] <- update_fail_htlcs -- node[2]
7826-
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
7827-
assert!(updates.update_add_htlcs.is_empty());
7828-
assert!(updates.update_fulfill_htlcs.is_empty());
7829-
assert_eq!(updates.update_fail_htlcs.len(), 1);
7830-
assert!(updates.update_fail_malformed_htlcs.is_empty());
7831-
assert!(updates.update_fee.is_none());
7832-
nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
7833-
check_added_monitors!(nodes[1], 0);
7834-
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, true);
7835-
7836-
// node[0] <- update_fail_htlcs -- node[1]
7837-
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
7838-
assert!(updates.update_add_htlcs.is_empty());
7839-
assert!(updates.update_fulfill_htlcs.is_empty());
7840-
assert_eq!(updates.update_fail_htlcs.len(), 1);
7841-
assert!(updates.update_fail_malformed_htlcs.is_empty());
7842-
assert!(updates.update_fee.is_none());
7843-
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
7844-
check_added_monitors!(nodes[0], 0);
7845-
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
7846-
7847-
let mut events = nodes[0].node.get_and_clear_pending_events();
7848-
assert_eq!(events.len(), 1);
7849-
match events.drain(..).next().unwrap() {
7850-
crate::util::events::Event::ProbeSuccessful { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
7851-
assert_eq!(payment_id, ev_pid);
7852-
assert_eq!(payment_hash, ev_ph);
7853-
},
7854-
_ => panic!(),
7855-
};
7856-
}
7857-
7858-
#[test]
7859-
fn failed_probe_yields_event() {
7860-
let chanmon_cfgs = create_chanmon_cfgs(3);
7861-
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
7862-
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
7863-
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
7864-
7865-
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
7866-
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000, InitFeatures::known(), InitFeatures::known());
7867-
7868-
let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
7869-
7870-
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42);
7871-
7872-
let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
7873-
7874-
// node[0] -- update_add_htlcs -> node[1]
7875-
check_added_monitors!(nodes[0], 1);
7876-
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
7877-
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
7878-
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
7879-
check_added_monitors!(nodes[1], 0);
7880-
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
7881-
expect_pending_htlcs_forwardable!(nodes[1]);
7882-
7883-
// node[0] <- update_fail_htlcs -- node[1]
7884-
check_added_monitors!(nodes[1], 1);
7885-
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
7886-
// Skip the PendingHTLCsForwardable event
7887-
let _events = nodes[1].node.get_and_clear_pending_events();
7888-
assert!(updates.update_add_htlcs.is_empty());
7889-
assert!(updates.update_fulfill_htlcs.is_empty());
7890-
assert_eq!(updates.update_fail_htlcs.len(), 1);
7891-
assert!(updates.update_fail_malformed_htlcs.is_empty());
7892-
assert!(updates.update_fee.is_none());
7893-
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
7894-
check_added_monitors!(nodes[0], 0);
7895-
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
7896-
7897-
let mut events = nodes[0].node.get_and_clear_pending_events();
7898-
assert_eq!(events.len(), 1);
7899-
match events.drain(..).next().unwrap() {
7900-
crate::util::events::Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
7901-
assert_eq!(payment_id, ev_pid);
7902-
assert_eq!(payment_hash, ev_ph);
7903-
},
7904-
_ => panic!(),
7905-
};
7906-
}
79077755
}
79087756

79097757
#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]

‎lightning/src/ln/payment_tests.rs

+129
Original file line numberDiff line numberDiff line change
@@ -845,3 +845,132 @@ fn get_ldk_payment_preimage() {
845845
pass_along_path(&nodes[0], &[&nodes[1]], amt_msat, payment_hash, Some(payment_secret), events.pop().unwrap(), true, Some(payment_preimage));
846846
claim_payment_along_route(&nodes[0], &[&[&nodes[1]]], false, payment_preimage);
847847
}
848+
849+
#[test]
850+
fn sent_probe_is_probe_of_sending_node() {
851+
let chanmon_cfgs = create_chanmon_cfgs(3);
852+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
853+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
854+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
855+
856+
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
857+
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
858+
859+
// First check we refuse to build a single-hop probe
860+
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
861+
assert!(nodes[0].node.send_probe(route.paths[0].clone()).is_err());
862+
863+
// Then build an actual two-hop probing path
864+
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);
865+
866+
match nodes[0].node.send_probe(route.paths[0].clone()) {
867+
Ok((payment_hash, payment_id)) => {
868+
assert!(nodes[0].node.payment_is_probe(&payment_hash, &payment_id));
869+
assert!(!nodes[1].node.payment_is_probe(&payment_hash, &payment_id));
870+
assert!(!nodes[2].node.payment_is_probe(&payment_hash, &payment_id));
871+
},
872+
_ => panic!(),
873+
}
874+
875+
get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
876+
check_added_monitors!(nodes[0], 1);
877+
}
878+
879+
#[test]
880+
fn successful_probe_yields_event() {
881+
let chanmon_cfgs = create_chanmon_cfgs(3);
882+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
883+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
884+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
885+
886+
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
887+
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
888+
889+
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);
890+
891+
let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
892+
893+
// node[0] -- update_add_htlcs -> node[1]
894+
check_added_monitors!(nodes[0], 1);
895+
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
896+
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
897+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
898+
check_added_monitors!(nodes[1], 0);
899+
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
900+
expect_pending_htlcs_forwardable!(nodes[1]);
901+
902+
// node[1] -- update_add_htlcs -> node[2]
903+
check_added_monitors!(nodes[1], 1);
904+
let updates = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
905+
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
906+
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &probe_event.msgs[0]);
907+
check_added_monitors!(nodes[2], 0);
908+
commitment_signed_dance!(nodes[2], nodes[1], probe_event.commitment_msg, true, true);
909+
910+
// node[1] <- update_fail_htlcs -- node[2]
911+
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
912+
nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
913+
check_added_monitors!(nodes[1], 0);
914+
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, true);
915+
916+
// node[0] <- update_fail_htlcs -- node[1]
917+
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
918+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
919+
check_added_monitors!(nodes[0], 0);
920+
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
921+
922+
let mut events = nodes[0].node.get_and_clear_pending_events();
923+
assert_eq!(events.len(), 1);
924+
match events.drain(..).next().unwrap() {
925+
crate::util::events::Event::ProbeSuccessful { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
926+
assert_eq!(payment_id, ev_pid);
927+
assert_eq!(payment_hash, ev_ph);
928+
},
929+
_ => panic!(),
930+
};
931+
}
932+
933+
#[test]
934+
fn failed_probe_yields_event() {
935+
let chanmon_cfgs = create_chanmon_cfgs(3);
936+
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
937+
let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None, None]);
938+
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
939+
940+
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
941+
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000, InitFeatures::known(), InitFeatures::known());
942+
943+
let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
944+
945+
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42);
946+
947+
let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
948+
949+
// node[0] -- update_add_htlcs -> node[1]
950+
check_added_monitors!(nodes[0], 1);
951+
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
952+
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
953+
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
954+
check_added_monitors!(nodes[1], 0);
955+
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
956+
expect_pending_htlcs_forwardable!(nodes[1]);
957+
958+
// node[0] <- update_fail_htlcs -- node[1]
959+
check_added_monitors!(nodes[1], 1);
960+
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
961+
// Skip the PendingHTLCsForwardable event
962+
let _events = nodes[1].node.get_and_clear_pending_events();
963+
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
964+
check_added_monitors!(nodes[0], 0);
965+
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);
966+
967+
let mut events = nodes[0].node.get_and_clear_pending_events();
968+
assert_eq!(events.len(), 1);
969+
match events.drain(..).next().unwrap() {
970+
crate::util::events::Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
971+
assert_eq!(payment_id, ev_pid);
972+
assert_eq!(payment_hash, ev_ph);
973+
},
974+
_ => panic!(),
975+
};
976+
}

‎lightning/src/util/events.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ pub enum Event {
368368
/// with channels in the public network graph.
369369
///
370370
/// If this is `Some`, then the corresponding channel should be avoided when the payment is
371-
/// retried.
371+
/// retried. May be `None` for older [`Event`] serializations.
372372
short_channel_id: Option<u64>,
373373
/// Parameters needed to compute a new [`Route`] when retrying the failed payment path.
374374
///

0 commit comments

Comments
 (0)