Skip to content

Add storage for forwarded payments #772

Open
benthecarman wants to merge 3 commits intolightningdevkit:mainfrom
benthecarman:save-fwd-payment
Open

Add storage for forwarded payments #772
benthecarman wants to merge 3 commits intolightningdevkit:mainfrom
benthecarman:save-fwd-payment

Conversation

@benthecarman
Copy link
Contributor

Routing nodes and LSPs want to track forwarded payments so they can run accounting on fees earned and track profitability across time. We now store these to make it easier to track and allows for future accounting utils in the future.

This shouldn't effect edge user nodes as they should never be forwarding payments.

Implementation is mostly just copied how we currently handle normal payments and adapted for forwarded payments.

@benthecarman benthecarman requested a review from tnull January 27, 2026 17:33
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jan 27, 2026

👋 Thanks for assigning @tnull 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.

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I do wonder if we should really enable storing all forwards by default, or rather make this optional.

Also more generally I wonder if users really expect us to store all forwarded payments forever, or if we should only keep the last X entries in the store? Also, with general-purpose HTLC interception coming up, maybe storing forwards might even be something we entirely want to leave entirely to the user after all?

What do you think?

@benthecarman
Copy link
Contributor Author

Addressed comments.

I think this makes sense to include in ldk-node and not just leaving it up to the user. If we are focusing on LSPs this will be an essential feature, especially if we want to add accounting tools down the line.

I think it can make sense to disable this and/or add a function to prune the storage for it. Maybe just an option that tracks totals per channel rather than individual htlcs

}

/// Retrieves all forwarded payments.
pub fn list_forwarded_payments(&self) -> Vec<ForwardedPaymentDetails> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes we wonder if we should wait for lightningdevkit/rust-lightning#4347 (and some follow-up PRs for implementations here ) to land so this can directly use pagination from the start?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured we weren't doing a release until the next ldk release so that'd be a given but can hold off on it if that's not the case

Routing nodes and LSPs want to track forwarded payments so they can run accounting on fees earned and track profitability across time. We now store these to make it easier to track and allows for future accounting utils in the future.

This shouldn't effect edge user nodes as they should never be forwarding payments.

Implementation is mostly just copied how we currently handle normal payments and adapted for forwarded payments.
Track aggregate stats (fees earned, payment counts, amounts) per channel.
Add ForwardedPaymentTrackingMode config: Stats (default) for lightweight
metrics only, or Detailed to also store individual payment records.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO storing granular forwarding information long-term is dangerous. Instead, can we store information that allows for easier compaction? eg total forwarding on a per-X basis between channel pairs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants