Skip to content

Commit

Permalink
Make send_payment_with_route take Route by value
Browse files Browse the repository at this point in the history 
Now that `ChannelManager::send_payment_with_route` is deprecated,
we don't care too much about making it as effecient as possible, so
there's not much cost to making it take `Route` by value. This
avoids bindings being unsure if the by-reference `Route` passed
needs to outlive the `ChannelManager` itself or if it only needs to
outlive the method call, creating some call overhead by forcing a
`Route::clone`, but avoiding a memory leak.
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Aug 4, 2024
1 parent bebd9d0 commit 753a7ac
Show file tree
Hide file tree
Showing 13 changed files with 160 additions and 160 deletions.
4 changes: 2 additions & 2 deletions fuzz/src/chanmon_consistency.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ fn send_payment(
.map(|chan| (chan.next_outbound_htlc_minimum_msat, chan.next_outbound_htlc_limit_msat))
.unwrap_or((0, 0));
if let Err(err) = source.send_payment_with_route(
&Route {
Route {
paths: vec![Path {
hops: vec![RouteHop {
pubkey: dest.get_our_node_id(),
Expand Down Expand Up @@ -619,7 +619,7 @@ fn send_hop_payment(
.unwrap_or((0, 0));
let first_hop_fee = 50_000;
if let Err(err) = source.send_payment_with_route(
&Route {
Route {
paths: vec![Path {
hops: vec![
RouteHop {
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/chain/channelmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4902,7 +4902,7 @@ mod tests {
// If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
// the update through to the ChannelMonitor which will refuse it (as the channel is closed).
let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000);
unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, payment_hash,
unwrap_send_err!(nodes[1].node.send_payment_with_route(route, payment_hash,
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
), false, APIError::MonitorUpdateInProgress, {});
check_added_monitors!(nodes[1], 1);
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/async_signer_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn do_test_async_commitment_signature_for_commitment_signed_revoke_and_ack(enabl
let src = &nodes[0];
let dst = &nodes[1];
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(src, dst, 8000000);
src.node.send_payment_with_route(&route, our_payment_hash,
src.node.send_payment_with_route(route, our_payment_hash,
RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).unwrap();
check_added_monitors!(src, 1);

Expand Down Expand Up @@ -333,7 +333,7 @@ fn do_test_async_raa_peer_disconnect(test_case: UnblockSignerAcrossDisconnectCas
let src = &nodes[0];
let dst = &nodes[1];
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(src, dst, 8000000);
src.node.send_payment_with_route(&route, our_payment_hash,
src.node.send_payment_with_route(route, our_payment_hash,
RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).unwrap();
check_added_monitors!(src, 1);

Expand Down Expand Up @@ -457,7 +457,7 @@ fn do_test_async_commitment_signature_peer_disconnect(test_case: UnblockSignerAc
let src = &nodes[0];
let dst = &nodes[1];
let (route, our_payment_hash, _our_payment_preimage, our_payment_secret) = get_route_and_payment_hash!(src, dst, 8000000);
src.node.send_payment_with_route(&route, our_payment_hash,
src.node.send_payment_with_route(route, our_payment_hash,
RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).unwrap();
check_added_monitors!(src, 1);

Expand Down Expand Up @@ -574,7 +574,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool) {
// Start to send the second update_add_htlc + commitment_signed, but don't actually make it
// to the peer.
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 1);

Expand Down
54 changes: 27 additions & 27 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);

{
unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash_1,
unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)
), false, APIError::MonitorUpdateInProgress, {});
check_added_monitors!(nodes[0], 1);
Expand Down Expand Up @@ -190,7 +190,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
let (route, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000);
{
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash_2,
unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)
), false, APIError::MonitorUpdateInProgress, {});
check_added_monitors!(nodes[0], 1);
Expand Down Expand Up @@ -257,7 +257,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash_2,
unwrap_send_err!(nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)
), false, APIError::MonitorUpdateInProgress, {});
check_added_monitors!(nodes[0], 1);
Expand Down Expand Up @@ -607,7 +607,7 @@ fn test_monitor_update_fail_cs() {

let (route, our_payment_hash, payment_preimage, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, our_payment_hash,
nodes[0].node.send_payment_with_route(route, our_payment_hash,
RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -700,7 +700,7 @@ fn test_monitor_update_fail_no_rebroadcast() {

let (route, our_payment_hash, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, our_payment_hash,
nodes[0].node.send_payment_with_route(route, our_payment_hash,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(our_payment_hash.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -748,15 +748,15 @@ fn test_monitor_update_raa_while_paused() {
send_payment(&nodes[0], &[&nodes[1]], 5000000);
let (route, our_payment_hash_1, payment_preimage_1, our_payment_secret_1) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, our_payment_hash_1,
nodes[0].node.send_payment_with_route(route, our_payment_hash_1,
RecipientOnionFields::secret_only(our_payment_secret_1), PaymentId(our_payment_hash_1.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
let send_event_1 = SendEvent::from_event(nodes[0].node.get_and_clear_pending_msg_events().remove(0));

let (route, our_payment_hash_2, payment_preimage_2, our_payment_secret_2) = get_route_and_payment_hash!(nodes[1], nodes[0], 1000000);
{
nodes[1].node.send_payment_with_route(&route, our_payment_hash_2,
nodes[1].node.send_payment_with_route(route, our_payment_hash_2,
RecipientOnionFields::secret_only(our_payment_secret_2), PaymentId(our_payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[1], 1);
}
Expand Down Expand Up @@ -845,7 +845,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
// holding cell.
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[2], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand All @@ -870,7 +870,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
// being paused waiting a monitor update.
let (route, payment_hash_3, _, payment_secret_3) = get_route_and_payment_hash!(nodes[0], nodes[2], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_3,
nodes[0].node.send_payment_with_route(route, payment_hash_3,
RecipientOnionFields::secret_only(payment_secret_3), PaymentId(payment_hash_3.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand All @@ -890,7 +890,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
let (payment_preimage_4, payment_hash_4) = if test_ignore_second_cs {
// Try to route another payment backwards from 2 to make sure 1 holds off on responding
let (route, payment_hash_4, payment_preimage_4, payment_secret_4) = get_route_and_payment_hash!(nodes[2], nodes[0], 1000000);
nodes[2].node.send_payment_with_route(&route, payment_hash_4,
nodes[2].node.send_payment_with_route(route, payment_hash_4,
RecipientOnionFields::secret_only(payment_secret_4), PaymentId(payment_hash_4.0)).unwrap();
check_added_monitors!(nodes[2], 1);

Expand Down Expand Up @@ -1197,10 +1197,10 @@ fn raa_no_response_awaiting_raa_state() {
// requires only an RAA response due to AwaitingRAA) we can deliver the RAA and require the CS
// generation during RAA while in monitor-update-failed state.
{
nodes[0].node.send_payment_with_route(&route, payment_hash_1,
nodes[0].node.send_payment_with_route(route.clone(), payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)).unwrap();
check_added_monitors!(nodes[0], 1);
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route.clone(), payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 0);
}
Expand Down Expand Up @@ -1250,7 +1250,7 @@ fn raa_no_response_awaiting_raa_state() {
// chanmon_fail_consistency test required it to actually find the bug (by seeing out-of-sync
// commitment transaction states) whereas here we can explicitly check for it.
{
nodes[0].node.send_payment_with_route(&route, payment_hash_3,
nodes[0].node.send_payment_with_route(route, payment_hash_3,
RecipientOnionFields::secret_only(payment_secret_3), PaymentId(payment_hash_3.0)).unwrap();
check_added_monitors!(nodes[0], 0);
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
Expand Down Expand Up @@ -1344,7 +1344,7 @@ fn claim_while_disconnected_monitor_update_fail() {
// the monitor still failed
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -1440,7 +1440,7 @@ fn monitor_failed_no_reestablish_response() {
// on receipt).
let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_1,
nodes[0].node.send_payment_with_route(route, payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -1517,7 +1517,7 @@ fn first_message_on_recv_ordering() {
// can deliver it and fail the monitor update.
let (route, payment_hash_1, payment_preimage_1, payment_secret_1) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_1,
nodes[0].node.send_payment_with_route(route, payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand All @@ -1541,7 +1541,7 @@ fn first_message_on_recv_ordering() {
// Route the second payment, generating an update_add_htlc/commitment_signed
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -1626,7 +1626,7 @@ fn test_monitor_update_fail_claim() {

let (route, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(nodes[2], nodes[0], 1_000_000);
{
nodes[2].node.send_payment_with_route(&route, payment_hash_2,
nodes[2].node.send_payment_with_route(route.clone(), payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[2], 1);
}
Expand All @@ -1645,7 +1645,7 @@ fn test_monitor_update_fail_claim() {
expect_pending_htlcs_forwardable_ignore!(nodes[1]);

let (_, payment_hash_3, payment_secret_3) = get_payment_preimage_hash!(nodes[0]);
nodes[2].node.send_payment_with_route(&route, payment_hash_3,
nodes[2].node.send_payment_with_route(route, payment_hash_3,
RecipientOnionFields::secret_only(payment_secret_3), PaymentId(payment_hash_3.0)).unwrap();
check_added_monitors!(nodes[2], 1);

Expand Down Expand Up @@ -1743,7 +1743,7 @@ fn test_monitor_update_on_pending_forwards() {

let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[2], nodes[0], 1000000);
{
nodes[2].node.send_payment_with_route(&route, payment_hash_2,
nodes[2].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[2], 1);
}
Expand Down Expand Up @@ -1808,7 +1808,7 @@ fn monitor_update_claim_fail_no_response() {
// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
{
nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 1);
}
Expand Down Expand Up @@ -2007,7 +2007,7 @@ fn test_path_paused_mpp() {
// the second got a MonitorUpdateInProgress err. This implies
// PaymentSendFailure::PartialFailure as some paths succeeded, preventing retry.
if let Err(PaymentSendFailure::PartialFailure { results, ..}) = nodes[0].node.send_payment_with_route(
&route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
route, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
) {
assert_eq!(results.len(), 2);
if let Ok(()) = results[0] {} else { panic!(); }
Expand Down Expand Up @@ -2055,7 +2055,7 @@ fn test_pending_update_fee_ack_on_reconnect() {
send_payment(&nodes[0], &[&nodes[1]], 100_000_00);

let (route, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[1], nodes[0], 1_000_000);
nodes[1].node.send_payment_with_route(&route, payment_hash,
nodes[1].node.send_payment_with_route(route, payment_hash,
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
check_added_monitors!(nodes[1], 1);
let bs_initial_send_msgs = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand Down Expand Up @@ -2315,13 +2315,13 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
// (c) will not be freed from the holding cell.
let (payment_preimage_0, payment_hash_0, ..) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

nodes[0].node.send_payment_with_route(&route, payment_hash_1,
nodes[0].node.send_payment_with_route(route.clone(), payment_hash_1,
RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)).unwrap();
check_added_monitors!(nodes[0], 1);
let send = SendEvent::from_node(&nodes[0]);
assert_eq!(send.msgs.len(), 1);

nodes[0].node.send_payment_with_route(&route, payment_hash_2,
nodes[0].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors!(nodes[0], 0);

Expand Down Expand Up @@ -2494,7 +2494,7 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
// In order to get the HTLC claim into the holding cell at nodes[1], we need nodes[1] to be
// awaiting a remote revoke_and_ack from nodes[0].
let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
nodes[0].node.send_payment_with_route(&route, second_payment_hash,
nodes[0].node.send_payment_with_route(route, second_payment_hash,
RecipientOnionFields::secret_only(second_payment_secret), PaymentId(second_payment_hash.0)).unwrap();
check_added_monitors!(nodes[0], 1);

Expand Down Expand Up @@ -3584,7 +3584,7 @@ fn do_test_glacial_peer_cant_hang(hold_chan_a: bool) {

let (route, payment_hash_2, _, payment_secret_2) = get_route_and_payment_hash!(&nodes[1], nodes[2], 1_000_000);

nodes[1].node.send_payment_with_route(&route, payment_hash_2,
nodes[1].node.send_payment_with_route(route, payment_hash_2,
RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
check_added_monitors(&nodes[1], 0);

Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4147,11 +4147,11 @@ where
/// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events
/// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
#[cfg_attr(not(any(test, feature = "_test_utils")), deprecated(note = "Use `send_payment` instead"))]
pub fn send_payment_with_route(&self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
pub fn send_payment_with_route(&self, route: Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
let best_block_height = self.best_block.read().unwrap().height;
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
self.pending_outbound_payments
.send_payment_with_route(route, payment_hash, recipient_onion, payment_id,
.send_payment_with_route(&route, payment_hash, recipient_onion, payment_id,
&self.entropy_source, &self.node_signer, best_block_height,
|args| self.send_payment_along_path(args))
}
Expand Down Expand Up @@ -12990,7 +12990,7 @@ mod tests {

// Next, attempt a regular payment and make sure it fails.
let payment_secret = PaymentSecret([43; 32]);
nodes[0].node.send_payment_with_route(&route, payment_hash,
nodes[0].node.send_payment_with_route(route.clone(), payment_hash,
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
check_added_monitors!(nodes[0], 1);
let mut events = nodes[0].node.get_and_clear_pending_msg_events();
Expand Down Expand Up @@ -13177,7 +13177,7 @@ mod tests {
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;

match nodes[0].node.send_payment_with_route(&route, payment_hash,
match nodes[0].node.send_payment_with_route(route, payment_hash,
RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0))
.unwrap_err() {
PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => {
Expand Down
Loading

0 comments on commit 753a7ac

Please sign in to comment.