Skip to content

Commit ecc567e

Browse files
lklimekcodexclaude
authored
feat(key-wallet): restore spent outpoints from host storage at wallet load (#1112)
* feat(key-wallet): restore externally persisted spent claims Restore spent guards without inventing transaction history or observation heights. Preserve unknown and unrelated claimants during conflict and abandonment release, and propagate legitimate releases across accounts. Co-authored-by: Codex <noreply@openai.com> * fix(key-wallet): release restored claims separately from spent marks Restored claims now guard on their own instead of being copied into `spent_outpoints`, so settling a claim can never drop a mark an account derived from its own records. - `release_spent_marks` is back to its account-local role: it runs only in an account that removed a record, on that record's inputs. The wallet-level loops that ran it on every funding account dropped the mark a ChainLocked, txid-only spend leaves behind in an account that never recorded the removed transaction, and let the coin be credited again. - `sweep_conflicts` and `abandon_transaction_with_spends` settle the claims of removed claimants wallet-wide: an outpoint the final winner spends becomes a permanent guard, one a live record still spends stays marked, any other is released. This also works when the claimant has no in-memory record. - `released_outpoints` lists an outpoint only once no funding account guards it, and never an output of a swept transaction. - Restoring an outpoint twice merges the claimants: any disagreement leaves the permanent `None` guard. - `restore_spent_outpoints` returns, and logs at warn, the restored outpoints still held as UTXOs; its docs state when to call it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): pass a restored claim to the surviving spender When a claimant is removed while another live record still spends the claimed outpoint, the claim now names that record instead of being dropped with a bare mark left behind. A mark inserted into accounts that never recorded the survivor had no owner: nothing released it once the survivor was removed too, so the coin stayed hidden until reload and was never reported as released. Removing the survivor settles the claim again, which releases the outpoint in every funding account and reports it once. The claim pass no longer writes to `spent_outpoints` at all, so that set holds only record-derived marks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): make a restored claim permanent once a final spend lands A confirmed or InstantSend-locked transaction spending a restored outpoint now turns the claim on it into the permanent `None` guard in every funding account, whoever the claimant is and whether or not the wallet holds a record of it. Before, only a swept claimant got this treatment. A claimant with no in-memory record cannot be swept, so its claim kept naming it, and a later abandon of that transaction released an outpoint the final transaction had spent. The check runs per final transaction and costs one claim lookup per input and funding account. It replaces the winner-input branch of the claim release pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): keep an outpoint guarded while another account's record spends it An account releases a spent mark from its own records alone. When a conflict sweep or an abandon removed a record there, and a live record held only by another account spent the same outpoint, the first account dropped its mark. The wallet withheld the outpoint from `released_outpoints`, but nothing guarded it in the account that owns it, so the coin was credited when its funding transaction arrived. Every funding account not guarding such an outpoint now takes a claim naming the surviving spender. The claim follows the existing rules: it is released when that spender is removed too, passes on to another live spender, and becomes permanent once a final transaction spends the outpoint. `apply_abandon` now returns the marks it released, so the abandon path can run the same check the sweep already made for reporting. Not changed: a spend recorded only in another account, with no record ever removed from the funding account, still leaves the funding account without a guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): record the spender of a claim-guarded coin as a spend A claim keeps its outpoint out of `utxos`, and the funding output was dropped altogether, so a spender delivered after the funding resolved no input: it was recorded as an incoming payment of its change, or not at all when it paid no wallet address. Keep such an output in `claim_guarded_outputs`, a third fallback for input matching next to `utxos` and `spent_before_funded`. It is never a skip condition and is not serialized: an entry is dropped when its spender is recorded, when its claim is released, and when its funding transaction is removed, so it cannot guard an outpoint past its claim and snapshots carry neither the claim nor the output. A claim restored after the account already recorded the funding takes the output from that record, so the spender is matched without the funding being delivered again. A funding record pruned to its txid holds no output to take. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): replay a spend seen in a block before its claimed funding A claim on an outpoint was checked before the observed block spends, so a funding whose spend had already been seen in a block left nothing in `spent_before_funded`. `unrecorded_spend_heights` then reported no height, the spending block was not replayed, and the spender was never recorded. Check in the order: own mark, observed spend, claim. An outpoint that is both claimed and observed spent now lands in `spent_before_funded`, as in a wallet that restored nothing. That entry is serialized and skips the outpoint on its own, which is sound here: the guard is the spend observed in a block, not the claim, so it does not depend on the claim being held. Also rewrap a comment in `record_transaction`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): never pick a removed transaction as a surviving spender A sweep drops a loser from funds accounts only, so a keys-only account keeps the record of a swept asset lock. `live_record_spenders` read that record as a live spend of the lock's other inputs: every funding account inherited a claim naming the transaction just removed, and a restored claim naming it was re-pointed to itself. The coin then stayed guarded until the host abandoned the lock, and was withheld from `released_outpoints`. Pass the removed txids to `live_record_spenders` and skip their records, on the sweep and on the abandon path, for both the inherit and the re-point step. Abandon already removes the records from keys-only accounts, so its test cases pin behaviour that held before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): pin InstantSend settlement, permanent claims and three accounts Cover three things the suite let through: - an InstantSend-locked spend makes a recordless claim permanent, as a spend in a block does; - an inherited claim does not replace a permanent one; - with three funding accounts, a coin owned by an account that records neither spend stays guarded until every spend is removed, for a restored claim and for record-derived marks, by sweep and by abandon. The first two fail when the InstantSend half of the settle condition in `sweep_conflicts` is dropped and when the account-level `inherit_spent_claims` inserts unconditionally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: bring released_outpoints, is_empty and restore docs in line with claims - `WalletEvent::TransactionsSwept::released_outpoints`, `TransactionCheckResult::released_outpoints` and the `dash-spv-ffi` swept callback state the rule of `WalletConflictSweep::released_outpoints` or point at it, and the ChainLock-pruning limitation is narrowed to a second spend recorded in the same account. - `WalletConflictSweep::is_empty` and `AbandonOutcome::is_empty` say what they cover: claims can be settled or released on an empty result. - `restore_spent_outpoints` documents how a spender of a guarded output is recorded and where that does not hold, that later claim changes stay in memory, what repeating the call with stale rows does, and that a recordless claimant is released by `abandon_transaction` only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): fold the spent-output claim entries into one `sweep_conflicts`, `abandon_transaction_with_spends` and `released_outpoints` are not part of 0.44.0, so the guard kept for a spend recorded in another account is not a fix to shipped behaviour: state it in the entry describing how both calls treat claims and drop the `Fixed` entry. Name `released_outpoints` as the sweep's field and the surfaces it reaches consumers through, say that abandoning reports none, and list the entry after the breaking ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): name the record summary type and the spend inputs Give the tuple the spender tests compare a type alias, which clippy's `type_complexity` asks for, and stop reusing one name for the inputs and the spends built from them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): cover claims for a later account and a held UTXO Two cases the per-account claim storage gets wrong, both failing at this commit: - an account added after `restore_spent_outpoints` credits a restored outpoint, because the claims were copied only into the accounts that existed at the time of the call; - a claim restored onto an outpoint the wallet already holds as a UTXO freezes that UTXO's confirmation state, so a funding transaction that later lands in a block leaves the coin in the unconfirmed bucket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): hold spent-output claims once per wallet Claims restored with `restore_spent_outpoints` were copied into every funding account. They now live in one map on `ManagedWalletInfo`, which transaction processing hands to each account next to the observed spends. `sweep_conflicts` and `abandon_transaction_with_spends` settle them in one place: a final spend makes an existing claim permanent, and removing a claimant releases its claims together with the outputs kept for them. Behaviour changes: - An account added after the restore is guarded without a second call. - Claims guard every account that holds funds, a contact's watch-only DashPay account included; before, only the wallet's own funding accounts. The same set decides what a restore reports as already held and which marks withhold an outpoint from `released_outpoints`. - A claim restored onto an outpoint the wallet holds as a UTXO no longer freezes that UTXO: its confirmation state follows its funding, and the claim guards only once the coin has left `utxos`. - A claim no longer passes to a second live transaction spending the outpoint when its claimant is removed; the outpoint is released. - An input of a removed transaction is no longer claimed on behalf of a live transaction recorded only in another account. Both cases are back to what `dev` does, tracked in #1114. - `released_outpoints` again counts a swept transaction's record left in a keys-only account as a surviving spender, as on `dev`. The public `ManagedAccountRefMut::record_transaction` and `confirm_transaction` drive one account and do not see claims. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: describe wallet-level spent-output claims `released_outpoints` lists an outpoint once nothing in the wallet guards it, rather than once no funding account does. The changelog drops the claim passing to a surviving spender and the cross-account guard, which are gone, and notes that claims cover accounts added later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): drop spent-claim tests that duplicate a neighbour Seven tests could only fail together with another one in the file. Each is removed after its assertions moved into the survivor where they were missing, and after a mutation of the code it pinned was shown to fail the survivor: - the snapshot-encoding check folds into the kept-output snapshot test - the recordless-abandon and kept-output-release tests fold into the contact-account test, which runs the same sequence - the cross-account mark test folds into its contact-account twin - "released once" becomes an exact-vector assertion in the claimant matrix, which also covers the claim filter of the sweep - the two held-UTXO tests become one - the no-claim replay case is `dev` behaviour, pinned in observed_spent_outpoints_tests The file's own helpers replace hand-rolled copies of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): stop the restore warning prescribing an order `restore_spent_outpoints` warned "restore them before their funding transactions are delivered" whenever a restored outpoint was still a UTXO, while its rustdoc allows restoring after the host has replayed its records. The message now states what happens to such an outpoint and when its claim takes effect, without naming an order. The rustdoc said those outpoints are logged; only their count is. It also loses the description of how the guarded output is kept, which `claim_guarded_outputs` documents, and states each limit once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: state the released-outpoints rule once `WalletConflictSweep::released_outpoints` keeps the full rule; the `sweep_conflicts` rustdoc and `WalletEvent::TransactionsSwept` point at it instead of restating it. The event doc goes back to `dev`'s wording and changes only what the rule changed, as do two comments in `ManagedCoreFundsAccount::record_transaction`. A released claim's outpoint need not be an input of a removed transaction, since the claimant comes from the host unchecked. The sentence saying a consumer holds every input of every transaction it deletes is therefore dropped from `WalletConflictSweep` too, as it already was from the event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): cover a fork dropping the spender of a claimed coin `truncate_above` gives back the coins a dropped transaction spent. A coin that only a spent-output claim kept out of `utxos` must not come back that way: the truncation reports nothing restored, the claim stays, and the funding delivered again is still not credited. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): import MasternodeNetInfo in the finalized-transaction tests `proreg_tx_and_owner_match` is compiled only without the `keep-finalized-transactions` feature and names `MasternodeNetInfo` without importing it, so `cargo test -p key-wallet --lib` does not compile with default features. CI builds with `--all-features` and does not reach it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): keep a claimed coin out of utxos across a fork A spender seen before its funding parks the funding output in `spent_before_funded` instead of the claim-kept outputs, so `truncate_above` dropping that spend put the coin back in `utxos` despite a permanent claim. Skip claimed outpoints when a fork restores spent coins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): keep a claimed coin aside again after a fork When a fork drops the spender of a claimed coin, rebuild its entry in `claim_guarded_outputs` from the surviving funding record, so the spender mined again on the new branch is still recorded as a spend. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9dea26e commit ecc567e

13 files changed

Lines changed: 1064 additions & 33 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
88

99
### Added
1010

11+
- Restore externally persisted spent-output claims with `ManagedWalletInfo::restore_spent_outpoints` in `key-wallet`.
12+
Claims are held once per wallet and guard every account that holds funds, including one added later.
13+
It returns the restored outpoints the wallet still holds as UTXOs, which stay credited.
1114
- Make `PublicKey::with_serialized` public
1215
- `EcdsaPublicKey`, `EcdsaSecretKey`, `EcdsaSignature`, `EcdsaRecSignature`,
1316
`EcdsaError` and the `ECDSA_*_LEN` constants in `dashcore::ecdsa`
@@ -90,6 +93,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
9093
- Relicense `dash-network`, `dash-network-seeds`, `dash-spv`, `dash-spv-ffi`,
9194
`dash-spv-bench`, `git-state` and `masternode-seeds-fetcher` to CC0-1.0
9295
- All workspace crates declare a MSRV of 1.89
96+
- `ManagedWalletInfo::sweep_conflicts` and `abandon_transaction_with_spends` in `key-wallet` release
97+
a restored spent-output claim that names a transaction they remove. `released_outpoints` of a
98+
sweep, as `WalletEvent::TransactionsSwept` and the `dash-spv-ffi` swept callback carry it, lists
99+
an outpoint only once no claim and no account's own spent mark guards it.
93100
- Human-readable serde of `PublicKey`, `ShortId` and `PlatformNodeId` also
94101
accepts raw bytes, and their deserialization errors read differently
95102
- `RootExtendedPubKey` now supports hybrid SEC1 public keys in human-readable

‎dash-spv-ffi/src/callbacks.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,8 +797,11 @@ impl FFIOutPoint {
797797
/// `txids` points to `txids_count` consecutive 32-byte txids.
798798
/// `superseded_by` is the transaction whose arrival settled the inputs.
799799
/// `released_outpoints` points to `released_outpoints_count` outpoints freed
800-
/// by the removal: inputs the removed transactions claimed to spend that no
801-
/// surviving record spends too. Mark these coins spendable again. This is
800+
/// by the removal, as `WalletEvent::TransactionsSwept::released_outpoints`
801+
/// documents: inputs of the removed transactions that no surviving record
802+
/// spends too, and outpoints whose spent-output claim named a removed
803+
/// transaction, each listed only once nothing in the wallet guards it any
804+
/// more. Mark these coins spendable again. This is
802805
/// not the same set as `txids`' inputs — a loser spending A+B against a
803806
/// winner spending only A leaves A marked and frees only B — and it cannot
804807
/// be recomputed from `txids` on the consumer side: `superseded_by` need not

‎key-wallet-manager/src/events.rs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,17 +273,21 @@ pub enum WalletEvent {
273273
/// possibly-invisible transaction also claims — so guessing either
274274
/// re-credits a coin the chain has already spent or leaves a
275275
/// genuinely free one stranded as spent forever. Wallet-scoped
276-
/// rather than attributed per removed transaction: a consumer holds
277-
/// every input of every transaction it deletes here, so it only
278-
/// needs to know which of them came free, not which removal freed
276+
/// rather than attributed per removed transaction: a consumer only
277+
/// needs to know which coins came free, not which removal freed
279278
/// which.
280279
///
280+
/// An outpoint whose spent-output claim named a removed transaction
281+
/// is released too, and either kind is listed only once nothing in
282+
/// the wallet guards it any more — see
283+
/// `WalletConflictSweep::released_outpoints` in key-wallet.
284+
///
281285
/// One pre-existing limitation, inherited from `release_spent_marks`
282286
/// rather than introduced with this field: it decides what stays
283287
/// spent from the wallet's *live* records, and under the default
284288
/// `keep-finalized-transactions = off` a chainlocked record is pruned
285-
/// to just its txid. So if this wallet ever recorded a second spend
286-
/// of a coin an already-pruned chainlocked transaction took — which
289+
/// to just its txid. So if an account ever recorded a second spend
290+
/// of a coin its own already-pruned chainlocked transaction took — which
287291
/// needs that second spend to arrive after the pruning, since
288292
/// otherwise the chainlocked arrival would have swept it — and that
289293
/// second spend is later swept on a different input, the coin is

‎key-wallet/CLAUDE.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ ManagedCoreFundsAccount (Mutable)
6464
├── balance: WalletCoreBalance
6565
├── transactions: BTreeMap<Txid, TransactionRecord>
6666
├── utxos: BTreeMap<OutPoint, Utxo>
67-
└── spent_outpoints: HashSet<OutPoint> // private, rebuilt on deserialization
67+
├── spent_outpoints: HashSet<OutPoint> // private, rebuilt on deserialization
68+
└── claim_guarded_outputs: BTreeMap<OutPoint, Utxo> // not serialized; outputs a wallet-level claim (ManagedWalletInfo::spent_claims, see restore_spent_outpoints) keeps out of utxos, for input matching
6869
```
6970

7071
### Address Pool Architecture

‎key-wallet/src/managed_account/managed_account_ref.rs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,10 @@ impl<'a> ManagedAccountRefMut<'a> {
309309
/// without wallet-level block processing. The wallet checker uses
310310
/// `Self::record_transaction_with_observed_spends` instead, which
311311
/// reconciles the record against the wallet-level observed-spent set.
312+
///
313+
/// Spent-output claims are wallet-level state too, so this entry point
314+
/// does not see them: an output restored as spent with
315+
/// `ManagedWalletInfo::restore_spent_outpoints` is credited here.
312316
pub fn record_transaction(
313317
&mut self,
314318
tx: &Transaction,
@@ -322,6 +326,7 @@ impl<'a> ManagedAccountRefMut<'a> {
322326
context,
323327
transaction_type,
324328
&BTreeMap::new(),
329+
&BTreeMap::new(),
325330
&BTreeSet::new(),
326331
)
327332
}
@@ -331,15 +336,20 @@ impl<'a> ManagedAccountRefMut<'a> {
331336
/// (dashpay/rust-dashcore#649); only the funds variant consults it (keys
332337
/// accounts track no UTXOs/output details).
333338
///
339+
/// `spent_claims` is the wallet-level `ManagedWalletInfo::spent_claims`
340+
/// view; an output it names is not credited.
341+
///
334342
/// `external_final_parents` is the wallet-level view of input parents held
335343
/// by sibling accounts, used for the trusted-self-send determination.
344+
#[expect(clippy::too_many_arguments, reason = "one argument per wallet-level view")]
336345
pub(crate) fn record_transaction_with_observed_spends(
337346
&mut self,
338347
tx: &Transaction,
339348
account_match: &AccountMatch,
340349
context: TransactionContext,
341350
transaction_type: TransactionType,
342351
observed_spent: &BTreeMap<OutPoint, CoreBlockHeight>,
352+
spent_claims: &BTreeMap<OutPoint, Option<Txid>>,
343353
external_final_parents: &BTreeSet<OutPoint>,
344354
) -> TransactionRecord {
345355
match self {
@@ -349,6 +359,7 @@ impl<'a> ManagedAccountRefMut<'a> {
349359
context,
350360
transaction_type,
351361
observed_spent,
362+
spent_claims,
352363
external_final_parents,
353364
),
354365
ManagedAccountRefMut::Keys(a) => {
@@ -366,6 +377,10 @@ impl<'a> ManagedAccountRefMut<'a> {
366377
/// pre-#649 behavior, appropriate for callers driving an account directly
367378
/// without wallet-level block processing. The wallet checker uses
368379
/// `Self::confirm_transaction_with_observed_spends` instead.
380+
///
381+
/// Spent-output claims are wallet-level state too, so this entry point
382+
/// does not see them: an output restored as spent with
383+
/// `ManagedWalletInfo::restore_spent_outpoints` is credited here.
369384
pub fn confirm_transaction(
370385
&mut self,
371386
tx: &Transaction,
@@ -379,6 +394,7 @@ impl<'a> ManagedAccountRefMut<'a> {
379394
context,
380395
transaction_type,
381396
&BTreeMap::new(),
397+
&BTreeMap::new(),
382398
&BTreeSet::new(),
383399
)
384400
}
@@ -387,15 +403,20 @@ impl<'a> ManagedAccountRefMut<'a> {
387403
/// against `observed_spent` — the wallet-level `observed_spent_outpoints`
388404
/// view (dashpay/rust-dashcore#649); only the funds variant consults it.
389405
///
406+
/// `spent_claims` is the wallet-level `ManagedWalletInfo::spent_claims`
407+
/// view; an output it names is not credited.
408+
///
390409
/// `external_final_parents` is the wallet-level view of input parents held
391410
/// by sibling accounts, used for the trusted-self-send determination.
411+
#[expect(clippy::too_many_arguments, reason = "one argument per wallet-level view")]
392412
pub(crate) fn confirm_transaction_with_observed_spends(
393413
&mut self,
394414
tx: &Transaction,
395415
account_match: &AccountMatch,
396416
context: TransactionContext,
397417
transaction_type: TransactionType,
398418
observed_spent: &BTreeMap<OutPoint, CoreBlockHeight>,
419+
spent_claims: &BTreeMap<OutPoint, Option<Txid>>,
399420
external_final_parents: &BTreeSet<OutPoint>,
400421
) -> Option<TransactionRecord> {
401422
match self {
@@ -405,6 +426,7 @@ impl<'a> ManagedAccountRefMut<'a> {
405426
context,
406427
transaction_type,
407428
observed_spent,
429+
spent_claims,
408430
external_final_parents,
409431
),
410432
ManagedAccountRefMut::Keys(a) => {

0 commit comments

Comments
 (0)