Make payment_id a required field in Events

The switch to tracking payments by ID happend with LDK Node v0.3.0,
which is >1.5 years old by now. We can be pretty certain that nobody is
upgrading from an older version to the upcoming v0.8.

Here we hence make the `payment_id` fields in `Event` required which is
a nice API simplification that will also be utilized in the next commit.

Author: tnull

bindings/ldk_node.udl

@@ -413,9 +413,9 @@ enum VssHeaderProviderError {
 
 [Enum]
 interface Event {
-	PaymentSuccessful(PaymentId? payment_id, PaymentHash payment_hash, PaymentPreimage? payment_preimage, u64? fee_paid_msat);
-	PaymentFailed(PaymentId? payment_id, PaymentHash? payment_hash, PaymentFailureReason? reason);
-	PaymentReceived(PaymentId? payment_id, PaymentHash payment_hash, u64 amount_msat, sequence<CustomTlvRecord> custom_records);
+	PaymentSuccessful(PaymentId payment_id, PaymentHash payment_hash, PaymentPreimage? payment_preimage, u64? fee_paid_msat);
+	PaymentFailed(PaymentId payment_id, PaymentHash? payment_hash, PaymentFailureReason? reason);
+	PaymentReceived(PaymentId payment_id, PaymentHash payment_hash, u64 amount_msat, sequence<CustomTlvRecord> custom_records);
 	PaymentClaimable(PaymentId payment_id, PaymentHash payment_hash, u64 claimable_amount_msat, u32? claim_deadline, sequence<CustomTlvRecord> custom_records);
 	PaymentForwarded(ChannelId prev_channel_id, ChannelId next_channel_id, UserChannelId?
 	prev_user_channel_id, UserChannelId? next_user_channel_id, PublicKey? prev_node_id, PublicKey? next_node_id, u64? total_fee_earned_msat, u64? skimmed_fee_msat, boolean claim_from_onchain_tx, u64? outbound_amount_forwarded_msat);

src/event.rs

@@ -62,9 +62,7 @@ pub enum Event {
 	/// A sent payment was successful.
 	PaymentSuccessful {
 		/// A local identifier used to track the payment.
-		///
-		/// Will only be `None` for events serialized with LDK Node v0.2.1 or prior.
-		payment_id: Option<PaymentId>,
+		payment_id: PaymentId,
 		/// The hash of the payment.
 		payment_hash: PaymentHash,
 		/// The preimage to the `payment_hash`.
@@ -79,9 +77,7 @@ pub enum Event {
 	/// A sent payment has failed.
 	PaymentFailed {
 		/// A local identifier used to track the payment.
-		///
-		/// Will only be `None` for events serialized with LDK Node v0.2.1 or prior.
-		payment_id: Option<PaymentId>,
+		payment_id: PaymentId,
 		/// The hash of the payment.
 		///
 		/// This will be `None` if the payment failed before receiving an invoice when paying a
@@ -97,9 +93,7 @@ pub enum Event {
 	/// A payment has been received.
 	PaymentReceived {
 		/// A local identifier used to track the payment.
-		///
-		/// Will only be `None` for events serialized with LDK Node v0.2.1 or prior.
-		payment_id: Option<PaymentId>,
+		payment_id: PaymentId,
 		/// The hash of the payment.
 		payment_hash: PaymentHash,
 		/// The value, in thousandths of a satoshi, that has been received.
@@ -262,17 +256,17 @@ impl_writeable_tlv_based_enum!(Event,
 	(0, PaymentSuccessful) => {
 		(0, payment_hash, required),
 		(1, fee_paid_msat, option),
-		(3, payment_id, option),
+		(3, payment_id, required),
 		(5, payment_preimage, option),
 	},
 	(1, PaymentFailed) => {
 		(0, payment_hash, option),
 		(1, reason, upgradable_option),
-		(3, payment_id, option),
+		(3, payment_id, required),
 	},
 	(2, PaymentReceived) => {
 		(0, payment_hash, required),
-		(1, payment_id, option),
+		(1, payment_id, required),
 		(2, amount_msat, required),
 		(3, custom_records, optional_vec),
 	},
@@ -1002,7 +996,7 @@ where
 				}
 
 				let event = Event::PaymentReceived {
-					payment_id: Some(payment_id),
+					payment_id,
 					payment_hash,
 					amount_msat,
 					custom_records: onion_fields
@@ -1063,7 +1057,7 @@ where
 					);
 				});
 				let event = Event::PaymentSuccessful {
-					payment_id: Some(payment_id),
+					payment_id,
 					payment_hash,
 					payment_preimage: Some(payment_preimage),
 					fee_paid_msat,
@@ -1098,8 +1092,7 @@ where
 					},
 				};
 
-				let event =
-					Event::PaymentFailed { payment_id: Some(payment_id), payment_hash, reason };
+				let event = Event::PaymentFailed { payment_id, payment_hash, reason };
 				match self.event_queue.add_event(event).await {
 					Ok(_) => return Ok(()),
 					Err(e) => {

tests/common/mod.rs

@@ -122,7 +122,7 @@ macro_rules! expect_payment_received_event {
 			ref e @ Event::PaymentReceived { payment_id, amount_msat, .. } => {
 				println!("{} got event {:?}", $node.node_id(), e);
 				assert_eq!(amount_msat, $amount_msat);
-				let payment = $node.payment(&payment_id.unwrap()).unwrap();
+				let payment = $node.payment(&payment_id).unwrap();
 				if !matches!(payment.kind, PaymentKind::Onchain { .. }) {
 					assert_eq!(payment.fee_paid_msat, None);
 				}
@@ -171,7 +171,7 @@ macro_rules! expect_payment_successful_event {
 				if let Some(fee_msat) = $fee_paid_msat {
 					assert_eq!(fee_paid_msat, fee_msat);
 				}
-				let payment = $node.payment(&$payment_id.unwrap()).unwrap();
+				let payment = $node.payment(&$payment_id).unwrap();
 				assert_eq!(payment.fee_paid_msat, fee_paid_msat);
 				assert_eq!(payment_id, $payment_id);
 				$node.event_handled().unwrap();
@@ -973,7 +973,7 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>(
 		.claim_for_hash(manual_payment_hash, claimable_amount_msat, manual_preimage)
 		.unwrap();
 	expect_payment_received_event!(node_b, claimable_amount_msat);
-	expect_payment_successful_event!(node_a, Some(manual_payment_id), None);
+	expect_payment_successful_event!(node_a, manual_payment_id, None);
 	assert_eq!(node_a.payment(&manual_payment_id).unwrap().status, PaymentStatus::Succeeded);
 	assert_eq!(node_a.payment(&manual_payment_id).unwrap().direction, PaymentDirection::Outbound);
 	assert_eq!(

tests/integration_tests_rust.rs

@@ -1028,7 +1028,7 @@ async fn run_splice_channel_test(bitcoind_chain_source: bool) {
 	let payment_id =
 		node_b.spontaneous_payment().send(amount_msat, node_a.node_id(), None).unwrap();
 
-	expect_payment_successful_event!(node_b, Some(payment_id), None);
+	expect_payment_successful_event!(node_b, payment_id, None);
 	expect_payment_received_event!(node_a, amount_msat);
 
 	// Mine a block to give time for the HTLC to resolve
@@ -1123,7 +1123,7 @@ async fn simple_bolt12_send_receive() {
 		.send(&offer, expected_quantity, expected_payer_note.clone(), None)
 		.unwrap();
 
-	expect_payment_successful_event!(node_a, Some(payment_id), None);
+	expect_payment_successful_event!(node_a, payment_id, None);
 	let node_a_payments =
 		node_a.list_payments_with_filter(|p| matches!(p.kind, PaymentKind::Bolt12Offer { .. }));
 	assert_eq!(node_a_payments.len(), 1);
@@ -1189,7 +1189,7 @@ async fn simple_bolt12_send_receive() {
 		)
 		.unwrap();
 
-	expect_payment_successful_event!(node_a, Some(payment_id), None);
+	expect_payment_successful_event!(node_a, payment_id, None);
 	let node_a_payments = node_a.list_payments_with_filter(|p| {
 		matches!(p.kind, PaymentKind::Bolt12Offer { .. }) && p.id == payment_id
 	});
@@ -1262,7 +1262,7 @@ async fn simple_bolt12_send_receive() {
 		.first()
 		.unwrap()
 		.id;
-	expect_payment_successful_event!(node_b, Some(node_b_payment_id), None);
+	expect_payment_successful_event!(node_b, node_b_payment_id, None);
 
 	let node_b_payments = node_b.list_payments_with_filter(|p| {
 		matches!(p.kind, PaymentKind::Bolt12Refund { .. }) && p.id == node_b_payment_id
@@ -1439,7 +1439,7 @@ async fn async_payment() {
 
 	node_receiver.start().unwrap();
 
-	expect_payment_successful_event!(node_sender, Some(payment_id), None);
+	expect_payment_successful_event!(node_sender, payment_id, None);
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -1647,7 +1647,7 @@ async fn unified_send_receive_bip21_uri() {
 			},
 		};
 
-	expect_payment_successful_event!(node_a, Some(offer_payment_id), None);
+	expect_payment_successful_event!(node_a, offer_payment_id, None);
 
 	// Cut off the BOLT12 part to fallback to BOLT11.
 	let uri_str_without_offer = uri_str.split("&lno=").next().unwrap();
@@ -1667,7 +1667,7 @@ async fn unified_send_receive_bip21_uri() {
 				panic!("Expected Bolt11 payment but got error: {:?}", e);
 			},
 		};
-	expect_payment_successful_event!(node_a, Some(invoice_payment_id), None);
+	expect_payment_successful_event!(node_a, invoice_payment_id, None);
 
 	let expect_onchain_amount_sats = 800_000;
 	let onchain_uni_payment =
@@ -1801,9 +1801,9 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 
 	let service_fee_msat = (jit_amount_msat * channel_opening_fee_ppm as u64) / 1_000_000;
 	let expected_received_amount_msat = jit_amount_msat - service_fee_msat;
-	expect_payment_successful_event!(payer_node, Some(payment_id), None);
+	expect_payment_successful_event!(payer_node, payment_id, None);
 	let client_payment_id =
-		expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap();
+		expect_payment_received_event!(client_node, expected_received_amount_msat);
 	let client_payment = client_node.payment(&client_payment_id).unwrap();
 	match client_payment.kind {
 		PaymentKind::Bolt11Jit { counterparty_skimmed_fee_msat, .. } => {
@@ -1830,7 +1830,7 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 	// are working as expected.
 	println!("Paying regular invoice!");
 	let payment_id = payer_node.bolt11_payment().send(&invoice, None).unwrap();
-	expect_payment_successful_event!(payer_node, Some(payment_id), None);
+	expect_payment_successful_event!(payer_node, payment_id, None);
 	expect_event!(service_node, PaymentForwarded);
 	expect_payment_received_event!(client_node, amount_msat);
 
@@ -1876,9 +1876,9 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) {
 		.unwrap();
 
 	expect_event!(service_node, PaymentForwarded);
-	expect_payment_successful_event!(payer_node, Some(payment_id), None);
+	expect_payment_successful_event!(payer_node, payment_id, None);
 	let client_payment_id =
-		expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap();
+		expect_payment_received_event!(client_node, expected_received_amount_msat);
 	let client_payment = client_node.payment(&client_payment_id).unwrap();
 	match client_payment.kind {
 		PaymentKind::Bolt11Jit { counterparty_skimmed_fee_msat, .. } => {
@@ -1983,7 +1983,7 @@ async fn spontaneous_send_with_custom_preimage() {
 		.unwrap();
 
 	// check payment status and verify stored preimage
-	expect_payment_successful_event!(node_a, Some(payment_id), None);
+	expect_payment_successful_event!(node_a, payment_id, None);
 	let details: PaymentDetails =
 		node_a.list_payments_with_filter(|p| p.id == payment_id).first().unwrap().clone();
 	assert_eq!(details.status, PaymentStatus::Succeeded);
@@ -2168,9 +2168,9 @@ async fn lsps2_client_trusts_lsp() {
 		.claim_for_hash(manual_payment_hash, jit_amount_msat, manual_preimage)
 		.unwrap();
 
-	expect_payment_successful_event!(payer_node, Some(payment_id), None);
+	expect_payment_successful_event!(payer_node, payment_id, None);
 
-	let _ = expect_payment_received_event!(client_node, expected_received_amount_msat).unwrap();
+	let _ = expect_payment_received_event!(client_node, expected_received_amount_msat);
 
 	// Check the nodes pick up on the confirmed funding tx now.
 	wait_for_tx(&electrsd.client, funding_txo.txid).await;