Repository navigation
fix(wallet-storage)!: reconcile historical AssetUnlock identities - #5783
Draft
Claudius-Maginificent wants to merge 4 commits into
Draft
Claudius-Maginificent wants to merge 4 commits into
Claudius-Maginificent wants to merge 4 commits into
Conversation
Adopt canonical history records while retaining validated legacy identity evidence in the immutable archive and preserving physical metadata anchors. Resolve historical prevouts for accounting without rewriting signed inputs, coins, claims or proof keys. Keep physical backend reads separate from the public alias projection and roll back incompatible confirmed contexts. Cover migration and redelivery with old-writer fixtures, including a real SIGHASH_ALL consumer, plus alias families, rollback and Recovery policy. <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Merge the published #5781 feature parent beneath the historical identity alias follow-up. Keep the physical bulk decoder's identity validation and the separate alias projection without copying fixes between PR diffs. <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub> Co-Authored-By: OpenAI Codex (GPT-6 Astra) <[email protected]>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
6 of 15 tasks
Collaborator
|
🕓 Review not started yet because this PR is a draft.
Commit 268a84f. Normal review starts when eligible; priority review starts as soon as a slot is available. |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…e rust-dashcore bump The rust-dashcore update inherited from v5.1-dev makes the byte-array constructors and accessors inherent on the hash newtypes, so these `dashcore::hashes::Hash` imports fail clippy with -D warnings. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basic explanation
TL;DR: Keep one complete wallet history entry when an old withdrawal is delivered again after upgrading.
Value: Upgrades retain one complete history entry and the amounts of transactions spending its outputs.
Risks: Looking up an adopted old identifier returns the current identifier. Exceptional withdrawal lookups scan wallet-scoped evidence; no performance benchmark is claimed.
User story
As a wallet user, I want previously recorded withdrawals and their spending history to retain their amounts and details after an upgrade.
Scenario
Base flow
A wallet upgrades an old database and later receives the same withdrawal again.
Actual behavior
The changed transaction identifier can produce two history entries, split ownership details, and leave an older spending entry's amount unrepaired.
Expected behavior
History retains one complete entry and the correct spending amounts. Signed transaction bytes remain unchanged.
Detailed discussion
Issue being fixed or feature implemented
Follow-up to #5150, specifically canonical redelivery of historical AssetUnlock records. Addresses 5150-11, including the duplicate report. This separate PR is stacked on #5781, the bulk-validation prerequisite on #5150. Shared replay in #5220 remains a later integration gate.
What was done?
The normal exact-canonical upsert policy is unchanged. No FFI ABI, binding or dependency-pin changes. Exceptional AssetUnlock lookups scan wallet-scoped history/archive evidence; no performance benchmark is claimed. A later #5220 integration must validate physical-record replay against these aliases; it is not part of this PR.
How Has This Been Tested?
--no-deps -D warnings; format check passed.2876cf47.Breaking Changes
After canonical adoption, public history lookup by a legacy alias returns the canonical record and identifier. Legacy-only records keep their old identifiers until adoption.
WalletStorageErrorgainsHistoryAliasContextConflict; it is already non-exhaustive, so downstream matches already require a wildcard. No database-schema or FFI-ABI break.Checklist:
No GroveDB structure, system contract schema, or shipped consensus generation changes.
🤖 Co-authored by Claudius the Magnificent AI Agent