Skip to content

feat: core derive_wallet_id for hardware watch-only wallets#108

Merged
coreyphillips merged 3 commits into
masterfrom
feat/wallet-id-derivation
Jun 24, 2026
Merged

feat: core derive_wallet_id for hardware watch-only wallets#108
coreyphillips merged 3 commits into
masterfrom
feat/wallet-id-derivation

Conversation

@coreyphillips

Copy link
Copy Markdown
Collaborator

Summary

Subtask 1 of #107. Core now owns the wallet_id derivation for hardware (watch-only) wallets so iOS and Android produce the same id for the same physical device by construction, instead of each platform reimplementing it. The Swift side currently improvises HwWalletId.derive = "trezor:" + SHA256(sorted xpubs) and flagged it as interim pending agreement with Android — if the two platforms derive independently they drift and activity won't reconcile cross-platform.

What changed

  • New #[uniffi::export] pub fn derive_wallet_id(device_type: String, xpubs: Vec<String>) -> String.
  • Device-agnostic (device_type keeps trezor / ledger / … distinct), so it generalises beyond Trezor.

Canonical derivation

  1. sort xpubs lexicographically (input order doesn't matter),
  2. join with a single \n,
  3. SHA256 the UTF-8 bytes, hex-encode lowercase,
  4. return "{device_type}:{hash}" (e.g. trezor:ab12…).

Trezor watch-only is new, so core defines the canonical form; platforms call this instead of rolling their own.

Tests

cargo test derive_wallet_id — order-independence, device_type distinctness, a fixed known-vector, and the empty-xpubs case.

🤖 Generated with Claude Code

Adds a device-agnostic `derive_wallet_id(device_type, xpubs)` so iOS and
Android derive the same `wallet_id` for the same physical device by
construction, instead of each platform reimplementing it (the Swift side
currently improvises "trezor:" + SHA256(sorted xpubs)).

Canonical derivation: sort xpubs lexicographically, join with "\n",
SHA256, hex-encode, return "{device_type}:{hash}". Order-independent.

Exported via UniFFI alongside get_default_wallet_id.

Closes subtask 1 of #107.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
So consumers of the checked-in iOS/Python packages can call the new
derive_wallet_id() export. (iOS xcframework / Android Kotlin / dylib are
rebuilt at release via build.sh, per repo convention.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@ben-kaufman ben-kaufman 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.

Two things I think we should tighten before merging:

  1. derive_wallet_id should reject an empty xpubs list instead of returning trezor:<sha256 empty string>. Otherwise any caller that reaches this with no account xpubs gets the same wallet id for that device type, which can collapse separate setup failures into the same activity scope. Could we make this return an error and update the empty-input test?

  2. The bindings look stale or partially regenerated. The Python wrapper references uniffi_bitkitcore_fn_func_derive_wallet_id, but the committed dylib does not export it, so python3 -c "import bitkitcore" fails with symbol not found. The Swift wrapper also calls the new symbol while the iOS headers and XCFramework do not include it, and Android has no deriveWalletId wrapper or JNI declaration. Could we regenerate and commit all platform bindings and artifacts from this Rust head?

Addresses @ben-kaufman review:

1. derive_wallet_id now returns Result<String, ActivityError> and rejects an
   empty xpubs list (and blank entries / blank device_type). An empty set would
   otherwise hash to the same id for every device of a type, collapsing distinct
   wallets into one activity scope.
2. Regenerated all platform bindings + native artifacts from this Rust head via
   build.sh all (Swift + bitkitcoreFFI.h + XCFramework, Kotlin + jniLibs .so,
   Python wrapper + libbitkitcore.dylib), so the new export is callable from the
   committed packages instead of failing with a missing symbol.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coreyphillips

Copy link
Copy Markdown
Collaborator Author

Thanks @ben-kaufman, both addressed in 80dadad.

  1. derive_wallet_id now returns Result<String, ActivityError> and rejects an empty xpubs list (also blank entries and a blank device_type), so an empty set can no longer collapse separate devices into one activity scope. The empty-input test was updated to assert the error.

  2. Regenerated all platform bindings and native artifacts from this Rust head via build.sh all: Swift + bitkitcoreFFI.h + the XCFramework, Kotlin + the jniLibs .so files, and the Python wrapper + libbitkitcore.dylib. The new export is now present and callable in all three, so import bitkitcore no longer fails with a missing symbol.

@ben-kaufman ben-kaufman 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.

Looks good, thanks for fixing the empty xpubs case and regenerating the bindings.

One release note before publishing: since the regenerated Swift zip checksum changed while Package.swift still points at v0.3.3, we may need to bump the bindings/package version so SwiftPM does not fetch the existing v0.3.3 asset with the old checksum.

@coreyphillips coreyphillips merged commit 2fcb16d into master Jun 24, 2026
@coreyphillips coreyphillips deleted the feat/wallet-id-derivation branch June 24, 2026 15:19
@ovitrif

ovitrif commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

also LGTM 👍🏻 🎉

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.

3 participants