Skip to content

fix(platform-wallet): lowering synced_height does not invalidate in-flight SPV filter scans #4955

Description

@lklimek

Problem

Lowering a wallet's filter-scan checkpoint (synced_height) with update_synced_height does not invalidate a scan batch that is already in flight. The batch can then commit and raise the checkpoint past a range that was never checked for this wallet, so payments in that range are missed.

Found by the automated review on #4740. It was verified against the pinned rust-dashcore e4208c9, and the problem already exists on v4.3-dev.

Mechanism (dash-spv @ e4208c9)

  • scan_batch (manager.rs ~1100-1135) snapshots each wallet's synced checkpoint and account_generation under one read lock. Filter attribution then skips heights <= wallet_synced (~1195).
  • try_commit_batches (~724-760) checks only that the generation is equal and that effective_synced + 1 >= batch_start, using the current checkpoint rather than the one captured at scan time.
  • Example: a batch covering 5000-9999 scans this wallet at checkpoint 9000. Before the batch commits, the wallet lowers its checkpoint to 7499. The generation is unchanged and 7500 >= 5000, so the commit passes and sets the checkpoint to 9999. The range 7500..9000 is never matched for this wallet.

account_generation is pub(crate) in key-wallet. It is bumped only by the private rewind_sync_checkpoint_for_new_account, which only the ManagedAccountOperations::add_managed_* functions call. No public API exists to lower the checkpoint and invalidate in-flight scans at the same time.

Affected callers in rs-platform-wallet

  • DashPayView::reconcile_dashpay_rescan (wallet/identity/network/payments.rs) lowers the checkpoint for existing DashPay receiving accounts. After this lowering the contact is marked in rescan_triggered, so nothing repairs the missed range in that process.
  • PlatformWalletManager::spv_rescan_filters_blocking (manager/accessors.rs) has the same race.

Proposed fix

  1. Add a public key-wallet API in rust-dashcore, e.g. ManagedWalletInfo::rewind_synced_height(height), that lowers the checkpoint and bumps account_generation atomically.
  2. Bump the dependency and route both callers above through it.

A workaround that uses only the existing public API was considered and rejected: remove the account, then add_managed_account (which bumps the generation), then re-insert the old account, then lower the checkpoint. It depends on upstream internals.

Related limitation (same area)

sent_sweep_completed in DashPayState is set after a budget-limited sent fetch. fetch_contact_requests_paginated returns Ok after 50×100 documents even when more remain. For identities with more than 5000 sent contact requests, some contacts may be reconciled before their earliest request height is known. A fix needs the rs-sdk helper to report exhaustion or continuation status.

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