Skip to content

Found-021: TransactionRecord::update_context silently drops InstantLock on InBlock/InChainLockedBlock promotion #763

Open

Description

@lklimek

User Story

As a Dash Platform application developer using rs-platform-wallet (which depends on key-wallet) to fund identity operations from a Core wallet, I need the wallet's TransactionRecord to preserve the InstantLock proof material when the underlying transaction transitions from InstantSend to InBlock (and onward to InChainLockedBlock), so that:

1. InstantAssetLockProof construction works at any time. Today, AssetLockManager::wait_for_proof only finds the IS-lock if it reads the record before block confirmation lands. Once update_context(InBlock(info)) runs, the lock is gone and the manager has to wait for a chainlock before falling back to ChainAssetLockProof — that adds minutes of latency, bounded by the 5-minute wait_for_proof timeout hard-coded at dashpay/platform packages/rs-platform-wallet/src/wallet/asset_lock/build.rs:334. Identity registration and top-up flows are on this critical path; any production caller whose transaction happens to be block-confirmed before they read the record silently drops onto the slow proof path.

Timeline of a wallet that misses the IS-lock and falls onto the chainlock branch (illustrative; concrete numbers depend on block-time and chainlock-quorum scheduling and are not normatively specified):

Time Event TransactionContext
T+0 tx broadcast Mempool
T+~5 s IS-lock arrives at SPV (if it does) InstantSend(lock) — caller can return proof immediately. If wait_for_proof reads here, returns InstantAssetLockProof.
T+~block-time next block mined containing tx InBlock(info) — InstantLock dropped by this bug if the wallet had reached the InstantSend state; loop continues waiting because Platform consensus does not accept a bare InBlock as proof.
T+~block-time + chainlock-quorum latency ChainLock for that block arrives InChainLockedBlock(info) — caller returns ChainAssetLockProof { core_chain_locked_height, out_point }.
T+300 s (worst case) wait_for_proof timeout fires before chainlock —

The chainlock-fallback path is legitimate: Dash Platform consensus accepts ChainAssetLockProof as valid finality. The cost of this bug is therefore latency on the proof, not correctness — but on identity-registration and top-up flows that minute-scale wait is a measurable user-experience regression vs the IS-lock path.

2. Wallet recovery / reload preserves proof material. If a wallet is taken offline (process restart, disk-state reload, SPV catch-up after disconnect) and the block confirmation arrives during the offline window, the IS-lock is permanently unrecoverable from local state once the wallet re-syncs — the SPV layer does not redeliver IS-locks for already-confirmed transactions. The wallet's only chance to retain the lock is to merge it into the persistent TransactionRecord before the block-arrival overwrite. Today it doesn't.

3. Audit trails remain accurate. Operators inspecting tracked asset-locks should be able to answer "was this transaction instant-locked?" by reading TransactionRecord directly. With the bug, the answer flips from "yes" to "no" silently the moment a block lands. Forensic, debugging, and monitoring queries get inconsistent results depending on when they run.

4. Symmetric ordering is safe. If a block confirmation arrives before the IS-lock notification (less common but possible under network partitions or SPV catch-up), the next update_context(InstantSend(lock)) would strip the InBlock(info) symmetrically. Today both directions are lossy.

Current Behavior

On v0.42-dev HEAD 5297d61a, TransactionRecord::update_context at key-wallet/src/managed_account/transaction_record.rs:182-184 is a naive replace:

/// Update the transaction context
pub fn update_context(&mut self, context: TransactionContext) {
    self.context = context;
}

TransactionContext (at key-wallet/src/transaction_checking/transaction_context.rs:41) has four variants:

pub enum TransactionContext {
    Mempool,
    InstantSend(InstantLock),
    InBlock(BlockInfo),
    InChainLockedBlock(BlockInfo),
}

There is no merged variant carrying both InstantLock and BlockInfo. When a tracked transaction transitions InstantSend(lock) → InBlock(info), the InstantLock is silently overwritten and unrecoverable from the record.

Related amplifier

The production transition InBlock(info) → InChainLockedBlock(info) at key-wallet/src/managed_account/managed_core_keys_account.rs:129-139 calls record.update_context(TransactionContext::InChainLockedBlock(info)) for already-InBlock records. By the time a record reaches that path, the IS-lock is already gone (lost at the prior IS→InBlock hop). Two compounding lossy hops to chain-lock proof, both caused by the same naive replace.

Reproduction

Pure unit test (no harness, no network, no async) at dashpay/platform packages/rs-platform-wallet/tests/e2e/cases/found_021_instant_lock_dropped_on_context_promotion.rs (on branch feat/rs-platform-wallet-e2e, PR #3549).

The test executes:

let lock = InstantLock::default();
let mut record = TransactionRecord::new(
    dummy_tx(),
    AccountType::Standard { index: 0, standard_account_type: StandardAccountType::BIP44Account },
    TransactionContext::InstantSend(lock),
    TransactionType::AssetLock,
    TransactionDirection::Outgoing,
    vec![], vec![],
    -100_000,
);
// Pre-condition: record.context is InstantSend(lock) ✓
record.update_context(TransactionContext::InBlock(block_info));
// Assertion: record.context still carries the InstantLock — FAILS today

Run with:

cargo test -p platform-wallet --test e2e -- --ignored found_021_instant_lock_dropped_on_context_promotion

Expected (after fix): record.context after update_context(InBlock(..)) either (a) carries the original InstantLock via a merged variant (e.g. InBlockWithInstantLock { info, lock }) or (b) the record exposes a dedicated instant_lock field that retains the lock independently of context.

Actual (today): record.context == InBlock(info) with no lock accessible. The IS-lock is gone.

Deterministic across runs — pure unit test, 0.00s execution time.

Root Cause

  1. Primary defect: TransactionRecord::update_context at transaction_record.rs:182-184 performs an unconditional field replacement. There is no inspection of the prior context to determine whether information is being lost.
  2. Secondary cause — type system: TransactionContext has no variant that carries both InstantLock and BlockInfo simultaneously. Even if update_context wanted to merge, there is no target type to merge into. The fix shape requires either a new variant or a new dedicated field on TransactionRecord.
  3. Amplifier: managed_core_keys_account.rs:129-139 performs a similar replace for InBlock → InChainLockedBlock. The chain is two-step lossy: any record that has accumulated an InstantLock loses it at either hop.

Affected Versions

  • v0.42-dev HEAD 5297d61a (today) — confirmed via direct unit test
  • All prior key-wallet versions since TransactionRecord::update_context was introduced (search returned 0 issues / 0 PRs touching the function — this surface has never been addressed)

Cross-Repository Impact

In dashpay/platform (rs-platform-wallet):

Surface File / line Exposure
AssetLockManager::wait_for_proof packages/rs-platform-wallet/src/wallet/asset_lock/sync/proof.rs:341-419 Slow-waiter paths drop onto chainlock fallback (minutes of added latency, bounded by the 5-minute proof timeout at build.rs:334) instead of returning InstantAssetLockProof
AssetLockManager::recover_asset_lock_blocking packages/rs-platform-wallet/src/wallet/asset_lock/sync/recovery.rs:36-280 Wallet-recovery path is maximally exposed — re-read happens AFTER any persistence-bracketed block confirmation. The IS-lock is unrecoverable once the wallet re-syncs.

e2e tests that surface the bug today or will after dependencies clear:

Severity

HIGH — silent data loss on the asset-lock proof critical path. The flow degrades in failure rather than failing loudly (minutes of added latency per affected transaction in the wait_for_proof slow-waiter case; recovery scenarios outright lose proof material). Production callers driving identity registration / top-up flows hit this whenever block confirmation outruns the proof read.

Notes

Per repo conventions for issue filing, no fix proposal in this body. Two options are referenced in dashpay/platform packages/rs-platform-wallet/tests/e2e/TEST_SPEC.md Found-021 entry (Option A: extend TransactionContext with InBlockWithInstantLock { info, lock }; Option B: store the most recent InstantLock on TransactionRecord independently of context) as implementer notes — the choice is left to the implementer.

The fix is a precondition for:

  • Implementing dashpay/platform packages/rs-platform-wallet/tests/e2e/cases/found_013_recover_asset_lock_silent_failure.rs end-to-end (currently blocked-scaffold)
  • Removing the chainlock-fallback latency hit in concurrent / slow-waiter paths through AssetLockManager::wait_for_proof

Activity

  1. Claudius-Maginificent commented on Jul 6, 2026

    @Claudius-Maginificent

    Confirmed still live on dev (647fa98), and there is now a
    crate-level repro inside key-wallet's own test suite (previously the only
    runnable pin lived in the dashpay/platform e2e suite,
    found_021_instant_lock_dropped_on_context_promotion.rs from platform#3549).

    Crate-level repro: key-wallet/tests/instant_lock_context_promotion.rs, test
    found_021_instant_lock_dropped_on_context_promotion

    (branch repro/pr3549-rdc). Pure unit test, no SPV harness or network — it drives
    the exact production call sequence through the public TransactionRecord
    API: construct a record in TransactionContext::InstantSend(lock), then
    update_context(TransactionContext::InBlock(info)), i.e. the two hops the
    wallet sync path performs when a block confirmation arrives for an
    already-IS-locked transaction: wallet_checker.rs's block-confirmation branch
    (wallet_checker.rs:166)
    calls into ManagedCoreFundsAccount::confirm_transaction,
    which promotes the existing record via tx_record.update_context(context.clone())
    at managed_core_funds_account.rs:325.

    Current output (RED as expected):

    test found_021_instant_lock_dropped_on_context_promotion ... FAILED
    InstantLock was silently dropped on InBlock promotion: record.context after
    update_context(InBlock(..)) is InBlock(BlockInfo { height: 100000, ... }).
    

    Defect site on dev (647fa98): key-wallet/src/managed_account/transaction_record.rs:182-184

    pub fn update_context(&mut self, context: TransactionContext) {
        self.context = context;
    }

    The naive replacement drops the IS-lock on IS → InBlock promotion, so any
    consumer reading the record after block confirmation for proof material
    (e.g. building an InstantAssetLockProof) finds no lock. The follow-on
    InBlock → InChainLockedBlock hop compounds it — by then the lock is
    already gone.

    The test asserts the merging invariant (lock still recoverable after
    promotion) and is written to flip GREEN once update_context preserves the
    lock, whether via a dedicated field or a merged variant such as
    InBlockWithInstantLock { info, lock }; its context_has_instant_lock
    helper has a marked extension point for the new representation.

    Happy to push the branch / open a PR with the test if useful.

    🤖 Co-authored by Claudius the Magnificent AI Agent

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