You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(platform-wallet): require only atomic tracked-lock writes for reconciliation - #5207
What this does: Lets a wallet save an updated asset-lock status when its storage supports the necessary atomic writes, even if it cannot restore an entire wallet.
Value: SQLite and callback-based storage can reconcile already-loaded asset locks without claiming unrelated restore support.
Risks: Low; the stronger wallet-drain requirements remain unchanged. No consensus changes.
Issue being fixed or feature implemented
Asset-lock reconciliation currently requires full wallet restoration in addition to atomic tracked-lock persistence. That prevents otherwise capable backends from updating already-loaded locks.
Final diff against 9766bab24a14af7cfbcafe33b466a777db8f868c is exactly the original five-file capability change. Full workspace tests were not run.
Breaking Changes
No API or consensus breaking changes. The reconciliation capability set is intentionally less restrictive; wallet drain retains its previous requirements.
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed
If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request
rs-platform-wallet-ffi (packages/rs-platform-wallet-ffi/src/persistence.rs) — HashEngineering or ZocoLini or llbartekll or romchornyi
wallet-storage — you own it
rs-platform-wallet (packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs, packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs, packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs) — HashEngineering or ZocoLini or llbartekll or romchornyi
When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.
Summary by CodeRabbit
Improvements
Asset-lock reconciliation can now work with persistence systems that support atomic changes and tracked asset-lock storage, without requiring wallet-restore support.
Recovered asset locks are verified after loading and wallet-manager initialization, including their status and associated transaction details.
Bump every rust-dashcore git dependency from e4208c90 to 719de34b (dashpay/rust-dashcore dev, the merge of #1049). The breaking change in range is rust-dashcore #1042: secp256k1 0.30 -> 0.33, rand 0.9, getrandom 0.4, and the secp256k1 context argument dropped API-wide. The range also carries key-wallet fixes#1035 (DIP-15 contact pools extend as payments arrive), #1009 (provider transactions consult every fund-bearing account), #1048, and test-only #920/#1038.
Mechanical call-site migration: drop Secp256k1 contexts (derive_priv/from_priv/public_key/from_secret_key/Keypair::new take no context); secp.sign_ecdsa/recover_ecdsa/verify_ecdsa become methods on the key/signature; SecretKey::from_slice/from_byte_array become from_secret_bytes (slices go through <[u8; 32]>::try_from mapped to Error::InvalidSecretKey, the error from_slice returned); secret_bytes -> to_secret_bytes; SecretKey/SharedSecret lost AsRef<[u8]>, so use as_secret_bytes; thread_rng -> rng; StdRng::from_entropy -> from_os_rng. The secp256k1::hashes re-export is gone, so hex rendering uses the hex crate (same output).
Seeded RNG bridging (rand 0.8 caller -> secp's rand 0.9 StdRng) now fills a 32-byte seed and calls from_seed, which is what rand_core 0.6 from_rng did, so seeded keys are unchanged; drive-abci tests that feed seed_from_u64 into Keypair::new use the secp re-exported StdRng (seed_from_u64 and ChaCha12 are identical across rand_core 0.6/0.9).
rs-platform-encryption moves its own secp256k1 to 0.33.1 so only one secp256k1 is in the graph. simple-signer takes a direct rand 0.8 dep for the RngCore its callers pass. rs-dpp enables getrandom 0.3/0.4 wasm_js on wasm32-unknown-unknown, which the new transitive getrandom versions require there. Rust toolchain already at 1.98.1, matching rust-dashcore.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Build on rust-dashcore PR #979 and account-local correction/event fixes at ed4c02e119898f1bb510cf79abc8a125a9bc9bea. Cargo metadata --locked validates the pin; wallet integration checks follow with the storage changes.
Repair fully resolved history from durable TXOs at round commit and wallet
load, preserve funded asset-lock accounting during context-only recovery,
and scope history presentation to the selected wallet.
Swift tests require macOS tooling and were not executable on this host.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Use one ownership predicate for persisted inputs, outputs, and scoped
history amounts, including legacy watch-only TXOs. Preserve accountless
rows with known wallet ownership.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Index historical inputs to repair late funding and existing SQLite history,
preserve known ownership across partial replay, and project corrected bridge
topology without double-counting account snapshots. Keep unknown credit
verdicts from inventing spends or resurrecting coins.
Co-Authored-By: Codex GPT-6 <[email protected]>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Replace the PR 979-based dependency with the isolated accounting correction
backported onto the original e4208c90 base. The identical upstream patch is
available on current rust-dashcore dev without SPV or address-pool changes.
Validated 24 Core storage and 132 changeset tests with the new pin; scoped
Clippy with CI flags and formatting checks pass.
Co-Authored-By: Codex <[email protected]>
#5043 introduced PLATFORM_V15 and made it latest, but the committed
grovedb-structure.json and one structure test still expected protocol 14,
so `structure::tests` has failed on v4.3-dev since it merged:
- should_match_committed_grovedb_structure_json: regenerate the snapshot.
Only the origin labels and latest_protocol_version change (14 -> 15);
v15 has the same state structure as v14.
- should_record_a_contract_layer_with_its_documents_on_top: read the
expected origin from PlatformVersion::latest() instead of a hard-coded
14, so the next protocol bump does not break it again.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
…sync
Replay persisted confirmed transactions through the wallet checker on load,
retain only persisted unspent outputs not consumed by confirmed history,
and restore finality before sync checkpoint pruning. Use dash-async for
synchronous persistence loading.
Cover restart, stale projections, funding redelivery, finality, same-block
spends, reserved inputs, and synchronous/current-thread/multi-thread callers.
Co-authored-by: Codex <[email protected]>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Accept atomic tracked-asset-lock writes with either full wallet restore
or the narrower tracked-asset-lock restore capability. SQLite attests
only the latter, matching its public load and manager hydration paths.
Preserve the typed consumption report, nonterminal recovery marker,
proof, and persistence failure behavior.
Co-Authored-By: Codex GPT-6 <[email protected]>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
No actionable comments were generated in the recent review.
Walkthrough
Asset-lock reconciliation now requires atomic changesets and tracked asset-lock persistence, but not wallet restore. Callback and SQLite integration tests now verify this capability contract and the restored asset-lock state.
ASSET_LOCK_RECONCILIATION now requires atomic changesets and tracked asset-lock persistence. WALLET_DRAIN still includes wallet restore. The capability tests cover the required bit combinations.
Persistence callback and test setup packages/rs-platform-wallet-ffi/src/persistence.rs, packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs
The callback capability test no longer requires wallet-list load and free callbacks. The test persistence implementation now selects reconciliation and full-restore capabilities separately.
SQLite persistence and manager hydration packages/rs-platform-wallet-storage/tests/integration/sqlite_persist_roundtrip.rs
The async test checks SQLite’s reconciliation and wallet-restore capabilities, then compares restored asset-lock fields after loading and manager hydration. It also checks manager shutdown.
dashpay/platform#5210: Changes SQLite capability reporting and wallet snapshot restoration, which connect to the capability contract updated here.
dashpay/platform#5150: Changes SQLite restoration capabilities that intersect with the wallet-restore requirement removed here.
dashpay/platform#5460: Changes asset-lock reconciliation and persistence-backed proof handling in the same flow.
Suggested reviewers:llbartekll, quantumexplorer
Merge Risk: Minimal · up to 6c01f
No actionable merge-blocking issue is established. The capability change and accompanying persistence tests appear consistent; merge after normal checks pass.
Pre-merge checks: 4 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: reconciliation now requires only atomic tracked-lock writes.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 4 files. (1 skipped: 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
♻️ No change to this PR's diff since commit 452a460 (an empty commit, a merge of the base or a rebase onto it): that review stands — approved; @thepastaclaw review asks for a review of this commit (commit 6c01f7e)
…count
The load projection parks an unattributable UTXO in the first funds
account; replaying confirmed history then credits the same outpoint to
its real owner. The post-replay filter only checked a wallet-wide
unspent set, so both copies survived and update_balance double counted.
Drop the load-time fallback copy whenever replay credits the outpoint to
a different funds account, while still refusing to re-credit anything
persistence excluded.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…dable
Pre-fix builds persisted DashPay contact (watch-only) outputs as unspent
core_utxos rows. V019 re-labels their history as Sent but left the rows
unspent, so load routed them to the first funds account: the balance
included coins the wallet cannot sign and coin selection could pick one.
load_state now skips unspent contact-only rows. The rows are kept in
SQLite (no destructive migration step), so the exclusion is reversible.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…eset
loadWalletList saved or rolled back the background context
unconditionally after reconciling transaction accounting. Inside an open
begin/endChangeset round that committed half of the round or silently
discarded its staged writes while the round still reported success.
Guard the pass with !inChangeset like every other load-time writer; the
round reconciles its own dirty rows and the next load runs the full pass.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Make the existing tracked-lock capability cover persistence and nonterminal
restart restore, and require only that contract plus atomic changesets for
reconciliation. Admit the FFI bit only with persistence and paired restore
callbacks. Fail Swift wallet restore if its tracked-lock fetch fails.
Co-Authored-By: Codex <[email protected]>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
lklimek
changed the title
fix(wallet): allow scoped SQLite asset-lock reconciliation
fix(wallet): require tracked asset locks to persist and restore
Sep 29, 2026
lklimek
changed the title
fix(wallet): require tracked asset locks to persist and restore
fix(platform-wallet): allow scoped SQLite asset-lock reconciliation
Sep 29, 2026
lklimek
changed the title
fix(platform-wallet): allow scoped SQLite asset-lock reconciliation
fix(wallet)!: require tracked asset locks to persist and restore
Sep 29, 2026
Require only atomic tracked-lock persistence for reconciliation of an
already-loaded wallet. Preserve existing capability meanings and FFI host
admission behavior, with wallet restore work outside this change.
Keep regression coverage for SQLite reopen/hydration, typed consumption
errors, and persistence failure handling.
Co-Authored-By: Codex <[email protected]>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
lklimek
changed the title
fix(wallet)!: require tracked asset locks to persist and restore
fix(wallet): require only atomic tracked-lock writes for reconciliation
Sep 29, 2026
…allet-level row
A block re-emits only the slices key-wallet's re-check still matches. For a
no-change asset lock (the wallet drain: every funding UTXO spent, the burn
as the only output) the funding account has nothing left to match, so the
block carries only the keys account's detail-less marker. Folded alone,
the marker became the wallet-level row (0 inputs, net 0) and replaced the
funding accounting the host had stored.
The BlockProcessed projection now completes the event's slices with every
slice the manager still holds for the same transaction under an account the
event lacks, then folds. The event stays authoritative for its own
accounts; contact watch-only slices stay out; an unknown wallet keeps the
event's slices unchanged.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ow folds in
Two edge cases of the held-sibling fold in the BlockProcessed projection:
the event's own slice wins over a held slice of the same account, and a
held contact watch-only slice stays out of the row. Both fail if the fold
admits every held slice. The no-change lock fixture is shared with the
existing keys-marker test.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…orical codec
Gate kept bodies and identifier keys inside SQLite before materialization,
and use a V019 record adapter with immutable historical fixtures. Validate
birth heights before destructive migration SQL and bind saturating rewinds.
Update SCHEMA.md and the SQL fingerprint without adding a migration.
Tests: storage all-features suite, clippy all-targets/all-features with
--no-deps -D warnings, and cargo fmt --all.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: Codex GPT-6 <[email protected]>
…action keys
Keep the physical transaction keys alongside decoded records so Recovery
removes unauthorized drifted rows from its snapshot. Clean related evidence
under both persisted and body txids. Repeated recovery loads release the
reservation without modifying the durable database.
Tests: should_remove_drifted_unauthorized_rows_from_the_recovery_snapshot,
full storage suite, clippy, and cargo fmt --all.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: Codex GPT-6 <[email protected]>
…igration rows
Pin the complete historical decoder in a storage-owned crate, including
leaf serde and input-key encoding. Add immutable pending-context fixtures,
type-safe materialization gates, and logged full rescans for invalid birth
heights under Recovery while preserving Strict failures. Document the
unreleased V020 recreation requirement and extend rewind boundary coverage.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: Codex GPT-6 <[email protected]>
…e drift
Delete rejected transaction bodies by persisted keys while retaining
evidence belonging to surviving logical transactions. Restore protected
claims cleared by the delete trigger and document the physical-key map.
Add a regression that fails when survivor outputs or claims are removed.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: Codex GPT-6 <[email protected]>
…le fixtures
Remove the pinned V019 codec crate and use the live record decoder and
input-index writer. Preserve pending and mined fixture bytes, strengthen
live decoding and migration checks, and correct the 5.0 writer provenance.
Recovery rewinds invalid birth heights without overwriting corrupt wallet
metadata, preserving loader diagnostics. Keep temporary V020 compatibility
notes in PR material rather than the schema reference.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: Codex GPT-6 <[email protected]>
Preserve the feature behavior while incorporating the current base.
Resolve additive conflicts and adapt regression fixtures to current APIs.
Adapt the optional built-lock observer and high-S fixture assertions.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: OpenAI Codex <[email protected]>
Preserve the feature behavior while incorporating the current base.
Resolve additive conflicts and adapt regression fixtures to current APIs.
Retain atomic tracked-lock reconciliation with the updated wallet APIs.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: OpenAI Codex <[email protected]>
The reason will be displayed to describe this comment to others. Learn more.
Re-review — Final validation — Phase 1 + Phase 2
Reviewed the complete five-file diff at 452a460 and found no actionable defects. Reconciliation now requires atomic tracked-lock persistence, while WALLET_DRAIN preserves its existing restore requirement and recovery remains nonterminal. This was a static review; current CI confirms wallet, Swift, frozen-schema, and Kotlin checks passed, while broader Rust workspace and WASM suites were skipped.
Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, contained capability-mask correction while preserving wallet-drain requirements, with the remaining changes limited to documentation and focused regression tests.
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
…unting stack
Keep PR 5207's five-file capability change on v5.1-dev. Restore inherited
PR 5150 accounting, replay and migration paths to that base without
rewriting branch history. Atomic tracked-lock reconciliation still
requires 0x201, while WALLET_DRAIN retains 0x2285 and full restore.
Verified the final base diff is byte-identical to the original PR 5207
delta. Run focused capability, reconciliation, FFI callback and SQLite
hydration regressions plus scoped formatting and CI-equivalent clippy.
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Co-Authored-By: OpenAI Codex <[email protected]>
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.
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
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.
lklimek
changed the title
thepastaclaw
left a comment
github-actions
Bot
added
the
waiting-bots
Basic explanation
What this does: Lets a wallet save an updated asset-lock status when its storage supports the necessary atomic writes, even if it cannot restore an entire wallet.
Value: SQLite and callback-based storage can reconcile already-loaded asset locks without claiming unrelated restore support.
Risks: Low; the stronger wallet-drain requirements remain unchanged. No consensus changes.
Issue being fixed or feature implemented
Asset-lock reconciliation currently requires full wallet restoration in addition to atomic tracked-lock persistence. That prevents otherwise capable backends from updating already-loaded locks.
What was done?
v5.1-devdirectly; this PR is independent of fix(platform-wallet): correct wallet-level transaction history accounting #5150.ATOMIC_CHANGESETS | TRACKED_ASSET_LOCKSforASSET_LOCK_RECONCILIATION(0x201). KeepWALLET_DRAINat0x2285, includingWALLET_RESTORE.How Has This Been Tested?
Validated on Linux at
6c01f7e279336cc6dd81347d2a069975876e3912:platform-wallet,platform-wallet-storageandplatform-wallet-ffi: stable masks, reconciliation without restore, missing-capability refusal, drain behavior, callback requirements and SQLite hydration.cargo fmt -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --checkpassed.cargo clippy -p platform-wallet -p platform-wallet-storage -p platform-wallet-ffi --all-targets --all-features --locked -- --no-deps -D warningspassed.9766bab24a14af7cfbcafe33b466a777db8f868cis exactly the original five-file capability change. Full workspace tests were not run.Breaking Changes
No API or consensus breaking changes. The reconciliation capability set is intentionally less restrictive; wallet drain retains its previous requirements.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Co-authored by Claudius the Magnificent AI Agent
PR Hygiene ·
6c01f7e/self-reviewedrs-platform-wallet-ffi(packages/rs-platform-wallet-ffi/src/persistence.rs) — HashEngineering or ZocoLini or llbartekll or romchornyiwallet-storage— you own itrs-platform-wallet(packages/rs-platform-wallet/src/changeset/persistence_capabilities.rs,packages/rs-platform-wallet/src/wallet/asset_lock/sync/tracking.rs,packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs) — HashEngineering or ZocoLini or llbartekll or romchornyiWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit