Skip to content

feat: prepare original-wallet migration proposals - #34

Open
ben-kaufman wants to merge 1 commit into
feat/backup-migration-statusfrom
feat/backup-migration-plan
Open

ben-kaufman wants to merge 1 commit into
feat/backup-migration-statusfrom
feat/backup-migration-plan

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Prepare immutable migration content from the original wallet session's app and LDK observations. Retain the existing logical WALLET at its original encrypted key/AAD and exact raw-envelope digest, encrypt the initial app head once without uploading it, and calculate the candidate server's canonical all-tuples digest. Explicit absence stays distinct from unreadable data.

Preparation rejects mixed owners/domains, revoked sessions, superseded or committed policies, duplicate/conflicting component keys and oversized input. Starting either domain's next status read invalidates earlier proposal access even when that read fails or is abandoned. Historical observations remain readable. Exact physical principal/store matches coalesce while retaining both domain labels; equal store IDs alone never combine identities. Mutable LDK objects cannot become immutable components.

Stacked on #33. This is in-crate Rust source preparation for terminal complete-WALLET work in synonymdev/bitkit-ios#1121 and synonymdev/bitkit-android#1686. It adds no activation request, UniFFI export, release, server change or app activation. Published native API versions/checksums are unchanged.

Validation

  • Full Rust suite: 76 passed, 0 failed/ignored. Debug/release builds and docs pass. Whole-repository formatting and diff checks pass. Complete Clippy diagnostics contain only two verified unchanged base test-module warnings.
  • Two independent security/recovery and integration reviews completed with affected-delta rechecks. Corrected mutable LDK component admission and head key bounds, and added exact production Unicode sorting/digest coverage. The initial compile failure is preserved.
  • Required iOS and four-ABI Android builds completed once in isolated validation version 0.5.27-dev.5. Both iOS17 consumer links and Kotlin consumer compile pass. Android lint XML: 0 errors, 3 verified unchanged dependency warnings. Generated Swift/header/modulemap/Kotlin interfaces are byte-identical to the validated parent. Complete logs, archive CRCs/hashes, 862 arm64 members per iOS slice, four Android ABIs/16 KiB alignment/debug IDs and all 93 existing FFI symbols inspected. Hosted validation on exact-tree synthetic 7e7bc007 also passes: 4 attempt-one workflows, 6 jobs, 76 tests, zero failures/ignored tests and no introduced diagnostics. The complete 8,264 log lines and 14 annotations were inspected. Both uploaded native artifacts passed independent SHA/CRC, interface, platform/ABI and 93-symbol checks; Android debug/release build IDs match. Each artifact downloaded once. Five conditional Rustup bootstrap skips and two Android NO-SOURCE tasks are not test passes.

Controlled cases cover original head key/AAD authentication, retained exact archive bytes including protobuf unknown fields, explicit absent versus unreadable WALLET, wrong/replaced owners, revocation, pending/failed status refreshes, committed policy refusal, coalesced-domain LDK rejection, duplicate/conflicting components, encoded size limits, fixed Rust/Java canonical vectors and an independently encoded present-archive/component Unicode vector. Same-handle ciphertext stability is asserted directly. These are native/component results, not authenticated app migration acceptance.

Remaining integration boundaries

App callers must prove complete/coherent capture and choose only already-immutable app components; ordinary same-key retries that re-encrypt are unsuitable. Every independent preparation requires a fresh UUID; later dispatch must retain one exact attempt across uncertain outcomes. This primitive supplies no durable UUID registry or replay authority.

The unpublished candidate server contract is not evidence of deployed enforcement. Future mutation requires fresh original-provider proofs and atomic tuple/object checks, lifecycle drains, running-node/RN and old reader/writer exclusion, durable outcome reconciliation and causal app restoration. In particular, a coalesced tuple retains two provider labels, but the preserved server's one-proof-per-target contract cannot freshly verify both mappings. That enforcement gap remains open before activation. This PR does not close terminal complete-WALLET acceptance.

@ben-kaufman
ben-kaufman requested review from dzdidi and ovitrif November 4, 2026 06:26

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve

Findings: none.

QA for 4b1bccc:

Rust test suite and lint at the head ✅ cargo test passes 76 of 76; clippy adds no new warnings.

cargo test: 76 passed, 0 failed, 0 ignored, including the new preparation and canonical-vector tests. cargo clippy --all-targets: only the two module_inception warnings already present in the base's test modules.

Log: RUN/media/cargo-test-clippy.log

Not driven on a device: the PR adds in-crate Rust only, with no UniFFI export, release or app change, so no app behaviour changes.

Review covered the full range 435bb2a..4b1bccc. Checked in the code:

  • Canonical digest order is UTF-8 byte order, for stores and component keys.
  • Every u32 length prefix is bounded first: 2 stores, 32 objects, 600-byte keys, 120-byte identities, 10 MiB head.
  • The head is size-checked both before and after encryption.
  • The archive keeps build_key("WALLET") with its raw-envelope digest, and absence stays None.
  • Archive/component and head/component conflicts are refused.
  • Stores coalesce only when principal and store ID both match.
  • A status read replaces the revision before its first await, so a pending, failed or dropped read still invalidates earlier proposals.
  • No new type derives Debug.

Not checked here: that the digest matches the candidate server's real implementation (only the fixed vectors in the tests were checked), and that the size limits match the deployed server.


Reviewed by claude-opus-5-5-medium

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings. Adds in-crate preparation of immutable original-wallet migration proposals: the retained WALLET archive at its original key and envelope digest, an initial app head encrypted once under the app key/AAD, app-only components, exact principal/store coalescing, and the canonical all-tuples digest, with per-domain status revisions that invalidate earlier proposals. Reviewed 4b1bccc, full tier, statically: stacked on #33, so the delta against feat/backup-migration-status is what I read.

What I checked, and 7 candidates I ruled out

Read in full: src/backup_session.rs, src/backup_session/preparation.rs, preparation/canonical.rs, plus the code they call in migration_status.rs, receipt.rs (get_with_receipt), implementation.rs (build_key, the StorableBuilder use in store)
Build: skipped, talos-pr-run could not confirm the author's permission on this repo and fails closed, so nothing from the branch ran here
CI: Tests, Clippy, iOS Bindgen, manifests green; Android Bindgen still pending at review time

Ruled out

  • Archive key differs from what was read: prepare_archive uses self.app.build_key("WALLET"), the same mapping get_with_receipt applied to the observed key, and the target is pinned to App { key: "WALLET" }.
  • Archive store not checked against the status tuple: the receipt's store_id is the session's app store_id, and decode in migration_status.rs rejects a status whose store_id differs from it, so they cannot diverge.
  • Head key bound on plaintext only: the 600-byte check runs again on the obfuscated key via canonical::valid_key.
  • Stale status admitted after a concurrent refresh: the revision is replaced before the read starts, and both preparation_status and prepared_migration compare by Arc::ptr_eq; a racing older read just yields StalePolicy. prepare_migration has no await, so nothing interleaves inside it. Covered by preparation_pending_then_abandoned_status_cannot_reuse_previous_policy.
  • Byte vs char identity limit differs from valid_identity: deliberate, the byte limit matches the activation bound and has its own test.
  • Head envelope version 1 vs the -1 used by store: documented as the authenticated version for an expected-version-zero insert; plausibly the contract, not a slip.
  • Archive also listed as a component: allowed, with the digest conflict check mirroring the server-side rule in committed().

Merge confidence: 4/5, no findings but the branch was not built here and Android Bindgen had not finished.

@dzdidi dzdidi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review of exact head 4b1bccc3563acd0857ba75924d3786ca52b916b9 against base 435bb2a48459a556f3504981c4dc471c21166d02 found no blocking correctness, security, persistence, API-contract, or documentation issues. All six applicable exact-head hosted checks completed successfully.

Residual boundaries: canonical digest parity relies on frozen vectors rather than an independently executed external candidate server; server contract remains unpublished/undeployed; multi-read coherence, lifecycle exclusion, durable UUID/replay handling, and dual-provider proof enforcement remain explicit future activation requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants