Add fees value for recent payment details by randomlogin · Pull Request #4674 · lightningdevkit/rust-lightning

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin · 2026-06-10T13:59:53Z

Previously it was not possible to obtain fees of pending/fulfilled payments.
Also we can add and populate the fee field for RecentPaymentDetails::Abandoned, but I doubt that it is useful, as the payment was not made.

I ran cargo fmt, so unrelated parts of code were changed.

Removed unnecessary clarification "in msat" for total_msat field.

ldk-reviews-bot · 2026-06-10T13:59:56Z

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-06-10T14:04:07Z

I re-examined the full data flow and all construction/match sites:

mark_fulfilled (outbound_payment.rs:304) and claim_htlc (2275) both derive the fee from get_pending_fee_msat() on the still-Retryable/Abandoned payment, with nothing mutating state between the PaymentSent push and mark_fulfilled at 2285 — so Fulfilled.fee_paid_msat and PaymentSent.fee_paid_msat are guaranteed identical. mark_fulfilled has only one caller.
get_pending_fee_msat now also covers the Fulfilled arm so the value survives MPP second-path/onchain re-claims and abandon→succeed transitions.
TLV (7, fee_paid_msat, option) for Fulfilled does not collide (existing: 0,1,3,5).
Version wording "prior to 0.3.0" matches the current crate version (0.3.0+git).
All other RecentPaymentDetails::{Pending,Fulfilled} sites (e.g. payment_tests.rs:1788, offers_tests.rs expect_recent_payment!) use .. or pattern-only matching, so no compile breakage.

No issues found.

wpaulino · 2026-06-10T17:48:46Z

 bool,
- ) -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),
+ )
+ -> (Option<MonitorUpdateCompletionAction>, Option<RAAMonitorUpdateBlockingAction>),


These should go away if you format with 1.75

wpaulino · 2026-06-10T17:49:29Z

+ /// Total routing fees paid for this payment, as also reported via the `fee_paid_msat`
+ /// field of [`Event::PaymentSent`].
+ ///
+ /// `None` for payments fulfilled before this field was introduced.


We should note the specific version

added specific version (0.3.0)

ldk-reviews-bot · 2026-06-10T17:50:46Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

wpaulino · 2026-06-11T21:40:13Z

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

ldk-reviews-bot · 2026-06-13T00:01:09Z

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-06-15T00:02:38Z

🔔 2nd Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

@TheBlueMatt not sure if you want this backported, it's just adding a new field.

I mean at this point, no, ideally not, but its trivial and @tnull wants it for lightningdevkit/ldk-node#815 so I figure we can.

TheBlueMatt · 2026-06-15T20:46:23Z

 total_msat: u64,
+ /// Total routing fees of the HTLCs currently in-flight for this payment.
+ ///
+ /// `None` for payments serialized by LDK versions prior to 0.0.103.


Ha, we don't support upgrading from 0.0.103 at this point, I don't think we need to even document this.

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

randomlogin mentioned this pull request Jun 10, 2026
Add probing service lightningdevkit/ldk-node#815
Merged

ldk-reviews-bot requested a review from wpaulino June 10, 2026 14:10

wpaulino reviewed Jun 10, 2026
View reviewed changes

Add fees value for recent payments
af19ed5
Introduce fields `pending_fee_msat` for `RecentPaymentDetails::Pending` and `fee_paid_msat` for `RecentPaymentDetails::Fulfilled`.

randomlogin force-pushed the expose-fees-in-payment-details branch from 95ece4a to af19ed5Compare June 10, 2026 21:51

randomlogin requested a review from wpaulino June 10, 2026 21:53

tnull added the backport 0.3 label Jun 11, 2026

TheBlueMatt approved these changes Jun 15, 2026
View reviewed changes

TheBlueMatt merged commit bfaedff into lightningdevkit:mainJun 15, 2026
1 check passed

randomlogin deleted the expose-fees-in-payment-details branch June 22, 2026 13:29

Uh oh!

Conversation

randomlogin commented Jun 10, 2026

Uh oh!

ldk-reviews-bot commented Jun 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-claude-review-bot commented Jun 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jun 10, 2026

Uh oh!

wpaulino commented Jun 11, 2026

Uh oh!

ldk-reviews-bot commented Jun 13, 2026

Uh oh!

ldk-reviews-bot commented Jun 15, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

randomlogin commented Jun 10, 2026

Uh oh!

ldk-reviews-bot commented Jun 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-claude-review-bot commented Jun 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jun 10, 2026

Uh oh!

wpaulino commented Jun 11, 2026

Uh oh!

ldk-reviews-bot commented Jun 13, 2026

Uh oh!

ldk-reviews-bot commented Jun 15, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ldk-reviews-bot commented Jun 10, 2026•
edited
Loading

ldk-claude-review-bot commented Jun 10, 2026•
edited
Loading

ldk-reviews-bot commented Jun 10, 2026•
edited
Loading

ldk-claude-review-bot commented Jun 10, 2026•
edited
Loading

ldk-reviews-bot commented Jun 10, 2026•
edited
Loading

ldk-claude-review-bot commented Jun 10, 2026•
edited
Loading