Lumen snapshot invariant #5059
Open
Lumen snapshot invariant #5059
+589
−38
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
sisuresh
commented
Dec 10, 2025
There was a problem hiding this comment.
Pull request overview
This PR implements a snapshot-based invariant for conservation of lumens to validate that the total native asset supply matches the totalCoins value in the ledger header. The implementation scans both live and hot archive buckets to calculate the sum of all native balances and compares it against the recorded total.
Key changes:
- Adds
checkSnapshotmethod toConservationOfLumensinvariant that scans bucket lists - Introduces
isStoppingcallback parameter to allow early termination during application shutdown - Adds test method
getLastClosedLedgerStateForTestingto access ledger state for testing
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/AppConnector.h/cpp | Adds isStopping() method to expose application shutdown state |
| src/ledger/LedgerManager.h | Adds getLastClosedLedgerStateForTesting() virtual method declaration |
| src/ledger/LedgerManagerImpl.h/cpp | Implements getLastClosedLedgerStateForTesting() for test access and updates snapshot invariant call with isStopping callback |
| src/invariant/InvariantManager.h | Updates runStateSnapshotInvariant signature to include isStopping parameter |
| src/invariant/InvariantManagerImpl.h/cpp | Forwards isStopping callback to individual invariant checks |
| src/invariant/Invariant.h | Adds isStopping parameter to checkSnapshot virtual method |
| src/invariant/ConservationOfLumens.h/cpp | Implements snapshot validation by scanning live and hot archive buckets to sum native balances |
| src/invariant/ArchivedStateConsistency.h/cpp | Updates signature to accept isStopping parameter (unused in implementation) |
| src/invariant/test/ConservationOfLumensTests.cpp | Adds test case validating the snapshot invariant correctly detects mismatches in total lumens |
sisuresh
force-pushed
the
lumen-inv
branch
2 times, most recently
from
26c82e7 to
bbd59fa
Compare
SirTyson
requested changes
Dec 16, 2025
Contributor
SirTyson
left a comment
SirTyson
left a comment
There was a problem hiding this comment.
Thanks for working on this! I think it looks good overall, just a few small nits and a suggestion on a memory/IO optimization. I think we should also improve the test to verify a non-empty and corrupt archive.
graydon
requested changes
Dec 20, 2025
graydon
requested changes
Dec 20, 2025
Contributor
graydon
left a comment
graydon
left a comment
There was a problem hiding this comment.
Just the one nit same as Garand, otherwise LGTM
Previously, processEntryIfNew would log overflow errors but allow the invariant to continue, causing it to pass despite data corruption. Now passes error string by reference and returns it from the invariant, causing proper failure on overflow. Changes: - Add errorMsg parameter to processEntryIfNew - Remove CLOG_ERROR calls, use fmt::format to set errorMsg - Update scanLiveBucket and scanHotArchiveBucket to accept and pass errorMsg - Check errorMsg after scan phases and return immediately if set 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Allows early termination during shutdown, preventing unnecessary work. Changes: - Check isStopping at start of checkIfLiveEntryInArchive lambda - Check isStopping between type iterations - Return empty string (not error) when stopping 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add comprehensive bucket-level test using BucketTestApplication pattern. Tests three scenarios: 1. Invariant passes with correct bucket state 2. Invariant fails when bucket balance doesn't match totalCoins 3. Invariant handles shadowing correctly (LIVE entry shadows INIT entry) Addresses feedback from PR review to test actual bucket corruption rather than just modifying totalCoins. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
SirTyson
previously approved these changes
Dec 23, 2025
Contributor
SirTyson
left a comment
SirTyson
left a comment
There was a problem hiding this comment.
Overall LGTM, with a few small cleanups in the unit test.
SirTyson
approved these changes
Jan 2, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Description
Resolves #5028
Checklist
clang-formatv8.0.0 (viamake formator the Visual Studio extension)