multi: add DeleteForwardingHistory to selectively purge old forwarding history#10666
Conversation
Summary of ChangesHello, 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 significantly enhances LND's operational capabilities by providing a non-disruptive method for managing forwarding history data. It addresses privacy concerns and facilitates compliance with data retention policies by allowing operators to purge old forwarding events without requiring node downtime or database resets. The new functionality is robust, user-friendly, and includes safeguards to prevent accidental data loss, making it a valuable tool for routing node operators. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to delete old forwarding history events from the database, primarily for privacy and data retention purposes. This includes the database logic, a new lncli command, RPC definitions, and extensive testing and documentation. The review comments suggest removing //nolint:ll comments as they do not adhere to the style guide's principle of explaining code intention.
667fd57 to
89456c1
Compare
In this commit, we add a new DeleteForwardingEvents method to the ForwardingLog that allows callers to permanently delete all forwarding events with a timestamp at or before a specified cutoff time. The deletion is performed in batches (default 10k, max 50k events per transaction) to avoid holding large database locks that would block concurrent operations. Each batch runs in its own transaction, so other database operations can proceed between batches. Context cancellation is checked at the start of each batch, allowing callers to abort mid-way through a large deletion. Any batches already committed are permanent and will not be rolled back on cancellation. The method returns a DeleteStats struct containing the number of events deleted and the sum of fees (AmtIn - AmtOut) earned during that period. This allows operators to maintain aggregate financial records for accounting purposes even after purging the detailed event history.
In this commit, we add test coverage for the new DeleteForwardingEvents method. The tests cover basic deletion, partial deletion by time range, batch processing across multiple transactions, idempotency, empty database handling, and exact boundary conditions. Property-based tests using the rapid package validate key invariants across randomized inputs: correct event counts, fee calculation accuracy, time boundary enforcement, and idempotent behaviour.
In this commit, we define the DeleteForwardingHistory RPC in the Router sub-server protocol and regenerate all derived Go stubs, JSON bindings, and Swagger documentation. 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 (delete_before_duration, e.g. "-30d", "-1M"). The response includes the count of deleted events and total fees earned in millisatoshis, allowing operators to maintain financial records while purging detailed routing surveillance data.
In this commit, we introduce a parseDuration helper that extends Go's standard time.ParseDuration with additional user-friendly time units: d (days), w (weeks), M (months, averaged to 30.44 days), and y (years, averaged to 365.25 days). Fractional values are supported for all units. All durations must be negative to indicate "time ago" semantics, making invocations like "-30d" or "-1M" unambiguous at the call site. The standard library parser is tried first, so all existing Go duration strings (e.g. "-24h", "-1.5h") continue to work as expected.
In this commit, we implement the server-side handler for the DeleteForwardingHistory RPC, connecting the proto definition to the database layer through the ForwardingLogDB interface on RouterBackend. The handler resolves the time specification from the request oneof: an absolute Unix timestamp is used directly, while a relative duration string is parsed via parseDuration and resolved against the current clock time. We use the injected clock (RouterBackend.Clock) rather than time.Now to keep the handler testable. A configurable minimum age guard (MinFwdHistoryAge, defaulting to 1h) prevents accidental deletion of recent events. The minimum age can be overridden via --routerrpc.min-fwd-history-age for environments such as integration tests that need a shorter threshold. The context is threaded through to DeleteForwardingEvents so that client cancellation or deadline expiry aborts the deletion between batches. Batches already committed at cancellation time are permanent, but the operation is safe to re-run since deletion is idempotent.
89456c1 to
b4ee823
Compare
calvinrzachman
left a comment
There was a problem hiding this comment.
All comments from #10319 addressed + some nice additions like context based cancellation. Looks good to me.
In this commit, we pass the node's ForwardingLog into the RouterBackend alongside the MinFwdHistoryAge configuration value, completing the dependency injection chain from the RPC handler down to the database layer.
In this commit, we extend the test harness RPC wrapper to expose the new DeleteForwardingHistory method, following the established pattern for router RPC calls with automatic error handling and logging.
In this commit, we add the lncli deletefwdhistory command that wraps the DeleteForwardingHistory RPC. The command accepts a time specification in one of two forms: --age=<duration> relative duration, e.g. "-90d", "-1M", "-720h" --before=<unix> absolute Unix timestamp in seconds An interactive confirmation prompt is shown before deletion proceeds, which can be suppressed with --force/-f for unattended automation. The --batch_size flag controls events deleted per database transaction (default 10000, max 50000). The response is printed as JSON, consistent with other lncli commands.
In this commit, we add integration tests for the DeleteForwardingHistory RPC covering four scenarios: basic deletion of all events, partial deletion by time range, empty database handling, and idempotency. A time format test validates both the relative duration and absolute timestamp code paths end-to-end. Bob's node is started with --routerrpc.min-fwd-history-age=2s so the tests can exercise the minimum age guard without waiting an hour.
In this commit, we add a guide explaining the privacy implications of retaining forwarding history and how to use DeleteForwardingHistory to implement a data retention policy. The guide covers the CLI interface, batch size tuning, cron-based automation, database compaction, fee accounting considerations, and privacy best practices.
b4ee823 to
a5fcc81
Compare
This PR supersedes #10319.
Overview
Routing nodes and LSPs accumulate forwarding history over time which, if
compromised or subpoenaed, could reveal sensitive information about payment
flows. Prior to this change, the only way to purge old forwarding history was
to reset the database and restore from backup — a disruptive operation with
significant operational risk.
This PR adds a
DeleteForwardingHistoryRPC to the Router sub-server thatallows operators to permanently delete forwarding events older than a specified
time threshold, without any node downtime.
Changes
channeldbDeleteForwardingEventstoForwardingLog, which deletes all eventswith a timestamp at or before a specified cutoff time
transaction) to avoid holding large database locks
committed batches are permanent and the operation is safe to re-run
DeleteStatswith event count and total fees (msat) for accountingrouterrpcDeleteForwardingHistoryRPC inrouter.protowith aoneoftime spec: absolute Unix timestamp (
delete_before_time) or relativeduration string (
delete_before_duration, e.g."-90d","-1M")parseDurationhelper extending Go's standard duration parser withd,w,M,yunits and fractional value support(
--routerrpc.min-fwd-history-age, default 1h) to prevent accidentaldeletion of recent data
clock.Clockfor testabilitylnclideletefwdhistorycommand with--age(relative) and--before(absolute Unix timestamp) flags
--force/-fforautomation
--batch_sizeflag (default 10000)Tests
DeleteForwardingEventscovering basic deletion, partialdeletion, batching, idempotency, empty database, and boundary conditions
rapidvalidating key invariants acrossrandomised inputs
parseDurationcovering all supported units and error casesDocs
docs/forwarding_history_privacy.mdcovering privacy implications,CLI usage, batch size tuning, cron automation, database compaction, and
fee accounting
Notes
DeleteForwardingHistoryRPC lives inrouterrpc(consistent withkeeping routing-related operations out of the main Lightning service)
Closes #10319