Summary
The spend-first ordering handled in #1001 has a second shape it does not cover: the spend is relevant on its own, because it pays one of our addresses.
The account records it before the funding arrives, and that record stays wrong for good. Balance is unaffected.
Reproduced on fix/key-wallet-spend-seen-before-funding (03c72b8). Funding pays address A (1 000 000 sat); the spend consumes it and pays address B,
also ours — an ordinary self-send. The spend is delivered first:
|
recorded |
correct |
input_details |
0 |
1 |
net_amount |
+999 000 |
−1 000 |
direction |
Incoming |
Internal |
| balance |
999 000 |
999 000 |
The user sees an incoming 0.00999 DASH transaction where they made a self-send that cost 1 000 sat in fees. Direction and amount are both wrong, which
is worse than the #1001 case, where only the input list was missing.
Reachable for the same reason #1001 is: chain order guarantees the funding is at or below the spend's height, but delivery order does not. A rescan
queues a funding block only once a later block derives the address it pays — on one restore, 404 of 405 outputs skipped this way had the funding block
below the spend in height, i.e. delivered after it. Here the spend's block matched the filter because output B was already watched, while A was not
yet.
Why it happens
In update_utxos, the is_outpoint_spent guard sits above the observed_spent branch and continues. Because the spend was relevant on its own,
update_utxos already ran for it and put its input in spent_outpoints, so when the funding arrives the first guard fires and spent_before_funded
is never populated.
Why the obvious fix is not enough
Populating spent_before_funded in that branch too does not repair anything. record_transaction is only reached from confirm_transaction under if !self.keys.has_transaction(&txid). The spend is already recorded, so every later delivery takes the update path, which touches context and
confirmation but not input_details, net_amount or direction.
Adding a rebuild there does work — when account_match.sent > 0 and the stored record has no input_details, the record was provably built before its
parent was resolvable — but only outside a chainlock.
The blocker
confirm_transaction opens with if self.keys.transaction_is_finalized(&txid) { return None; }, above any repair. With keep-finalized-transactions
on — the configuration dash-spv-ffi builds with, so the one the app ships — a spend delivered in a chainlocked block is finalized on first sighting
and its wrong record is frozen:
has_record=true finalized=true
input_details=0 net=999000 dir=Incoming (after the funding arrives)
On a restore, essentially all of history arrives below the chainlock frontier, so this is the common case, not the edge one. Without the feature the
record is dropped at finalization instead, so there is nothing to repair.
Fixing this means allowing a finalized record to be rebuilt when it is demonstrably incomplete — a change to the "finalized is immutable" contract, on
the hot path for every already-known transaction. That is why it is not in #1001.
Second-order note for whoever picks this up
Holding the output in the is_outpoint_spent branch also fires in the normal flow: any redelivery of a funding transaction whose output has since been
spent adds an entry to spent_before_funded, removed only when the spend is reprocessed. Measured on the plain funding-then-spend order, one
redelivery of the funding takes the map from 0 to 1 entry. On a restore that redelivers 2 904 of 3 039 relevant heights this accumulates in a map that
is serialized. Any fix should scope that branch so it does not trigger on the ordinary path.
Repro
Add to key-wallet/src/tests/observed_spent_outpoints_tests.rs; fails today with input_details=0. For the chainlock variant, deliver the spend with
TransactionContext::InChainLockedBlock and run with --features keep-finalized-transactions.
#[tokio::test]
async fn spend_first_paying_us_is_repaired_once_the_funding_arrives() {
use dashcore::blockdata::script::ScriptBuf;
use dashcore::TxOut;
let mut ctx = TestWalletContext::new_random();
let change = ctx
.managed_wallet
.first_bip44_managed_account_mut()
.expect("BIP44 account")
.next_receive_address(Some(&ctx.xpub), true)
.expect("second receive address");
let funding = Transaction::dummy(&ctx.receive_address, 0..1, &[SPEND_FIRST_FUNDING_VALUE]);
let change_value = SPEND_FIRST_FUNDING_VALUE - 1_000;
let spend = Transaction {
version: 2,
lock_time: 0,
input: vec![TxIn {
previous_output: OutPoint::new(funding.txid(), 0),
script_sig: ScriptBuf::new(),
sequence: 0xffffffff,
witness: dashcore::Witness::new(),
}],
output: vec![TxOut { value: change_value, script_pubkey: change.script_pubkey() }],
special_transaction_payload: None,
};
let s_txid = spend.txid();
assert!(ctx.check_transaction(&spend, in_block(200, 2)).await.is_relevant);
assert!(ctx.check_transaction(&funding, in_block(100, 1)).await.is_relevant);
ctx.check_transaction(&spend, in_block(200, 2)).await;
let account = ctx.managed_wallet.first_bip44_managed_account().expect("BIP44 account");
let record = account.transactions().get(&s_txid).expect("the spend stays in history");
assert_eq!(record.input_details.len(), 1, "the funding must be attributed to the input");
assert_eq!(record.input_details[0].value, SPEND_FIRST_FUNDING_VALUE);
assert_eq!(record.input_details[0].address, ctx.receive_address);
assert_eq!(
record.net_amount,
change_value as i64 - SPEND_FIRST_FUNDING_VALUE as i64,
"a self-send nets the fee, not the whole change"
);
assert_eq!(record.direction, TransactionDirection::Internal);
assert_eq!(ctx.managed_wallet.balance.total(), change_value);
}
Found by CodeRabbit on #1001 (managed_core_funds_account.rs:353), verified independently.
Summary
The spend-first ordering handled in #1001 has a second shape it does not cover: the spend is relevant on its own, because it pays one of our addresses.
The account records it before the funding arrives, and that record stays wrong for good. Balance is unaffected.
Reproduced on
fix/key-wallet-spend-seen-before-funding(03c72b8). Funding pays address A (1 000 000 sat); the spend consumes it and pays address B,also ours — an ordinary self-send. The spend is delivered first:
input_detailsnet_amountdirectionIncomingInternalThe user sees an incoming 0.00999 DASH transaction where they made a self-send that cost 1 000 sat in fees. Direction and amount are both wrong, which
is worse than the #1001 case, where only the input list was missing.
Reachable for the same reason #1001 is: chain order guarantees the funding is at or below the spend's height, but delivery order does not. A rescan
queues a funding block only once a later block derives the address it pays — on one restore, 404 of 405 outputs skipped this way had the funding block
below the spend in height, i.e. delivered after it. Here the spend's block matched the filter because output B was already watched, while A was not
yet.
Why it happens
In
update_utxos, theis_outpoint_spentguard sits above theobserved_spentbranch andcontinues. Because the spend was relevant on its own,update_utxosalready ran for it and put its input inspent_outpoints, so when the funding arrives the first guard fires andspent_before_fundedis never populated.
Why the obvious fix is not enough
Populating
spent_before_fundedin that branch too does not repair anything.record_transactionis only reached fromconfirm_transactionunderif !self.keys.has_transaction(&txid). The spend is already recorded, so every later delivery takes the update path, which touches context andconfirmation but not
input_details,net_amountordirection.Adding a rebuild there does work — when
account_match.sent > 0and the stored record has noinput_details, the record was provably built before itsparent was resolvable — but only outside a chainlock.
The blocker
confirm_transactionopens withif self.keys.transaction_is_finalized(&txid) { return None; }, above any repair. Withkeep-finalized-transactionson — the configuration
dash-spv-ffibuilds with, so the one the app ships — a spend delivered in a chainlocked block is finalized on first sightingand its wrong record is frozen:
On a restore, essentially all of history arrives below the chainlock frontier, so this is the common case, not the edge one. Without the feature the
record is dropped at finalization instead, so there is nothing to repair.
Fixing this means allowing a finalized record to be rebuilt when it is demonstrably incomplete — a change to the "finalized is immutable" contract, on
the hot path for every already-known transaction. That is why it is not in #1001.
Second-order note for whoever picks this up
Holding the output in the
is_outpoint_spentbranch also fires in the normal flow: any redelivery of a funding transaction whose output has since beenspent adds an entry to
spent_before_funded, removed only when the spend is reprocessed. Measured on the plain funding-then-spend order, oneredelivery of the funding takes the map from 0 to 1 entry. On a restore that redelivers 2 904 of 3 039 relevant heights this accumulates in a map that
is serialized. Any fix should scope that branch so it does not trigger on the ordinary path.
Repro
Add to
key-wallet/src/tests/observed_spent_outpoints_tests.rs; fails today withinput_details=0. For the chainlock variant, deliver the spend withTransactionContext::InChainLockedBlockand run with--features keep-finalized-transactions.