key-wallet: a coin spent by a transaction recorded only in another account is credited when its funding arrives late

[lklimek]

TL;DR: A wallet with more than one account can show a coin as available although one of its own pending transactions already spent it, when the wallet learns about the spend before it learns about the coin.

User story

As a wallet user with several accounts, I want my balance to count each coin once, so I am not shown money I have already sent and cannot spend.

Scenario

Base flow

A coin belongs to one account. A transaction spends it and pays an address in a second account of the same wallet. The wallet sees that transaction first, while it is still pending or only InstantSend-locked. The transaction that created the coin is delivered afterwards, as happens during a restore or rescan.

Actual behavior

The coin is added to the first account as available, while the second account already holds what the coin paid for. The balance counts the same money twice, and the coin can be picked for a new payment that the network will reject. The error goes away once the spending transaction is mined.

Expected behavior

The coin is not shown as available, because the wallet already knows a transaction that spends it.

Detailed discussion

Reproduced on dev at 314f106 with a two-account BIP44 wallet (key-wallet unit test, no network). funding pays account 0 150 000 duffs; spend consumes that output and pays account 1 140 000 duffs. spend is delivered first, then funding.

spend context	funding context	coin credited in account 0	wallet total
Mempool	Mempool	yes	290 000
Mempool	InBlock	yes	290 000
InstantSend	InBlock	yes	290 000
InBlock	InBlock	no	140 000

In every row spend is recorded in account 1 only. In the three failing rows balance.spendable() is also 290 000. Delivering spend again in a block removes the coin and brings the total to 140 000, and the spend is then recorded in account 0 too.

Why: the guard that stops a spent outpoint from being credited is per account. update_utxos consults is_outpoint_spent of the account being credited, and spent_outpoints holds only the inputs of that account's own records. spend is relevant to account 1 through its output, and not to account 0, whose coin is not in utxos yet, so the mark lands in account 1 and account 0 never learns of it. The wallet-level observed_spent_outpoints set closes this for block spends, and by design never holds mempool spends, which is why only the last row is correct. An InstantSend-locked spend is final against a double spend and is not covered either.

Reach: the spend has to be seen before the funding. Chain order rules that out, delivery order does not: a rescan fetches a funding block only once the address it pays is derived, as #1003 describes. Live operation is not affected, since the wallet holds the coin before it can spend it.

Possible directions, not evaluated:

• Check every funding account's spent marks when crediting an output, not only the credited account's.
• Record a spend's inputs as marks in every funding account, the way restore_spent_outpoints in feat(key-wallet): restore externally persisted spent claims #1112 copies restored claims.

Either needs care with release: sweep_conflicts and abandon_transaction release a mark from one account's records alone.

Reproduction test, to drop into key-wallet/src/tests/:

cross_account_issue_repro.rs

use dashcore::blockdata::transaction::{OutPoint, Transaction};
use dashcore::ephemerealdata::instant_lock::InstantLock;
use dashcore::{BlockHash, TxIn};

use crate::managed_account::managed_account_trait::ManagedAccountTrait;
use crate::test_utils::TestWalletContext;
use crate::transaction_checking::{BlockInfo, TransactionContext};
use crate::wallet::initialization::WalletAccountCreationOptions;
use crate::wallet::{ManagedWalletInfo, Wallet};
use crate::Network;

fn two_account_context() -> (TestWalletContext, dashcore::Address) {
    let wallet = Wallet::from_seed_bytes(
        [42; 64],
        Network::Testnet,
        WalletAccountCreationOptions::BIP44AccountsOnly([0, 1].into_iter().collect()),
    )
    .unwrap();
    let mut managed_wallet = ManagedWalletInfo::from_wallet(&wallet, 0);
    let mut address = |index| {
        let xpub = wallet.accounts.standard_bip44_accounts[&index].account_xpub;
        managed_wallet
            .accounts
            .standard_bip44_accounts
            .get_mut(&index)
            .unwrap()
            .next_receive_address(Some(&xpub), true)
            .unwrap()
    };
    let receive_address = address(0);
    let second_address = address(1);
    let xpub = wallet.accounts.standard_bip44_accounts[&0].account_xpub;
    (
        TestWalletContext {
            wallet,
            managed_wallet,
            receive_address,
            xpub,
        },
        second_address,
    )
}

fn block(height: u32) -> TransactionContext {
    TransactionContext::InBlock(BlockInfo::new(
        height,
        BlockHash::from([height as u8; 32]),
        1_700_000_000,
    ))
}

async fn coin_is_credited(spend_context: TransactionContext, funding_context: TransactionContext) -> bool {
    let (mut ctx, second_address) = two_account_context();
    let funding = Transaction::dummy(&ctx.receive_address, 20..21, &[150_000]);
    let coin = OutPoint::new(funding.txid(), 0);
    let spend = Transaction {
        version: 1,
        lock_time: 0,
        input: vec![TxIn {
            previous_output: coin,
            ..Default::default()
        }],
        output: Transaction::dummy(&second_address, 30..31, &[140_000]).output,
        special_transaction_payload: None,
    };
    ctx.check_transaction(&spend, spend_context).await;
    let accounts = &ctx.managed_wallet.accounts.standard_bip44_accounts;
    assert!(accounts[&1].has_transaction(&spend.txid()));
    assert!(!accounts[&0].has_transaction(&spend.txid()));
    ctx.check_transaction(&funding, funding_context).await;
    ctx.managed_wallet.accounts.standard_bip44_accounts[&0].utxos.contains_key(&coin)
}

#[tokio::test]
async fn coin_spent_by_another_accounts_record_is_not_credited() {
    assert!(!coin_is_credited(block(110), block(100)).await);
    assert!(!coin_is_credited(TransactionContext::Mempool, TransactionContext::Mempool).await);
    assert!(!coin_is_credited(TransactionContext::Mempool, block(100)).await);
    assert!(
        !coin_is_credited(TransactionContext::InstantSend(InstantLock::default()), block(100)).await
    );
}

Not tested: dashd integration, and whether a live dash-spv restore actually delivers the two transactions in this order.

Prior work

• key-wallet: a spend-first coin whose spend pays us keeps a wrong record, and the repair is blocked by finalization #1003 — same spend-before-funding ordering within one account: the guard fires there and the balance is right, but the record is wrong.
• fix(key-wallet): keep a spend-first coin recognisable so its spend stays in history #1001 — made a spend-first coin recognisable (spent_before_funded) for block spends.
• dash-spv: a spend with no wallet-owned output is never recorded — coin stays unspent, balance overstated #992 — a spend with no wallet-owned output is never recorded, so the coin stays unspent; here the spend is recorded, in another account.
• fix(wallet): reconcile late inputs and publish accounting corrections #1082 — reconciles late inputs and added the wallet-wide conflict sweep.
• feat(key-wallet): restore externally persisted spent claims #1112 — found while verifying a review thread there; its 7f27562 covers only the case where a record of the coin's spend is removed from the owning account while another account's record still spends it.
• feat(key-wallet)!: report and restore spent-outpoint claims #1113 — single-map rewrite of the spent set; this case reproduces at 6d5a7ba as well.

_{🤖 Co-authored by Claudius the Magnificent AI Agent}