Skip to content

dash-spv: masternode list is never persisted — every client start rebuilds it from the network (persistence orphaned since #414) #988

Description

@PastaPastaPasta

Summary

The masternode list is never persisted to disk. The persistence code exists (storage/masternode.rs) but has had zero call sites since the legacy sync engine was removed, and the client never attempts a restore at startup. As a result, every DashSpvClient start rebuilds the entire masternode list from the network — a full QRInfo plus every MnListDiff from scratch — while headers, filters, and ChainLocks all resume instantly from storage.

On desktop this is an invisible slow start. On mobile it makes the masternode list effectively unsyncable: the host app stops/restarts the SPV client on lifecycle events (backgrounding, wallet switches) every minute or two, so the multi-minute rebuild keeps being killed before it completes. Combined with #954 (QRInfo retry exhaustion silently reports the phase complete), the client can run indefinitely with no masternode list at all despite having fully synced one in a previous session.

The bug in one log excerpt

A fully-synced client is cleanly shut down, then a new client is constructed on the same storage directory, before any network traffic:

lifecycle: Headers already exist in storage, skipping genesis initialization
block_headers::manager: BlockHeadersManager initialized at height 406
  Headers:        WaitingForConnections 406/406 (100.0%)
  Masternodes:    WaitingForConnections 0/406 | diffs_processed: 0, qr_infos_requested: 0

Headers restore to the tip; masternodes restore to nothing. The subsequent sync then logs Requesting QRInfo for tip at height 406 with 0 base hash(es) — no base, full rebuild.

Root cause

Two independent halves, both missing:

  1. Nothing ever saves. dash-spv/src/storage/masternode.rs defines MasternodeStateStorage::{store_masternode_state, load_masternode_state} (writes masternodestate/masternodestate.json), and DiskStorageManager implements it — but grep -rn "store_masternode_state\|load_masternode_state" dash-spv/src | grep -v storage/ returns nothing. The trait is dead code. The current MasternodesManager (sync/masternodes/manager.rs) holds only header_storage for height lookups.
  2. Nothing could restore even if something saved. DashSpvClient::new (client/lifecycle.rs:68-76) unconditionally constructs MasternodeListEngine::default_for_network(config.network). There is no path that loads persisted state and feeds it back into the engine.

When it was introduced

  • The legacy sync engine persisted and resumed masternode state (sync/legacy/masternodes/manager.rs called store_masternode_state at three sites; sync/legacy/post_sync.rs / transitions.rs / message_handlers.rs called load_masternode_state to resume from a base height).
  • feat: rewrite, fix and improve the sync architecture #411 (12abdc2f, 2026-02-04) rewrote the sync architecture without wiring persistence into the new masternode manager.
  • chore: remove legacy sync code #414 (245a330d, 2026-02-04) deleted the legacy sync module — removing the last remaining call sites and orphaning the storage trait.
  • First shipped in v0.42.0; present in 0.45.0 and current dev.

Reproduction (dashd regtest integration test)

Branch PastaPastaPasta/rust-dashcore@test/mnlist-persistence-repro (commit c1afdb89) adds two tests to dash-spv/tests/dashd_masternode/tests_sync.rs, written to assert the correct behavior — both fail on current dev:

  • test_masternode_restart_loses_state: sync to completion against regtest dashd (4 masternodes, 2 rotated QRInfo cycles), clean shutdown, then assert persistence. Failure output:

    After a clean shutdown of a fully-synced client (4 masternodes, 2 rotated cycles):
    - masternodestate/masternodestate.json exists: false (expected true)
    - storage dir contains: ["block_headers/", "blocks/", "filter_headers/", "filters/", "logs/", "metadata/", "peers/"]
    - a fresh client on the same dir restores 0 masternodes (expected 4)
    - headers, by contrast, restored to height 406 from that same dir
    

    The header restore is the control: same directory, shutdown demonstrably flushed, only the masternode state is missing.

  • test_masternode_state_unrecoverable_offline_after_restart: same first sync, then restart with the peer's p2p disabled → Offline restart: 0 masternodes, header height 406. A client that had a complete, verified list ends up with none because the network isn't there to rebuild it.

eval $(python3 contrib/setup-dashd.py)
DASHD_TEST_RETAIN_DIR=/tmp/dashd-test-logs DASHD_TEST_RETAIN_ALWAYS=1 \
  cargo test -p dash-spv --test dashd_masternode test_masternode_ -- --test-threads=1 --nocapture

Real-world impact (how this was found)

DashWallet iOS (Swift SDK, dash-spv 0.45.0, testnet): headers/filters/blocks report synced within a second of every launch, while the masternode phase restarts at 0/1542249 with stored_cycle_height=None. The rebuild there costs a full QRInfo (first response ~27s against the 10s timeout, causing duplicate requests) plus ~44 MnListDiff verifications at ~1.5s each — minutes per launch. The app restarts the SPV client on wallet lifecycle events every ~1–2 minutes, so the rebuild rarely completes; one attempt exhausted the QRInfo retry budget and hit the #954 path (skipping masternode sync → phase reports complete, No masternode lists available). Sixteen logged sessions over five days show Masternode sync complete reached in most runs and thrown away every time.

Why the test suite never caught it

test_masternode_list_sync_with_restart passes on the buggy code: it compares post-restart sync progress, and a from-scratch network re-sync produces identical progress to a restored one. Only a disk-level assertion (state file exists / engine non-empty before network traffic) distinguishes them.

Suggested fix shape

  • Save: persist engine state (masternode lists, quorum cycles, last synced height/hash) from the masternode sync path — at minimum on sync completion, ideally incrementally after each applied diff/cycle.
  • Restore: load at client startup and seed MasternodeListEngine before the sync phase runs, so QRInfo/MnListDiff requests use the persisted tip as base (with N base hash(es) instead of 0) and only fetch the delta.
  • Decide staleness policy (how old a persisted list may be before a full rebuild is forced), and how a restored list interacts with the dash-spv: exhausted QRInfo retry budget silently disables masternode features #954 give-up path — a stale-but-present list is strictly better than none for ChainLock/InstantSend verification.
  • Invert the two repro tests into regression guards once wired up.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions