Skip to content

Mark ChannelManager::send_payment_with_route as deprecated and take Route by value #3224

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions fuzz/src/chanmon_consistency.rs
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
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
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
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
13 changes: 9 additions & 4 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4093,6 +4093,10 @@ where

/// Sends a payment along a given route.
///
/// This method is *DEPRECATED*, use [`Self::send_payment`] instead. If you wish to fix the
/// route for a payment, do so by matching the [`PaymentId`] passed to
/// [`Router::find_route_with_id`].
///
/// Value parameters are provided via the last hop in route, see documentation for [`RouteHop`]
/// fields for more info.
///
Expand Down Expand Up @@ -4142,11 +4146,12 @@ where
/// [`UpdateHTLCs`]: events::MessageSendEvent::UpdateHTLCs
/// [`PeerManager::process_events`]: crate::ln::peer_handler::PeerManager::process_events
/// [`ChannelMonitorUpdateStatus::InProgress`]: crate::chain::ChannelMonitorUpdateStatus::InProgress
pub fn send_payment_with_route(&self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, payment_id: PaymentId) -> Result<(), PaymentSendFailure> {
#[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> {
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 @@ -12985,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 @@ -13172,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
Loading