Skip to content

Commit 1016e1f

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2139 from TheBlueMatt/2023-03-metadata-prefactors
Add a new `RecipientOnionFields` and replace `PaymentSecret` with it
2 parents c8441d2 + 77b1a22 commit 1016e1f

21 files changed

+661
-438
lines changed

‎fuzz/src/chanmon_consistency.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use lightning::chain::keysinterface::{KeyMaterial, InMemorySigner, Recipient, En
4141
use lightning::events;
4242
use lightning::events::MessageSendEventsProvider;
4343
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
44-
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs, PaymentId};
44+
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentSendFailure, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
4545
use lightning::ln::channel::FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
4646
use lightning::ln::msgs::{self, CommitmentUpdate, ChannelMessageHandler, DecodeError, UpdateAddHTLC, Init};
4747
use lightning::ln::script::ShutdownScript;
@@ -351,7 +351,7 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
351351
let mut payment_id = [0; 32];
352352
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
353353
*payment_idx += 1;
354-
if let Err(err) = source.send_payment(&Route {
354+
if let Err(err) = source.send_payment_with_route(&Route {
355355
paths: vec![vec![RouteHop {
356356
pubkey: dest.get_our_node_id(),
357357
node_features: dest.node_features(),
@@ -361,7 +361,7 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
361361
cltv_expiry_delta: 200,
362362
}]],
363363
payment_params: None,
364-
}, payment_hash, &Some(payment_secret), PaymentId(payment_id)) {
364+
}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
365365
check_payment_err(err);
366366
false
367367
} else { true }
@@ -373,7 +373,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
373373
let mut payment_id = [0; 32];
374374
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
375375
*payment_idx += 1;
376-
if let Err(err) = source.send_payment(&Route {
376+
if let Err(err) = source.send_payment_with_route(&Route {
377377
paths: vec![vec![RouteHop {
378378
pubkey: middle.get_our_node_id(),
379379
node_features: middle.node_features(),
@@ -390,7 +390,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
390390
cltv_expiry_delta: 200,
391391
}]],
392392
payment_params: None,
393-
}, payment_hash, &Some(payment_secret), PaymentId(payment_id)) {
393+
}, payment_hash, RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_id)) {
394394
check_payment_err(err);
395395
false
396396
} else { true }

‎fuzz/src/full_stack.rs

+10-18
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,13 @@ use lightning::chain::transaction::OutPoint;
3737
use lightning::chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, EntropySource, NodeSigner, SignerProvider};
3838
use lightning::events::Event;
3939
use lightning::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
40-
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId};
40+
use lightning::ln::channelmanager::{ChainParameters, ChannelDetails, ChannelManager, PaymentId, RecipientOnionFields, Retry};
4141
use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor,IgnoringMessageHandler};
4242
use lightning::ln::msgs::{self, DecodeError};
4343
use lightning::ln::script::ShutdownScript;
4444
use lightning::routing::gossip::{P2PGossipSync, NetworkGraph};
4545
use lightning::routing::utxo::UtxoLookup;
46-
use lightning::routing::router::{find_route, InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router};
47-
use lightning::routing::scoring::FixedPenaltyScorer;
46+
use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router};
4847
use lightning::util::config::UserConfig;
4948
use lightning::util::errors::APIError;
5049
use lightning::util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
@@ -449,10 +448,8 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
449448
// keys subsequently generated in this test. Rather than regenerating all the messages manually,
450449
// it's easier to just increment the counter here so the keys don't change.
451450
keys_manager.counter.fetch_sub(3, Ordering::AcqRel);
452-
let our_id = &keys_manager.get_node_id(Recipient::Node).unwrap();
453451
let network_graph = Arc::new(NetworkGraph::new(network, Arc::clone(&logger)));
454452
let gossip_sync = Arc::new(P2PGossipSync::new(Arc::clone(&network_graph), None, Arc::clone(&logger)));
455-
let scorer = FixedPenaltyScorer::with_penalty(0);
456453

457454
let peers = RefCell::new([false; 256]);
458455
let mut loss_detector = MoneyLossDetector::new(&peers, channelmanager.clone(), monitor.clone(), PeerManager::new(MessageHandler {
@@ -514,18 +511,16 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
514511
payment_params,
515512
final_value_msat,
516513
};
517-
let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes();
518-
let route = match find_route(&our_id, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
519-
Ok(route) => route,
520-
Err(_) => return,
521-
};
522514
let mut payment_hash = PaymentHash([0; 32]);
523515
payment_hash.0[0..8].copy_from_slice(&be64_to_array(payments_sent));
524516
let mut sha = Sha256::engine();
525517
sha.input(&payment_hash.0[..]);
526518
payment_hash.0 = Sha256::from_engine(sha).into_inner();
527519
payments_sent += 1;
528-
match channelmanager.send_payment(&route, payment_hash, &None, PaymentId(payment_hash.0)) {
520+
match channelmanager.send_payment(payment_hash,
521+
RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), params,
522+
Retry::Attempts(0))
523+
{
529524
Ok(_) => {},
530525
Err(_) => return,
531526
}
@@ -537,12 +532,6 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
537532
payment_params,
538533
final_value_msat,
539534
};
540-
let random_seed_bytes: [u8; 32] = keys_manager.get_secure_random_bytes();
541-
let mut route = match find_route(&our_id, &params, &network_graph, None, Arc::clone(&logger), &scorer, &random_seed_bytes) {
542-
Ok(route) => route,
543-
Err(_) => return,
544-
};
545-
route.paths.push(route.paths[0].clone());
546535
let mut payment_hash = PaymentHash([0; 32]);
547536
payment_hash.0[0..8].copy_from_slice(&be64_to_array(payments_sent));
548537
let mut sha = Sha256::engine();
@@ -552,7 +541,10 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
552541
let mut payment_secret = PaymentSecret([0; 32]);
553542
payment_secret.0[0..8].copy_from_slice(&be64_to_array(payments_sent));
554543
payments_sent += 1;
555-
match channelmanager.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)) {
544+
match channelmanager.send_payment(payment_hash,
545+
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0),
546+
params, Retry::Attempts(0))
547+
{
556548
Ok(_) => {},
557549
Err(_) => return,
558550
}

‎lightning-invoice/src/payment.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ use bitcoin_hashes::Hash;
1616
use lightning::chain;
1717
use lightning::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
1818
use lightning::chain::keysinterface::{NodeSigner, SignerProvider, EntropySource};
19-
use lightning::ln::{PaymentHash, PaymentSecret};
20-
use lightning::ln::channelmanager::{ChannelManager, PaymentId, Retry, RetryableSendFailure};
19+
use lightning::ln::PaymentHash;
20+
use lightning::ln::channelmanager::{ChannelManager, PaymentId, Retry, RetryableSendFailure, RecipientOnionFields};
2121
use lightning::routing::router::{PaymentParameters, RouteParameters, Router};
2222
use lightning::util::logger::Logger;
2323

@@ -146,6 +146,7 @@ fn pay_invoice_using_amount<P: Deref>(
146146
) -> Result<(), PaymentError> where P::Target: Payer {
147147
let payment_hash = PaymentHash((*invoice.payment_hash()).into_inner());
148148
let payment_secret = Some(*invoice.payment_secret());
149+
let recipient_onion = RecipientOnionFields { payment_secret };
149150
let mut payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key(),
150151
invoice.min_final_cltv_expiry_delta() as u32)
151152
.with_expiry_time(expiry_time_from_unix_epoch(invoice).as_secs())
@@ -158,7 +159,7 @@ fn pay_invoice_using_amount<P: Deref>(
158159
final_value_msat: amount_msats,
159160
};
160161

161-
payer.send_payment(payment_hash, &payment_secret, payment_id, route_params, retry_strategy)
162+
payer.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
162163
}
163164

164165
fn expiry_time_from_unix_epoch(invoice: &Invoice) -> Duration {
@@ -182,7 +183,7 @@ trait Payer {
182183
///
183184
/// [`Route`]: lightning::routing::router::Route
184185
fn send_payment(
185-
&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>,
186+
&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
186187
payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry
187188
) -> Result<(), PaymentError>;
188189
}
@@ -199,10 +200,10 @@ where
199200
L::Target: Logger,
200201
{
201202
fn send_payment(
202-
&self, payment_hash: PaymentHash, payment_secret: &Option<PaymentSecret>,
203+
&self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields,
203204
payment_id: PaymentId, route_params: RouteParameters, retry_strategy: Retry
204205
) -> Result<(), PaymentError> {
205-
self.send_payment_with_retry(payment_hash, payment_secret, payment_id, route_params, retry_strategy)
206+
self.send_payment(payment_hash, recipient_onion, payment_id, route_params, retry_strategy)
206207
.map_err(PaymentError::Sending)
207208
}
208209
}
@@ -212,7 +213,7 @@ mod tests {
212213
use super::*;
213214
use crate::{InvoiceBuilder, Currency};
214215
use bitcoin_hashes::sha256::Hash as Sha256;
215-
use lightning::ln::PaymentPreimage;
216+
use lightning::ln::{PaymentPreimage, PaymentSecret};
216217
use lightning::ln::functional_test_utils::*;
217218
use secp256k1::{SecretKey, Secp256k1};
218219
use std::collections::VecDeque;
@@ -249,7 +250,7 @@ mod tests {
249250

250251
impl Payer for TestPayer {
251252
fn send_payment(
252-
&self, _payment_hash: PaymentHash, _payment_secret: &Option<PaymentSecret>,
253+
&self, _payment_hash: PaymentHash, _recipient_onion: RecipientOnionFields,
253254
_payment_id: PaymentId, route_params: RouteParameters, _retry_strategy: Retry
254255
) -> Result<(), PaymentError> {
255256
self.check_value_msats(Amount(route_params.final_value_msat));

‎lightning-invoice/src/utils.rs

+10-25
Original file line numberDiff line numberDiff line change
@@ -664,13 +664,13 @@ mod test {
664664
use crate::{Currency, Description, InvoiceDescription, SignOrCreationError, CreationError};
665665
use bitcoin_hashes::{Hash, sha256};
666666
use bitcoin_hashes::sha256::Hash as Sha256;
667-
use lightning::chain::keysinterface::{EntropySource, PhantomKeysManager};
667+
use lightning::chain::keysinterface::PhantomKeysManager;
668668
use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event};
669669
use lightning::ln::{PaymentPreimage, PaymentHash};
670-
use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId};
670+
use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry};
671671
use lightning::ln::functional_test_utils::*;
672672
use lightning::ln::msgs::ChannelMessageHandler;
673-
use lightning::routing::router::{PaymentParameters, RouteParameters, find_route};
673+
use lightning::routing::router::{PaymentParameters, RouteParameters};
674674
use lightning::util::test_utils;
675675
use lightning::util::config::UserConfig;
676676
use crate::utils::create_invoice_from_channelmanager_and_duration_since_epoch;
@@ -712,20 +712,12 @@ mod test {
712712
payment_params,
713713
final_value_msat: invoice.amount_milli_satoshis().unwrap(),
714714
};
715-
let first_hops = nodes[0].node.list_usable_channels();
716-
let network_graph = &node_cfgs[0].network_graph;
717-
let logger = test_utils::TestLogger::new();
718-
let scorer = test_utils::TestScorer::new();
719-
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
720-
let route = find_route(
721-
&nodes[0].node.get_our_node_id(), &route_params, network_graph,
722-
Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
723-
).unwrap();
724-
725715
let payment_event = {
726716
let mut payment_hash = PaymentHash([0; 32]);
727717
payment_hash.0.copy_from_slice(&invoice.payment_hash().as_ref()[0..32]);
728-
nodes[0].node.send_payment(&route, payment_hash, &Some(*invoice.payment_secret()), PaymentId(payment_hash.0)).unwrap();
718+
nodes[0].node.send_payment(payment_hash,
719+
RecipientOnionFields::secret_only(*invoice.payment_secret()),
720+
PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
729721
let mut added_monitors = nodes[0].chain_monitor.added_monitors.lock().unwrap();
730722
assert_eq!(added_monitors.len(), 1);
731723
added_monitors.clear();
@@ -1132,19 +1124,12 @@ mod test {
11321124
payment_params,
11331125
final_value_msat: invoice.amount_milli_satoshis().unwrap(),
11341126
};
1135-
let first_hops = nodes[0].node.list_usable_channels();
1136-
let network_graph = &node_cfgs[0].network_graph;
1137-
let logger = test_utils::TestLogger::new();
1138-
let scorer = test_utils::TestScorer::new();
1139-
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
1140-
let route = find_route(
1141-
&nodes[0].node.get_our_node_id(), &params, network_graph,
1142-
Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer, &random_seed_bytes
1143-
).unwrap();
11441127
let (payment_event, fwd_idx) = {
11451128
let mut payment_hash = PaymentHash([0; 32]);
11461129
payment_hash.0.copy_from_slice(&invoice.payment_hash().as_ref()[0..32]);
1147-
nodes[0].node.send_payment(&route, payment_hash, &Some(*invoice.payment_secret()), PaymentId(payment_hash.0)).unwrap();
1130+
nodes[0].node.send_payment(payment_hash,
1131+
RecipientOnionFields::secret_only(*invoice.payment_secret()),
1132+
PaymentId(payment_hash.0), params, Retry::Attempts(0)).unwrap();
11481133
let mut added_monitors = nodes[0].chain_monitor.added_monitors.lock().unwrap();
11491134
assert_eq!(added_monitors.len(), 1);
11501135
added_monitors.clear();
@@ -1173,7 +1158,7 @@ mod test {
11731158
nodes[fwd_idx].node.process_pending_htlc_forwards();
11741159

11751160
let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
1176-
expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, route.paths[0].last().unwrap().pubkey);
1161+
expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, invoice.recover_payee_pub_key());
11771162
do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
11781163
let events = nodes[0].node.get_and_clear_pending_events();
11791164
assert_eq!(events.len(), 2);

‎lightning/src/chain/chainmonitor.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -812,7 +812,7 @@ mod tests {
812812
use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Watch};
813813
use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
814814
use crate::events::{Event, ClosureReason, MessageSendEvent, MessageSendEventsProvider};
815-
use crate::ln::channelmanager::{PaymentSendFailure, PaymentId};
815+
use crate::ln::channelmanager::{PaymentSendFailure, PaymentId, RecipientOnionFields};
816816
use crate::ln::functional_test_utils::*;
817817
use crate::ln::msgs::ChannelMessageHandler;
818818
use crate::util::errors::APIError;
@@ -964,8 +964,9 @@ mod tests {
964964
// If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
965965
// the update through to the ChannelMonitor which will refuse it (as the channel is closed).
966966
chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
967-
unwrap_send_err!(nodes[0].node.send_payment(&route, second_payment_hash, &Some(second_payment_secret), PaymentId(second_payment_hash.0)),
968-
true, APIError::ChannelUnavailable { ref err },
967+
unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, second_payment_hash,
968+
RecipientOnionFields::secret_only(second_payment_secret), PaymentId(second_payment_hash.0)
969+
), true, APIError::ChannelUnavailable { ref err },
969970
assert!(err.contains("ChannelMonitor storage failure")));
970971
check_added_monitors!(nodes[0], 2); // After the failure we generate a close-channel monitor update
971972
check_closed_broadcast!(nodes[0], true);

‎lightning/src/chain/channelmonitor.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -4067,7 +4067,7 @@ mod tests {
40674067
use crate::ln::{PaymentPreimage, PaymentHash};
40684068
use crate::ln::chan_utils;
40694069
use crate::ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
4070-
use crate::ln::channelmanager::{PaymentSendFailure, PaymentId};
4070+
use crate::ln::channelmanager::{PaymentSendFailure, PaymentId, RecipientOnionFields};
40714071
use crate::ln::functional_test_utils::*;
40724072
use crate::ln::script::ShutdownScript;
40734073
use crate::util::errors::APIError;
@@ -4129,8 +4129,9 @@ mod tests {
41294129
// If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
41304130
// the update through to the ChannelMonitor which will refuse it (as the channel is closed).
41314131
let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000);
4132-
unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret), PaymentId(payment_hash.0)),
4133-
true, APIError::ChannelUnavailable { ref err },
4132+
unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, payment_hash,
4133+
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
4134+
), true, APIError::ChannelUnavailable { ref err },
41344135
assert!(err.contains("ChannelMonitor storage failure")));
41354136
check_added_monitors!(nodes[1], 2); // After the failure we generate a close-channel monitor update
41364137
check_closed_broadcast!(nodes[1], true);

‎lightning/src/events/mod.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ pub enum Event {
387387
/// Note for MPP payments: in rare cases, this event may be preceded by a `PaymentPathFailed`
388388
/// event. In this situation, you SHOULD treat this payment as having succeeded.
389389
PaymentSent {
390-
/// The id returned by [`ChannelManager::send_payment`].
390+
/// The `payment_id` passed to [`ChannelManager::send_payment`].
391391
///
392392
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
393393
payment_id: Option<PaymentId>,
@@ -420,11 +420,9 @@ pub enum Event {
420420
/// [`Retry`]: crate::ln::channelmanager::Retry
421421
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
422422
PaymentFailed {
423-
/// The id returned by [`ChannelManager::send_payment`] and used with
424-
/// [`ChannelManager::abandon_payment`].
423+
/// The `payment_id` passed to [`ChannelManager::send_payment`].
425424
///
426425
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
427-
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
428426
payment_id: PaymentId,
429427
/// The hash that was given to [`ChannelManager::send_payment`].
430428
///
@@ -436,7 +434,7 @@ pub enum Event {
436434
/// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See
437435
/// [`Event::PaymentSent`] for obtaining the payment preimage.
438436
PaymentPathSuccessful {
439-
/// The id returned by [`ChannelManager::send_payment`].
437+
/// The `payment_id` passed to [`ChannelManager::send_payment`].
440438
///
441439
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
442440
payment_id: PaymentId,
@@ -460,8 +458,7 @@ pub enum Event {
460458
///
461459
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
462460
PaymentPathFailed {
463-
/// The id returned by [`ChannelManager::send_payment`] and used with
464-
/// [`ChannelManager::abandon_payment`].
461+
/// The `payment_id` passed to [`ChannelManager::send_payment`].
465462
///
466463
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
467464
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment

0 commit comments

Comments
 (0)