Read the RGS sync timestamp from the network graph#881
Open
tnull wants to merge 2 commits intolightningdevkit:mainfrom
Open
Read the RGS sync timestamp from the network graph#881tnull wants to merge 2 commits intolightningdevkit:mainfrom
tnull wants to merge 2 commits intolightningdevkit:mainfrom
Conversation
Extract the repeated "acquire write lock on `node_metrics`, mutate a field or two, then write the encoded struct to the kv-store" idiom into a single helper in `io::utils`. As a side effect, `write_node_metrics` is inlined into the helper. Co-Authored-By: HAL 9000
Previously, after each successful Rapid Gossip Sync update the background task wrote `latest_rgs_snapshot_timestamp` to the persisted `NodeMetrics` immediately, while the network graph itself is only flushed to disk later by LDK's background processor. A crash in that window left the on-disk metric ahead of the on-disk graph — on restart we'd resume RGS from the newer timestamp and permanently skip the updates that were never persisted together with the graph. Instead, seed the RGS start timestamp from `NetworkGraph::get_last_rapid_gossip_sync_timestamp`, which is part of the graph's own serialized state and therefore lands on disk atomically with the channel updates it describes. The same source now backs the RGS timestamp reported via `NodeStatus::latest_rgs_snapshot_timestamp`, so the reported value always matches what's reflected in the graph. Worst case after a crash is that we refetch the snapshots since the last persisted graph — an idempotent operation — rather than silently losing them. The `latest_rgs_snapshot_timestamp` field is retired from `NodeMetrics`, and TLV slot 6 is kept readable for backwards compatibility via LDK's `legacy` TLV grammar. Old persisted records still deserialize; new records no longer carry slot 6. The dead "reset RGS timestamp on gossip-source switch" block in the P2P builder branch also goes away, since the graph's timestamp remains the correct resume point across a P2P→RGS switch. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #615.
As a slightly related prefactor, we also move
NodeMetricspersistence to a newupdate_and_persist_node_metricshelper.