Skip to content

Commit

Permalink
Addressed nits.
Browse files Browse the repository at this point in the history
  • Loading branch information
@tnull
tnull committed Jul 6, 2022
1 parent fe6dd2b commit e9432be
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 156 deletions.
2 changes: 1 addition & 1 deletion fuzz/src/full_stack.rs
View file
Expand Up @@ -393,7 +393,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
// Adding new calls to `KeysInterface::get_secure_random_bytes` during startup can change all the
// keys subsequently generated in this test. Rather than regenerating all the messages manually,
// it's easier to just increment the counter here so the keys don't change.
keys_manager.counter.fetch_sub(1, Ordering::AcqRel);
keys_manager.counter.fetch_sub(2, Ordering::AcqRel);
let our_id = PublicKey::from_secret_key(&Secp256k1::signing_only(), &keys_manager.get_node_secret(Recipient::Node).unwrap());
let network_graph = Arc::new(NetworkGraph::new(genesis_block(network).block_hash(), Arc::clone(&logger)));
let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));
Expand Down
156 changes: 2 additions & 154 deletions lightning/src/ln/channelmanager.rs
View file
Expand Up @@ -2756,7 +2756,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
}

let route = Route { paths: vec![hops], payment_params: None };

match self.send_payment_internal(&route, payment_hash, &None, None, Some(payment_id), None) {
Ok(payment_id) => Ok((payment_hash, payment_id)),
Err(e) => Err(e)
Expand All @@ -2765,7 +2765,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana

/// Returns whether a payment with the given [`PaymentHash`] and [`PaymentId`] is, in fact, a
/// payment probe.
fn payment_is_probe(&self, payment_hash: &PaymentHash, payment_id: &PaymentId) -> bool {
pub(crate) fn payment_is_probe(&self, payment_hash: &PaymentHash, payment_id: &PaymentId) -> bool {
let target_payment_hash = self.probing_cookie_from_id(payment_id);
target_payment_hash == *payment_hash
}
Expand Down Expand Up @@ -7752,158 +7752,6 @@ mod tests {
// Check that using the original payment hash succeeds.
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());
}

#[test]
fn sent_probe_is_probe_of_sending_node() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

// First check we refuse to build a single-hop probe
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
assert!(nodes[0].node.send_probe(route.paths[0].clone()).is_err());

// Then build an actual two-hop probing path
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);

match nodes[0].node.send_probe(route.paths[0].clone()) {
Ok((payment_hash, payment_id)) => {
assert!(nodes[0].node.payment_is_probe(&payment_hash, &payment_id));
assert!(!nodes[1].node.payment_is_probe(&payment_hash, &payment_id));
assert!(!nodes[2].node.payment_is_probe(&payment_hash, &payment_id));
},
_ => panic!(),
}

let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(msg_events.len(), 1);
match msg_events.pop().unwrap() {
MessageSendEvent::UpdateHTLCs { node_id, .. } => {
assert_eq!(node_id, nodes[1].node.get_our_node_id());
},
_ => panic!(),
}
check_added_monitors!(nodes[0], 1);
}

#[test]
fn successful_probe_yields_event() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);

let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();

// node[0] -- update_add_htlcs -> node[1]
check_added_monitors!(nodes[0], 1);
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);

// node[1] -- update_add_htlcs -> node[2]
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
assert_eq!(updates.update_add_htlcs.len(), 1);
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[2], 0);
commitment_signed_dance!(nodes[2], nodes[1], probe_event.commitment_msg, true, true);

// node[1] <- update_fail_htlcs -- node[2]
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
assert!(updates.update_fulfill_htlcs.is_empty());
assert_eq!(updates.update_fail_htlcs.len(), 1);
assert!(updates.update_fail_malformed_htlcs.is_empty());
assert!(updates.update_fee.is_none());
nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, true);

// node[0] <- update_fail_htlcs -- node[1]
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
assert!(updates.update_fulfill_htlcs.is_empty());
assert_eq!(updates.update_fail_htlcs.len(), 1);
assert!(updates.update_fail_malformed_htlcs.is_empty());
assert!(updates.update_fee.is_none());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[0], 0);
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);

let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events.drain(..).next().unwrap() {
crate::util::events::Event::ProbeSuccessful { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
assert_eq!(payment_id, ev_pid);
assert_eq!(payment_hash, ev_ph);
},
_ => panic!(),
};
}

#[test]
fn failed_probe_yields_event() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000, InitFeatures::known(), InitFeatures::known());

let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());

let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42);

let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();

// node[0] -- update_add_htlcs -> node[1]
check_added_monitors!(nodes[0], 1);
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);

// node[0] <- update_fail_htlcs -- node[1]
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
// Skip the PendingHTLCsForwardable event
let _events = nodes[1].node.get_and_clear_pending_events();
assert!(updates.update_add_htlcs.is_empty());
assert!(updates.update_fulfill_htlcs.is_empty());
assert_eq!(updates.update_fail_htlcs.len(), 1);
assert!(updates.update_fail_malformed_htlcs.is_empty());
assert!(updates.update_fee.is_none());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[0], 0);
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);

let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events.drain(..).next().unwrap() {
crate::util::events::Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
assert_eq!(payment_id, ev_pid);
assert_eq!(payment_hash, ev_ph);
},
_ => panic!(),
};
}
}

#[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]
Expand Down
129 changes: 129 additions & 0 deletions lightning/src/ln/payment_tests.rs
View file
Expand Up @@ -845,3 +845,132 @@ fn get_ldk_payment_preimage() {
pass_along_path(&nodes[0], &[&nodes[1]], amt_msat, payment_hash, Some(payment_secret), events.pop().unwrap(), true, Some(payment_preimage));
claim_payment_along_route(&nodes[0], &[&[&nodes[1]]], false, payment_preimage);
}

#[test]
fn sent_probe_is_probe_of_sending_node() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

// First check we refuse to build a single-hop probe
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[1], 100_000);
assert!(nodes[0].node.send_probe(route.paths[0].clone()).is_err());

// Then build an actual two-hop probing path
let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);

match nodes[0].node.send_probe(route.paths[0].clone()) {
Ok((payment_hash, payment_id)) => {
assert!(nodes[0].node.payment_is_probe(&payment_hash, &payment_id));
assert!(!nodes[1].node.payment_is_probe(&payment_hash, &payment_id));
assert!(!nodes[2].node.payment_is_probe(&payment_hash, &payment_id));
},
_ => panic!(),
}

get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
check_added_monitors!(nodes[0], 1);
}

#[test]
fn successful_probe_yields_event() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], 100_000);

let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();

// node[0] -- update_add_htlcs -> node[1]
check_added_monitors!(nodes[0], 1);
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);

// node[1] -- update_add_htlcs -> node[2]
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[2].node.get_our_node_id());
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[2], 0);
commitment_signed_dance!(nodes[2], nodes[1], probe_event.commitment_msg, true, true);

// node[1] <- update_fail_htlcs -- node[2]
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
nodes[1].node.handle_update_fail_htlc(&nodes[2].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, true);

// node[0] <- update_fail_htlcs -- node[1]
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[0], 0);
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);

let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events.drain(..).next().unwrap() {
crate::util::events::Event::ProbeSuccessful { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
assert_eq!(payment_id, ev_pid);
assert_eq!(payment_hash, ev_ph);
},
_ => panic!(),
};
}

#[test]
fn failed_probe_yields_event() {
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, None, None, None]);
let nodes = create_network(3, &node_cfgs, &node_chanmgrs);

create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 90000000, InitFeatures::known(), InitFeatures::known());

let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());

let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42);

let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();

// node[0] -- update_add_htlcs -> node[1]
check_added_monitors!(nodes[0], 1);
let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
let probe_event = SendEvent::from_commitment_update(nodes[1].node.get_our_node_id(), updates);
nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &probe_event.msgs[0]);
check_added_monitors!(nodes[1], 0);
commitment_signed_dance!(nodes[1], nodes[0], probe_event.commitment_msg, false);
expect_pending_htlcs_forwardable!(nodes[1]);

// node[0] <- update_fail_htlcs -- node[1]
check_added_monitors!(nodes[1], 1);
let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
// Skip the PendingHTLCsForwardable event
let _events = nodes[1].node.get_and_clear_pending_events();
nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]);
check_added_monitors!(nodes[0], 0);
commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false);

let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events.drain(..).next().unwrap() {
crate::util::events::Event::ProbeFailed { payment_id: ev_pid, payment_hash: ev_ph, .. } => {
assert_eq!(payment_id, ev_pid);
assert_eq!(payment_hash, ev_ph);
},
_ => panic!(),
};
}
2 changes: 1 addition & 1 deletion lightning/src/util/events.rs
View file
Expand Up @@ -368,7 +368,7 @@ pub enum Event {
/// with channels in the public network graph.
///
/// If this is `Some`, then the corresponding channel should be avoided when the payment is
/// retried.
/// retried. May be `None` for older [`Event`] serializations.
short_channel_id: Option<u64>,
/// Parameters needed to compute a new [`Route`] when retrying the failed payment path.
///
Expand Down

0 comments on commit e9432be

Please sign in to comment.