Add storage for forwarded payments by benthecarman · Pull Request #772 · lightningdevkit/ldk-node

benthecarman · 2026-01-27T17:33:31Z

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.

ldk-reviews-bot · 2026-01-27T17:33:34Z

👋 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.

tnull

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 · 2026-01-28T14:56:40Z

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

tnull · 2026-01-29T12:36:19Z

+	}
+
+	/// Retrieves all forwarded payments.
+	pub fn list_forwarded_payments(&self) -> Vec<ForwardedPaymentDetails> {


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?

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

Yeah, given that the PaginatedKVStore is already pretty far along, let's wait on in here. Thankfully we can then directly bump the LDK dependency.

TheBlueMatt

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?

tnull · 2026-02-05T08:22:11Z

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?

Agree it's not great to store everything forever. However, it's also hard to guess which values users are interested in. For example, one metric they def. want to see is fee revenue, and I suspect they might even want individual values for each payment forwarded rather than aggregated numbers.

benthecarman · 2026-02-05T14:56:04Z

I started working on a version where you set an interval, say 12 hours, and we store individual events and after the time has lapsed we combine them into a single entry. Does that sound good?

TheBlueMatt · 2026-02-05T15:46:16Z

I suspect they might even want individual values for each payment forwarded rather than aggregated numbers.

Yea, I kinda wonder what kind of aggregate stats they might want. Pairwise totals and counts over discrete time horizons suffices for things like "average forwarded per payment over this channel pair", but median is of course trickier. Maybe that's okay?

TheBlueMatt · 2026-02-05T15:46:44Z

I started working on a version where you set an interval, say 12 hours, and we store individual events and after the time has lapsed we combine them into a single entry. Does that sound good?

I think its totally fair to store the last N hours of individual forwards, yea! After that question is format.

benthecarman · 2026-02-06T01:26:16Z

Made it so the Detailed mode now only stores for a configured time period and will aggregate them into channel pair stats

tnull · 2026-06-11T10:52:55Z

It seems this should be unblocked by now, but needs a considerable rebase?

benthecarman · 2026-07-01T00:21:32Z

Rebased and updated for pagination.

However probably worth holding off on until https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4766 is fixed

tnull · 2026-07-01T10:41:53Z

Rebased and updated for pagination.

However probably worth holding off on until https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4766 is fixed

Ugh, okay, that means that the fix needs to happen for 0.3 still then.

benthecarman · 2026-07-10T10:45:42Z

Rebased and now uses the updated HTLCLocator. Main open questions for this is that we currently persist the aggregate bucket length and don't let the user change it as there's no real way to reconcile previous ones. This is kinda ugly, we could alternatively not allow this to be configured and just have a hard coded number.

tnull

Needs a rebase, some comments.

tnull · 2026-07-21T08:29:35Z

 }

 impl StorableObjectId for PaymentId {
-	fn encode_to_hex_str(&self) -> String {


I'm a bit dubious why we need this refactor? Can you elaborate?

ChannelPairStatsId is 80 bytes, so its hex would be 160 characters, which is larger than the KVStore key limit of 120. For ChannelPairStatsId I used base64 instead, which is 107 characters so it can fit. Couldn't really trim ChannelPairStatsId either

tnull · 2026-07-21T08:31:47Z

 	},
 });

+uniffi::custom_type!(ForwardedPaymentId, String, {


Would be good to avoid the custom types if we can. Note that they are error prone as if conversion fails in a path where Error can't be emitted (i.e., we can't bubble up a Result<X, Error>), it will have the node panic. While most callsites should be safe, we therefore still aim to reduce the number of custom type implementations over time.

tnull · 2026-07-21T08:36:31Z

+	Detailed {
+		/// The length of each aggregation bucket, in minutes. Must be non-zero.
+		///
+		/// The first detailed configuration persists this value. Later detailed configurations for


This seems like an error prone API. If users can't change this config after the fact we maybe shouldn't allow them to configure it in the first place or provide an API design that communicates that outside of docs (who users tend to read after the fact).

Switched to a fixed retention time

tnull · 2026-07-21T08:40:33Z

+		}]);
+	}
+
+	// LDK reports the incoming and outgoing HTLCs, but not which incoming HTLC funded each


It seems like here we're trying to re-establish an API contract that LDK doesn't provide, and it seems error prone. Can we avoid this? If we think this matching is important we might need to change upstream first?

There isn't a mapping of the sats for when we receive them to when we send them, they are all in a single batch. I did FIFO because it's a simple way to split the amounts while keeping the totals correct. I don't know if this is something we really can upstream in ldk because there really is no "pairs" its just one batch of htlcs coming in and going out.

tnull · 2026-07-21T08:44:48Z

+			let channel_pair_stats_store = Arc::clone(&self.channel_pair_forwarding_stats_store);
+			let logger = Arc::clone(&self.logger);
+			self.runtime.spawn_cancellable_background_task(async move {
+				run_forwarded_payment_aggregation(


I'm not sure aggregating stats needs its own background task? I mean they are relatively cheap, but there's still some overhead. Do we think doing it on-demand would be very costly?

Or maybe, if we want to do it in the background, doing some prefactoring to have some of these tasks merged into a single 'maintenance task' could make sense (as we have a few by now).

I'd rather not do it on-demand, that always feels flaky and confusing to me, especially when handling potential storage errors.

I think a maintenance task could make sense as a follow up

tnull · 2026-07-21T08:46:32Z

+						}
+					}
+
+					for inbound_stats in inbound_stats_by_channel.into_values() {


Codex:

[P1] Forwarding accounting is not idempotent across event replay.
The handler performs additive channel-stat updates and creates detailed records with fresh random IDs before all later persistence—including the user event queue write—has succeeded. If a later write fails, ReplayEvent causes the same forward to be processed again, doubling counts,
amounts, and fees. LDK also explicitly permits duplicate PaymentForwarded events when the fee is unknown. See /home/tnull/worktrees/ldk-node/pr-772-latest-20260721/src/event.rs:1769, /home/tnull/worktrees/ldk-node/pr-772-latest-20260721/src/payment/store.rs:1419, and LDK’s event contract
(https://github.com/lightningdevkit/rust-lightning/blob/f680fd37d8bea69383e1b18730423f347e6a8341/lightning/src/events/mod.rs#L1489-L1514). A stable idempotency marker or atomic persistence strategy is needed.

True, though unclear how to tackle without an idempotency marker / event ID.

yeah i dont really know a way around this unless we add the payment hash to the forwarded event

Update LDK and payment instructions to compatible revisions and adapt forwarded and unified payment handling to their API changes. Keep both native and UniFFI test targets compiling. AI-assisted-by: OpenAI Codex

Allow storable objects to choose compact non-hexadecimal key encodings. Forwarding statistics use this to keep composite identifiers within the backing KVStore's key-length limit. AI-assisted-by: OpenAI Codex

Store individual forwarding events and aggregate them into channel and channel-pair statistics for fee and profitability tracking. Use fixed one-hour buckets for detailed records and expose their identifiers as opaque strings. Infer channel-pair allocations for multi-HTLC forwards by matching incoming and outgoing amounts in FIFO order. AI-assisted-by: OpenAI Codex

benthecarman requested a review from tnull January 27, 2026 17:33

benthecarman force-pushed the save-fwd-payment branch from 1feb7ae to ba15c40 Compare January 27, 2026 17:40

tnull reviewed Jan 28, 2026

View reviewed changes

Comment thread src/io/utils.rs Outdated

Comment thread src/io/utils.rs Outdated

Comment thread src/payment/store.rs Outdated

benthecarman force-pushed the save-fwd-payment branch from ba15c40 to 20877bd Compare January 28, 2026 14:53

tnull reviewed Jan 29, 2026

View reviewed changes

benthecarman self-assigned this Jan 29, 2026

benthecarman added this to Weekly Goals Jan 29, 2026

benthecarman force-pushed the save-fwd-payment branch 3 times, most recently from 970a395 to c3f7714 Compare February 4, 2026 21:48

TheBlueMatt reviewed Feb 4, 2026

View reviewed changes

benthecarman force-pushed the save-fwd-payment branch 2 times, most recently from 19c5b89 to cbb235e Compare February 6, 2026 01:26

benthecarman force-pushed the save-fwd-payment branch from cbb235e to ad559cc Compare February 6, 2026 01:39

benthecarman mentioned this pull request Feb 6, 2026

Beta Release lightningdevkit/ldk-server#121

Open

25 tasks

benthecarman mentioned this pull request Feb 23, 2026

Update LDK dependency for splicing, http, and wallet_utils #794

Merged

benthecarman mentioned this pull request Mar 20, 2026

Add timestamp to ForwardedPayment lightningdevkit/ldk-server#162

Open

tnull added this to the 0.8 milestone Apr 20, 2026

benthecarman force-pushed the save-fwd-payment branch from ad559cc to d8a4671 Compare July 1, 2026 00:20

benthecarman force-pushed the save-fwd-payment branch from d8a4671 to a831210 Compare July 1, 2026 01:47

benthecarman force-pushed the save-fwd-payment branch from a831210 to f1c31b4 Compare July 10, 2026 10:40

benthecarman force-pushed the save-fwd-payment branch from f1c31b4 to 81249d0 Compare July 12, 2026 16:30

tnull self-requested a review July 14, 2026 10:11

benthecarman force-pushed the save-fwd-payment branch from 81249d0 to 6319183 Compare July 16, 2026 18:15

tnull reviewed Jul 21, 2026

View reviewed changes

benthecarman added 3 commits July 21, 2026 12:32

Update LDK dependencies

0b05a8a

Update LDK and payment instructions to compatible revisions and adapt forwarded and unified payment handling to their API changes. Keep both native and UniFFI test targets compiling. AI-assisted-by: OpenAI Codex

Generalize storable object keys

0d201d6

Allow storable objects to choose compact non-hexadecimal key encodings. Forwarding statistics use this to keep composite identifiers within the backing KVStore's key-length limit. AI-assisted-by: OpenAI Codex

benthecarman force-pushed the save-fwd-payment branch from 6319183 to 4a2b945 Compare July 22, 2026 01:33

Uh oh!

Conversation

benthecarman commented Jan 27, 2026

Uh oh!

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

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benthecarman commented Jan 28, 2026

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!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

tnull commented Feb 5, 2026

Uh oh!

benthecarman commented Feb 5, 2026

Uh oh!

TheBlueMatt commented Feb 5, 2026

Uh oh!

TheBlueMatt commented Feb 5, 2026

Uh oh!

benthecarman commented Feb 6, 2026

Uh oh!

tnull commented Jun 11, 2026

Uh oh!

benthecarman commented Jul 1, 2026

Uh oh!

tnull commented Jul 1, 2026

Uh oh!

benthecarman commented Jul 10, 2026

Uh oh!

tnull left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

tnull Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ldk-reviews-bot commented Jan 27, 2026 •

edited

Loading

tnull Jul 21, 2026 •

edited

Loading