multi: add DeleteForwardingEvents to selectively purge old forwarding history #10319
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In this commit, we add a new method to delete old forwarding events from the database, addressing issue lightningnetwork#9963 where LSPs need a way to implement data retention policies without node migration. The implementation uses batched deletion to avoid holding large database transactions that could block other operations. Events are deleted in configurable batches (default 10k, maximum 50k) with cursor-based iteration for memory efficiency. Each batch runs in its own transaction, allowing concurrent operations to proceed between batches. The method calculates and returns total fees (sum of AmtIn - AmtOut) from deleted events, allowing operators to maintain aggregate financial records for tax reporting even after deleting detailed surveillance data. This separation of financial accounting from detailed event logs is essential for privacy-conscious operators. Security validations include a minimum age requirement (currently 1 second for testing, intended to be configurable) to prevent accidental deletion of recent data, and batch size limits to prevent resource exhaustion attacks.
In this commit, we add extensive test coverage for the forwarding event deletion functionality, achieving over 85% coverage as required. The test suite includes eight distinct test cases covering all critical paths and edge cases. The basic tests verify correct deletion counts, fee calculations, and time boundary handling. We test partial deletion (ensuring only events before a cutoff are deleted), batch processing (verifying multiple batches work correctly), and idempotency (confirming repeated deletions are safe). The most interesting addition is property-based testing using the rapid package. This test generates 100 randomized scenarios and validates five key invariants: deleted count accuracy, fee calculation correctness, query result consistency, timestamp boundary enforcement, and idempotent behavior. This approach catches edge cases that traditional example-based tests might miss, particularly around boundary conditions and concurrent access patterns. We also test degenerate cases like empty databases and exact time boundaries to ensure robust error handling throughout.
In this commit, we define the new DeleteForwardingHistory RPC in the Router sub-server protocol, providing a clean API for privacy-focused deletion of old forwarding events. The RPC uses a oneof for time specification, allowing callers to provide either an absolute Unix timestamp (delete_before_time) or a relative duration string (duration like "-1w" or "-1M"). This flexibility accommodates both precise compliance deadlines and convenient regular cleanup schedules. The duration format supports seconds, minutes, hours, days, weeks, months, and years. The response includes the count of deleted events and total fees in millisatoshis, enabling operators to maintain financial records while discarding detailed routing surveillance data. The status field provides human-readable feedback about the operation.
In this commit, we regenerate the protocol buffer code using make rpc to produce the Go client/server interfaces, JSON bindings, and Swagger documentation for the new DeleteForwardingHistory RPC.
In this commit, we implement the server-side handler for the DeleteForwardingHistory RPC, connecting the protocol definition to the database layer through our ForwardingLogDB interface. The handler supports two time specification formats via the oneof field. For absolute timestamps, we convert directly from Unix seconds to a time.Time. For relative duration strings, we implement a parseDuration helper that supports natural time units (s, m, h, d, w, M for months, y for years) with negative values indicating "time ago" semantics. This makes invocations like "-1M" (one month ago) work intuitively at the CLI. Security is a key consideration in this implementation. We validate that the delete_before_time is at least 1 second in the past to prevent accidental deletion of very recent data. The current 1-second threshold is intentionally minimal for integration testing; in production deployments, operators would typically configure a longer minimum age (like 1 hour or 1 day) to provide additional safety margins. We also enforce batch size limits and provide comprehensive logging for audit trails. The response includes aggregated fee totals to support operators who need to maintain financial records separate from detailed routing surveillance data. This aligns with privacy-preserving accounting practices where aggregate revenue can be tracked without retaining granular event history.
In this commit, we extend the test harness RPC wrapper to include the new DeleteForwardingHistory method, following the established pattern for router RPC calls. This enables integration tests to invoke the deletion functionality with automatic error handling and logging.
In this commit, we connect the forwarding log database interface to the router RPC backend, completing the dependency injection chain from the RPC handler down to the database layer. This single-line change enables the DeleteForwardingHistory RPC to access the actual forwarding event database through the clean ForwardingLogDB interface abstraction.
In this commit, we add a new lncli command for deleting forwarding
history, providing a user-friendly interface to the underlying RPC.
The command follows the pattern established by similar commands like
deletepayments.
Users can specify deletion criteria using either --duration for
relative time ("--duration=-1M" for events older than one month) or
--before for absolute Unix timestamps. The --batch_size flag allows
tuning the deletion batch size for performance, though the default of
10k events per batch should work well for most deployments.
The implementation includes an interactive confirmation prompt that
displays exactly what will be deleted and requests explicit user
confirmation. This safety measure helps prevent accidental deletion
of important data. The prompt can be bypassed in scripted
environments by piping "yes" to the command.
After successful deletion, the command displays comprehensive
statistics including the number of events deleted and total fees
earned during that period. This fee information is critical for
operators who need to maintain financial records for tax reporting
even after purging detailed routing surveillance data for privacy.
In this commit, we add end-to-end integration tests that exercise the complete forwarding history deletion flow through multi-node Lightning Network topologies. The tests verify the feature works correctly in realistic scenarios with actual payment routing and database operations. We implement five distinct test scenarios. The basic deletion test creates a three-node network (Alice → Bob → Carol), routes multiple payments through Bob to generate forwarding events, then verifies that deletion works correctly with accurate event counts and fee calculations. The partial deletion test validates that only events before a specific cutoff time are deleted, leaving newer events intact. The empty database test ensures graceful handling when no events exist. The idempotency test confirms that repeated deletions with the same parameters safely do nothing after the first deletion. Finally, the time formats test validates both absolute timestamp and relative duration specifications work correctly. A critical implementation detail is timing. The minimum age validation requires events to be at least 1 second old before deletion. To satisfy this in tests without excessive delays, we sleep for 1 second after each payment, then use a timestamp 2 seconds in the past for deletion. This ensures events are old enough while keeping total test time under 125 seconds for all five scenarios.
In this commit, we add detailed documentation explaining the privacy implications of forwarding history retention and how to use the new deletion feature effectively. The document is written in natural prose with technical depth appropriate for node operators and LSPs. The guide begins by explaining why this feature matters. Lightning routing nodes accumulate detailed surveillance data about payment flows over time. While this data is useful for debugging and analytics, it poses significant privacy risks if the node is compromised or subpoenaed. For LSPs handling customer payments, retention of detailed forwarding logs may conflict with data protection regulations like GDPR. The deletion feature allows operators to implement sensible retention policies without the extreme measure of migrating to a new node instance. We include two mermaid diagrams to visualize the system. The sequence diagram shows the complete RPC flow including batch processing and fee calculation. The graph diagram illustrates how fees are accumulated from individual events during deletion to provide aggregate financial records. The usage section provides concrete examples for common scenarios: deleting old data on a regular schedule, one-time historical purges, and automated cron-based cleanup. We explain both time specification formats (relative durations and absolute timestamps) with examples of when each is appropriate. The document also covers automation strategies, troubleshooting common issues, performance characteristics of batch deletion, and legal/compliance considerations. We emphasize best practices like maintaining separate financial records, testing deletion policies in staging environments, and understanding that deletion is permanent and irreversible. This documentation ensures operators can use the feature safely and effectively while understanding the privacy and compliance trade-offs involved.
Roasbeef
added
enhancement
Summary of ChangesHello @Roasbeef, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a crucial privacy-enhancing feature for LND nodes by providing a mechanism to selectively delete old forwarding history. This allows node operators to manage data retention policies effectively, reducing the privacy exposure associated with long-term storage of routing event data, while still providing necessary accounting information for the purged records. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new RPC call, DeleteForwardingEvents, to selectively purge old forwarding history. The implementation is well-structured, including batched database deletions to avoid long-held locks, and it returns useful statistics about the deleted events. The addition of extensive unit, property-based, and integration tests is commendable. I have two main points of feedback: one regarding a safety check's duration and another about clarifying a function's behavior in its comment.
Comment on lines
+1869
to
+1876
| minimumAge := 1 * time.Second | ||
| if time.Since(deleteBeforeTime) < minimumAge { | ||
| return nil, fmt.Errorf("delete_before_time must be at "+ | ||
| "least %v "+ | ||
| "in the past to prevent accidental deletion of recent "+ | ||
| "data (requested time: %v, current time: %v)", | ||
| minimumAge, deleteBeforeTime, time.Now()) | ||
| } |
There was a problem hiding this comment.
The minimumAge for deleting forwarding history is set to 1 second. This is a very short duration and could lead to accidental deletion of recent data. The new documentation in docs/forwarding_history_privacy.md states that a one-hour minimum age is enforced. To improve safety and align with the documentation, this value should be increased to one hour. The TODO comment also suggests this was the original intention.
Please note that integration tests in itest/lnd_forward_delete_test.go will need to be adjusted to account for the longer minimum age if this change is made.
Suggested change
| minimumAge := 1 * time.Second | |
| if time.Since(deleteBeforeTime) < minimumAge { | |
| return nil, fmt.Errorf("delete_before_time must be at "+ | |
| "least %v "+ | |
| "in the past to prevent accidental deletion of recent "+ | |
| "data (requested time: %v, current time: %v)", | |
| minimumAge, deleteBeforeTime, time.Now()) | |
| } | |
| minimumAge := 1 * time.Hour | |
| if time.Since(deleteBeforeTime) < minimumAge { | |
| return nil, fmt.Errorf("delete_before_time must be at "+ | |
| "least %v "+ | |
| "in the past to prevent accidental deletion of recent "+ | |
| "data (requested time: %v, current time: %v)", | |
| minimumAge, deleteBeforeTime, time.Now()) | |
| } |
Comment on lines
+431
to
+432
| // DeleteForwardingEvents deletes all forwarding events older than the specified | ||
| // endTime from the database. The deletion is performed in batches to avoid |
There was a problem hiding this comment.
The function comment states that it deletes events "older than" endTime. However, the implementation and tests show that it deletes events with a timestamp less than or equal to endTime. To avoid confusion, it would be clearer to state that it deletes events "at or before" the specified endTime.
Suggested change
| // DeleteForwardingEvents deletes all forwarding events older than the specified | |
| // endTime from the database. The deletion is performed in batches to avoid | |
| // DeleteForwardingEvents deletes all forwarding events with a timestamp at or | |
| // before the specified endTime from the database. The deletion is performed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this PR, we add a new RPC call
DeleteForwardingEvents. The new calls allows users to specify a timestamp, or a duration (-1d, -1y, etc) to be used to delete all forwarding events before said timestamp. After deletion, the call also returns the total amount of fees earned in that period.Some additional documentation, is added to explain how the call works, and best practices w.r.t using it.
Fixes #9963