chore(deps): bump platform pin to PR #3968 tip (fix empty-script UTXO wallet-brick) - #953
Merged
Merged
Conversation
… wallet-brick) Moves all four dashpay/platform git dependencies -- dash-sdk, rs-sdk-trusted-context-provider, platform-wallet, platform-wallet-storage -- from a18bd158 to 762c66cf, the tip of PR #3968 (feat/platform-wallet-storage-rehydration). The four share one rev by necessity: splitting them would resolve two git sources for one repo and duplicate every shared platform type. 762c66cf is "fix(platform-wallet-storage): stop one bad script row from bricking a wallet file", a direct child of the previous pin touching only rs-platform-wallet-storage (core_state.rs, core_pool.rs, and a test). A single core_utxos row with an empty script made an entire persisted wallet file unloadable -- one fabricated row out of 668 locked 12 wallets in a real user database. Upstream stops manufacturing the row on write and makes both reuse-guard readers of load_used_addresses skip-and-warn per row. load_state stays fail-hard by design: it reads only spent = 0 rows and is the balance source, so a corrupt balance-bearing row still aborts loudly rather than under-report a balance. No DET-side change is required. load_used_addresses and AddressDecode appear nowhere in this repository, and all seven Address::from_script call sites here decode asset-lock credit outputs or withdrawal documents, already tolerate a decode failure, and never touch the persisted-UTXO path. The user-facing string this fix stops triggering is TaskError's "Saved wallet data appears damaged", which is a Display message rather than a behavioural dependency. Cargo.lock carries only the 28 platform source lines. A plain cargo update -p also re-pointed prost-build, prost-derive and winapi-util onto already-present older copies of itertools and windows-sys; the package version set was identical either way, so that churn was dropped in favour of a rev rewrite verified by building with --locked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe update advances Dash Platform Git dependencies to a shared revision and adds a changelog entry for resilient wallet-file loading. Unreadable payment records are skipped, while records with balances still prevent loading. ChangesWallet loading resilience
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Collaborator
|
✅ Final review complete — no blockers (commit be46a17) |
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
3 participants
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.
TL;DR: Pull in an upstream fix so a single damaged payment record can no longer make an entire wallet file — every wallet in it — refuse to open.
User story
As a Dash Evo Tool user, I want one unreadable internal record to never take down my whole wallet file, so a wallet that was working keeps working after a restart.
Scenario
Base flow
DET keeps all of a user's wallets in one on-disk file. Normal use — syncing, migrating from an older install, topping up an identity — can produce internal bookkeeping records for spent payments the wallet never had full details for.
Actual behavior
One such record could end up with no readable script attached. On the next launch, DET tries to read every record in the file to build its address-reuse safeguard, hits the unreadable one, and — because that check historically failed the entire file's load rather than skipping the one bad record — rejects every wallet in it. A real user hit this: one bad record out of 668 locked all 12 of their wallets, reporting "Saved wallet data appears damaged and cannot be loaded. Restore the wallet from its recovery phrase to keep using it." — even though nothing about their actual funds or keys was damaged.
Expected behavior
An unreadable record is skipped, with the rest of the wallet loading normally. The record that caused this is no longer written in the first place going forward.
Detailed discussion
What was done
Bumps the pinned
dashpay/platformgit dependency (dash-sdk,rs-sdk-trusted-context-provider,platform-wallet,platform-wallet-storage— all four share one revision by necessity, since splitting them would resolve two git sources for one repo and duplicate every shared platform type) froma18bd158to762c66cf, the current tip of upstream PR dashpay/platform#3968.762c66cf(fix(platform-wallet-storage): stop one bad script row from bricking a wallet file) is a direct child of the previous pin, touching onlyrs-platform-wallet-storage. It stops manufacturing a placeholder record with no script for a spent payment the wallet has no prior record of, and makes the address-reuse-guard readers skip and warn on an unreadable record instead of failing the whole load. The balance-bearing load path stays deliberately fail-hard — it only ever reads confirmed-unspent records, so a genuinely corrupt balance-affecting record still refuses to open rather than silently under-reporting funds.No DET-side code change was needed:
load_used_addressesandAddressDecode(the upstream types involved) don't appear anywhere in this repository, and everyAddress::from_scriptcall site here already tolerates a decode failure and never touches the affected persisted-record path — verified by grep, not assumed.Root cause was originally traced against a real user's broken database (a full investigation, separately documented) and led to two other, unrelated fixes already in #951 (a duplicate-identity-index guard and a top-up-account persistence fix) — those are real, independently confirmed bugs, but this PR is what actually fixes that user's specific incident.
Testing
cargo clippy --locked --all-features --all-targets -- -D warnings— clean (this is also the build check; a superset of compilation).cargo test --locked --all-features --lib wallet(coverswallet_backend::{loader,hydration,single_key,snapshot,wallet_seed_store,wallet_meta}andcontext::wallet_lifecycle::tests, the load/rehydration surface this touches) — 722 passed, 0 failed, 1 pre-existing ignored.cargo fmt --all— no source reformatted.Cargo.lockrewritten as a targeted rev substitution (28 platformsource =lines only) rather than a plaincargo update, which would have also re-pointed unrelated dependencies (prost-build,prost-derive,winapi-util) onto older already-present versions for no reason. Both verification runs used--locked, which hard-fails on any lock/manifest mismatch.Breaking changes
None. Dependency bump only; no public API changed upstream, no DET code changed.
Checklist
cargo fmt --all,clippy --all-features --all-targets -- -D warningscleanUnreleased→Fixed, Everyday User voice)Prior work
762c66cf.Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Bug Fixes
Documentation