feat(dash-spv): masternode diff storage - #993
Draft
ZocoLini wants to merge 7 commits into
Draft
Conversation
`test_masternode_list_sync_with_restart` compared masternode sync progress either side of a restart. A from-scratch network re-sync produces the same progress as a restored one, so the test passed while the list was being rebuilt from nothing every time (#988). It now looks at the disk. After the first session's clean shutdown every directory that session earned must hold a file, and across the restart no directory may disappear or lose files. Fails as written: the first session builds four masternodes and writes no `masternodestate/`, while `block_headers/`, `filter_headers/`, `metadata/` and `peers/` all persist through the same shutdown to the same directory — so the storage layer and the shutdown are ruled out as causes. `filters/` and `blocks/` are left out of the must-hold set on purpose: the client stops as soon as the masternode phase reports `Synced`, which is before the filter phase leaves `WaitForEvents`, so they are legitimately empty here. The no-shrink check still covers them. The engine is read before the shutdown and the count carried into the failure message, so the assertion cannot be satisfied by a session that synced nothing — which is the shape #954 produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NhHBDGiKfiGpy7FwooyfS
`storage/masternode.rs` has had no callers outside `storage/` since the legacy sync engine was deleted, and `DashSpvClient::new` always built a fresh `MasternodeListEngine`. Every start therefore rebuilt the whole list from the network — a full QRInfo plus every MnListDiff — while headers, filters and ChainLocks resumed from disk. On mobile, where the host app restarts the client every minute or two, the rebuild rarely finishes, so a client can run with no masternode list at all despite having synced one in a previous session (#988). Both halves are wired here. `MasternodesManager` takes the state store and writes the engine wherever it reports `MasternodeStateUpdated` — the same condition that makes the new state worth keeping. `DashSpvClient::new` loads the state and seeds the engine, before the managers are built: `MasternodesManager::new` already recovers its resume point from the engine's stored lists, so a restore landing after it would be ignored. Both directions fail soft. An unwritten list costs a rebuild next start; a failed sync costs the list now. Likewise state that cannot be read is logged and rebuilt, which is exactly the old behaviour. `test_masternode_list_sync_with_restart` now passes, and the log shows why: `0 base hash(es)` on the first sync, `Restored masternode state from height 406`, then `1 base hash(es)` on the second — the delta, not a rebuild. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NhHBDGiKfiGpy7FwooyfS
`MasternodeStateStorage` took and returned `MasternodeState`, the on-disk shape, so both callers had to build it: the manager serialized the engine, stamped a timestamp and assembled the struct, and the client took it apart again. Two places knew the encoding, and neither was the one that owns it. The trait now takes and returns the engine. `MasternodeState` stays as the file format and is built and read inside `masternode.rs` alone — it is no longer named outside `storage/`. Changing how the engine is encoded, which the current JSON-array-of-bytes shape will want, is now an edit to one file rather than three. `load_engine` also absorbs the case that is not an error: nothing persisted yet yields the network's default, which is where a first run starts anyway, so the caller loses an `Option` it only ever mapped one way. A file that exists and cannot be read stays an `Err`, because that one is worth seeing — the client logs it and rebuilds from the network. The masternode manager's persistence path goes from 24 lines to 5, the client's restore from 22 to 8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015NhHBDGiKfiGpy7FwooyfS
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #993 +/- ##
========================================
Coverage 77.14% 77.14%
========================================
Files 329 329
Lines 82998 83689 +691
========================================
+ Hits 64026 64561 +535
- Misses 18972 19128 +156
|
…ed copy The masternode storage persisted the engine's block container alongside the quorum statuses, so every hash/height pair the replay needed was written a second time next to the header storage that already holds it. Two copies of the same mapping can only diverge: a header reorg rewrites one of them. The replay now works exactly like the live sync path, only reading its messages from disk instead of waiting for peers. The header storage is injected at construction, `PersistentMasternodeStorage<H: BlockHeaderStorage>` keeps the shared handle, and heights are resolved against it: QRInfo through the same `feed_qrinfo_heights_to_engine` the sync manager uses (moved to the storage module, where both callers reach it), MnListDiff through its file name plus a lookup of the base hash it extends. What is left of the old context file is the quorum statuses, so it is named for them: `quorum_statuses.dat`, written by `store_quorum_statuses`. One that fails to decode is now ignored with a warning instead of failing the whole load, since the statuses are a verification cache the replay re-derives. Tying the storage to `H` also ties it to the same header storage its manager already carries, instead of hardcoding the concrete type. `PersistentStorage` is gone from this storage: its `persist` was a no-op, the messages are written as they arrive, so the background worker no longer wakes it up. Verified against dashd regtest: the whole `dash-spv` suite passes (569 unit, 10 dashd_masternode, 30 dashd_sync). The restart test's log shows the replay rebuilding the lists purely from headers plus stored messages: "Replayed 1/1 QRInfo and 1/1 MnListDiff messages into 7 masternode lists". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uep5dcAKWZ3GA2Rm7GqUbr
ZocoLini
force-pushed
the
feat/masternode-diff-storage
branch
from
August 31, 2026 10:11
15ef160 to
ae334bc
Compare
ZocoLini
changed the title
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
1 participant
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.
proof of concept to improve the current masternode storage
Why??
The current implementation takes 1.3GB of disk and hundreds of MB in memory, with this refactor I am aiming for something more align with the network msg and, at the same time, reduce the amount of disk and memory the storage takes to feed the dash-spv client
This is still a prototype that I am validating and currently writing