From 107c47aa198214b8e03111e78b147ec77b41863d Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:43:09 -0500 Subject: [PATCH 01/15] Pin audited build dependencies Use reviewed revisions for git dependencies and OpenSSL 3.6.3. Require the lockfile in CI, local checks, and mobile binding builds so dependency resolution cannot move without review. --- .github/workflows/ci.yml | 6 ++-- .github/workflows/mobile-artifacts.yml | 2 +- .github/workflows/regenerate-bindings.yml | 2 +- justfile | 34 +++++++++++------------ rust/Cargo.lock | 32 ++++++++++----------- rust/Cargo.toml | 6 ++-- rust/xtask/src/android.rs | 13 +++++---- rust/xtask/src/ios.rs | 10 +++---- 8 files changed, 53 insertions(+), 52 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ed687cba..1e8f0b663 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -91,7 +91,7 @@ jobs: java-version: "17" - name: Install build dependencies - run: cd rust && cargo xtask install-deps + run: cd rust && cargo --locked xtask install-deps - name: Build Rust FFI and generate Kotlin bindings run: just build-android @@ -185,7 +185,7 @@ jobs: prefix-key: v1-rust - name: Run tests - run: cd rust && cargo test --workspace + run: cd rust && cargo test --locked --workspace clippy: runs-on: ubuntu-latest @@ -204,4 +204,4 @@ jobs: cache-bin: false prefix-key: v1-rust - - run: cd rust && cargo clippy --all-targets --all-features -- -D warnings + - run: cd rust && cargo clippy --locked --all-targets --all-features -- -D warnings diff --git a/.github/workflows/mobile-artifacts.yml b/.github/workflows/mobile-artifacts.yml index 87e23c948..8851ee130 100644 --- a/.github/workflows/mobile-artifacts.yml +++ b/.github/workflows/mobile-artifacts.yml @@ -67,7 +67,7 @@ jobs: java-version: '17' - name: Install build dependencies - run: cd rust && cargo xtask install-deps + run: cd rust && cargo --locked xtask install-deps - name: Build Rust FFI and generate Kotlin bindings run: just build-android diff --git a/.github/workflows/regenerate-bindings.yml b/.github/workflows/regenerate-bindings.yml index ce688107b..97f41054d 100644 --- a/.github/workflows/regenerate-bindings.yml +++ b/.github/workflows/regenerate-bindings.yml @@ -42,7 +42,7 @@ jobs: java-version: '17' - name: Install build dependencies - run: cd rust && cargo xtask install-deps + run: cd rust && cargo --locked xtask install-deps - name: Build Rust FFI and generate Kotlin bindings run: just build-android diff --git a/justfile b/justfile index 1250d00fb..e11ee79f4 100644 --- a/justfile +++ b/justfile @@ -19,7 +19,7 @@ list: # [variable] Run an xtask command [group('utils')] xtask *args: - cd rust && cargo xtask {{ args }} + cd rust && cargo --locked xtask {{ args }} # [bounded] Rebase current branch onto new-base after choosing the old squash-merged base [group('utils')] @@ -40,11 +40,11 @@ sign-psbt psbt: mkdir -p "$OUTPUT_DIR" echo "Signing PSBT and outputting to: $OUTPUT_DIR" cd rust - cargo xtask util sign-psbt --psbt "{{ psbt }}" -f base64 -O "$OUTPUT_DIR/signed.base64.txt" - cargo xtask util sign-psbt --psbt "{{ psbt }}" -f hex -O "$OUTPUT_DIR/signed.hex.txt" - cargo xtask util sign-psbt --psbt "{{ psbt }}" -f binary -O "$OUTPUT_DIR/signed.psbt" - cargo xtask util sign-psbt --psbt "{{ psbt }}" -f bbqr-gif -O "$OUTPUT_DIR/signed-bbqr.gif" - cargo xtask util sign-psbt --psbt "{{ psbt }}" -f ur-gif -O "$OUTPUT_DIR/signed-ur.gif" + cargo --locked xtask util sign-psbt --psbt "{{ psbt }}" -f base64 -O "$OUTPUT_DIR/signed.base64.txt" + cargo --locked xtask util sign-psbt --psbt "{{ psbt }}" -f hex -O "$OUTPUT_DIR/signed.hex.txt" + cargo --locked xtask util sign-psbt --psbt "{{ psbt }}" -f binary -O "$OUTPUT_DIR/signed.psbt" + cargo --locked xtask util sign-psbt --psbt "{{ psbt }}" -f bbqr-gif -O "$OUTPUT_DIR/signed-bbqr.gif" + cargo --locked xtask util sign-psbt --psbt "{{ psbt }}" -f ur-gif -O "$OUTPUT_DIR/signed-ur.gif" echo "" echo "All formats saved to: $OUTPUT_DIR" ls -la "$OUTPUT_DIR" @@ -223,7 +223,7 @@ android-preview-screenshots-validate: [group('test')] [script('bash')] ios-ui-background device="iPhone 17" test="CoveUITests/OnboardingFullLaunchUITests": - cd rust && cargo build --package xtask -q && ./target/debug/xtask ios-ui --device "{{ device }}" --test "{{ test }}" + cd rust && cargo build --locked --package xtask -q && ./target/debug/xtask ios-ui --device "{{ device }}" --test "{{ test }}" alias iub := ios-ui-background @@ -231,7 +231,7 @@ alias iub := ios-ui-background [group('test')] [script('bash')] ios-ui-foreground device="iPhone 17" test="CoveUITests/OnboardingFullLaunchUITests": - cd rust && cargo build --package xtask -q && ./target/debug/xtask ios-ui --foreground --device "{{ device }}" --test "{{ test }}" + cd rust && cargo build --locked --package xtask -q && ./target/debug/xtask ios-ui --foreground --device "{{ device }}" --test "{{ test }}" alias iuf := ios-ui-foreground @@ -239,19 +239,19 @@ alias iuf := ios-ui-foreground [group('test')] [working-directory('rust')] test test="" flags="": - cargo nextest run {{ test }} --workspace {{ flags }} + cargo nextest run --locked {{ test }} --workspace {{ flags }} # [bounded] Run tests the same way as GitHub Actions [group('test')] [working-directory('rust')] test-gh test="" flags="": - cargo test {{ test }} --workspace {{ flags }} + cargo test --locked {{ test }} --workspace {{ flags }} # [bounded] Run tests with cargo test [group('test')] [working-directory('rust')] ctest test="" flags="": - cargo test {{ test }} --workspace -- {{ flags }} + cargo test --locked {{ test }} --workspace -- {{ flags }} # [indefinite] Run tests with bacon [group('test')] @@ -280,7 +280,7 @@ alias wtest := watch-test [group('lint')] [working-directory('rust')] lint-rust *flags="": - cargo clippy --all-targets --all-features -- -D warnings {{ flags }} + cargo clippy --locked --all-targets --all-features -- -D warnings {{ flags }} # [bounded] Lint Android code [group('lint')] @@ -301,13 +301,13 @@ lint-swift *flags="": [group('lint')] [working-directory('rust')] clippy *flags="": - cargo clippy {{ flags }} + cargo clippy --locked {{ flags }} # [bounded] Run pedantic clippy checks (excluding must_use, truncation, single_match, if_not_else, needless_continue, option_if_let_else) [group('lint')] [working-directory('rust')] pedantic *flags="": - cargo clippy -- -D clippy::pedantic -D clippy::nursery \ + cargo clippy --locked -- -D clippy::pedantic -D clippy::nursery \ -A clippy::must_use_candidate \ -A clippy::cast_possible_truncation \ -A clippy::single_match \ @@ -325,7 +325,7 @@ pedantic *flags="": [group('lint')] [working-directory('rust')] pedantic-all *flags="": - cargo clippy -- -D clippy::pedantic -D clippy::nursery {{ flags }} + cargo clippy --locked -- -D clippy::pedantic -D clippy::nursery {{ flags }} # ------------------------------------------------------------------------------ # format @@ -379,7 +379,7 @@ bcheck: [group('dev')] [working-directory('rust')] check *flags="--workspace --all-targets --all-features": - cargo check {{ flags }} + cargo check --locked {{ flags }} # [indefinite] Watch and rebuild iOS on file changes [group('dev')] @@ -392,7 +392,7 @@ alias wb := watch-build [group('dev')] [working-directory('rust')] fix *flags="": - cargo fix --workspace {{ flags }} + cargo fix --locked --workspace {{ flags }} # ------------------------------------------------------------------------------ # release diff --git a/rust/Cargo.lock b/rust/Cargo.lock index abd537702..17313000c 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -3691,7 +3691,7 @@ dependencies = [ [[package]] name = "numfmt" version = "1.1.1" -source = "git+https://github.com/bitcoinppl/numfmt#a130b90092457526a336d3d13eb63493271aa190" +source = "git+https://github.com/bitcoinppl/numfmt?rev=a130b90092457526a336d3d13eb63493271aa190#a130b90092457526a336d3d13eb63493271aa190" dependencies = [ "dtoa", "itoa", @@ -3732,9 +3732,9 @@ checksum = "7c87def4c32ab89d880effc9e097653c8da5d6ef28e6b539d313baaacfbafcbe" [[package]] name = "openssl-src" -version = "300.6.0+3.6.2" +version = "300.6.1+3.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8e8cbfd3a4a8c8f089147fd7aaa33cf8c7450c4d09f8f80698a0cf093abeff4" +checksum = "46eb8fb9fb3b61ce1c0f8a026c4c1a0714d3a9e138e7fbde78753ce2babc3846" dependencies = [ "cc", ] @@ -4590,7 +4590,7 @@ dependencies = [ [[package]] name = "rust-cktap" version = "0.1.0" -source = "git+https://github.com/bitcoinppl/rust-cktap#97d780f8a8680661ac476d469f9513dba219ea45" +source = "git+https://github.com/bitcoinppl/rust-cktap?rev=97d780f8a8680661ac476d469f9513dba219ea45#97d780f8a8680661ac476d469f9513dba219ea45" dependencies = [ "bitcoin", "ciborium", @@ -5638,7 +5638,7 @@ dependencies = [ [[package]] name = "uniffi" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "camino", @@ -5680,7 +5680,7 @@ dependencies = [ [[package]] name = "uniffi_bindgen" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "askama 0.16.0", @@ -5717,7 +5717,7 @@ dependencies = [ [[package]] name = "uniffi_build" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "camino", @@ -5746,7 +5746,7 @@ dependencies = [ [[package]] name = "uniffi_core" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "async-compat", @@ -5771,7 +5771,7 @@ dependencies = [ [[package]] name = "uniffi_internal_macros" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "indexmap", @@ -5800,7 +5800,7 @@ dependencies = [ [[package]] name = "uniffi_macros" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "camino", "fs-err 3.3.0", @@ -5828,7 +5828,7 @@ dependencies = [ [[package]] name = "uniffi_meta" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "siphasher 1.0.3", @@ -5852,7 +5852,7 @@ dependencies = [ [[package]] name = "uniffi_pipeline" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "heck", @@ -5864,7 +5864,7 @@ dependencies = [ [[package]] name = "uniffi_testing" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "camino", @@ -5888,12 +5888,12 @@ dependencies = [ [[package]] name = "uniffi_udl" version = "0.31.1" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "anyhow", "textwrap", "uniffi_meta 0.31.1", - "weedle2 5.0.0 (git+https://github.com/mozilla/uniffi-rs?branch=main)", + "weedle2 5.0.0 (git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57)", ] [[package]] @@ -6174,7 +6174,7 @@ dependencies = [ [[package]] name = "weedle2" version = "5.0.0" -source = "git+https://github.com/mozilla/uniffi-rs?branch=main#42e834a4371547d9da47fd06e34062a91fa25b57" +source = "git+https://github.com/mozilla/uniffi-rs?rev=42e834a4371547d9da47fd06e34062a91fa25b57#42e834a4371547d9da47fd06e34062a91fa25b57" dependencies = [ "nom 7.1.3", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index ab19e5478..797b93810 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -13,7 +13,7 @@ members = ["crates/*", "xtask"] ## MARK: WORKSPACE DEPENDENCIES [workspace.dependencies] # bindings -uniffi = { git = "https://github.com/mozilla/uniffi-rs", branch = "main", features = ["tokio"] } +uniffi = { git = "https://github.com/mozilla/uniffi-rs", rev = "42e834a4371547d9da47fd06e34062a91fa25b57", features = ["tokio"] } # sync parking_lot = { version = "0.12.5" } @@ -78,7 +78,7 @@ ahash = "0.8.12" once_cell = "1.21.4" # fmt currency -numfmt = { git = "https://github.com/bitcoinppl/numfmt" } +numfmt = { git = "https://github.com/bitcoinppl/numfmt", rev = "a130b90092457526a336d3d13eb63493271aa190" } # num num-bigint = "0.5" @@ -157,7 +157,7 @@ bip329 = { workspace = true } payjoin = { workspace = true } # tapsigner / satscard -rust-cktap = { git = "https://github.com/bitcoinppl/rust-cktap" } +rust-cktap = { git = "https://github.com/bitcoinppl/rust-cktap", rev = "97d780f8a8680661ac476d469f9513dba219ea45" } # actors act-zero = { version = "0.4.0", features = ["default-tokio"] } diff --git a/rust/xtask/src/android.rs b/rust/xtask/src/android.rs index 681a3abe1..00c12a16c 100644 --- a/rust/xtask/src/android.rs +++ b/rust/xtask/src/android.rs @@ -167,7 +167,7 @@ pub fn build_android( // check for cargo-ndk if !command_exists("cargo-ndk") { - print_error("cargo-ndk not found. Please run: cargo xtask install-deps"); + print_error("cargo-ndk not found. Please run: cargo --locked xtask install-deps"); color_eyre::eyre::bail!("cargo-ndk is required for Android builds"); } @@ -220,14 +220,14 @@ pub fn build_android( // build with cargo-ndk let flags = crate::common::parse_build_flags(&build_flag); let build_result = if flags.is_empty() { - let cmd = cmd!(sh, "cargo ndk --target {target} build"); + let cmd = cmd!(sh, "cargo ndk --target {target} build --locked"); if verbose { cmd.run() } else { cmd.quiet().run() } } else if flags.len() == 1 && flags[0] == "--release" { - let cmd = cmd!(sh, "cargo ndk --target {target} build --release"); + let cmd = cmd!(sh, "cargo ndk --target {target} build --release --locked"); if verbose { cmd.run() } else { @@ -235,14 +235,15 @@ pub fn build_android( } } else if flags.len() == 2 && flags[0] == "--profile" { let profile_name = &flags[1]; - let cmd = cmd!(sh, "cargo ndk --target {target} build --profile {profile_name}"); + let cmd = + cmd!(sh, "cargo ndk --target {target} build --profile {profile_name} --locked"); if verbose { cmd.run() } else { cmd.quiet().run() } } else { - let cmd = cmd!(sh, "cargo ndk --target {target} build"); + let cmd = cmd!(sh, "cargo ndk --target {target} build --locked"); if verbose { cmd.run() } else { @@ -294,7 +295,7 @@ pub fn build_android( print_info(&format!("Generating Kotlin bindings into {}", BINDINGS_DIR)); cmd!( sh, - "cargo run -p uniffi_cli -- generate {dynamic_lib_path} --library --language kotlin --no-format --out-dir {BINDINGS_DIR}" + "cargo run --locked -p uniffi_cli -- generate {dynamic_lib_path} --library --language kotlin --no-format --out-dir {BINDINGS_DIR}" ) .run() .wrap_err("Failed to generate Kotlin bindings")?; diff --git a/rust/xtask/src/ios.rs b/rust/xtask/src/ios.rs index 0a6313599..4f8900792 100644 --- a/rust/xtask/src/ios.rs +++ b/rust/xtask/src/ios.rs @@ -426,14 +426,14 @@ pub fn build_ios(build_type: IosBuildType, device: bool, _sign: bool, verbose: b // build with cargo let flags = crate::common::parse_build_flags(&build_flag); let build_result = if flags.is_empty() { - let cmd = cmd!(sh, "cargo build --target {target}"); + let cmd = cmd!(sh, "cargo build --locked --target {target}"); if verbose { cmd.run() } else { cmd.quiet().run() } } else if flags.len() == 1 && flags[0] == "--release" { - let cmd = cmd!(sh, "cargo build --target {target} --release"); + let cmd = cmd!(sh, "cargo build --locked --target {target} --release"); if verbose { cmd.run() } else { @@ -441,14 +441,14 @@ pub fn build_ios(build_type: IosBuildType, device: bool, _sign: bool, verbose: b } } else if flags.len() == 2 && flags[0] == "--profile" { let profile_name = &flags[1]; - let cmd = cmd!(sh, "cargo build --target {target} --profile {profile_name}"); + let cmd = cmd!(sh, "cargo build --locked --target {target} --profile {profile_name}"); if verbose { cmd.run() } else { cmd.quiet().run() } } else { - let cmd = cmd!(sh, "cargo build --target {target}"); + let cmd = cmd!(sh, "cargo build --locked --target {target}"); if verbose { cmd.run() } else { @@ -487,7 +487,7 @@ pub fn build_ios(build_type: IosBuildType, device: bool, _sign: bool, verbose: b let _ = sh.remove_path(BINDINGS_DIR); cmd!( sh, - "cargo run -p uniffi_cli -- {static_lib_path} {BINDINGS_DIR} --swift-sources --headers --modulemap --module-name {UNIFFI_MODULE_NAME} --modulemap-filename {MODULEMAP_FILENAME}" + "cargo run --locked -p uniffi_cli -- {static_lib_path} {BINDINGS_DIR} --swift-sources --headers --modulemap --module-name {UNIFFI_MODULE_NAME} --modulemap-filename {MODULEMAP_FILENAME}" ) .run() .wrap_err("Failed to generate Swift bindings")?; From dd49bac319c2aded6968af85363ad6abdbceffbb Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:46:20 -0500 Subject: [PATCH 02/15] Protect wallet storage boundaries Reserve wallet identifiers before restore writes and remove only artifacts created by a failed attempt. Keep preview wallets in memory so test keys cannot enter production storage. --- .../CoinControlFlow/UtxoListScreen.swift | 4 +- .../CoinControlFlow/UtxoRowPreview.swift | 2 +- .../VerificationCompleteScreen.swift | 2 +- .../VerifyWords/VerifyWordsScreen.swift | 2 +- .../ChooseWalletTypeView.swift | 2 +- .../SelectedWalletFlow/MoreInfoPopover.swift | 2 +- .../SelectedWalletFlow/ReceiveView.swift | 2 +- .../SelectedWalletScreen.swift | 2 +- .../TransactionDetailsLabelView.swift | 6 +- .../TransactionDetailsView.swift | 8 +- .../TransactionsCardView.swift | 16 +- .../WalletBalanceHeaderView.swift | 12 +- .../Common/SendFlowAccountSection.swift | 2 +- .../Common/SendFlowDetailsSheetView.swift | 2 +- .../SendFlow/Common/SendFlowDetailsView.swift | 4 +- .../SendFlowFlowAdvancedDetailsView.swift | 4 +- .../SendFlow/Common/SendFlowHeaderView.swift | 4 +- .../SendFlowUtxoCustomAmountSheetView.swift | 2 +- .../SendFlowCoinControlSetAmountScreen.swift | 2 +- .../SendFlow/SendFlowConfirmScreen.swift | 6 +- .../SendFlow/SendFlowCustomFeeRateView.swift | 4 +- .../SendFlow/SendFlowHardwareScreen.swift | 2 +- .../SendFlow/SendFlowSetAmountScreen.swift | 4 +- .../SetAmountScreen/AddressTextEditor.swift | 4 +- .../SetAmountScreen/EnterAddressView.swift | 2 +- .../SendFlowSelectFeeRateView.swift | 8 +- .../WalletSettings/WalletSettingsView.swift | 2 +- ios/Cove/WalletManager.swift | 8 +- .../HotWalletCreateScreenLayoutTests.swift | 12 +- rust/crates/cove-device/src/keychain.rs | 7 + .../cove-device/src/keychain/wallet_public.rs | 4 + .../src/keychain/wallet_secrets.rs | 5 + rust/src/backup/error.rs | 5 + rust/src/backup/import.rs | 299 +++++++++++++++--- rust/src/bdk_store.rs | 40 ++- rust/src/database/wallet_data.rs | 54 +++- rust/src/database/wallet_data/label.rs | 7 +- rust/src/label_manager.rs | 8 + .../src/manager/cloud_backup_manager/error.rs | 6 + .../cloud_backup_manager/ops/test_support.rs | 16 + .../cloud_backup_manager/ops/tests/enable.rs | 15 +- .../ops/tests/other_backups.rs | 13 +- .../cloud_backup_manager/ops/tests/restore.rs | 69 ++-- rust/src/manager/wallet_manager.rs | 103 +++++- rust/src/manager/wallet_manager/actor.rs | 36 ++- rust/src/manager/wallet_manager/actor/node.rs | 6 + rust/src/manager/wallet_manager/exports.rs | 8 + .../wallet_manager/unsigned_transactions.rs | 12 + .../manager/wallet_manager/wallet_admin.rs | 49 +-- rust/src/transaction.rs | 41 ++- rust/src/wallet.rs | 98 +++++- rust/src/wallet/addressing.rs | 38 ++- rust/src/wallet/builder.rs | 74 ++++- 53 files changed, 930 insertions(+), 215 deletions(-) diff --git a/ios/Cove/Flows/CoinControlFlow/UtxoListScreen.swift b/ios/Cove/Flows/CoinControlFlow/UtxoListScreen.swift index 35ebbfc4e..e0297620c 100644 --- a/ios/Cove/Flows/CoinControlFlow/UtxoListScreen.swift +++ b/ios/Cove/Flows/CoinControlFlow/UtxoListScreen.swift @@ -615,7 +615,7 @@ private struct UtxoRow: View { UtxoListScreen( manager: CoinControlManager(RustCoinControlManager.previewNew()) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -626,6 +626,6 @@ private struct UtxoRow: View { RustCoinControlManager.previewNew(outputCount: 0, changeCount: 0) ) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } diff --git a/ios/Cove/Flows/CoinControlFlow/UtxoRowPreview.swift b/ios/Cove/Flows/CoinControlFlow/UtxoRowPreview.swift index c6f7eaac1..3f59a22a1 100644 --- a/ios/Cove/Flows/CoinControlFlow/UtxoRowPreview.swift +++ b/ios/Cove/Flows/CoinControlFlow/UtxoRowPreview.swift @@ -56,6 +56,6 @@ struct UtxoRowPreview: View { AsyncPreview { let manager = CoinControlManager(RustCoinControlManager.previewNew()) UtxoRowPreview(displayAmount: manager.displayAmount, utxo: manager.utxos[0]) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } diff --git a/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerificationCompleteScreen.swift b/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerificationCompleteScreen.swift index ec25c6a9b..bfe2cf6e8 100644 --- a/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerificationCompleteScreen.swift +++ b/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerificationCompleteScreen.swift @@ -145,7 +145,7 @@ private struct VerificationCompleteContent: View { #Preview { AsyncPreview { - VerificationCompleteScreen(manager: WalletManager(preview: "preview_only")) + VerificationCompleteScreen(manager: WalletManager(preview: .only)) .environment(AppManager.shared) } } diff --git a/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerifyWordsScreen.swift b/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerifyWordsScreen.swift index 37794ebf3..039268a14 100644 --- a/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerifyWordsScreen.swift +++ b/ios/Cove/Flows/NewWalletFlow/HotWallet/VerifyWords/VerifyWordsScreen.swift @@ -719,7 +719,7 @@ private struct VerifyWordsCompactActions: View { #Preview { struct Container: View { - @State var manager = WalletManager(preview: "preview_only") + @State var manager = WalletManager(preview: .only) @State var stateMachine: WordVerifyStateMachine init() { diff --git a/ios/Cove/Flows/SelectedWalletFlow/ChooseWalletTypeView.swift b/ios/Cove/Flows/SelectedWalletFlow/ChooseWalletTypeView.swift index a7cc5dce6..fb45359f1 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/ChooseWalletTypeView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/ChooseWalletTypeView.swift @@ -125,7 +125,7 @@ private struct FoundWalletTypeButton: View { #Preview { AsyncPreview { ChooseWalletTypeView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), foundAddresses: [ previewNewLegacyFoundAddress(), previewNewWrappedFoundAddress(), diff --git a/ios/Cove/Flows/SelectedWalletFlow/MoreInfoPopover.swift b/ios/Cove/Flows/SelectedWalletFlow/MoreInfoPopover.swift index e382626f6..1217773d2 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/MoreInfoPopover.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/MoreInfoPopover.swift @@ -225,7 +225,7 @@ private struct MoreInfoWalletActions: View { #Preview { AsyncPreview { MoreInfoPopover( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), importLabels: {}, exportLabels: {}, exportXpub: {} diff --git a/ios/Cove/Flows/SelectedWalletFlow/ReceiveView.swift b/ios/Cove/Flows/SelectedWalletFlow/ReceiveView.swift index d1111e45c..df80e9c3a 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/ReceiveView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/ReceiveView.swift @@ -375,7 +375,7 @@ private struct AddressView: View { #Preview { AsyncPreview { - ReceiveView(manager: WalletManager(preview: "preview_only")) + ReceiveView(manager: WalletManager(preview: .only)) .environment(AppManager.shared) } } diff --git a/ios/Cove/Flows/SelectedWalletFlow/SelectedWalletScreen.swift b/ios/Cove/Flows/SelectedWalletFlow/SelectedWalletScreen.swift index 2a839d121..8d10cd9e5 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/SelectedWalletScreen.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/SelectedWalletScreen.swift @@ -755,7 +755,7 @@ struct VerifyReminder: View { AsyncPreview { NavigationStack { SelectedWalletScreen( - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ).environment(AppManager.shared) } } diff --git a/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsLabelView.swift b/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsLabelView.swift index f809ec9a0..b30693e18 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsLabelView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsLabelView.swift @@ -174,7 +174,7 @@ private struct TransactionDetailsEditingLabelField: View { AsyncPreview { TransactionDetailsLabelView( details: TransactionDetails.previewNewConfirmed(), - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } @@ -184,7 +184,7 @@ private struct TransactionDetailsEditingLabelField: View { AsyncPreview { TransactionDetailsLabelView( details: TransactionDetails.previewNewWithLabel(), - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } @@ -194,7 +194,7 @@ private struct TransactionDetailsEditingLabelField: View { AsyncPreview { TransactionDetailsLabelView( details: TransactionDetails.previewNewWithLabel(label: "Car payment"), - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), isEditing: true ) .environment(AppManager.shared) diff --git a/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsView.swift b/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsView.swift index 916296ede..859311f03 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/TransactionDetails/TransactionDetailsView.swift @@ -450,7 +450,7 @@ private struct TransactionLockErrorModifier: ViewModifier { id: WalletId(), txId: details.txId(), transactionDetailsPresentation: presentation, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } @@ -465,7 +465,7 @@ private struct TransactionLockErrorModifier: ViewModifier { id: WalletId(), txId: details.txId(), transactionDetailsPresentation: presentation, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } @@ -480,7 +480,7 @@ private struct TransactionLockErrorModifier: ViewModifier { id: WalletId(), txId: details.txId(), transactionDetailsPresentation: presentation, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } @@ -495,7 +495,7 @@ private struct TransactionLockErrorModifier: ViewModifier { id: WalletId(), txId: details.txId(), transactionDetailsPresentation: presentation, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) .environment(AppManager.shared) } diff --git a/ios/Cove/Flows/SelectedWalletFlow/TransactionsCardView.swift b/ios/Cove/Flows/SelectedWalletFlow/TransactionsCardView.swift index 7e46dffd3..2cc3de12e 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/TransactionsCardView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/TransactionsCardView.swift @@ -784,7 +784,7 @@ private struct TxnIcon: View { unsignedTransactions: [], metadata: walletMetadataPreview() ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -797,7 +797,7 @@ private struct TxnIcon: View { metadata: walletMetadataPreview() ) .background(.thickMaterial) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } } @@ -809,7 +809,7 @@ private struct TxnIcon: View { unsignedTransactions: [], metadata: walletMetadataPreview() ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -820,7 +820,7 @@ private struct TxnIcon: View { unsignedTransactions: [], metadata: walletMetadataPreview() ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -833,7 +833,7 @@ private struct TxnIcon: View { ], metadata: walletMetadataPreview() ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -847,7 +847,7 @@ private struct TxnIcon: View { unsignedTransactions: [], metadata: metadata ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -862,7 +862,7 @@ private struct TxnIcon: View { unsignedTransactions: [], metadata: metadata ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -893,6 +893,6 @@ private struct TxnIcon: View { } .ignoresSafeArea() } - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } diff --git a/ios/Cove/Flows/SelectedWalletFlow/WalletBalanceHeaderView.swift b/ios/Cove/Flows/SelectedWalletFlow/WalletBalanceHeaderView.swift index d33c3cff3..c2dfd6b89 100644 --- a/ios/Cove/Flows/SelectedWalletFlow/WalletBalanceHeaderView.swift +++ b/ios/Cove/Flows/SelectedWalletFlow/WalletBalanceHeaderView.swift @@ -346,7 +346,7 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -368,7 +368,7 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -389,7 +389,7 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -410,7 +410,7 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -432,7 +432,7 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } @@ -453,6 +453,6 @@ private struct WalletBalancePendingView: View { showReceiveSheet: {} ) .environment(AppManager.shared) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } } diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowAccountSection.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowAccountSection.swift index fd70966b8..149309162 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowAccountSection.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowAccountSection.swift @@ -70,7 +70,7 @@ struct SendFlowAccountSection: View { #Preview { AsyncPreview { SendFlowAccountSection( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), showsTitle: false ) } diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsSheetView.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsSheetView.swift index c9995b822..2272a6591 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsSheetView.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsSheetView.swift @@ -48,7 +48,7 @@ struct SendFlowDetailsSheetView: View { #Preview { AsyncPreview { SendFlowDetailsSheetView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), details: confirmDetailsPreviewNew() ) .padding() diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsView.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsView.swift index 27902883c..da75e2ac8 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsView.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowDetailsView.swift @@ -146,7 +146,7 @@ private struct SendFlowDetailsAmountRows: View { #Preview { AsyncPreview { SendFlowDetailsView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), details: confirmDetailsPreviewNew(), prices: nil ) @@ -154,7 +154,7 @@ private struct SendFlowDetailsAmountRows: View { .environment(AppManager.shared) .environment( SendFlowPresenter( - app: AppManager.shared, manager: WalletManager(preview: "preview_only") + app: AppManager.shared, manager: WalletManager(preview: .only) ) ) } diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowFlowAdvancedDetailsView.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowFlowAdvancedDetailsView.swift index 378471c83..e9cedd221 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowFlowAdvancedDetailsView.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowFlowAdvancedDetailsView.swift @@ -302,14 +302,14 @@ private struct SectionCard: View { #Preview { AsyncPreview { SendFlowAdvancedDetailsView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), details: confirmDetailsPreviewNew() ) .environment(AppManager.shared) .environment( SendFlowPresenter( app: AppManager.shared, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) ) } diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowHeaderView.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowHeaderView.swift index a00b48dcb..1dbde06fa 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowHeaderView.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowHeaderView.swift @@ -175,7 +175,7 @@ private struct SendFlowHeaderUnitMenu: View { #Preview { struct Container: View { - @State var manager: WalletManager = .init(preview: "preview_only") + @State var manager: WalletManager = .init(preview: .only) var body: some View { SendFlowHeaderView( @@ -189,7 +189,7 @@ private struct SendFlowHeaderUnitMenu: View { #Preview("small") { struct Container: View { - @State var manager: WalletManager = .init(preview: "preview_only") + @State var manager: WalletManager = .init(preview: .only) var body: some View { SendFlowHeaderView( diff --git a/ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift b/ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift index 4f6c3805c..4ee0d6123 100644 --- a/ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift +++ b/ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift @@ -364,7 +364,7 @@ private struct UtxoRowIdentity: View { #Preview { AsyncPreview { - let wm = WalletManager(preview: "preview_only") + let wm = WalletManager(preview: .only) let ap = AppManager.shared let presenter = SendFlowPresenter(app: ap, manager: wm) let utxos = previewNewUtxoList(outputCount: 2, changeCount: 1) diff --git a/ios/Cove/Flows/SendFlow/SendFlowCoinControlSetAmountScreen.swift b/ios/Cove/Flows/SendFlow/SendFlowCoinControlSetAmountScreen.swift index a626c11d2..3a28a2d0c 100644 --- a/ios/Cove/Flows/SendFlow/SendFlowCoinControlSetAmountScreen.swift +++ b/ios/Cove/Flows/SendFlow/SendFlowCoinControlSetAmountScreen.swift @@ -452,7 +452,7 @@ private struct SendFlowCoinControlFeeSelectionSheet: View { #Preview { AsyncPreview { NavigationStack { - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: AppManager.shared, manager: manager) if let rustSendFlowManager = try? manager.rust.newSendFlowManager(balance: manager.balance) { diff --git a/ios/Cove/Flows/SendFlow/SendFlowConfirmScreen.swift b/ios/Cove/Flows/SendFlow/SendFlowConfirmScreen.swift index f7e82d435..4bc9426e0 100644 --- a/ios/Cove/Flows/SendFlow/SendFlowConfirmScreen.swift +++ b/ios/Cove/Flows/SendFlow/SendFlowConfirmScreen.swift @@ -528,7 +528,7 @@ private enum SendConfirmationError: LocalizedError { } } .task { - manager = WalletManager(preview: "preview_only", metadata) + manager = WalletManager(preview: .only, metadata) manager?.dispatch(action: .updateUnit(.sat)) } } @@ -543,7 +543,7 @@ private enum SendConfirmationError: LocalizedError { AsyncPreview { SendFlowConfirmScreen( id: WalletId(), - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), details: confirmDetailsPreviewNew(), input: .unsigned, payjoinEndpoint: nil @@ -553,7 +553,7 @@ private enum SendConfirmationError: LocalizedError { .environment( SendFlowPresenter( app: AppManager.shared, - manager: WalletManager(preview: "preview_only") + manager: WalletManager(preview: .only) ) ) } diff --git a/ios/Cove/Flows/SendFlow/SendFlowCustomFeeRateView.swift b/ios/Cove/Flows/SendFlow/SendFlowCustomFeeRateView.swift index 6c9641f75..6ada68d24 100644 --- a/ios/Cove/Flows/SendFlow/SendFlowCustomFeeRateView.swift +++ b/ios/Cove/Flows/SendFlow/SendFlowCustomFeeRateView.swift @@ -311,11 +311,11 @@ private struct SendFlowCustomFeeDoneButton: View { ), selectedPresentationDetent: Binding.constant(PresentationDetent.large) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) .environment(AppManager.shared) .environment( SendFlowPresenter( - app: AppManager.shared, manager: WalletManager(preview: "preview_only") + app: AppManager.shared, manager: WalletManager(preview: .only) ) ) .frame(height: 300) diff --git a/ios/Cove/Flows/SendFlow/SendFlowHardwareScreen.swift b/ios/Cove/Flows/SendFlow/SendFlowHardwareScreen.swift index 28991cc78..be3fb2691 100644 --- a/ios/Cove/Flows/SendFlow/SendFlowHardwareScreen.swift +++ b/ios/Cove/Flows/SendFlow/SendFlowHardwareScreen.swift @@ -541,7 +541,7 @@ private struct SendFlowHardwareConfirmationDialog: View { AsyncPreview { SendFlowHardwareScreen( id: WalletId(), - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), details: confirmDetailsPreviewNew() ) .environment(AppManager.shared) diff --git a/ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift b/ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift index 2ac26d640..3ca07876c 100644 --- a/ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift +++ b/ios/Cove/Flows/SendFlow/SendFlowSetAmountScreen.swift @@ -495,7 +495,7 @@ private struct SendFlowFeeSelectionSheet: View { #Preview("with address") { AsyncPreview { NavigationStack { - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) SendFlowSetAmountScreen( id: WalletId() @@ -510,7 +510,7 @@ private struct SendFlowFeeSelectionSheet: View { #Preview("no address") { AsyncPreview { NavigationStack { - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) SendFlowSetAmountScreen( id: WalletId() diff --git a/ios/Cove/Flows/SendFlow/SetAmountScreen/AddressTextEditor.swift b/ios/Cove/Flows/SendFlow/SetAmountScreen/AddressTextEditor.swift index 388c47282..78d85a9af 100644 --- a/ios/Cove/Flows/SendFlow/SetAmountScreen/AddressTextEditor.swift +++ b/ios/Cove/Flows/SendFlow/SetAmountScreen/AddressTextEditor.swift @@ -68,7 +68,7 @@ struct AddressTextEditor: View { #Preview("focused") { AsyncPreview { let app = AppManager.shared - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: app, manager: manager) AddressTextEditor( @@ -84,7 +84,7 @@ struct AddressTextEditor: View { #Preview("not focused") { AsyncPreview { let app = AppManager.shared - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: app, manager: manager) AddressTextEditor( diff --git a/ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift b/ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift index 9a8a16fc9..75356e4c5 100644 --- a/ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift +++ b/ios/Cove/Flows/SendFlow/SetAmountScreen/EnterAddressView.swift @@ -86,7 +86,7 @@ private struct EnterAddressHeader: View { #Preview { AsyncPreview { let app = AppManager.shared - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: app, manager: manager) EnterAddressView(address: Binding.constant("bc1qdgxdn046v8tvxtx2k6ml7q7mcanj6dy63atva9")) diff --git a/ios/Cove/Flows/SendFlow/SetAmountScreen/SendFlowSelectFeeRateView.swift b/ios/Cove/Flows/SendFlow/SetAmountScreen/SendFlowSelectFeeRateView.swift index a6b7a3f13..fce6bd04f 100644 --- a/ios/Cove/Flows/SendFlow/SetAmountScreen/SendFlowSelectFeeRateView.swift +++ b/ios/Cove/Flows/SendFlow/SetAmountScreen/SendFlowSelectFeeRateView.swift @@ -187,14 +187,14 @@ private struct FeeOptionView: View { AsyncPreview { VStack { SendFlowSelectFeeRateView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), feeOptions: Binding.constant(FeeRateOptionsWithTotalFee.previewNew()), selectedOption: Binding.constant( FeeRateOptionsWithTotalFee.previewNew().medium() ), selectedPresentationDetent: Binding.constant(PresentationDetent.large) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) .environment(AppManager.shared) .frame(height: 440) } @@ -207,7 +207,7 @@ private struct FeeOptionView: View { AsyncPreview { VStack { SendFlowSelectFeeRateView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), feeOptions: Binding.constant( FeeRateOptionsWithTotalFee.previewNew().addCustomFeeRate( feeRate: FeeRateOptionWithTotalFee( @@ -222,7 +222,7 @@ private struct FeeOptionView: View { ), selectedPresentationDetent: Binding.constant(PresentationDetent.large) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) .environment(AppManager.shared) .frame(height: 550) } diff --git a/ios/Cove/Flows/SettingsFlow/WalletSettings/WalletSettingsView.swift b/ios/Cove/Flows/SettingsFlow/WalletSettings/WalletSettingsView.swift index d30e1c454..f08b9444d 100644 --- a/ios/Cove/Flows/SettingsFlow/WalletSettings/WalletSettingsView.swift +++ b/ios/Cove/Flows/SettingsFlow/WalletSettings/WalletSettingsView.swift @@ -243,7 +243,7 @@ private struct WalletSettingsContent: View { #Preview { AsyncPreview { - WalletSettingsView(manager: WalletManager(preview: "preview_only")) + WalletSettingsView(manager: WalletManager(preview: .only)) .environment(AppManager.shared) .environment(AuthManager.shared) .environment(\.navigate) { _ in diff --git a/ios/Cove/WalletManager.swift b/ios/Cove/WalletManager.swift index 09fc6a907..8acf22c77 100644 --- a/ios/Cove/WalletManager.swift +++ b/ios/Cove/WalletManager.swift @@ -46,6 +46,10 @@ private struct WalletManagerBootstrap { let initialState: WalletInitialState } +enum WalletManagerPreview { + case only +} + @Observable final class WalletManager: ReconcilingManager, WalletManagerReconciler { typealias Message = WalletManagerReconcileMessage typealias Action = WalletManagerAction @@ -669,9 +673,7 @@ private struct WalletManagerBootstrap { } /// PREVIEW only - convenience init(preview: String, _ walletMetadata: WalletMetadata? = nil) { - assert(preview == "preview_only") - + convenience init(preview _: WalletManagerPreview, _ walletMetadata: WalletMetadata? = nil) { let rust = if let walletMetadata { RustWalletManager.previewNewWalletWithMetadata(metadata: walletMetadata) diff --git a/ios/CoveTests/HotWalletCreateScreenLayoutTests.swift b/ios/CoveTests/HotWalletCreateScreenLayoutTests.swift index b21877d5f..7cfb2efff 100644 --- a/ios/CoveTests/HotWalletCreateScreenLayoutTests.swift +++ b/ios/CoveTests/HotWalletCreateScreenLayoutTests.swift @@ -84,7 +84,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { let image = render( view: NavigationStack { VerificationCompleteScreen( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), onVerified: {} ) } @@ -248,7 +248,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { let feeOptions = FeeRateOptionsWithTotalFee.previewNew() let image = render( view: SendFlowSelectFeeRateView( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), feeOptions: .constant(feeOptions), selectedOption: .constant(feeOptions.medium()), selectedPresentationDetent: .constant(.large) @@ -275,7 +275,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { try await bootstrapIfNeeded() let size = CGSize(width: 375, height: 667) - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: AppManager.shared, manager: manager) let image = render( view: NavigationStack { @@ -309,7 +309,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { try await bootstrapIfNeeded() let size = CGSize(width: 430, height: 932) - let manager = WalletManager(preview: "preview_only") + let manager = WalletManager(preview: .only) let presenter = SendFlowPresenter(app: AppManager.shared, manager: manager) let image = render( view: NavigationStack { @@ -373,7 +373,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { let size = CGSize(width: 375, height: 667) let view = NavigationStack { VerifyWordsScreen( - manager: WalletManager(preview: "preview_only"), + manager: WalletManager(preview: .only), stateMachine: WordVerifyStateMachine( validator: WordValidator.preview(preview: true), startingWordNumber: 1 @@ -446,7 +446,7 @@ final class HotWalletCreateScreenLayoutTests: XCTestCase { UtxoListScreen( manager: CoinControlManager(RustCoinControlManager.previewNew()) ) - .environment(WalletManager(preview: "preview_only")) + .environment(WalletManager(preview: .only)) } .frame(width: size.width, height: size.height), size: size diff --git a/rust/crates/cove-device/src/keychain.rs b/rust/crates/cove-device/src/keychain.rs index 3b642945f..004758a36 100644 --- a/rust/crates/cove-device/src/keychain.rs +++ b/rust/crates/cove-device/src/keychain.rs @@ -317,6 +317,13 @@ impl Keychain { key_ok && xpub_ok && descriptor_ok && tap_signer_ok } + /// Checks whether any wallet-specific keychain entry exists + pub fn wallet_items_exist(&self, id: &WalletId) -> bool { + self.wallet_secrets.has_any(id) + || self.wallet_public.has_any(id) + || matches!(self.get_tap_signer_backup(id), Ok(Some(_)) | Err(_)) + } + fn from_access(access: SharedAccess) -> Self { Self { local_encryption: LocalEncryptionKeyStore::new(access.clone()), diff --git a/rust/crates/cove-device/src/keychain/wallet_public.rs b/rust/crates/cove-device/src/keychain/wallet_public.rs index 33b1e276b..2b2e59e85 100644 --- a/rust/crates/cove-device/src/keychain/wallet_public.rs +++ b/rust/crates/cove-device/src/keychain/wallet_public.rs @@ -78,6 +78,10 @@ impl WalletPublicDataStore { pub(crate) fn delete_descriptors(&self, id: &WalletId) -> bool { delete_if_present(&self.0, descriptor_key_name(id)) } + + pub(crate) fn has_any(&self, id: &WalletId) -> bool { + self.0.get(xpub_key_name(id)).is_some() || self.0.get(descriptor_key_name(id)).is_some() + } } fn delete_if_present(access: &SharedAccess, key: String) -> bool { diff --git a/rust/crates/cove-device/src/keychain/wallet_secrets.rs b/rust/crates/cove-device/src/keychain/wallet_secrets.rs index 72eb867d3..891545354 100644 --- a/rust/crates/cove-device/src/keychain/wallet_secrets.rs +++ b/rust/crates/cove-device/src/keychain/wallet_secrets.rs @@ -238,6 +238,11 @@ impl WalletSecretStore { self.pair(id).delete_existing_value_then_cryptor() } + pub(crate) fn has_any(&self, id: &WalletId) -> bool { + let _guard = self.1.lock(); + !matches!(self.pair(id).state(), State::Empty) + } + fn pair(&self, id: &WalletId) -> EncryptedPair { EncryptedPair::new(self.0.clone(), mnemonic_key_name(id), mnemonic_cryptor_key_name(id)) } diff --git a/rust/src/backup/error.rs b/rust/src/backup/error.rs index 2a1249e7b..f5705abc9 100644 --- a/rust/src/backup/error.rs +++ b/rust/src/backup/error.rs @@ -1,4 +1,5 @@ use crate::wallet_identity::WalletIdentityError; +use cove_types::WalletId; #[derive(Debug, Clone, uniffi::Error, thiserror::Error)] #[uniffi::export(Display)] @@ -46,6 +47,10 @@ pub enum BackupError { #[error("Failed to access database: {0}")] Database(String), + /// The wallet id is already used by local wallet state or restore artifacts + #[error("Wallet id is already occupied: {0}")] + WalletIdOccupied(WalletId), + #[error("Failed to decompress: {0}")] Decompression(String), } diff --git a/rust/src/backup/import.rs b/rust/src/backup/import.rs index 5e10349d1..012b69489 100644 --- a/rust/src/backup/import.rs +++ b/rust/src/backup/import.rs @@ -1,16 +1,23 @@ -use std::{collections::BTreeMap, str::FromStr as _}; +use std::{ + collections::{BTreeMap, HashSet}, + path::PathBuf, + str::FromStr as _, + sync::LazyLock, +}; use bip39::Mnemonic; use cove_device::keychain::{Keychain, WalletSecret as KeychainWalletSecret, WalletXprv}; use cove_types::network::Network; use cove_util::ResultExt as _; +use parking_lot::Mutex; +use strum::IntoEnumIterator as _; use tracing::{error, info, warn}; use zeroize::Zeroizing; use crate::database::global_config::{GlobalConfigKey, GlobalConfigTable, GlobalConfigTableError}; use crate::database::{Database, Error as DatabaseError}; use crate::label_manager::LabelManager; -use crate::wallet::metadata::{WalletId, WalletMetadata, WalletType}; +use crate::wallet::metadata::{WalletId, WalletMetadata, WalletMode, WalletType}; use crate::wallet_identity::{ ExistingWalletIdentitySet, WalletIdentityKey, collect_existing_wallet_identities, fallback_identity_key_for_backup, identity_key_for_backup, @@ -174,6 +181,205 @@ enum RestoreSaveBehavior { SkipCloudBackup, } +static ACTIVE_WALLET_RESTORE_RESERVATIONS: LazyLock>> = + LazyLock::new(|| Mutex::new(HashSet::new())); + +#[derive(Clone, Default)] +struct RestoreArtifactSnapshot { + metadata: bool, + keychain_items: bool, + bdk_paths: HashSet, + wallet_data_paths: HashSet, + wallet_data_occupied: bool, +} + +impl RestoreArtifactSnapshot { + fn capture(metadata: &WalletMetadata) -> Result { + let database = Database::global(); + let mut metadata_present = false; + + for network in Network::iter() { + for mode in WalletMode::iter() { + let wallets = database + .wallets + .get_all(network, mode) + .map_err(|error| BackupError::Database(error.to_string()))?; + + if wallets.iter().any(|wallet| wallet.id == metadata.id) { + metadata_present = true; + } + } + } + + let keychain_items = Keychain::global().wallet_items_exist(&metadata.id); + let bdk_paths = crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id) + .into_iter() + .filter(|path| path.exists()) + .collect(); + let wallet_data_paths = + crate::database::wallet_data::wallet_data_artifact_paths(&metadata.id) + .into_iter() + .filter(|path| path.exists()) + .collect(); + let wallet_data_occupied = + crate::database::wallet_data::wallet_data_artifacts_exist(&metadata.id); + + Ok(Self { + metadata: metadata_present, + keychain_items, + bdk_paths, + wallet_data_paths, + wallet_data_occupied, + }) + } + + fn is_occupied(&self) -> bool { + self.metadata + || self.keychain_items + || !self.bdk_paths.is_empty() + || self.wallet_data_occupied + } +} + +struct WalletRestoreReservation { + id: WalletId, + snapshot: RestoreArtifactSnapshot, +} + +impl WalletRestoreReservation { + fn acquire(metadata: &WalletMetadata) -> Result { + let mut active = ACTIVE_WALLET_RESTORE_RESERVATIONS.lock(); + + if active.contains(&metadata.id) { + return Err(BackupError::WalletIdOccupied(metadata.id.clone())); + } + + let snapshot = RestoreArtifactSnapshot::capture(metadata)?; + if snapshot.is_occupied() { + return Err(BackupError::WalletIdOccupied(metadata.id.clone())); + } + + active.insert(metadata.id.clone()); + + Ok(Self { id: metadata.id.clone(), snapshot }) + } +} + +impl Drop for WalletRestoreReservation { + fn drop(&mut self) { + ACTIVE_WALLET_RESTORE_RESERVATIONS.lock().remove(&self.id); + } +} + +struct RestoreJournal { + metadata: WalletMetadata, + initial: RestoreArtifactSnapshot, +} + +impl RestoreJournal { + fn new(metadata: &WalletMetadata, initial: RestoreArtifactSnapshot) -> Self { + Self { metadata: metadata.clone(), initial } + } + + fn rollback(&self) -> Vec { + let mut failures = Vec::new(); + + self.rollback_keychain(&mut failures); + self.rollback_paths( + crate::bdk_store::BdkStore::wallet_store_artifact_paths(&self.metadata.id), + &self.initial.bdk_paths, + "BDK store", + &mut failures, + ); + self.rollback_paths( + crate::database::wallet_data::wallet_data_artifact_paths(&self.metadata.id), + &self.initial.wallet_data_paths, + "wallet data", + &mut failures, + ); + self.rollback_metadata(&mut failures); + + failures + } + + fn rollback_keychain(&self, failures: &mut Vec) { + if self.initial.keychain_items { + return; + } + + let keychain = Keychain::global(); + if keychain.wallet_items_exist(&self.metadata.id) + && !keychain.delete_wallet_items(&self.metadata.id) + { + failures.push(format!("{}: incomplete keychain deletion", self.metadata.name)); + } + } + + fn rollback_paths( + &self, + paths: impl IntoIterator, + initial: &HashSet, + description: &str, + failures: &mut Vec, + ) { + let mut paths = paths.into_iter().collect::>(); + paths.sort_by_key(|path| std::cmp::Reverse(path.components().count())); + + for path in paths { + if initial.contains(&path) || !path.exists() { + continue; + } + + let result = if path.is_dir() { + std::fs::remove_dir(&path) + } else { + std::fs::remove_file(&path) + }; + + if let Err(error) = result + && error.kind() != std::io::ErrorKind::NotFound + { + failures.push(format!( + "{}: failed to delete {description} {}: {error}", + self.metadata.name, + path.display() + )); + } + } + } + + fn rollback_metadata(&self, failures: &mut Vec) { + if self.initial.metadata { + return; + } + + let database = Database::global(); + match database.wallets.get_all(self.metadata.network, self.metadata.wallet_mode) { + Ok(mut wallets) => { + let before = wallets.len(); + wallets.retain(|wallet| wallet.id != self.metadata.id); + + if wallets.len() < before + && let Err(error) = database.wallets.save_all_wallets( + self.metadata.network, + self.metadata.wallet_mode, + wallets, + ) + { + failures.push(format!( + "{}: failed to delete metadata: {error}", + self.metadata.name + )); + } + } + Err(error) => failures.push(format!( + "{}: failed to read wallets for cleanup: {error}", + self.metadata.name + )), + } + } +} + #[derive(Clone)] struct RestoredWalletMetadataStore(Database); @@ -310,10 +516,11 @@ fn with_cleanup(metadata: &WalletMetadata, f: F) -> Result<(), (BackupError, where F: FnOnce() -> Result<(), BackupError>, { - f().map_err(|e| { - let cleanup_failures = cleanup_failed_wallet(metadata); - (e, cleanup_failures) - }) + let reservation = + WalletRestoreReservation::acquire(metadata).map_err(|error| (error, Vec::new()))?; + let journal = RestoreJournal::new(metadata, reservation.snapshot.clone()); + + f().map_err(|error| (error, journal.rollback())) } pub(crate) fn restore_mnemonic_wallet( @@ -492,43 +699,6 @@ fn restore_descriptor_wallet_inner( Ok(()) } -/// Clean up a partially-imported wallet on failure -/// -/// Returns a list of cleanup failures; empty means fully cleaned -pub(crate) fn cleanup_failed_wallet(metadata: &WalletMetadata) -> Vec { - let wallet_id = &metadata.id; - let name = &metadata.name; - let mut failures = Vec::new(); - - let keychain_ok = Keychain::global().delete_wallet_items(wallet_id); - if !keychain_ok { - failures.push(format!("{name}: incomplete keychain deletion")); - } - - if let Err(e) = crate::wallet::delete_wallet_specific_data(wallet_id) { - failures.push(format!("{name}: failed to delete wallet data: {e}")); - } - - let db = Database::global(); - match db.wallets.get_all(metadata.network, metadata.wallet_mode) { - Ok(mut wallets) => { - let before = wallets.len(); - wallets.retain(|w| w.id != *wallet_id); - if wallets.len() < before - && let Err(e) = - db.wallets.save_all_wallets(metadata.network, metadata.wallet_mode, wallets) - { - failures.push(format!("{name}: failed to delete metadata: {e}")); - } - } - Err(e) => { - failures.push(format!("{name}: failed to read wallets for cleanup: {e}")); - } - } - - failures -} - fn import_labels(id: &WalletId, jsonl: &str) -> Result<(), BackupError> { let manager = LabelManager::new(id.clone()); manager.import(jsonl).map_err(|e| BackupError::Restore(e.to_string())) @@ -827,4 +997,47 @@ mod tests { Err(error) => panic!("expected duplicate skip, got {}", error.error), } } + + #[test] + fn wallet_id_reservation_rejects_existing_keychain_items() { + let _guard = crate::test_support::global_state_test_lock().blocking_lock(); + crate::database::test_support::delete_database(); + crate::test_support::init_test_keychain(); + crate::test_support::shared_mock_keychain().reset(); + + let mut metadata = hot_metadata("Occupied wallet"); + metadata.id = WalletId::preview_new_random(); + let xpub = bdk_wallet::bitcoin::bip32::Xpub::from_str( + "xpub6CiKnWv7PPyyeb4kCwK4fidKqVjPfD9TP6MiXnzBVGZYNanNdY3mMvywcrdDc6wK82jyBSd95vsk26QujnJWPrSaPfYeyW7NyX37HHGtfQM", + ) + .unwrap(); + Keychain::global().save_wallet_xpub(&metadata.id, xpub).unwrap(); + + let result = WalletRestoreReservation::acquire(&metadata); + + assert!(matches!(result, Err(BackupError::WalletIdOccupied(id)) if id == metadata.id)); + assert_eq!(Keychain::global().get_wallet_xpub(&metadata.id).unwrap(), Some(xpub)); + } + + #[test] + fn restore_journal_preserves_preexisting_bdk_artifact() { + let _guard = crate::test_support::global_state_test_lock().blocking_lock(); + let mut metadata = hot_metadata("Existing BDK artifact"); + metadata.id = WalletId::preview_new_random(); + let artifact = + crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id)[2].clone(); + std::fs::write(&artifact, b"pre-existing WAL").unwrap(); + + let initial = RestoreArtifactSnapshot { + metadata: true, + keychain_items: true, + bdk_paths: HashSet::from([artifact.clone()]), + ..RestoreArtifactSnapshot::default() + }; + let journal = RestoreJournal::new(&metadata, initial); + + assert!(journal.rollback().is_empty()); + assert!(artifact.exists()); + std::fs::remove_file(artifact).unwrap(); + } } diff --git a/rust/src/bdk_store.rs b/rust/src/bdk_store.rs index 9e19bf3cf..d64c59b43 100644 --- a/rust/src/bdk_store.rs +++ b/rust/src/bdk_store.rs @@ -18,9 +18,17 @@ pub struct BdkStore { id: WalletId, network: Network, pub conn: bdk_wallet::rusqlite::Connection, + storage: BdkStoreStorage, +} + +#[derive(Debug, Clone, Copy)] +pub(crate) enum BdkStoreStorage { + Persistent, + InMemory, } impl BdkStore { + /// Open the persistent BDK wallet store for a real wallet pub fn try_new(id: &WalletId, network: impl Into) -> Result { crate::bootstrap::ensure_storage_bootstrapped() .map_err(|e| eyre::eyre!("storage bootstrap failed: {e}"))?; @@ -51,7 +59,12 @@ impl BdkStore { // in pages (4096 bytes) 2000 pages = 8MB conn.pragma_update(None, "cache_size", 2000)?; - let mut me = Self { id: id.clone(), network: network.into(), conn }; + let mut me = Self { + id: id.clone(), + network: network.into(), + conn, + storage: BdkStoreStorage::Persistent, + }; if let Err(e) = me.check_and_migrate_from_file_store() { tracing::error!("{id} failed to migrate from file store: {e:?}"); @@ -61,6 +74,19 @@ impl BdkStore { Ok(me) } + /// Open an in-memory BDK wallet store for ephemeral wallet views + pub fn in_memory(id: &WalletId, network: impl Into) -> Result { + let network = network.into(); + let conn = bdk_wallet::rusqlite::Connection::open_in_memory() + .context("unable to open in-memory rusqlite connection")?; + + Ok(Self { id: id.clone(), network, conn, storage: BdkStoreStorage::InMemory }) + } + + pub(crate) const fn is_in_memory(&self) -> bool { + matches!(self.storage, BdkStoreStorage::InMemory) + } + // check if we have a file store // if we do, migrate to the new SQLite store fn check_and_migrate_from_file_store(&mut self) -> Result { @@ -158,6 +184,18 @@ impl BdkStore { Ok(()) } + + pub(crate) fn wallet_store_artifact_paths(wallet_id: &WalletId) -> [PathBuf; 5] { + let sqlite_data_path = sqlite_data_path(wallet_id); + + [ + file_store_data_path(wallet_id), + sqlite_data_path.clone(), + sqlite_auxiliary_path(&sqlite_data_path, "wal"), + sqlite_auxiliary_path(&sqlite_data_path, "shm"), + sqlite_data_path.with_extension("db-journal"), + ] + } } fn remove_sqlite_auxiliary_files(db_path: &Path) { diff --git a/rust/src/database/wallet_data.rs b/rust/src/database/wallet_data.rs index ae1ca932c..87e5e008f 100644 --- a/rust/src/database/wallet_data.rs +++ b/rust/src/database/wallet_data.rs @@ -125,6 +125,13 @@ pub struct WalletDataDb { pub id: WalletId, pub db: Arc, pub labels: LabelsTable, + storage: WalletDataStorage, +} + +#[derive(Debug, Clone, Copy)] +pub(crate) enum WalletDataStorage { + Persistent, + InMemory, } #[derive(Debug, thiserror::Error, uniffi::Error)] @@ -155,8 +162,28 @@ impl WalletDataDb { Self::new_with_db_location(id, &WALLET_DATA_DIR) } + /// Creates an ephemeral wallet-data database that never touches the wallet-data directory + pub(crate) fn new_in_memory(id: WalletId) -> Result { + let db = redb::Database::builder() + .create_with_backend(redb::backends::InMemoryBackend::new()) + .map_err(|error| WalletDataError::DatabaseAccess { + id: id.clone(), + error: error.to_string(), + })?; + + Self::new_with_db(id, Arc::new(db), WalletDataStorage::InMemory) + } + fn new_with_db_location(id: WalletId, db_location: &Path) -> Result { let db = get_or_create_database(&id, db_location)?; + Self::new_with_db(id, db, WalletDataStorage::Persistent) + } + + fn new_with_db( + id: WalletId, + db: Arc, + storage: WalletDataStorage, + ) -> Result { let write_txn = db.begin_write().map_err(|e| WalletDataError::DatabaseAccess { id: id.clone(), error: e.to_string(), @@ -174,7 +201,11 @@ impl WalletDataDb { error: e.to_string(), })?; - Ok(Self { id, db, labels }) + Ok(Self { id, db, labels, storage }) + } + + pub(crate) const fn is_in_memory(&self) -> bool { + matches!(self.storage, WalletDataStorage::InMemory) } pub fn get_scan_state(&self, address_type: WalletAddressType) -> Result> { @@ -349,6 +380,27 @@ pub fn delete_database(id: &WalletId) -> Result<(), std::io::Error> { delete_database_at_location(id, &WALLET_DATA_DIR) } +pub(crate) fn wallet_data_artifact_paths(id: &WalletId) -> Vec { + let directory = WALLET_DATA_DIR.join(id.as_str()); + let mut paths = vec![directory.clone()]; + + let Ok(entries) = std::fs::read_dir(&directory) else { + return paths; + }; + + paths.extend(entries.filter_map(std::result::Result::ok).map(|entry| entry.path())); + paths +} + +pub(crate) fn wallet_data_artifacts_exist(id: &WalletId) -> bool { + let directory = WALLET_DATA_DIR.join(id.as_str()); + if directory.is_file() { + return true; + } + + std::fs::read_dir(directory).is_ok_and(|mut entries| entries.next().is_some()) +} + /// Drop all cached wallet data connections and open locks pub fn clear_database_connections() { DATABASE_CONNECTIONS.write().clear(); diff --git a/rust/src/database/wallet_data/label.rs b/rust/src/database/wallet_data/label.rs index 455892db1..8f86fe699 100644 --- a/rust/src/database/wallet_data/label.rs +++ b/rust/src/database/wallet_data/label.rs @@ -569,7 +569,10 @@ pub(crate) mod test_support { use redb::TableDefinition; use super::LabelsTable; - use crate::{database::wallet_data::WalletDataDb, wallet::metadata::WalletId}; + use crate::{ + database::wallet_data::{WalletDataDb, WalletDataStorage}, + wallet::metadata::WalletId, + }; const MISMATCHED_OUTPUT_TABLE: TableDefinition<&'static str, &'static str> = TableDefinition::new("output_records_v2.cbor"); @@ -589,7 +592,7 @@ pub(crate) mod test_support { write_txn.commit().expect("failed to commit write transaction"); let labels = LabelsTable { db: db.clone() }; - let wallet_db = WalletDataDb { id, db, labels }; + let wallet_db = WalletDataDb { id, db, labels, storage: WalletDataStorage::Persistent }; (wallet_db, tmp) } diff --git a/rust/src/label_manager.rs b/rust/src/label_manager.rs index f410994c5..265e831e4 100644 --- a/rust/src/label_manager.rs +++ b/rust/src/label_manager.rs @@ -262,6 +262,10 @@ impl LabelManager { } impl LabelManager { + pub(crate) fn try_new_with_db(db: WalletDataDb) -> Self { + Self { db } + } + pub fn try_new( id: WalletId, ) -> std::result::Result { @@ -376,6 +380,10 @@ impl LabelManager { } fn mark_cloud_backup_dirty(&self) { + if self.db.is_in_memory() { + return; + } + CLOUD_BACKUP_MANAGER.handle_wallet_backup_change(self.db.id.clone()); } diff --git a/rust/src/manager/cloud_backup_manager/error.rs b/rust/src/manager/cloud_backup_manager/error.rs index 611e25b2a..c8aaf1c54 100644 --- a/rust/src/manager/cloud_backup_manager/error.rs +++ b/rust/src/manager/cloud_backup_manager/error.rs @@ -504,6 +504,12 @@ impl From for CloudBackupError { } } +impl From for CloudBackupInternalError { + fn from(error: crate::backup::BackupError) -> Self { + Self(CloudBackupErrorSource::source(error)) + } +} + impl From for CloudBackupError { fn from(error: serde_json::Error) -> Self { Self::internal(error) diff --git a/rust/src/manager/cloud_backup_manager/ops/test_support.rs b/rust/src/manager/cloud_backup_manager/ops/test_support.rs index d52fd9eb5..caa895323 100644 --- a/rust/src/manager/cloud_backup_manager/ops/test_support.rs +++ b/rust/src/manager/cloud_backup_manager/ops/test_support.rs @@ -1338,6 +1338,22 @@ pub(crate) async fn encrypted_wallet_backup_bytes( serde_json::to_vec(&encrypted).unwrap() } +pub(crate) async fn encrypted_remote_wallet_backup_bytes( + metadata: &WalletMetadata, + master_key: &cove_cspp::master_key::MasterKey, + revision_hash: &str, + version: u32, +) -> Vec { + let bytes = encrypted_wallet_backup_bytes(metadata, master_key, revision_hash, version).await; + + assert!(Keychain::global().delete_wallet_items(&metadata.id)); + assert!(!Keychain::global().wallet_items_exist(&metadata.id)); + crate::wallet::delete_wallet_specific_data(&metadata.id) + .expect("remote restore fixture has no local wallet data"); + + bytes +} + pub(crate) fn wallet_entry_with_labels( metadata: &WalletMetadata, labels_jsonl: Option<&str>, diff --git a/rust/src/manager/cloud_backup_manager/ops/tests/enable.rs b/rust/src/manager/cloud_backup_manager/ops/tests/enable.rs index 5e82a6623..a35ad541b 100644 --- a/rust/src/manager/cloud_backup_manager/ops/tests/enable.rs +++ b/rust/src/manager/cloud_backup_manager/ops/tests/enable.rs @@ -525,18 +525,25 @@ async fn enable_with_multiple_matching_namespaces_merges_into_largest_namespace( globals.cloud.set_wallet_backup( first_namespace.clone(), first_record_id.clone(), - encrypted_wallet_backup_bytes(&first_wallet, &first_master_key, &first_revision, 1).await, + encrypted_remote_wallet_backup_bytes(&first_wallet, &first_master_key, &first_revision, 1) + .await, ); globals.cloud.set_wallet_backup( second_namespace.clone(), second_record_id.clone(), - encrypted_wallet_backup_bytes(&second_wallet, &second_master_key, &second_revision, 1) - .await, + encrypted_remote_wallet_backup_bytes( + &second_wallet, + &second_master_key, + &second_revision, + 1, + ) + .await, ); globals.cloud.set_wallet_backup( second_namespace.clone(), third_record_id.clone(), - encrypted_wallet_backup_bytes(&third_wallet, &second_master_key, &third_revision, 1).await, + encrypted_remote_wallet_backup_bytes(&third_wallet, &second_master_key, &third_revision, 1) + .await, ); globals.cloud.set_wallet_files( first_namespace.clone(), diff --git a/rust/src/manager/cloud_backup_manager/ops/tests/other_backups.rs b/rust/src/manager/cloud_backup_manager/ops/tests/other_backups.rs index c33338006..0d578e89b 100644 --- a/rust/src/manager/cloud_backup_manager/ops/tests/other_backups.rs +++ b/rust/src/manager/cloud_backup_manager/ops/tests/other_backups.rs @@ -224,7 +224,7 @@ async fn recover_other_backups_keeps_current_passkey_metadata() { globals.cloud.set_wallet_backup( other_namespace.clone(), record_id.clone(), - encrypted_wallet_backup_bytes(&wallet, &other_master_key, "other-revision", 1).await, + encrypted_remote_wallet_backup_bytes(&wallet, &other_master_key, "other-revision", 1).await, ); globals.cloud.set_wallet_files( other_namespace.clone(), @@ -342,8 +342,13 @@ async fn recover_other_backups_keeps_partially_moved_namespace() { globals.cloud.set_wallet_backup( other_namespace.clone(), restored_record_id.clone(), - encrypted_wallet_backup_bytes(&restored_wallet, &other_master_key, "other-revision", 1) - .await, + encrypted_remote_wallet_backup_bytes( + &restored_wallet, + &other_master_key, + "other-revision", + 1, + ) + .await, ); globals.cloud.set_wallet_files( other_namespace.clone(), @@ -395,7 +400,7 @@ async fn recover_other_backups_keeps_namespace_when_current_upload_fails() { globals.cloud.set_wallet_backup( other_namespace.clone(), record_id.clone(), - encrypted_wallet_backup_bytes(&wallet, &other_master_key, "other-revision", 1).await, + encrypted_remote_wallet_backup_bytes(&wallet, &other_master_key, "other-revision", 1).await, ); globals.cloud.set_wallet_files( other_namespace.clone(), diff --git a/rust/src/manager/cloud_backup_manager/ops/tests/restore.rs b/rust/src/manager/cloud_backup_manager/ops/tests/restore.rs index 6738cf20f..79fdb2120 100644 --- a/rust/src/manager/cloud_backup_manager/ops/tests/restore.rs +++ b/rust/src/manager/cloud_backup_manager/ops/tests/restore.rs @@ -151,14 +151,24 @@ async fn restore_counts_unsupported_wallet_versions_as_failures() { globals.cloud.set_wallet_backup( namespace.clone(), supported_record_id.clone(), - encrypted_wallet_backup_bytes(&supported_wallet, &master_key, "supported-revision", 2) - .await, + encrypted_remote_wallet_backup_bytes( + &supported_wallet, + &master_key, + "supported-revision", + 2, + ) + .await, ); globals.cloud.set_wallet_backup( namespace.clone(), unsupported_record_id.clone(), - encrypted_wallet_backup_bytes(&unsupported_wallet, &master_key, "unsupported-revision", 3) - .await, + encrypted_remote_wallet_backup_bytes( + &unsupported_wallet, + &master_key, + "unsupported-revision", + 3, + ) + .await, ); globals.cloud.set_wallet_files( namespace, @@ -220,7 +230,7 @@ async fn restore_queues_reupload_when_cloud_upload_confirmation_lags() { globals.cloud.set_wallet_backup( namespace.clone(), record_id.clone(), - encrypted_wallet_backup_bytes(&wallet, &master_key, "restored-revision", 1).await, + encrypted_remote_wallet_backup_bytes(&wallet, &master_key, "restored-revision", 1).await, ); globals.cloud.set_wallet_files(namespace, vec![wallet_filename_from_record_id(&record_id)]); globals.cloud.set_uploaded_wallets_pending_confirmation(true); @@ -306,13 +316,19 @@ async fn restore_with_one_passkey_restores_wallets_from_all_matching_namespaces( globals.cloud.set_wallet_backup( first_namespace.clone(), first_record_id.clone(), - encrypted_wallet_backup_bytes(&first_wallet, &first_master_key, "first-revision", 1).await, + encrypted_remote_wallet_backup_bytes(&first_wallet, &first_master_key, "first-revision", 1) + .await, ); globals.cloud.set_wallet_backup( second_namespace.clone(), second_record_id.clone(), - encrypted_wallet_backup_bytes(&second_wallet, &second_master_key, "second-revision", 1) - .await, + encrypted_remote_wallet_backup_bytes( + &second_wallet, + &second_master_key, + "second-revision", + 1, + ) + .await, ); globals .cloud @@ -397,7 +413,7 @@ async fn restore_activation_upload_failure_keeps_restore_successful_and_queues_u globals.cloud.set_wallet_backup( namespace.clone(), record_id.clone(), - encrypted_wallet_backup_bytes(&wallet, &master_key, "restore-revision", 1).await, + encrypted_remote_wallet_backup_bytes(&wallet, &master_key, "restore-revision", 1).await, ); globals .cloud @@ -683,7 +699,7 @@ async fn restore_refresh_finds_namespace_that_appears_late() { Keychain::global().save_wallet_xpub(&wallet.id, sample_xpub(&wallet).parse().unwrap()).unwrap(); let record_id = cove_cspp::backup_data::wallet_record_id(wallet.id.as_ref()); let encrypted_wallet = - encrypted_wallet_backup_bytes(&wallet, &master_key, "late-revision", 1).await; + encrypted_remote_wallet_backup_bytes(&wallet, &master_key, "late-revision", 1).await; globals.passkey.set_discover_result(Ok(DiscoveredPasskeyResult { prf_output: prf_key.to_vec(), credential_id: vec![1, 2, 3], @@ -753,10 +769,15 @@ async fn restore_grace_window_accumulates_a_namespace_that_appears_after_the_fir let first_record_id = wallet_record_id(first_wallet.id.as_ref()); let second_record_id = wallet_record_id(second_wallet.id.as_ref()); let first_encrypted_wallet = - encrypted_wallet_backup_bytes(&first_wallet, &first_master_key, "first-revision", 1).await; - let second_encrypted_wallet = - encrypted_wallet_backup_bytes(&second_wallet, &second_master_key, "second-revision", 1) + encrypted_remote_wallet_backup_bytes(&first_wallet, &first_master_key, "first-revision", 1) .await; + let second_encrypted_wallet = encrypted_remote_wallet_backup_bytes( + &second_wallet, + &second_master_key, + "second-revision", + 1, + ) + .await; globals.cloud.set_master_key_backup( first_namespace.clone(), @@ -852,7 +873,7 @@ async fn restore_retries_platform_authorization_discover_failures() { globals.cloud.set_wallet_backup( namespace.clone(), record_id.clone(), - encrypted_wallet_backup_bytes(&wallet, &master_key, "revision", 1).await, + encrypted_remote_wallet_backup_bytes(&wallet, &master_key, "revision", 1).await, ); globals.cloud.set_wallet_files(namespace, vec![wallet_filename_from_record_id(&record_id)]); @@ -1017,8 +1038,13 @@ async fn restore_counts_listed_missing_wallet_backups_as_failures() { globals.cloud.set_wallet_backup( namespace.clone(), supported_record_id.clone(), - encrypted_wallet_backup_bytes(&supported_wallet, &master_key, "supported-revision", 1) - .await, + encrypted_remote_wallet_backup_bytes( + &supported_wallet, + &master_key, + "supported-revision", + 1, + ) + .await, ); globals.cloud.set_wallet_files( namespace, @@ -1067,8 +1093,13 @@ async fn restore_sanitizes_non_connectivity_wallet_download_errors() { globals.cloud.set_wallet_backup( namespace.clone(), supported_record_id.clone(), - encrypted_wallet_backup_bytes(&supported_wallet, &master_key, "supported-revision", 1) - .await, + encrypted_remote_wallet_backup_bytes( + &supported_wallet, + &master_key, + "supported-revision", + 1, + ) + .await, ); globals.cloud.fail_wallet_backup_download( namespace.clone(), @@ -1228,6 +1259,8 @@ async fn restore_all_matches_individual_restore_for_the_same_record() { .save_all_wallets(wallet.network, wallet.wallet_mode, Vec::new()) .unwrap(); assert!(Keychain::global().delete_wallet_items(&wallet.id)); + crate::wallet::delete_wallet_specific_data(&wallet.id) + .expect("individual restore artifacts are removed before the batch restore"); let mut prepared = manager .prepare_restore_all_cloud_wallets(vec![frozen_restore_all_wallet( diff --git a/rust/src/manager/wallet_manager.rs b/rust/src/manager/wallet_manager.rs index 4810b5b97..568148662 100644 --- a/rust/src/manager/wallet_manager.rs +++ b/rust/src/manager/wallet_manager.rs @@ -33,7 +33,7 @@ use cove_util::result_ext::ResultExt as _; use crate::{ app::FfiApp, converter::{Converter, ConverterError}, - database::{Database, error::DatabaseError}, + database::{Database, error::DatabaseError, wallet_data::WalletDataDb}, discovery_scanner::{ScannerResponse, WalletDiscoveryScanner}, fee_client::{FEE_CLIENT, FEES, FeeResponse}, fiat::client::PriceResponse, @@ -249,6 +249,10 @@ impl WalletBootstrapUnsignedTransactions { Self::InMemory(unsigned_transactions) } + pub(crate) const fn is_in_memory(&self) -> bool { + matches!(self, Self::InMemory(_)) + } + fn load(&self) -> Result>, Error> { match self { Self::Database(wallet_id) => unsigned_transactions_for_wallet(wallet_id), @@ -291,6 +295,9 @@ pub enum WalletManagerError { #[error("wallet does not exist")] WalletDoesNotExist, + #[error("operation is unavailable for a preview wallet")] + PreviewOperationUnavailable, + #[error("unable to retrieve the secret words for the wallet {0}")] SecretRetrievalError(#[from] KeychainError), @@ -857,6 +864,10 @@ impl RustWalletManager { #[uniffi::method] pub fn non_default_account_number(&self) -> Option { + if !self.uses_persistent_storage() { + return None; + } + wallet_account_number(&self.id).filter(|account| *account != 0) } @@ -1172,12 +1183,17 @@ impl RustWalletManager { } } - let candidate = match Database::global().wallets.update_wallet_metadata(candidate.clone()) { - Ok(candidate) => candidate, - Err(error) => { - error!("Unable to update wallet metadata: {error:?}"); - return; + let uses_persistent_storage = self.uses_persistent_storage(); + let candidate = if uses_persistent_storage { + match Database::global().wallets.update_wallet_metadata(candidate.clone()) { + Ok(candidate) => candidate, + Err(error) => { + error!("Unable to update wallet metadata: {error:?}"); + return; + } } + } else { + candidate }; *self.metadata.write() = candidate.clone(); @@ -1186,7 +1202,9 @@ impl RustWalletManager { self.reconciler.send(Message::LedgerStateChanged( WalletLedgerState::from_metadata_and_scan_status(&candidate, &scan_status), )); - CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &candidate); + if uses_persistent_storage { + CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &candidate); + } } pub fn shutdown(&self) { @@ -1203,12 +1221,20 @@ impl RustWalletManager { } impl RustWalletManager { + fn uses_persistent_storage(&self) -> bool { + !self.unsigned_transactions.is_in_memory() + } + fn current_scan_status(&self) -> WalletScanStatus { self.scan_status.read().clone() } fn current_metadata(&self) -> WalletMetadata { let cached_metadata = self.metadata.read().clone(); + if !self.uses_persistent_storage() { + return cached_metadata; + } + let database_metadata = Database::global().wallets().get( &self.id, cached_metadata.network, @@ -1247,6 +1273,10 @@ impl RustWalletManager { } fn refresh_metadata_from_database(&self) -> Result { + if !self.uses_persistent_storage() { + return Ok(self.metadata.read().clone()); + } + let before_metadata = self.metadata.read().clone(); let metadata = Database::global() .wallets() @@ -1288,17 +1318,20 @@ impl RustWalletManager { let channel = ReconcileChannel::new(100); let wallet = Wallet::preview_new_wallet_with_metadata(metadata.clone()); - let label_manager = LabelManager::new(wallet.metadata.id.clone()).into(); + let wallet_data_db = WalletDataDb::new_in_memory(wallet.metadata.id.clone()) + .expect("failed to open in-memory wallet data database for preview wallet"); + let label_manager = LabelManager::try_new_with_db(wallet_data_db.clone()).into(); let wallet_snapshot = Arc::new(RwLock::new(WalletSnapshot::from_wallet(&wallet))); let unsigned_transactions = WalletBootstrapUnsignedTransactions::in_memory(Vec::new()); let scan_status = Arc::new(RwLock::new(WalletScanStatus::Idle)); - let wallet_actor = WalletActor::new( + let wallet_actor = WalletActor::new_with_db( wallet, channel.raw_sender(), scan_status.clone(), wallet_snapshot.clone(), + wallet_data_db, ) - .expect("failed to open wallet database for preview wallet"); + .expect("failed to open in-memory wallet data database for preview wallet"); let actor = task::spawn_actor(wallet_actor); Self { @@ -1386,9 +1419,9 @@ mod tests { use bitcoin::Amount; use super::{ - Balance, BalancePresentation, Error, PREVIEW_FULL_SCAN_COMPLETED_AT, WalletLedgerState, - WalletLoadState, WalletManagerError, WalletScanPhase, WalletScanProgress, WalletScanStatus, - WalletSnapshot, initial_state_from_snapshot, + Balance, BalancePresentation, Error, PREVIEW_FULL_SCAN_COMPLETED_AT, RustWalletManager, + WalletLedgerState, WalletLoadState, WalletManagerError, WalletScanPhase, + WalletScanProgress, WalletScanStatus, WalletSnapshot, initial_state_from_snapshot, initial_state_from_snapshot_with_pending_unsigned_transactions, ledger_state, preview_ledger_ready_metadata, }; @@ -1416,6 +1449,50 @@ mod tests { })) } + #[test] + fn preview_manager_construction_has_no_persistent_side_effects() { + crate::test_support::ensure_tokio_runtime(); + + let metadata = WalletMetadata::preview_new(); + let bdk_paths = crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id); + let wallet_data_paths = + crate::database::wallet_data::wallet_data_artifact_paths(&metadata.id); + + let manager = RustWalletManager::preview_new_wallet_with_metadata(metadata); + + assert!(manager.get_unsigned_transactions().unwrap().is_empty()); + assert_eq!(manager.non_default_account_number(), None); + assert!(bdk_paths.iter().all(|path| !path.exists())); + assert!(wallet_data_paths.iter().all(|path| !path.exists())); + } + + #[tokio::test] + async fn preview_manager_rejects_xpub_export_without_persistent_side_effects() { + crate::test_support::ensure_tokio_runtime(); + + let metadata = WalletMetadata::preview_new(); + let bdk_paths = crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id); + let wallet_data_paths = + crate::database::wallet_data::wallet_data_artifact_paths(&metadata.id); + let manager = RustWalletManager::preview_new_wallet_with_metadata(metadata); + + assert!(matches!( + manager.export_xpub_for_share().await, + Err(Error::PreviewOperationUnavailable) + )); + assert!(bdk_paths.iter().all(|path| !path.exists())); + assert!(wallet_data_paths.iter().all(|path| !path.exists())); + } + + #[test] + fn preview_manager_rejects_wallet_deletion() { + crate::test_support::ensure_tokio_runtime(); + + let manager = RustWalletManager::preview_new_wallet(); + + assert_eq!(manager.delete_wallet(), Err(Error::PreviewOperationUnavailable)); + } + #[test] fn preview_wallet_metadata_is_ledger_ready_for_spend() { let metadata = preview_ledger_ready_metadata(WalletMetadata::preview_new()); diff --git a/rust/src/manager/wallet_manager/actor.rs b/rust/src/manager/wallet_manager/actor.rs index 7c068724f..4b59164a7 100644 --- a/rust/src/manager/wallet_manager/actor.rs +++ b/rust/src/manager/wallet_manager/actor.rs @@ -157,6 +157,17 @@ impl WalletActor { wallet_snapshot: Arc>, ) -> Result { let db = WalletDataDb::new_or_existing(wallet.id.clone())?; + + Self::new_with_db(wallet, reconciler, scan_status, wallet_snapshot, db) + } + + pub(crate) fn new_with_db( + wallet: Wallet, + reconciler: Sender, + scan_status: Arc>, + wallet_snapshot: Arc>, + db: WalletDataDb, + ) -> Result { let seed = rand::rng().random(); Ok(Self { @@ -243,7 +254,9 @@ impl WalletActor { let sent_and_received = self.wallet.bdk.sent_and_received(&tx.tx_node.tx).into(); (tx, sent_and_received) }) - .map(|(tx, sent_and_received)| Transaction::new(&self.wallet.id, sent_and_received, tx)) + .map(|(tx, sent_and_received)| { + Transaction::new_with_labels(sent_and_received, tx, &self.db.labels) + }) .filter(|tx| tx.sent_and_received().amount() > zero) .inspect(|tx| { if let Transaction::Unconfirmed(unconfirmed) = &tx { @@ -816,6 +829,10 @@ impl WalletActor { // the balance is not updated after the second full scan if I don't reload // the wallet from the file storage fn reload_wallet(&mut self) { + if !self.wallet.uses_persistent_storage() { + return; + } + match Wallet::try_load_persisted(self.wallet.id.clone()) { Ok(wallet) => self.wallet = wallet, Err(error) => error!("failed to reload wallet: {error:?}"), @@ -842,6 +859,11 @@ impl WalletActor { let now = UNIX_EPOCH.elapsed().unwrap_or_default(); self.last_scan_finished = Some(now); + if !self.wallet.uses_persistent_storage() { + self.wallet.metadata.internal.last_scan_finished = Some(now); + return Some(()); + } + let wallets = Database::global().wallets(); let mut metadata = wallets @@ -856,6 +878,11 @@ impl WalletActor { } fn record_full_scan_performed(&mut self, completed_at: u64) -> Result { + if !self.wallet.uses_persistent_storage() { + self.wallet.metadata.internal.performed_full_scan_at = Some(completed_at); + return Ok(self.wallet.metadata.clone()); + } + let wallets = Database::global().wallets(); let current_metadata = wallets .get(&self.wallet.id, self.wallet.network, self.wallet.metadata.wallet_mode) @@ -1436,10 +1463,13 @@ mod tests { crate::test_support::ensure_tokio_runtime(); test_keychain(); - let wallet = Wallet::preview_new_wallet_with_metadata(metadata.clone()); + let _ = crate::wallet::delete_wallet_specific_data(&metadata.id); + let wallet = + Wallet::try_new_persisted_from_mnemonic_segwit(metadata, test_mnemonic(), None) + .expect("test wallet is persisted"); crate::database::Database::global() .wallets - .save_new_wallet_metadata(metadata) + .save_new_wallet_metadata(wallet.metadata.clone()) .expect("wallet metadata is persisted"); wallet diff --git a/rust/src/manager/wallet_manager/actor/node.rs b/rust/src/manager/wallet_manager/actor/node.rs index 2db8bcba3..e36cbfbd6 100644 --- a/rust/src/manager/wallet_manager/actor/node.rs +++ b/rust/src/manager/wallet_manager/actor/node.rs @@ -344,6 +344,12 @@ impl WalletActor { let now = std::time::UNIX_EPOCH.elapsed().unwrap_or_default(); self.last_height_fetched = Some((now, block_height)); + if !self.wallet.uses_persistent_storage() { + self.wallet.metadata.internal.last_height_fetched = + Some(BlockSizeLast { block_height: block_height as u64, last_seen: now }); + return Some(()); + } + let wallets = Database::global().wallets(); let mut metadata = wallets diff --git a/rust/src/manager/wallet_manager/exports.rs b/rust/src/manager/wallet_manager/exports.rs index 225d301fc..07f097a10 100644 --- a/rust/src/manager/wallet_manager/exports.rs +++ b/rust/src/manager/wallet_manager/exports.rs @@ -60,6 +60,10 @@ impl RustWalletManager { /// Export public descriptors (xpub) for share #[uniffi::method] pub async fn export_xpub_for_share(&self) -> Result { + if !self.uses_persistent_storage() { + return Err(Error::PreviewOperationUnavailable); + } + let id = self.id.clone(); let name = self.metadata.read().name.clone(); @@ -84,6 +88,10 @@ impl RustWalletManager { /// Export public descriptors (xpub) as QR codes #[uniffi::method] pub async fn export_xpub_for_qr(&self, density: Arc) -> Result, Error> { + if !self.uses_persistent_storage() { + return Err(Error::PreviewOperationUnavailable); + } + use bbqr::{ encode::Encoding, file_type::FileType, diff --git a/rust/src/manager/wallet_manager/unsigned_transactions.rs b/rust/src/manager/wallet_manager/unsigned_transactions.rs index cd50e16a2..6032db7e4 100644 --- a/rust/src/manager/wallet_manager/unsigned_transactions.rs +++ b/rust/src/manager/wallet_manager/unsigned_transactions.rs @@ -16,6 +16,10 @@ impl RustWalletManager { &self, details: Arc, ) -> Result<(), Error> { + if self.unsigned_transactions.is_in_memory() { + return Ok(()); + } + let wallet_id = self.id.clone(); let tx_id = details.psbt.tx_id(); let db = Database::global(); @@ -48,6 +52,10 @@ impl RustWalletManager { pub(crate) fn get_unsigned_transactions_internal( &self, ) -> Result>, Error> { + if self.unsigned_transactions.is_in_memory() { + return Ok(Vec::new()); + } + let wallet_id = &self.id; let db = Database::global(); @@ -65,6 +73,10 @@ impl RustWalletManager { &self, tx_id: Arc, ) -> Result<(), Error> { + if self.unsigned_transactions.is_in_memory() { + return Ok(()); + } + debug!("deleting unsigned transaction: {tx_id:?}"); let db = Database::global(); diff --git a/rust/src/manager/wallet_manager/wallet_admin.rs b/rust/src/manager/wallet_manager/wallet_admin.rs index aede86327..3a1f0d7f1 100644 --- a/rust/src/manager/wallet_manager/wallet_admin.rs +++ b/rust/src/manager/wallet_manager/wallet_admin.rs @@ -20,6 +20,10 @@ use super::{Error, Message, RustWalletManager}; impl RustWalletManager { pub(crate) fn delete_wallet_internal(&self) -> Result<(), Error> { + if !self.uses_persistent_storage() { + return Err(Error::PreviewOperationUnavailable); + } + let wallet_id = self.metadata.read().id.clone(); tracing::debug!("deleting wallet {wallet_id}"); @@ -65,15 +69,19 @@ impl RustWalletManager { let mut metadata = before_metadata.clone(); metadata.wallet_type = wallet_type; - metadata = Database::global() - .wallets - .update_wallet_metadata(metadata.clone()) - .map_err_debug(Error::SetWalletTypeError)?; + if self.uses_persistent_storage() { + metadata = Database::global() + .wallets + .update_wallet_metadata(metadata.clone()) + .map_err_debug(Error::SetWalletTypeError)?; + } *self.metadata.write() = metadata.clone(); self.reconciler.send(Message::WalletMetadataChanged(Box::new(metadata.clone()))); - CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &metadata); + if self.uses_persistent_storage() { + CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &metadata); + } Ok(()) } @@ -90,28 +98,33 @@ impl RustWalletManager { let mut metadata = before_metadata.clone(); metadata.name = name; - let metadata = match Database::global().wallets.update_wallet_metadata(metadata.clone()) { - Ok(metadata) => metadata, - Err(error) => { - error!("Unable to update wallet metadata: {error:?}"); - return; + let metadata = if self.uses_persistent_storage() { + match Database::global().wallets.update_wallet_metadata(metadata.clone()) { + Ok(metadata) => metadata, + Err(error) => { + error!("Unable to update wallet metadata: {error:?}"); + return; + } } + } else { + metadata }; *self.metadata.write() = metadata.clone(); self.reconciler.send(Message::WalletMetadataChanged(Box::new(metadata.clone()))); - CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &metadata); + if self.uses_persistent_storage() { + CLOUD_BACKUP_MANAGER.handle_wallet_metadata_update(&before_metadata, &metadata); + } } pub(crate) fn mark_wallet_as_verified_internal(&self) -> Result<(), Error> { - // clone metadata and release lock before I/O - let metadata = { - let mut wallet_metadata = self.metadata.write(); - wallet_metadata.verified = true; - wallet_metadata.clone() - }; + let mut metadata = self.metadata.read().clone(); + metadata.verified = true; - Database::global().wallets.mark_wallet_as_verified(&metadata.id)?; + if self.uses_persistent_storage() { + Database::global().wallets.mark_wallet_as_verified(&metadata.id)?; + } + *self.metadata.write() = metadata.clone(); self.reconciler.send(Message::WalletMetadataChanged(Box::new(metadata.clone()))); Ok(()) diff --git a/rust/src/transaction.rs b/rust/src/transaction.rs index 61034c216..362aae6d3 100644 --- a/rust/src/transaction.rs +++ b/rust/src/transaction.rs @@ -11,7 +11,10 @@ use bdk_wallet::chain::{ use bip329::Labels; use crate::{ - database::{Database, wallet_data::WalletDataDb}, + database::{ + Database, + wallet_data::{WalletDataDb, label::LabelsTable}, + }, fiat::FiatAmount, wallet::metadata::WalletId, }; @@ -70,18 +73,42 @@ impl Transaction { wallet_id: &WalletId, sent_and_received: SentAndReceived, tx: CanonicalTx, ConfirmationBlockTime>, + ) -> Self { + let labels = WalletDataDb::new_or_existing(wallet_id.clone()) + .ok() + .and_then(|db| db.labels.all_labels_for_txn(tx.tx_node.txid).ok()) + .unwrap_or_default(); + + Self::new_with_label_values(sent_and_received, tx, labels.into()) + } + + pub(crate) fn new_with_labels( + sent_and_received: SentAndReceived, + tx: CanonicalTx, ConfirmationBlockTime>, + labels_table: &LabelsTable, + ) -> Self { + let labels = labels_table.all_labels_for_txn(tx.tx_node.txid).unwrap_or_default().into(); + + Self::new_with_label_values(sent_and_received, tx, labels) + } + + pub(crate) fn new_without_labels( + sent_and_received: SentAndReceived, + tx: CanonicalTx, ConfirmationBlockTime>, + ) -> Self { + Self::new_with_label_values(sent_and_received, tx, Labels::default()) + } + + fn new_with_label_values( + sent_and_received: SentAndReceived, + tx: CanonicalTx, ConfirmationBlockTime>, + labels: Labels, ) -> Self { let txid = tx.tx_node.txid.into(); let fiat_currency = Database::global().global_config.fiat_currency().unwrap_or_default(); let fiat = FiatAmount::try_new(&sent_and_received, fiat_currency).ok(); - let labels = WalletDataDb::new_or_existing(wallet_id.clone()) - .ok() - .and_then(|db| db.labels.all_labels_for_txn(tx.tx_node.txid).ok()) - .unwrap_or_default() - .into(); - match tx.chain_position { BdkChainPosition::Unconfirmed { last_seen, .. } => { let unconfirmed = UnconfirmedTransaction { diff --git a/rust/src/wallet.rs b/rust/src/wallet.rs index 0bfdfb93f..1951f5a84 100644 --- a/rust/src/wallet.rs +++ b/rust/src/wallet.rs @@ -27,7 +27,7 @@ use cove_util::result_ext::ResultExt as _; use eyre::Context as _; use metadata::{WalletBirthday, WalletId, WalletMetadata, WalletType}; use parking_lot::Mutex; -use tracing::{debug, warn}; +use tracing::warn; use builder::{WalletBuilder, WalletSource}; @@ -83,9 +83,33 @@ pub struct Wallet { pub network: Network, pub bdk: bdk_wallet::PersistedWallet, pub metadata: WalletMetadata, - // BDK's PersistedWallet

takes &mut P by reference on persist/load/create, - // it doesn't hold the connection itself - db: Mutex, + storage: WalletStorage, +} + +#[derive(Debug)] +pub(crate) enum WalletStorage { + Persistent(Mutex), + InMemory(Mutex), +} + +impl WalletStorage { + pub(crate) fn persistent(connection: Connection) -> Self { + Self::Persistent(Mutex::new(connection)) + } + + pub(crate) fn in_memory(connection: Connection) -> Self { + Self::InMemory(Mutex::new(connection)) + } + + pub(crate) fn connection(&self) -> &Mutex { + match self { + Self::Persistent(connection) | Self::InMemory(connection) => connection, + } + } + + pub(crate) const fn is_persistent(&self) -> bool { + matches!(self, Self::Persistent(_)) + } } #[derive( @@ -137,6 +161,10 @@ impl WalletAddressType { impl Wallet { fn current_database_metadata(&self) -> Result { + if !self.uses_persistent_storage() { + return Ok(self.metadata.clone()); + } + Database::global() .wallets .get(&self.id, self.network, self.metadata.wallet_mode)? @@ -147,6 +175,11 @@ impl Wallet { &mut self, metadata: WalletMetadata, ) -> Result<(), WalletError> { + if !self.uses_persistent_storage() { + self.metadata = metadata; + return Ok(()); + } + let metadata = Database::global().wallets.replace_wallet_metadata(metadata)?; self.metadata = metadata; @@ -208,7 +241,13 @@ impl Wallet { } } - Ok(Self { id, network, metadata, bdk: wallet, db: Mutex::new(store.conn) }) + Ok(Self { + id, + network, + metadata, + bdk: wallet, + storage: WalletStorage::persistent(store.conn), + }) } /// Create a new watch-only wallet from the given xpub @@ -230,7 +269,8 @@ impl Wallet { WalletBuilder::new(WalletSource::TapSigner { tap_signer, derive, backup, birthday }).build() } - fn try_new_persisted_from_mnemonic_segwit( + #[cfg(test)] + pub(crate) fn try_new_persisted_from_mnemonic_segwit( metadata: WalletMetadata, mnemonic: Mnemonic, passphrase: Option, @@ -278,7 +318,13 @@ impl Wallet { let sent_and_received = self.bdk.sent_and_received(&tx.tx_node.tx).into(); (tx, sent_and_received) }) - .map(|(tx, sent_and_received)| Transaction::new(&self.id, sent_and_received, tx)) + .map(|(tx, sent_and_received)| { + if self.uses_persistent_storage() { + Transaction::new(&self.id, sent_and_received, tx) + } else { + Transaction::new_without_labels(sent_and_received, tx) + } + }) .filter(|tx| tx.sent_and_received().amount() > zero) .collect::>(); @@ -287,10 +333,16 @@ impl Wallet { } pub fn persist(&mut self) -> Result<(), WalletError> { - self.bdk.persist(&mut self.db.lock()).map_err_str(WalletError::PersistError)?; + self.bdk + .persist(&mut self.storage.connection().lock()) + .map_err_str(WalletError::PersistError)?; Ok(()) } + + pub(crate) const fn uses_persistent_storage(&self) -> bool { + self.storage.is_persistent() + } } impl Wallet { @@ -298,15 +350,8 @@ impl Wallet { let mnemonic = Mnemonic::from_str("abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about").unwrap(); let passphrase = None; - if let Err(error) = delete_wallet_specific_data(&metadata.id) { - debug!("clean up failed, failed to delete wallet data: {error}"); - } - - if let Err(error) = Database::global().wallets.delete(&metadata.id) { - debug!("clean up failed, failed to delete wallet: {error}"); - } - - Self::try_new_persisted_from_mnemonic_segwit(metadata, mnemonic, passphrase).unwrap() + WalletBuilder::build_preview(metadata, mnemonic, passphrase) + .expect("failed to build in-memory preview wallet") } } @@ -358,3 +403,22 @@ impl Wallet { Self::try_new_persisted_from_pubport(export.into_format()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn preview_wallet_uses_only_ephemeral_storage() { + let metadata = WalletMetadata::preview_new(); + let bdk_paths = crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id); + let wallet_data_paths = + crate::database::wallet_data::wallet_data_artifact_paths(&metadata.id); + + let wallet = Wallet::preview_new_wallet_with_metadata(metadata); + + assert!(!wallet.uses_persistent_storage()); + assert!(bdk_paths.iter().all(|path| !path.exists())); + assert!(wallet_data_paths.iter().all(|path| !path.exists())); + } +} diff --git a/rust/src/wallet/addressing.rs b/rust/src/wallet/addressing.rs index b64df2891..6df724eb3 100644 --- a/rust/src/wallet/addressing.rs +++ b/rust/src/wallet/addressing.rs @@ -32,12 +32,20 @@ impl Wallet { let id = self.id.clone(); - // delete the bdk wallet filestore - BdkStore::delete_sqlite_store(&self.id).map_err(|error| { - WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) - })?; + let is_persistent = self.uses_persistent_storage(); - let store = BdkStore::try_new(&id, self.network); + if is_persistent { + // delete the bdk wallet filestore + BdkStore::delete_sqlite_store(&self.id).map_err(|error| { + WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) + })?; + } + + let store = if is_persistent { + BdkStore::try_new(&id, self.network) + } else { + BdkStore::in_memory(&id, self.network) + }; let mut db = store.map_err_str(WalletError::LoadError)?.conn; let descriptors: Descriptors = descriptors.into(); @@ -49,7 +57,11 @@ impl Wallet { // switch db and wallet self.bdk = wallet; - self.db = parking_lot::Mutex::new(db); + self.storage = if is_persistent { + super::WalletStorage::persistent(db) + } else { + super::WalletStorage::in_memory(db) + }; let metadata = self.current_database_metadata()?; let metadata = metadata_for_address_type_switch(metadata, address_type); self.persist_address_type_switch_metadata(metadata)?; @@ -64,10 +76,12 @@ impl Wallet { ) -> Result<(), WalletError> { debug!("switching private wallet to new address type"); - // delete the bdk wallet filestore - BdkStore::delete_sqlite_store(&self.id).map_err(|error| { - WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) - })?; + if self.uses_persistent_storage() { + // delete the bdk wallet filestore + BdkStore::delete_sqlite_store(&self.id).map_err(|error| { + WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) + })?; + } let secret = Keychain::global() .get_wallet_secret(&self.id) @@ -138,7 +152,9 @@ impl Wallet { let address_info = addresses[index_to_use].clone(); self.metadata.internal.set_last_seen_address_index(&addresses, index_to_use); - Database::global().wallets.update_internal_metadata(&self.metadata)?; + if self.uses_persistent_storage() { + Database::global().wallets.update_internal_metadata(&self.metadata)?; + } let public_descriptor = self.bdk.public_descriptor(KeychainKind::External); let derivation_path = public_descriptor.derivation_path().ok(); diff --git a/rust/src/wallet/builder.rs b/rust/src/wallet/builder.rs index 207a80e6b..32583e167 100644 --- a/rust/src/wallet/builder.rs +++ b/rust/src/wallet/builder.rs @@ -7,7 +7,6 @@ use bip39::Mnemonic; use cove_device::keychain::{WalletSecret, WalletXprv}; use cove_types::Network; use cove_util::result_ext::ResultExt as _; -use parking_lot::Mutex; use pubport::formats::Format; use tracing::{error, warn}; @@ -26,7 +25,7 @@ use crate::{ }; use super::{ - Wallet, WalletAddressType, WalletError, delete_wallet_specific_data, + Wallet, WalletAddressType, WalletError, WalletStorage, delete_wallet_specific_data, fingerprint::Fingerprint, metadata, metadata::{ @@ -101,6 +100,23 @@ impl WalletBuilder { } } + pub(crate) fn build_preview( + metadata: WalletMetadata, + mnemonic: Mnemonic, + passphrase: Option, + ) -> Result { + let id = metadata.id.clone(); + let network = metadata.network; + + Self::build_from_mnemonic_with_store( + metadata, + mnemonic, + passphrase, + WalletAddressType::NativeSegwit, + BdkStore::in_memory(&id, network).map_err_str(WalletError::LoadError)?, + ) + } + fn build_persisted_and_selected( metadata: WalletMetadata, secret: WalletSecret, @@ -289,7 +305,13 @@ impl WalletBuilder { database.wallets.save_new_wallet_metadata(metadata.clone())?; CLOUD_BACKUP_MANAGER.handle_wallet_set_change(); - Ok(Wallet { id, metadata, network, bdk: wallet, db: Mutex::new(store.conn) }) + Ok(Wallet { + id, + metadata, + network, + bdk: wallet, + storage: WalletStorage::persistent(store.conn), + }) } fn build_from_tap_signer( @@ -355,11 +377,17 @@ impl WalletBuilder { database.wallets.save_new_wallet_metadata(metadata.clone())?; CLOUD_BACKUP_MANAGER.handle_wallet_set_change(); - Ok(Wallet { id, metadata, network, bdk: wallet, db: Mutex::new(store.conn) }) + Ok(Wallet { + id, + metadata, + network, + bdk: wallet, + storage: WalletStorage::persistent(store.conn), + }) } fn build_from_mnemonic( - mut metadata: WalletMetadata, + metadata: WalletMetadata, mnemonic: Mnemonic, passphrase: Option, address_type: WalletAddressType, @@ -367,7 +395,20 @@ impl WalletBuilder { let network = metadata.network; let id = metadata.id.clone(); - let mut store = BdkStore::try_new(&id, network).map_err_str(WalletError::LoadError)?; + let store = BdkStore::try_new(&id, network).map_err_str(WalletError::LoadError)?; + + Self::build_from_mnemonic_with_store(metadata, mnemonic, passphrase, address_type, store) + } + + fn build_from_mnemonic_with_store( + mut metadata: WalletMetadata, + mnemonic: Mnemonic, + passphrase: Option, + address_type: WalletAddressType, + mut store: BdkStore, + ) -> Result { + let id = metadata.id.clone(); + let network = metadata.network; let descriptors = mnemonic.into_descriptors(passphrase, network, address_type); let origin = descriptors.origin().ok(); @@ -375,13 +416,24 @@ impl WalletBuilder { metadata.master_fingerprint = descriptors.fingerprint().map(|f| Arc::new(f.into())); metadata.origin = origin; + let storage = store.is_in_memory(); let wallet = descriptors .into_create_params() .network(network.into()) .create_wallet(&mut store.conn) .map_err_str(WalletError::BdkError)?; - Ok(Wallet { id, metadata, network, bdk: wallet, db: Mutex::new(store.conn) }) + Ok(Wallet { + id, + metadata, + network, + bdk: wallet, + storage: if storage { + WalletStorage::in_memory(store.conn) + } else { + WalletStorage::persistent(store.conn) + }, + }) } fn build_from_xpriv( @@ -404,7 +456,13 @@ impl WalletBuilder { .create_wallet(&mut store.conn) .map_err_str(WalletError::BdkError)?; - Ok(Wallet { id, metadata, network, bdk: wallet, db: Mutex::new(store.conn) }) + Ok(Wallet { + id, + metadata, + network, + bdk: wallet, + storage: WalletStorage::persistent(store.conn), + }) } fn upgrade_to_cold( From 6923168ebc04651de675d626e8ad0a8da9871ded Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:46:35 -0500 Subject: [PATCH 03/15] Validate remote fee estimates Reject HTTP errors and invalid or excessive fee rates before they reach transaction builders. Persist fetch times so offline fallback data expires after one hour across restarts. --- rust/src/database/global_cache.rs | 62 ++- rust/src/fee_client.rs | 434 ++++++++++++++---- rust/src/manager/send_flow_manager.rs | 18 +- .../send_flow_manager/fee_selection.rs | 2 +- rust/src/manager/wallet_manager.rs | 37 +- 5 files changed, 417 insertions(+), 136 deletions(-) diff --git a/rust/src/database/global_cache.rs b/rust/src/database/global_cache.rs index 1b95c2886..c5af648b6 100644 --- a/rust/src/database/global_cache.rs +++ b/rust/src/database/global_cache.rs @@ -7,7 +7,7 @@ use cove_util::result_ext::ResultExt as _; use crate::{ app::reconcile::{Update, Updater}, - fee_client::FeeResponse, + fee_client::{FeeResponse, FeeSnapshot}, fiat::client::PriceResponse, network::Network, }; @@ -44,8 +44,12 @@ impl GlobalCacheKey { #[derive(Debug, Clone, derive_more::From, serde::Serialize, serde::Deserialize)] pub enum GlobalCacheData { Prices(PriceResponse), + /// Legacy fee data without a fetch timestamp. It is retained for decoding old databases but + /// must never be treated as a current fee snapshot Fees(FeeResponse), BlockHeight(BlockSizeLast), + /// Fee data with a wall-clock timestamp for bounded offline fallback + FeesV2(FeeSnapshot), } #[derive(Debug, Clone)] @@ -62,6 +66,52 @@ impl GlobalCacheTable { } } +#[cfg(test)] +mod tests { + use super::*; + use crate::fee_client::{FeeFetchedAt, FeeSnapshot}; + + fn test_table() -> (tempfile::TempDir, GlobalCacheTable) { + let tmp = tempfile::tempdir().expect("failed to create temp dir"); + let db = Arc::new( + redb::Database::create(tmp.path().join("global_cache.redb")) + .expect("failed to create redb"), + ); + let write_txn = db.begin_write().expect("failed to begin write transaction"); + let table = GlobalCacheTable::new(db, &write_txn); + write_txn.commit().expect("failed to commit table creation"); + + (tmp, table) + } + + fn fees() -> FeeResponse { + FeeResponse { + fastest_fee: 5.0, + half_hour_fee: 3.0, + hour_fee: 2.0, + economy_fee: 1.0, + minimum_fee: 1.0, + } + } + + #[test] + fn legacy_fee_data_is_stale_and_timestamped_data_round_trips() { + let (_tmp, table) = test_table(); + let key = GlobalCacheKey::Fees(FeesKey); + + table.set(key, GlobalCacheData::Fees(fees())).expect("legacy fee data saves"); + assert!(table.get_fee_snapshot().expect("fee snapshot loads").is_none()); + + let snapshot = FeeSnapshot { + fees: fees(), + fetched_at: FeeFetchedAt::from_unix_seconds(1_700_000_000), + }; + table.set_fee_snapshot(snapshot).expect("timestamped fee data saves"); + + assert_eq!(table.get_fee_snapshot().expect("fee snapshot loads"), Some(snapshot)); + } +} + #[derive(Debug, Clone, Hash, Eq, PartialEq, uniffi::Error, thiserror::Error)] #[uniffi::export(Display)] pub enum GlobalCacheTableError { @@ -87,18 +137,18 @@ impl GlobalCacheTable { self.set(key, prices.into()) } - pub fn get_fees(&self) -> Result, Error> { + pub fn get_fee_snapshot(&self) -> Result, Error> { let key = GlobalCacheKey::Fees(FeesKey); - if let Some(GlobalCacheData::Fees(fees)) = self.get(key)? { - return Ok(Some(fees)); + if let Some(GlobalCacheData::FeesV2(snapshot)) = self.get(key)? { + return Ok(Some(snapshot)); } Ok(None) } - pub fn set_fees(&self, fees: FeeResponse) -> Result<(), Error> { + pub fn set_fee_snapshot(&self, snapshot: FeeSnapshot) -> Result<(), Error> { let key = GlobalCacheKey::Fees(FeesKey); - self.set(key, fees.into()) + self.set(key, GlobalCacheData::FeesV2(snapshot)) } pub fn get_block_height(&self, network: Network) -> Result, Error> { diff --git a/rust/src/fee_client.rs b/rust/src/fee_client.rs index c9abcd6c2..57a5a514d 100644 --- a/rust/src/fee_client.rs +++ b/rust/src/fee_client.rs @@ -3,7 +3,7 @@ use std::{ Arc, LazyLock, atomic::{AtomicBool, Ordering}, }, - time::{Duration, Instant}, + time::{Duration, Instant, SystemTime, UNIX_EPOCH}, }; use arc_swap::ArcSwap; @@ -29,6 +29,12 @@ const BACKGROUND_REFRESH_INTERVAL: u64 = 60; /// Hard limit: never fetch if < 30 seconds since last fetch const HARD_LIMIT: u64 = 30; +/// Do not use a persisted fee snapshot as a fallback after this amount of time +const STALE_FALLBACK_MAX_AGE: Duration = Duration::from_secs(60 * 60); + +/// Maximum fee rate accepted from the remote fee service, in sat/vB +const MAX_REMOTE_FEE_RATE: f32 = 500.0; + // Global client for getting fees pub static FEE_CLIENT: LazyLock = LazyLock::new(FeeClient::new); @@ -40,6 +46,75 @@ pub struct FeeClient { client: OnceCell, } +/// Errors raised while validating fee data from the remote service +#[derive(Debug, Clone, thiserror::Error)] +pub enum FeeValidationError { + #[error( + "{field} fee rate must be finite, positive, and at most {MAX_REMOTE_FEE_RATE} sat/vB (got {value})" + )] + InvalidRate { field: &'static str, value: f32 }, +} + +/// Errors raised while fetching or validating fee data +#[derive(Debug, thiserror::Error)] +pub enum FeeClientError { + #[error(transparent)] + Request(#[from] reqwest::Error), + + #[error(transparent)] + Validation(#[from] FeeValidationError), +} + +/// Wall-clock time at which a fee snapshot was fetched +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub struct FeeFetchedAt(u64); + +impl FeeFetchedAt { + /// Create a timestamp from Unix seconds + pub const fn from_unix_seconds(seconds: u64) -> Self { + Self(seconds) + } + + /// Return the timestamp as Unix seconds + pub const fn as_unix_seconds(self) -> u64 { + self.0 + } + + fn now() -> Self { + let seconds = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_secs()) + .unwrap_or_default(); + + Self::from_unix_seconds(seconds) + } + + fn age(self) -> Option { + let now = SystemTime::now().duration_since(UNIX_EPOCH).ok()?.as_secs(); + Some(Duration::from_secs(now.checked_sub(self.as_unix_seconds())?)) + } +} + +/// A fee response and its wall-clock fetch time +#[derive(Debug, Clone, Copy, PartialEq, serde::Serialize, serde::Deserialize)] +pub struct FeeSnapshot { + pub fees: FeeResponse, + pub fetched_at: FeeFetchedAt, +} + +impl FeeSnapshot { + fn new(fees: FeeResponse) -> Self { + Self { fees, fetched_at: FeeFetchedAt::now() } + } + + fn is_usable_fallback(self) -> bool { + self.fetched_at.age().is_some_and(|age| age <= STALE_FALLBACK_MAX_AGE) + } +} + +#[derive(Debug, Clone, Copy)] +struct ValidatedFeeResponse(FeeResponse); + impl FeeClient { pub fn new() -> Self { Self::new_with_url(FEE_URL.to_string()) @@ -52,76 +127,115 @@ impl FeeClient { /// Get cached fees, will trigger background refresh if stale /// Returns None if no cache exists (memory or database) pub fn fees(&self) -> Option { - // check in-memory cache first - if let Some(cached) = FEES.load().as_ref() { - let now = Instant::now(); - - // cache is fresh, no refresh needed - if now.duration_since(cached.last_fetched) - <= Duration::from_secs(BACKGROUND_REFRESH_INTERVAL) - { - return Some(cached.fees); - } + if let Some(cached) = self.cached_fees() { + if cached.snapshot().is_usable_fallback() { + if cached.last_fetched.elapsed() <= Duration::from_secs(BACKGROUND_REFRESH_INTERVAL) + { + return Some(cached.fees); + } - // refresh already in flight - if REFRESH_IN_FLIGHT - .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) - .is_err() - { + self.spawn_background_refresh(); return Some(cached.fees); } - cove_tokio::task::spawn(async move { - if let Err(e) = fetch_and_update_fees_if_needed().await { - warn!("background fee refresh failed: {e:?}"); - } - REFRESH_IN_FLIGHT.store(false, Ordering::SeqCst); - }); - - return Some(cached.fees); - } - - // fallback to database cache - if let Ok(Some(fees)) = Database::global().global_cache.get_fees() { - debug!("loaded cached fees from database"); - FEES.swap(Arc::new(Some(CachedFeeResponse { fees, last_fetched: Instant::now() }))); - return Some(fees); + self.spawn_background_refresh(); } - warn!("no cached fees found in memory or database"); + warn!("no usable cached fees found in memory or database"); None } /// Get fees, using cache if available and fresh, otherwise fetching new /// Respects 30-second hard limit to prevent excessive fetching - pub async fn fetch_and_get_fees(&self) -> Result { - if let Some(cached) = FEES.load().as_ref() { - let now = Instant::now(); - if now.duration_since(cached.last_fetched) < Duration::from_secs(HARD_LIMIT) { - return Ok(cached.fees); - } + pub async fn fetch_and_get_fees(&self) -> Result { + let cached = self.cached_fees(); + if let Some(cached) = cached + && cached.snapshot().is_usable_fallback() + && cached.last_fetched.elapsed() < Duration::from_secs(HARD_LIMIT) + { + return Ok(cached.fees); } debug!("fetching fees from network"); - let fees = self.get_new_fees().await?; - update_fees(fees); + match self.get_new_fees().await { + Ok(fees) => { + let fees = fees.0; + update_fees(fees); - Ok(fees) + Ok(fees) + } + Err(error) => { + if let Some(cached) = cached.filter(|cached| cached.snapshot().is_usable_fallback()) + { + warn!("fee refresh failed, using cached fees: {error}"); + return Ok(cached.fees); + } + + Err(error) + } + } } /// Always gets new fees from the server - async fn get_new_fees(&self) -> Result { - let response = self.client()?.get(&self.url).send().await?; + async fn get_new_fees(&self) -> Result { + let response = self.client()?.get(&self.url).send().await?.error_for_status()?; let fees: FeeResponse = response.json().await?; - Ok(fees) + Ok(fees.try_into()?) } fn client(&self) -> Result<&reqwest::Client, reqwest::Error> { self.client.get_or_try_init(cove_http::new_client) } + + fn cached_fees(&self) -> Option { + if let Some(cached) = FEES.load().as_ref() { + let cached = *cached; + if ValidatedFeeResponse::try_from(cached.fees).is_ok() { + return Some(cached); + } + + warn!("ignoring invalid fee snapshot from memory"); + FEES.swap(Arc::new(None)); + } + + let snapshot = match Database::global().global_cache.get_fee_snapshot() { + Ok(Some(snapshot)) => snapshot, + Ok(None) => return None, + Err(error) => { + warn!("unable to load fee snapshot from database: {error}"); + return None; + } + }; + + if let Err(error) = ValidatedFeeResponse::try_from(snapshot.fees) { + warn!("ignoring invalid fee snapshot from database: {error}"); + return None; + } + + let cached = CachedFeeResponse::from_persisted_snapshot(snapshot); + debug!("loaded cached fees from database"); + FEES.swap(Arc::new(Some(cached))); + Some(cached) + } + + fn spawn_background_refresh(&self) { + if REFRESH_IN_FLIGHT + .compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst) + .is_err() + { + return; + } + + cove_tokio::task::spawn(async move { + if let Err(error) = fetch_and_update_fees_if_needed().await { + warn!("background fee refresh failed: {error:?}"); + } + REFRESH_IN_FLIGHT.store(false, Ordering::SeqCst); + }); + } } -#[derive(Debug, Clone, Copy, serde::Serialize, serde::Deserialize, uniffi::Record)] +#[derive(Debug, Clone, Copy, PartialEq, serde::Serialize, serde::Deserialize, uniffi::Record)] #[serde(rename_all = "camelCase")] pub struct FeeResponse { pub fastest_fee: f32, @@ -131,49 +245,113 @@ pub struct FeeResponse { pub minimum_fee: f32, } +impl TryFrom for ValidatedFeeResponse { + type Error = FeeValidationError; + + fn try_from(fees: FeeResponse) -> Result { + for (field, value) in [ + ("fastestFee", fees.fastest_fee), + ("halfHourFee", fees.half_hour_fee), + ("hourFee", fees.hour_fee), + ("economyFee", fees.economy_fee), + ("minimumFee", fees.minimum_fee), + ] { + if !value.is_finite() || value <= 0.0 || value > MAX_REMOTE_FEE_RATE { + return Err(FeeValidationError::InvalidRate { field, value }); + } + } + + let options = derive_fee_rate_options(fees); + for (field, value) in [ + ("slow", options.slow.fee_rate.sat_per_vb()), + ("medium", options.medium.fee_rate.sat_per_vb()), + ("fast", options.fast.fee_rate.sat_per_vb()), + ] { + if !value.is_finite() || value <= 0.0 || value > MAX_REMOTE_FEE_RATE { + return Err(FeeValidationError::InvalidRate { field, value }); + } + } + + Ok(Self(fees)) + } +} + +impl ValidatedFeeResponse { + fn fee_rate_options(self) -> FeeRateOptions { + derive_fee_rate_options(self.0) + } +} + +impl FeeResponse { + /// Convert a validated remote fee response into display and builder fee tiers + pub fn fee_rate_options(self) -> Result { + Ok(ValidatedFeeResponse::try_from(self)?.fee_rate_options()) + } +} + #[derive(Debug, Clone, Copy)] pub struct CachedFeeResponse { pub fees: FeeResponse, pub last_fetched: Instant, + pub fetched_at: FeeFetchedAt, } -/// Convert fee response to fee rate options -impl From for FeeRateOptions { - fn from(fees: FeeResponse) -> Self { - /// Policy minimum fee rate in sat/vb - const POLICY_MIN_FEE_RATE: f32 = 1.0; +impl CachedFeeResponse { + fn from_fresh_snapshot(snapshot: FeeSnapshot) -> Self { + Self { fees: snapshot.fees, last_fetched: Instant::now(), fetched_at: snapshot.fetched_at } + } + + fn from_persisted_snapshot(snapshot: FeeSnapshot) -> Self { + let now = Instant::now(); + let last_fetched = + snapshot.fetched_at.age().and_then(|age| now.checked_sub(age)).unwrap_or(now); + + Self { fees: snapshot.fees, last_fetched, fetched_at: snapshot.fetched_at } + } - /// Minimum gap between fee tiers to ensure they're visually distinct - const TIER_GAP: f32 = 0.1; + fn snapshot(self) -> FeeSnapshot { + FeeSnapshot { fees: self.fees, fetched_at: self.fetched_at } + } +} - let min_relay_rate = fees.minimum_fee.max(POLICY_MIN_FEE_RATE); +fn derive_fee_rate_options(fees: FeeResponse) -> FeeRateOptions { + /// Policy minimum fee rate in sat/vB + const POLICY_MIN_FEE_RATE: f32 = 1.0; - let slow_rate = - f32::midpoint(fees.economy_fee, fees.hour_fee).min(fees.hour_fee).max(min_relay_rate); + /// Minimum gap between fee tiers to ensure they're visually distinct + const TIER_GAP: f32 = 0.1; - let medium_rate = fees.half_hour_fee.max(slow_rate + TIER_GAP); - let fast_rate = fees.fastest_fee.max(medium_rate + TIER_GAP); + let min_relay_rate = fees.minimum_fee.max(POLICY_MIN_FEE_RATE); - let slow = FeeRateOption { - fee_speed: FeeSpeed::Slow, - fee_rate: FeeRate::from_sat_per_vb(slow_rate), - }; - let medium = FeeRateOption { - fee_speed: FeeSpeed::Medium, - fee_rate: FeeRate::from_sat_per_vb(medium_rate), - }; - let fast = FeeRateOption { - fee_speed: FeeSpeed::Fast, - fee_rate: FeeRate::from_sat_per_vb(fast_rate), - }; + let slow_rate = + f32::midpoint(fees.economy_fee, fees.hour_fee).min(fees.hour_fee).max(min_relay_rate); - Self { fast, medium, slow } + let medium_rate = fees.half_hour_fee.max(slow_rate + TIER_GAP); + let fast_rate = fees.fastest_fee.max(medium_rate + TIER_GAP); + + let slow = + FeeRateOption { fee_speed: FeeSpeed::Slow, fee_rate: FeeRate::from_sat_per_vb(slow_rate) }; + let medium = FeeRateOption { + fee_speed: FeeSpeed::Medium, + fee_rate: FeeRate::from_sat_per_vb(medium_rate), + }; + let fast = + FeeRateOption { fee_speed: FeeSpeed::Fast, fee_rate: FeeRate::from_sat_per_vb(fast_rate) }; + + FeeRateOptions { fast, medium, slow } +} + +impl TryFrom for FeeRateOptions { + type Error = FeeValidationError; + + fn try_from(fees: FeeResponse) -> Result { + ValidatedFeeResponse::try_from(fees).map(ValidatedFeeResponse::fee_rate_options) } } /// get and update fees -pub async fn get_and_update_fees() -> Result<(), reqwest::Error> { - let fees = FEE_CLIENT.get_new_fees().await?; +pub async fn get_and_update_fees() -> Result<(), FeeClientError> { + let fees = FEE_CLIENT.get_new_fees().await?.0; update_fees(fees); Ok(()) } @@ -181,13 +359,14 @@ pub async fn get_and_update_fees() -> Result<(), reqwest::Error> { /// Update fees in memory cache and database fn update_fees(fees: FeeResponse) { debug!("update_fees"); - let cached = CachedFeeResponse { fees, last_fetched: Instant::now() }; + let snapshot = FeeSnapshot::new(fees); + let cached = CachedFeeResponse::from_fresh_snapshot(snapshot); FEES.swap(Arc::new(Some(cached))); Updater::send_update(AppMessage::FeesChanged(fees)); // persist to database let db = Database::global(); - if let Err(e) = db.global_cache.set_fees(fees) { + if let Err(e) = db.global_cache.set_fee_snapshot(snapshot) { error!("unable to save fees to database: {e:?}"); } } @@ -201,11 +380,12 @@ pub async fn init_and_update_fees() { return; } - // try loading from database first - if let Ok(Some(fees)) = Database::global().global_cache.get_fees() { + // try loading a timestamped database snapshot first + if let Some(cached) = + FEE_CLIENT.cached_fees().filter(|cached| cached.snapshot().is_usable_fallback()) + { debug!("loaded fees from database cache"); - FEES.swap(Arc::new(Some(CachedFeeResponse { fees, last_fetched: Instant::now() }))); - Updater::send_update(AppMessage::FeesChanged(fees)); + Updater::send_update(AppMessage::FeesChanged(cached.fees)); } // fetch from network @@ -219,22 +399,22 @@ pub async fn init_and_update_fees() { .await; match result { - Ok(fees) => update_fees(fees), + Ok(_) => {} Err(error) => warn!("unable to get fees: {error:?}"), } } /// Fetch and update fees if needed (respects hard limit) pub async fn fetch_and_update_fees_if_needed() -> Result<()> { - if let Some(cached) = FEES.load().as_ref() { - let now = Instant::now(); - if now.duration_since(cached.last_fetched) < Duration::from_secs(HARD_LIMIT) { - return Ok(()); - } + if let Some(cached) = FEE_CLIENT.cached_fees() + && cached.snapshot().is_usable_fallback() + && cached.last_fetched.elapsed() < Duration::from_secs(HARD_LIMIT) + { + return Ok(()); } debug!("fetching fees"); - let fees = FEE_CLIENT.get_new_fees().await.context("unable to get fees")?; + let fees = FEE_CLIENT.get_new_fees().await.context("unable to get fees")?.0; update_fees(fees); Ok(()) @@ -243,6 +423,7 @@ pub async fn fetch_and_update_fees_if_needed() -> Result<()> { #[cfg(test)] mod tests { use super::*; + use tokio::{io::AsyncReadExt as _, io::AsyncWriteExt as _, net::TcpListener}; fn fee_response(economy: f32, hour: f32, half_hour: f32, fastest: f32) -> FeeResponse { FeeResponse { @@ -254,11 +435,80 @@ mod tests { } } + #[test] + fn remote_fee_rates_must_be_finite_positive_and_bounded() { + for invalid in [ + fee_response(0.0, 1.0, 1.0, 1.0), + fee_response(f32::NAN, 1.0, 1.0, 1.0), + fee_response(1.0, f32::INFINITY, 1.0, 1.0), + fee_response(1.0, 1.0, 501.0, 1.0), + ] { + assert!(ValidatedFeeResponse::try_from(invalid).is_err()); + } + } + + #[test] + fn derived_fee_rates_are_bounded_after_tier_separation() { + let fees = fee_response(499.9, 499.9, 499.9, 499.9); + + assert!(matches!( + ValidatedFeeResponse::try_from(fees), + Err(FeeValidationError::InvalidRate { field: "medium" | "fast", .. }) + )); + } + + #[test] + fn legacy_timestamp_is_not_a_usable_snapshot() { + let now = UNIX_EPOCH.elapsed().expect("system clock is after Unix epoch").as_secs(); + let snapshot = FeeSnapshot { + fees: fee_response(1.0, 1.0, 1.0, 1.0), + fetched_at: FeeFetchedAt::from_unix_seconds(now.saturating_sub(3_601)), + }; + + assert!(!snapshot.is_usable_fallback()); + } + + #[test] + fn persisted_snapshot_keeps_wall_clock_age_for_refresh_limits() { + let now = UNIX_EPOCH.elapsed().expect("system clock is after Unix epoch").as_secs(); + let snapshot = FeeSnapshot { + fees: fee_response(1.0, 1.0, 1.0, 1.0), + fetched_at: FeeFetchedAt::from_unix_seconds(now.saturating_sub(120)), + }; + + let cached = CachedFeeResponse::from_persisted_snapshot(snapshot); + + assert!(cached.last_fetched.elapsed() >= Duration::from_secs(30)); + } + + #[tokio::test] + async fn fee_client_rejects_http_error_status() { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("listener binds"); + let address = listener.local_addr().expect("listener has an address"); + let server = tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.expect("request arrives"); + let mut request = [0; 1024]; + let _ = stream.read(&mut request).await; + stream + .write_all( + b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n", + ) + .await + .expect("response writes"); + }); + + let client = FeeClient::new_with_url(format!("http://{address}")); + let error = client.get_new_fees().await.expect_err("HTTP errors are rejected"); + + assert!(matches!(error, FeeClientError::Request(_))); + server.await.expect("server exits"); + } + #[test] fn low_fees_reported_bug() { // API returns sub-1 fees, app should show 1.0, 1.1, 1.2 let fees = fee_response(0.1, 0.5, 0.5, 1.0); - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 250); // 1.0 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 275); // 1.1 sat/vb @@ -268,7 +518,7 @@ mod tests { #[test] fn all_fees_below_one() { let fees = fee_response(0.1, 0.1, 0.1, 0.1); - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 250); // 1.0 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 275); // 1.1 sat/vb @@ -278,7 +528,7 @@ mod tests { #[test] fn all_fees_equal_at_one() { let fees = fee_response(1.0, 1.0, 1.0, 1.0); - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 250); // 1.0 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 275); // 1.1 sat/vb @@ -289,7 +539,7 @@ mod tests { fn normal_differentiated_fees() { // high enough fees should pass through without inflation let fees = fee_response(5.0, 10.0, 15.0, 20.0); - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 1875); // 7.5 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 3750); // 15.0 sat/vb @@ -299,7 +549,7 @@ mod tests { #[test] fn fees_slightly_above_one() { let fees = fee_response(1.0, 2.0, 3.0, 5.0); - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 375); // 1.5 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 750); // 3.0 sat/vb @@ -317,7 +567,7 @@ mod tests { economy_fee: 2.0, minimum_fee: 2.0, }; - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 500); // 2.0 sat/vb assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 525); // 2.1 sat/vb @@ -335,7 +585,7 @@ mod tests { economy_fee: 0.5, minimum_fee: 1.5, }; - let options = FeeRateOptions::from(fees); + let options = fees.fee_rate_options().expect("fee rates are valid"); assert_eq!(options.slow.fee_rate.to_sat_per_kwu(), 375); // 1.5 sat/vb (floor) assert_eq!(options.medium.fee_rate.to_sat_per_kwu(), 750); // 3.0 sat/vb diff --git a/rust/src/manager/send_flow_manager.rs b/rust/src/manager/send_flow_manager.rs index 1cf73232a..c78b7ef58 100644 --- a/rust/src/manager/send_flow_manager.rs +++ b/rust/src/manager/send_flow_manager.rs @@ -36,7 +36,7 @@ use btc_on_change::BtcOnChangeHandler; use cove_common::consts::LOW_SEND_WARNING_SATS; use cove_types::{ amount::Amount, - fees::{FeeRateOptionWithTotalFee, FeeRateOptions, FeeRateOptionsWithTotalFee, FeeSpeed}, + fees::{FeeRateOptionWithTotalFee, FeeRateOptionsWithTotalFee, FeeSpeed}, unit::BitcoinUnit, utxo::Utxo, }; @@ -167,8 +167,9 @@ impl RustSendFlowManager { let state = State::new(metadata, balance); // immediately populate cached values if available - let has_base_fees = if let Some(fee_response) = FEE_CLIENT.fees() { - let base_options = FeeRateOptions::from(fee_response); + let has_base_fees = if let Some(base_options) = + FEE_CLIENT.fees().and_then(|fees| fees.fee_rate_options().ok()) + { let fee_options = FeeRateOptionsWithTotalFee::without_totals(base_options); let selected = Arc::new(fee_options.medium); let fee_selection = FeeSelection::new(Arc::new(fee_options), selected); @@ -673,7 +674,9 @@ impl RustSendFlowManager { return; }; - let base_options = FeeRateOptions::from(fee_response); + let Ok(base_options) = fee_response.fee_rate_options() else { + return; + }; let fee_options = FeeRateOptionsWithTotalFee::without_totals(base_options); let previous_selected = state.lock().fee_selection.as_ref().map(|selection| selection.selected.clone()); @@ -745,7 +748,10 @@ mod tests { use std::sync::Arc; use cove_types::{ - fees::{FeeRateOption, FeeRateOptionWithTotalFee, FeeRateOptionsWithTotalFee, FeeSpeed}, + fees::{ + FeeRateOption, FeeRateOptionWithTotalFee, FeeRateOptions, FeeRateOptionsWithTotalFee, + FeeSpeed, + }, utxo::{UtxoList, ffi_preview::preview_new_utxo_list}, }; @@ -803,7 +809,7 @@ mod tests { } fn set_selected_fee_without_total(manager: &super::RustSendFlowManager) { - let base_options = super::FeeRateOptions::_ffi_preview_new(); + let base_options = FeeRateOptions::_ffi_preview_new(); let fee_option = FeeRateOption::new(FeeSpeed::Custom { duration_mins: 10 }, 1.0); let selected = FeeRateOptionWithTotalFee::without_total(fee_option); let options = FeeRateOptionsWithTotalFee::without_totals(base_options); diff --git a/rust/src/manager/send_flow_manager/fee_selection.rs b/rust/src/manager/send_flow_manager/fee_selection.rs index dc997d821..ac0ff211a 100644 --- a/rust/src/manager/send_flow_manager/fee_selection.rs +++ b/rust/src/manager/send_flow_manager/fee_selection.rs @@ -77,7 +77,7 @@ impl RustSendFlowManager { self: &Arc, ) -> Option> { let fee_response = FEE_CLIENT.fetch_and_get_fees().await.ok()?; - let fees = Arc::new(FeeRateOptions::from(fee_response)); + let fees = Arc::new(fee_response.fee_rate_options().ok()?); { let mut state = self.state.lock(); diff --git a/rust/src/manager/wallet_manager.rs b/rust/src/manager/wallet_manager.rs index 568148662..feb6bf0d8 100644 --- a/rust/src/manager/wallet_manager.rs +++ b/rust/src/manager/wallet_manager.rs @@ -11,10 +11,7 @@ mod transaction_locks; mod unsigned_transactions; mod wallet_admin; -use std::{ - sync::Arc, - time::{Duration, Instant}, -}; +use std::sync::Arc; use act_zero::{Addr, call, send}; use actor::WalletActor; @@ -35,7 +32,7 @@ use crate::{ converter::{Converter, ConverterError}, database::{Database, error::DatabaseError, wallet_data::WalletDataDb}, discovery_scanner::{ScannerResponse, WalletDiscoveryScanner}, - fee_client::{FEE_CLIENT, FEES, FeeResponse}, + fee_client::{FEE_CLIENT, FeeClientError, FeeResponse}, fiat::client::PriceResponse, keychain::{Keychain, KeychainError}, label_manager::LabelManager, @@ -439,7 +436,7 @@ impl From for WalletManagerError { #[derive(Debug, thiserror::Error)] pub(crate) enum WalletManagerFeesError { #[error(transparent)] - Fetch(#[from] reqwest::Error), + Fetch(#[from] FeeClientError), #[error(transparent)] Psbt(#[from] bitcoin::psbt::Error), @@ -657,7 +654,7 @@ impl RustWalletManager { let fee_client = &FEE_CLIENT; let fees = fee_client.fetch_and_get_fees().await.map_err(WalletManagerFeesError::from)?; - Ok(fees.into()) + fees.fee_rate_options().map_err(|error| Error::FeesError(error.to_string())) } #[uniffi::method] @@ -978,36 +975,14 @@ impl RustWalletManager { } pub fn fees(&self) -> Option { - let cached_fees = *FEES.load().as_ref(); - - match cached_fees { - Some(cached_fees) - if cached_fees.last_fetched > Instant::now() - Duration::from_secs(30) => - { - cove_tokio::task::spawn( - async move { crate::fee_client::get_and_update_fees().await }, - ); - } - None => { - cove_tokio::task::spawn( - async move { crate::fee_client::get_and_update_fees().await }, - ); - } - _ => {} - } - - if let Some(cached_fees) = cached_fees { - return Some(cached_fees.fees); - } - - None + FEE_CLIENT.fees() } pub async fn fee_rate_options(&self) -> Result { let fee_client = &FEE_CLIENT; let fees = fee_client.fetch_and_get_fees().await.map_err(WalletManagerFeesError::from)?; - Ok(fees.into()) + fees.fee_rate_options().map_err(|error| Error::FeesError(error.to_string())) } #[uniffi::method] From c4947cf4badf6c69c47262ecdb8fc57f6596c75f Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:46:50 -0500 Subject: [PATCH 04/15] Exclude untrusted pending inputs Apply one spend policy to automatic transaction and fee builders. Keep unconfirmed external outputs and locked outputs out of automatic selection while allowing unconfirmed internal change. --- rust/src/manager/wallet_manager/actor.rs | 119 +++++++++++++++++- .../wallet_manager/actor/transactions.rs | 14 +-- 2 files changed, 125 insertions(+), 8 deletions(-) diff --git a/rust/src/manager/wallet_manager/actor.rs b/rust/src/manager/wallet_manager/actor.rs index 4b59164a7..8b378965a 100644 --- a/rust/src/manager/wallet_manager/actor.rs +++ b/rust/src/manager/wallet_manager/actor.rs @@ -586,6 +586,12 @@ impl WalletActor { Ok(outpoints) } + fn automatic_spend_policy(&self) -> Result { + let locked_outpoints = self.locked_output_outpoints()?; + + Ok(SpendPolicy::from_wallet_outputs(self.wallet.bdk.list_unspent(), locked_outpoints)) + } + fn reject_locked_outpoints(&self, outpoints: &[OutPoint]) -> Result<(), Error> { let locked_outpoints = self.db.labels.locked_output_outpoints().map_err_str(Error::OutputLabelsError)?; @@ -1207,6 +1213,7 @@ fn reject_locked_selected_outpoints( Ok(()) } +#[cfg(test)] fn exclude_locked_outpoints( tx_builder: &mut TxBuilder<'_, Cs>, locked_outpoints: Vec, @@ -1214,6 +1221,39 @@ fn exclude_locked_outpoints( tx_builder.unspendable(locked_outpoints); } +#[derive(Debug, Clone, Eq, PartialEq)] +struct SpendPolicy { + locked_outpoints: HashSet, + unconfirmed_external_outpoints: HashSet, +} + +impl SpendPolicy { + fn from_wallet_outputs( + outputs: impl IntoIterator, + locked_outpoints: impl IntoIterator, + ) -> Self { + let unconfirmed_external_outpoints = outputs + .into_iter() + .filter(|output| { + output.keychain == KeychainKind::External + && matches!(output.chain_position, ChainPosition::Unconfirmed { .. }) + }) + .map(|output| output.outpoint) + .collect(); + + Self { + locked_outpoints: locked_outpoints.into_iter().collect(), + unconfirmed_external_outpoints, + } + } + + fn apply(&self, tx_builder: &mut TxBuilder<'_, Cs>) { + let mut unspendable = self.locked_outpoints.iter().copied().collect::>(); + unspendable.extend(self.unconfirmed_external_outpoints.iter().copied()); + tx_builder.unspendable(unspendable); + } +} + #[cfg(test)] mod tests { use act_zero::{runtimes::tokio::spawn_actor, *}; @@ -1228,7 +1268,7 @@ mod tests { use bip39::Mnemonic; use bitcoin::{ Address as BdkAddress, Amount, BlockHash, Network, OutPoint, ScriptBuf, - Transaction as BdkTransaction, TxOut, Txid, absolute::LockTime, hashes::Hash as _, + Transaction as BdkTransaction, TxIn, TxOut, Txid, absolute::LockTime, hashes::Hash as _, transaction::Version, }; use cove_bdk_progressive_scan::ScanUpdate; @@ -2045,6 +2085,83 @@ mod tests { assert!(spent_outpoints.contains(&unlocked)); } + #[test] + fn automatic_spend_policy_excludes_unconfirmed_external_outputs_but_keeps_internal_outputs() { + let (mut wallet, initial_txid) = get_funded_wallet_wpkh(); + let external = receive_output(&mut wallet, Amount::from_sat(80_000), ReceiveTo::Mempool(1)); + let internal_address = wallet.next_unused_address(KeychainKind::Internal).address; + let internal = receive_output_to_address( + &mut wallet, + internal_address, + Amount::from_sat(80_000), + ReceiveTo::Mempool(2), + ); + let policy = super::SpendPolicy::from_wallet_outputs( + wallet.list_unspent(), + [OutPoint { txid: initial_txid, vout: 0 }], + ); + + let mut tx_builder = wallet.build_tx(); + policy.apply(&mut tx_builder); + tx_builder.add_recipient(regtest_address().script_pubkey(), Amount::from_sat(40_000)); + tx_builder.fee_absolute(Amount::from_sat(500)); + + let psbt = tx_builder.finish().expect("trusted internal output can fund transaction"); + let spent_outpoints = spent_outpoints(&psbt); + + assert!(!spent_outpoints.contains(&external)); + assert!(spent_outpoints.contains(&internal)); + } + + #[test] + fn automatic_spend_policy_leaves_immature_coinbase_filtering_to_bdk() { + let (desc, change_desc) = bdk_wallet::test_utils::get_test_wpkh_and_change_desc(); + let mut wallet = bdk_wallet::Wallet::create(desc, change_desc) + .network(Network::Regtest) + .create_wallet_no_persist() + .expect("wallet is created"); + let confirmation_height = 5; + insert_checkpoint( + &mut wallet, + BlockId { height: confirmation_height, hash: BlockHash::all_zeros() }, + ); + + let coinbase = BdkTransaction { + version: Version::ONE, + lock_time: LockTime::ZERO, + input: vec![TxIn { previous_output: OutPoint::null(), ..Default::default() }], + output: vec![TxOut { + script_pubkey: wallet.next_unused_address(KeychainKind::External).script_pubkey(), + value: Amount::from_sat(25_000), + }], + }; + let coinbase_txid = coinbase.compute_txid(); + let confirmation = ConfirmationBlockTime { + block_id: BlockId { height: confirmation_height, hash: BlockHash::all_zeros() }, + confirmation_time: 30_000, + }; + let mut tx_update = bdk_wallet::chain::TxUpdate::default(); + tx_update.txs = vec![Arc::new(coinbase)]; + tx_update.anchors = [(confirmation, coinbase_txid)].into(); + wallet + .apply_update(bdk_wallet::Update { tx_update, ..Default::default() }) + .expect("confirmed coinbase update applies without a mempool timestamp"); + + let policy = super::SpendPolicy::from_wallet_outputs(wallet.list_unspent(), []); + let mut tx_builder = wallet.build_tx(); + policy.apply(&mut tx_builder); + tx_builder + .add_recipient(regtest_address().script_pubkey(), Amount::from_sat(10_000)) + .current_height(confirmation_height); + + assert!(matches!( + tx_builder.finish(), + Err(bdk_wallet::error::CreateTxError::CoinSelection( + bdk_wallet::coin_selection::InsufficientFunds { available: Amount::ZERO, .. } + )) + )); + } + #[test] fn drain_builder_excludes_locked_outpoints_from_psbt_inputs() { let (mut wallet, initial_txid) = get_funded_wallet_wpkh(); diff --git a/rust/src/manager/wallet_manager/actor/transactions.rs b/rust/src/manager/wallet_manager/actor/transactions.rs index fce652076..dd51c3120 100644 --- a/rust/src/manager/wallet_manager/actor/transactions.rs +++ b/rust/src/manager/wallet_manager/actor/transactions.rs @@ -28,7 +28,7 @@ use crate::{ manager::wallet_manager::{ Error, SendFlowErrorAlert, WalletManagerBuildTxError, WalletManagerError, WalletManagerFeesError, WalletManagerReconcileMessage, - actor::{WalletActor, current_wallet_unspent_outpoints_for_txid, exclude_locked_outpoints}, + actor::{WalletActor, current_wallet_unspent_outpoints_for_txid}, payjoin::{PayjoinActor, PayjoinSessionPersister, build_sender}, }, node::client::NodeClient, @@ -69,10 +69,10 @@ impl WalletActor { address: Address, ) -> Result { let coin_selection = CoveDefaultCoinSelection::new(self.seed); - let locked_outpoints = self.locked_output_outpoints()?; + let spend_policy = self.automatic_spend_policy()?; let mut tx_builder = self.wallet.bdk.build_tx().coin_selection(coin_selection); - exclude_locked_outpoints(&mut tx_builder, locked_outpoints); + spend_policy.apply(&mut tx_builder); tx_builder.ordering(TxOrdering::Untouched); tx_builder.add_recipient(address.script_pubkey(), amount); tx_builder.fee_rate(*option.fee_rate); @@ -103,10 +103,10 @@ impl WalletActor { debug!("build_ephemeral_drain_tx for fee rate {}", fee.sat_per_vb()); let script_pubkey = address.script_pubkey(); - let locked_outpoints = self.locked_output_outpoints()?; + let spend_policy = self.automatic_spend_policy()?; let mut tx_builder = self.wallet.bdk.build_tx(); - exclude_locked_outpoints(&mut tx_builder, locked_outpoints); + spend_policy.apply(&mut tx_builder); tx_builder.drain_wallet().drain_to(script_pubkey).fee_rate(fee.into()); let psbt = tx_builder.finish()?; self.wallet.unreserve_tx_change_addresses(&psbt.unsigned_tx); @@ -129,10 +129,10 @@ impl WalletActor { let script_pubkey = address.script_pubkey(); let coin_selection = CoveDefaultCoinSelection::new(self.seed); - let locked_outpoints = self.locked_output_outpoints()?; + let spend_policy = self.automatic_spend_policy()?; let mut tx_builder = self.wallet.bdk.build_tx().coin_selection(coin_selection); - exclude_locked_outpoints(&mut tx_builder, locked_outpoints); + spend_policy.apply(&mut tx_builder); tx_builder.ordering(TxOrdering::Untouched); tx_builder.add_recipient(script_pubkey, amount); tx_builder.fee_rate(fee_rate); From e567a9e52bd0f03f121b98a0d26ec8a57443a8df Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:47:02 -0500 Subject: [PATCH 05/15] Validate complete SeedQR groups Require exact four-digit ASCII groups before parsing word indexes. Reject truncated and Unicode input as typed errors instead of allowing a panic. --- rust/src/seed_qr.rs | 51 +++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/rust/src/seed_qr.rs b/rust/src/seed_qr.rs index 3307c29b9..301cb18b5 100644 --- a/rust/src/seed_qr.rs +++ b/rust/src/seed_qr.rs @@ -14,6 +14,9 @@ pub enum SeedQrError { #[error("Not a standard seed QR, contains non numeric chars")] ContainsNonNumericChars, + #[error("Seed QR digit length must be a multiple of four")] + InvalidLength, + #[error("Index out of bounds: {0}, max is: 2047")] IndexOutOfBounds(u16), @@ -108,33 +111,27 @@ impl SeedQr { } fn parse_str_into_word_indexes(qr: &str) -> Result, SeedQrError> { - if !qr.chars().all(char::is_numeric) { + if !qr.is_ascii() || !qr.bytes().all(|byte| byte.is_ascii_digit()) { return Err(SeedQrError::ContainsNonNumericChars); } - let max_index = qr.len(); - let mut indexes: Vec = Vec::with_capacity((qr.len() / 4) + 1); - let mut current_starting_index = 0; - - let end_index = |starting_index: usize| -> usize { - let index = starting_index + 4; - if index > max_index { max_index } else { index } - }; + if !qr.len().is_multiple_of(4) { + return Err(SeedQrError::InvalidLength); + } - while current_starting_index < max_index { - let starting_index = current_starting_index; - let ending_index = end_index(starting_index); + let mut indexes: Vec = Vec::with_capacity(qr.len() / 4); - let word_index: u16 = - qr[starting_index..ending_index].parse().expect("already checked all numeric"); + for group in qr.as_bytes().chunks_exact(4) { + let word_index = u16::from(group[0] - b'0') * 1000 + + u16::from(group[1] - b'0') * 100 + + u16::from(group[2] - b'0') * 10 + + u16::from(group[3] - b'0'); if word_index > 2047 { return Err(SeedQrError::IndexOutOfBounds(word_index)); } indexes.push(word_index); - - current_starting_index = ending_index; } match indexes.len() { @@ -179,6 +176,28 @@ pub mod tests { assert_eq!(parse_str_into_word_indexes(qr).unwrap(), expected); } + #[test] + fn rejects_non_multiple_of_four_without_panicking() { + assert_eq!(parse_str_into_word_indexes("123"), Err(SeedQrError::InvalidLength)); + assert_eq!(parse_str_into_word_indexes("12345"), Err(SeedQrError::InvalidLength)); + } + + #[test] + fn rejects_unicode_digits_without_panicking() { + let result = std::panic::catch_unwind(|| SeedQr::try_from_str("１２３４")); + + assert!(result.is_ok()); + assert!(matches!(result.unwrap(), Err(SeedQrError::ContainsNonNumericChars))); + } + + #[test] + fn preserves_leading_zeroes_in_four_digit_groups() { + let qr = "073318950739065415961602009907670428187212261116"; + let expected = vec![733, 1895, 739, 654, 1596, 1602, 99, 767, 428, 1872, 1226, 1116]; + + assert_eq!(parse_str_into_word_indexes(qr).unwrap(), expected); + } + #[test] fn test_get_words_from_str() { let qr = "192402220235174306311124037817700641198012901210"; From e9c1c99cbbd727b91f96e03535e949491dbde0af Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:47:35 -0500 Subject: [PATCH 06/15] Redact TapSigner PIN data Store validated PINs in zeroizing typed values and redact their debug output. Remove PIN, command, APDU, and raw error details from mobile logs and alerts. --- .../TapSignerFlow/TapSignerConfirmPinView.kt | 11 +- .../TapSignerFlow/TapSignerEnterPinView.kt | 2 +- .../TapSignerFlow/TapSignerImportRetryView.kt | 2 +- .../TapSignerImportSuccessView.kt | 11 +- .../flows/TapSignerFlow/TapSignerManager.kt | 19 +- .../flows/TapSignerFlow/TapSignerNfcHelper.kt | 12 +- .../TapSignerFlow/TapSignerSetupRetryView.kt | 2 +- .../TapSignerSetupSuccessView.kt | 2 +- .../bitcoinppl/cove/nfc/TapCardNfcManager.kt | 27 +- .../TapSignerConfirmPinView.swift | 17 +- .../TapSignerFlow/TapSignerContainer.swift | 19 +- .../TapSignerFlow/TapSignerEnterPinView.swift | 21 +- .../TapSignerImportRetryView.swift | 8 +- .../TapSignerImportSuccessView.swift | 2 +- .../TapSignerSetupRetryView.swift | 12 +- .../TapSignerSetupSuccessView.swift | 2 +- ios/Cove/TapSignerNFC.swift | 94 +++--- rust/src/tap_card/tap_signer_reader.rs | 267 ++++++++++++++---- 18 files changed, 377 insertions(+), 153 deletions(-) diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerConfirmPinView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerConfirmPinView.kt index 8b97d9067..6b54d3966 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerConfirmPinView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerConfirmPinView.kt @@ -261,7 +261,9 @@ private suspend fun setupTapSigner( app.sheetState = null app.alertState = TaggedItem( - AppAlertState.TapSignerSetupFailed(e.message ?: "Unknown error"), + AppAlertState.TapSignerSetupFailed( + "TapSigner setup failed. Please try again.", + ), ) } } @@ -310,7 +312,12 @@ private suspend fun changeTapSignerPin( Log.e("TapSignerConfirmPin", "Error changing PIN") // check error type and show appropriate alert - val errorMessage = e.message ?: "Unknown error" + val errorMessage = + if (isAuthError(e)) { + "Wrong PIN, please try again" + } else { + "TapSigner PIN change failed. Please try again." + } app.alertState = TaggedItem( AppAlertState.General( diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerEnterPinView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerEnterPinView.kt index e3816b927..166e2ae5c 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerEnterPinView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerEnterPinView.kt @@ -380,7 +380,7 @@ private suspend fun signAction( } } -private fun isAuthError(error: Exception): Boolean { +internal fun isAuthError(error: Exception): Boolean { // check if error is a bad auth error using type-safe FFI function return error is org.bitcoinppl.cove_core.TapSignerReaderException && error.isAuthError() diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt index a00aa2c76..a9c5536cb 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt @@ -143,7 +143,7 @@ fun TapSignerImportRetryView( app.alertState = TaggedItem( AppAlertState.TapSignerDeriveFailed( - e.message ?: "Unknown error occurred", + "TapSigner import failed. Please try again.", ), ) } diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportSuccessView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportSuccessView.kt index 5e5898a07..ab0b1410f 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportSuccessView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportSuccessView.kt @@ -71,8 +71,8 @@ fun TapSignerImportSuccessView( try { walletId = persistWallet(tapSigner, deriveInfo) } catch (e: Exception) { - android.util.Log.e("TapSignerImportSuccess", "Failed to save wallet", e) - error = e.message ?: "Failed to save wallet" + android.util.Log.e("TapSignerImportSuccess", "Failed to save TapSigner wallet") + error = "Failed to save wallet" } finally { saving = false } @@ -155,8 +155,11 @@ fun TapSignerImportSuccessView( try { walletId = persistWallet(tapSigner, deriveInfo) } catch (e: Exception) { - android.util.Log.e("TapSignerImportSuccess", "Failed to save wallet", e) - error = e.message ?: "Failed to save wallet" + android.util.Log.e( + "TapSignerImportSuccess", + "Failed to save TapSigner wallet", + ) + error = "Failed to save wallet" } finally { saving = false } diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerManager.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerManager.kt index 2e4386d2b..c6ce491a0 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerManager.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerManager.kt @@ -56,10 +56,27 @@ class TapSignerManager( return } - android.util.Log.d(tag, "Navigating to $to, current path: $path") + android.util.Log.d( + tag, + "Navigating to ${routeKind(to)}, current path count: ${path.size}", + ) path.add(to) } + private fun routeKind(route: org.bitcoinppl.cove_core.TapSignerRoute): String = + when (route) { + is org.bitcoinppl.cove_core.TapSignerRoute.InitSelect -> "initSelect" + is org.bitcoinppl.cove_core.TapSignerRoute.InitAdvanced -> "initAdvanced" + is org.bitcoinppl.cove_core.TapSignerRoute.StartingPin -> "startingPin" + is org.bitcoinppl.cove_core.TapSignerRoute.NewPin -> "newPin" + is org.bitcoinppl.cove_core.TapSignerRoute.ConfirmPin -> "confirmPin" + is org.bitcoinppl.cove_core.TapSignerRoute.SetupSuccess -> "setupSuccess" + is org.bitcoinppl.cove_core.TapSignerRoute.SetupRetry -> "setupRetry" + is org.bitcoinppl.cove_core.TapSignerRoute.ImportSuccess -> "importSuccess" + is org.bitcoinppl.cove_core.TapSignerRoute.ImportRetry -> "importRetry" + is org.bitcoinppl.cove_core.TapSignerRoute.EnterPin -> "enterPin" + } + private fun shouldPreventNavigation( from: org.bitcoinppl.cove_core.TapSignerRoute, to: org.bitcoinppl.cove_core.TapSignerRoute, diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt index 873a90f40..e4fa94e5d 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt @@ -23,9 +23,9 @@ class TapSignerNfcHelper( ): SetupCmdResponse = try { doSetupTapSigner(factoryPin, newPin, chainCode) - } catch (e: Exception) { - Log.e(tag, "Setup failed", e) - throw e + } catch (error: Exception) { + Log.e(tag, "TapSigner setup failed") + throw error } suspend fun derive(pin: String): DeriveInfo = @@ -94,9 +94,9 @@ class TapSignerNfcHelper( lastResponse?.destroy() lastResponse = response return result - } catch (e: Exception) { - Log.e(tag, "TapSigner command failed", e) - throw e + } catch (error: Exception) { + Log.e(tag, "TapSigner operation failed") + throw error } } diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt index 79d235f36..45199938b 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt @@ -161,7 +161,7 @@ fun TapSignerSetupRetryView( app.alertState = TaggedItem( AppAlertState.TapSignerSetupFailed( - e.message ?: "Unknown error", + "TapSigner setup failed. Please try again.", ), ) } diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupSuccessView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupSuccessView.kt index 8386b600c..79073d837 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupSuccessView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupSuccessView.kt @@ -58,7 +58,7 @@ fun TapSignerSetupSuccessView( walletId = walletManager.id } } catch (e: Exception) { - android.util.Log.e("TapSignerSetupSuccess", "Failed to save wallet", e) + android.util.Log.e("TapSignerSetupSuccess", "Failed to save TapSigner wallet") } } diff --git a/android/app/src/main/java/org/bitcoinppl/cove/nfc/TapCardNfcManager.kt b/android/app/src/main/java/org/bitcoinppl/cove/nfc/TapCardNfcManager.kt index c7cab0c4e..52c54a38c 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/nfc/TapCardNfcManager.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/nfc/TapCardNfcManager.kt @@ -32,7 +32,6 @@ class TapCardNfcManager private constructor() { private val mainHandler = android.os.Handler(android.os.Looper.getMainLooper()) // current operation state - private var currentCmd: TapSignerCmd? = null private var tagDetected = CompletableDeferred() private var isScanning = false private var pendingDisableRunnable: Runnable? = null @@ -68,7 +67,7 @@ class TapCardNfcManager private constructor() { throw Exception("NFC is disabled. Please enable it in Settings") } - Log.d(tag, "Starting NFC scan for command: $cmd") + Log.d(tag, "Starting NFC scan for operation: ${operationKind(cmd)}") return@withLock suspendCancellableCoroutine { continuation -> // cancel any pending disable from previous operation @@ -76,7 +75,6 @@ class TapCardNfcManager private constructor() { pendingDisableRunnable = null // reset state for new operation - currentCmd = cmd tagDetected = CompletableDeferred() isScanning = true @@ -138,7 +136,7 @@ class TapCardNfcManager private constructor() { Log.d( tag, - "Connected to IsoDep tag (timeout=${timeout}ms, cmd=${cmd::class.simpleName})", + "Connected to IsoDep tag (timeout=${timeout}ms, operation=${operationKind(cmd)})", ) // send proactive UX guidance for heavy NFC operations @@ -174,14 +172,14 @@ class TapCardNfcManager private constructor() { continuation.resume(resultPair) } } catch (e: TapSignerReaderException) { - Log.e(tag, "TapSigner error", e) + Log.e(tag, "TapSigner operation failed: ${operationKind(cmd)}") // guard against cancelled continuation if (continuation.isActive) { continuation.resumeWithException(e) } } catch (e: Exception) { - Log.e(tag, "NFC operation failed", e) + Log.e(tag, "NFC operation failed: ${operationKind(cmd)}") // guard against cancelled continuation if (continuation.isActive) { continuation.resumeWithException(e) @@ -228,6 +226,15 @@ class TapCardNfcManager private constructor() { } } + private fun operationKind(cmd: TapSignerCmd): String = + when (cmd) { + is TapSignerCmd.Setup -> "setup" + is TapSignerCmd.Backup -> "backup" + is TapSignerCmd.Derive -> "derive" + is TapSignerCmd.Change -> "change" + is TapSignerCmd.Sign -> "sign" + } + companion object { @Volatile private var instance: TapCardNfcManager? = null @@ -258,13 +265,13 @@ private class TapCardTransport( private var currentMessage = "" override fun setMessage(message: String) { - Log.d(tag, "Message: $message") + Log.d(tag, "TapSigner progress message updated") currentMessage = message onMessageUpdate?.invoke(currentMessage) } override fun appendMessage(message: String) { - Log.d(tag, "Append: $message") + Log.d(tag, "TapSigner progress message appended") currentMessage += message onMessageUpdate?.invoke(currentMessage) } @@ -281,8 +288,8 @@ private class TapCardTransport( val response = isoDep.transceive(commandApdu) Log.d(tag, "APDU response: ${response.size} bytes") response - } catch (e: Exception) { - Log.e(tag, "APDU error", e) + } catch (_: Exception) { + Log.e(tag, "TapSigner APDU transmission failed") throw TransportException.UnknownException( "Tag connection lost, please hold your phone still and try again" ) diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerConfirmPinView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerConfirmPinView.swift index 0e9649dc8..5134ccb01 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerConfirmPinView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerConfirmPinView.swift @@ -49,16 +49,20 @@ struct TapSignerConfirmPinView: View { manager.resetRoute(to: .setupSuccess(args.tapSigner, c)) case let .success(incomplete): manager.resetRoute(to: .setupRetry(args.tapSigner, incomplete)) - case let .failure(error): + case .failure: // failed to setup but we can continue if let incomplete = nfc.lastResponse()?.setupResponse { return manager.resetRoute(to: .setupRetry(args.tapSigner, incomplete)) } // failed to setup and can't continue from a screen, send back to home and ask them to restart the process - Log.error("Failed to setup TapSigner: \(error)") + Log.error("TapSigner setup failed") app.sheetState = .none - app.alertState = .init(.tapSignerSetupFailed(message: error.description)) + app.alertState = .init( + .tapSignerSetupFailed( + message: "TapSigner setup failed. Please try again." + ) + ) } } } @@ -81,7 +85,12 @@ struct TapSignerConfirmPinView: View { case let .failure(error): if error.isAuthError() { return app.alertState = .init(.tapSignerInvalidAuth) } if error.isNoBackupError() { return app.alertState = .init(.tapSignerNoBackup(tapSigner: args.tapSigner)) } - app.alertState = .init(.general(title: "Error", message: error.description)) + app.alertState = .init( + .general( + title: "Error", + message: "TapSigner PIN change failed. Please try again." + ) + ) } } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerContainer.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerContainer.swift index d2aff3e69..6353c857e 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerContainer.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerContainer.swift @@ -50,10 +50,27 @@ class TapSignerManager { } } - logger.debug("Navigating to \(newRoute), current path: \(path)") + logger.debug( + "Navigating to \(routeKind(newRoute)), current path count: \(path.count)" + ) path.append(newRoute) } + private func routeKind(_ route: TapSignerRoute) -> String { + switch route { + case .initSelect: "initSelect" + case .initAdvanced: "initAdvanced" + case .startingPin: "startingPin" + case .newPin: "newPin" + case .confirmPin: "confirmPin" + case .setupSuccess: "setupSuccess" + case .setupRetry: "setupRetry" + case .importSuccess: "importSuccess" + case .importRetry: "importRetry" + case .enterPin: "enterPin" + } + } + func popRoute() { if !path.isEmpty { path.removeLast() } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerEnterPinView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerEnterPinView.swift index 2dc27071c..6762d797e 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerEnterPinView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerEnterPinView.swift @@ -55,7 +55,11 @@ struct TapSignerEnterPin: View { app.sheetState = nil app.alertState = .init(.tapSignerWrongPin(tapSigner: tapSigner, action: .derive)) } else { - app.alertState = .init(.tapSignerDeriveFailed(message: error.description)) + app.alertState = .init( + .tapSignerDeriveFailed( + message: "TapSigner import failed. Please try again." + ) + ) } } @@ -85,7 +89,10 @@ struct TapSignerEnterPin: View { app.alertState = .init(.tapSignerWrongPin(tapSigner: tapSigner, action: .backup)) } else { app.alertState = .init( - .general(title: "Backup Failed!", message: error.description) + .general( + title: "Backup Failed!", + message: "TapSigner backup failed. Please try again." + ) ) } @@ -117,7 +124,10 @@ struct TapSignerEnterPin: View { } catch { await MainActor.run { app.alertState = .init( - .general(title: "Error", message: error.localizedDescription) + .general( + title: "Error", + message: "Unable to load the pending transaction." + ) ) self.pin = "" @@ -130,7 +140,10 @@ struct TapSignerEnterPin: View { app.alertState = .init(.tapSignerWrongPin(tapSigner: tapSigner, action: .sign(psbt))) } else { app.alertState = .init( - .general(title: "Signing Failed!", message: error.description) + .general( + title: "Signing Failed!", + message: "TapSigner signing failed. Please try again." + ) ) app.sheetState = .none } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift index 672a88991..64ac2c730 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift @@ -47,8 +47,12 @@ struct TapSignerImportRetry: View { switch await nfc.derive(pin: pin) { case let .success(deriveInfo): manager.resetRoute(to: .importSuccess(tapSigner, deriveInfo)) - case let .failure(error): - app.alertState = .init(.tapSignerDeriveFailed(message: error.description)) + case .failure: + app.alertState = .init( + .tapSignerDeriveFailed( + message: "TapSigner import failed. Please try again." + ) + ) } } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerImportSuccessView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerImportSuccessView.swift index 0821cc262..879705ab9 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerImportSuccessView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerImportSuccessView.swift @@ -22,7 +22,7 @@ struct TapSignerImportSuccess: View { let manager = try WalletManager(tapSigner: tapSigner, deriveInfo: deriveInfo) walletId = manager.id } catch { - Log.error("Failed to save wallet: \(error.localizedDescription)") + Log.error("Failed to save TapSigner wallet") } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift index 165f8ce09..7ac199565 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift @@ -44,16 +44,18 @@ struct TapSignerSetupRetry: View { case let .success(.complete(complete)): manager.resetRoute(to: .setupSuccess(tapSigner, complete)) case let .success(incomplete): - Log.error( - "Failed to complete TAPSIGNER setup, won't retry anymore \(incomplete)" - ) + Log.error("TapSigner setup retry returned an incomplete response") app.sheetState = nil app.alertState = .init( .tapSignerSetupFailed(message: "Failed to setup TapSigner") ) - case let .failure(error): + case .failure: app.sheetState = nil - app.alertState = .init(.tapSignerSetupFailed(message: error.description)) + app.alertState = .init( + .tapSignerSetupFailed( + message: "TapSigner setup failed. Please try again." + ) + ) } } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupSuccessView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupSuccessView.swift index d7680391d..02dd1c92f 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupSuccessView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupSuccessView.swift @@ -27,7 +27,7 @@ struct TapSignerSetupSuccess: View { walletId = manager.id } catch { - Log.error("Failed to save wallet: \(error.localizedDescription)") + Log.error("Failed to save TapSigner wallet") } } diff --git a/ios/Cove/TapSignerNFC.swift b/ios/Cove/TapSignerNFC.swift index 61a709ff6..38e3262eb 100644 --- a/ios/Cove/TapSignerNFC.swift +++ b/ios/Cove/TapSignerNFC.swift @@ -33,7 +33,7 @@ class TapSignerNFC { } catch let error as TapSignerReaderError { return .failure(error) } catch { - return .failure(TapSignerReaderError.Unknown(error.localizedDescription)) + return .failure(TapSignerReaderError.Unknown("TapSigner setup failed")) } } @@ -82,11 +82,11 @@ class TapSignerNFC { self.nfc.session?.invalidate() } else if let error = self.nfc.tapSignerError { continuation.resume(returning: .failure(error)) - self.nfc.session?.invalidate(errorMessage: error.description) - } else { - Log.error( - "Unknown error response: \(String(describing: self.nfc.tapSignerResponse)), error: \(String(describing: self.nfc.tapSignerError))" + self.nfc.session?.invalidate( + errorMessage: "TapSigner operation failed. Please try again." ) + } else { + Log.error("TapSigner operation returned no response") let error = TapSignerReaderError.Unknown("Unknown error occurred") continuation.resume(returning: .failure(error)) self.nfc.session?.invalidate(errorMessage: error.description) @@ -99,11 +99,13 @@ class TapSignerNFC { } } } catch let error as TapSignerReaderError { - self.nfc.session?.invalidate(errorMessage: error.description) + self.nfc.session?.invalidate( + errorMessage: "TapSigner operation failed. Please try again." + ) return .failure(error) } catch { nfc.session?.invalidate(errorMessage: "Something went wrong!") - return .failure(.Unknown(error.localizedDescription)) + return .failure(.Unknown("TapSigner operation failed")) } } @@ -111,8 +113,6 @@ class TapSignerNFC { async throws -> SetupCmdResponse { var errorCount = 0 - var lastError: TapSignerReaderError? = nil - let response = try await startSetupTapSigner( factoryPin: factoryPin, newPin: newPin, @@ -137,20 +137,17 @@ class TapSignerNFC { case let .success(other): errorCount += 1 - lastError = other.error incompleteResponse = other - case let .failure(error): + case .failure: nfc.session?.invalidate() - Log.error("Error count: \(errorCount), last error: \(error)") + Log.error("TapSigner setup failed during retry") return incompleteResponse } if errorCount > 5 { nfc.session?.invalidate() - Log.error( - "Error count: \(errorCount), last error: \(lastError ?? .Unknown("unknown error, no error found"))" - ) + Log.error("TapSigner setup retry limit reached") return incompleteResponse } } @@ -196,7 +193,7 @@ class TapSignerNFC { } catch let error as TapSignerReaderError { throw error } catch { - throw TapSignerReaderError.Unknown(error.localizedDescription) + throw TapSignerReaderError.Unknown("TapSigner setup failed") } } } @@ -252,7 +249,7 @@ class TapSignerNFC { } catch let error as TapSignerReaderError { return .failure(error) } catch { - return .failure(.Unknown(error.localizedDescription)) + return .failure(.Unknown("TapSigner setup retry failed")) } } } @@ -294,13 +291,7 @@ private class TapCardNFC: NSObject, NFCTagReaderSessionDelegate { func scan() { guard let tapSignerCmd else { return Log.error("cmd not set") } - switch tapSignerCmd { - case .setup: logger.info("started scanning for setup") - case .derive: logger.info("started scanning for derive") - case .change: logger.info("started scanning for change pin cmd") - case .backup: logger.info("started scanning for backup") - case .sign: logger.info("started scanning for sign") - } + logger.info("started scanning for \(operationKind(tapSignerCmd))") isScanning = true session = NFCTagReaderSession(pollingOption: [.iso14443, .iso15693], delegate: self) @@ -308,6 +299,16 @@ private class TapCardNFC: NSObject, NFCTagReaderSessionDelegate { session?.begin() } + private func operationKind(_ command: TapSignerCmd) -> String { + switch command { + case .setup: "setup" + case .derive: "derive" + case .change: "change" + case .backup: "backup" + case .sign: "sign" + } + } + func tagReaderSession(_ session: NFCTagReaderSession, didDetect tags: [NFCTag]) { self.session = session guard let tag = tags.first else { @@ -316,10 +317,9 @@ private class TapCardNFC: NSObject, NFCTagReaderSessionDelegate { } session.connect(to: tag) { error in - if let error { + if error != nil { session.invalidate( - errorMessage: - "Connection error: \(error.localizedDescription), please try again" + errorMessage: "Unable to connect to TapSigner. Please try again." ) return } @@ -370,15 +370,16 @@ private class TapCardNFC: NSObject, NFCTagReaderSessionDelegate { tapSignerResponse = response } } catch let error as TapSignerReaderError { - logger.error("TAPSIGNER error: \(error)") + let operation = tapSignerCmd.map(operationKind) ?? "unknown" + logger.error("TapSigner operation failed: \(operation)") tapSignerError = error if case .TapSignerError(.CkTap(.BadAuth)) = error { return session.invalidate(errorMessage: "Wrong PIN, please try again") } - session.invalidate(errorMessage: "TapSigner error: \(error.description)") + session.invalidate(errorMessage: "TapSigner operation failed. Please try again.") } catch { - logger.error("Error creating reader: \(error)") - session.invalidate(errorMessage: "Error creating reader: \(error.localizedDescription)") + logger.error("TapSigner operation failed while creating reader") + session.invalidate(errorMessage: "Unable to read TapSigner, please try again") } } @@ -393,12 +394,12 @@ private class TapCardNFC: NSObject, NFCTagReaderSessionDelegate { if let nfcError = error as? NFCReaderError, nfcError.code == .readerSessionInvalidationErrorUserCanceled { - logger.debug("tapcard reader session ended normally: \(error.localizedDescription)") + logger.debug("tapcard reader session ended normally") return } // actual error occurred - Log.error("tapcard reader session did invalidate with error: \(error.localizedDescription)") + Log.error("tapcard reader session did invalidate with an error") switch error as? NFCReaderError { case .none: tapSignerError = .Unknown("Unable to read NFC tag, try again") @@ -438,7 +439,7 @@ class TapCardTransport: TapcardTransportProtocol, @unchecked Sendable { } func transmitApdu(commandApdu: Data) async throws -> Data { - logger.debug("Transmitting APDU: \(commandApdu) bytes") + logger.debug("Transmitting APDU, bytes=\(commandApdu.count)") guard let apdu = NFCISO7816APDU(data: commandApdu) else { logger.error("Invalid APDU") @@ -449,10 +450,12 @@ class TapCardTransport: TapcardTransportProtocol, @unchecked Sendable { tag.sendCommand(apdu: apdu) { response, sw1Value, sw2Value, error in Log.debug("APDU response: \(response.count) bytes") - if let error { - logger.error("APDU error: \(error)") + if error != nil { + logger.error("TapSigner APDU transmission failed") continuation.resume( - throwing: TransportError.UnknownError(error.localizedDescription) + throwing: TransportError.UnknownError( + "Unable to communicate with TapSigner, please try again" + ) ) return } @@ -460,21 +463,8 @@ class TapCardTransport: TapcardTransportProtocol, @unchecked Sendable { // Check for success (0x9000) let statusWord = (Int(sw1Value) << 8) | Int(sw2Value) if statusWord != 0x9000 { - // Handle specific error codes - var errorMessage = "" - switch statusWord { - case 0x6D00: - errorMessage = "Instruction code not supported or invalid" - default: - errorMessage = - if !response.isEmpty { - "Card error: SW=\(String(format: "0x%04X", statusWord)), data: \(response.hexEncodedString())" - } else { - "Card error: SW=\(String(format: "0x%04X", statusWord))" - } - } - - logger.error("APDU ERROR: \(errorMessage)") + let errorMessage = "TapSigner card rejected the operation" + logger.error("TapSigner APDU operation rejected") continuation.resume( throwing: TransportError(code: statusWord, message: errorMessage) ) diff --git a/rust/src/tap_card/tap_signer_reader.rs b/rust/src/tap_card/tap_signer_reader.rs index 2fffd4744..d5a254844 100644 --- a/rust/src/tap_card/tap_signer_reader.rs +++ b/rust/src/tap_card/tap_signer_reader.rs @@ -1,5 +1,4 @@ -use std::hash::Hasher; -use std::sync::Arc; +use std::{fmt, hash::Hasher, sync::Arc}; use bitcoin::{bip32::Fingerprint, hashes::HashEngine as _, secp256k1}; use nid::Nanoid; @@ -14,6 +13,7 @@ use rust_cktap::{ use tokio::sync::Mutex; use tracing::debug; +use zeroize::Zeroize; use crate::{ database::Database, @@ -46,11 +46,11 @@ pub enum TapSignerReaderError { #[error("No command")] NoCommand, - #[error("Invalid pin length, must be betweeen 6 and 32, found {0}")] - InvalidPinLength(u8), + #[error("PIN must be between 6 and 32 digits")] + InvalidPinLength, - #[error("PIN must be numeric only, found {0}")] - NonNumericPin(String), + #[error("PIN must contain only ASCII digits")] + NonNumericPin, #[error("Setup is already complete")] SetupAlreadyComplete, @@ -65,12 +65,51 @@ pub enum TapSignerReaderError { type Error = TapSignerReaderError; type Result = std::result::Result; +const MIN_PIN_LENGTH: usize = 6; +const MAX_PIN_LENGTH: usize = 32; + +#[derive(Clone, Eq, Hash, PartialEq, zeroize::Zeroize, zeroize::ZeroizeOnDrop)] +struct TapSignerPin(String); + +impl TapSignerPin { + fn try_new(mut pin: String) -> Result { + let pin_length = pin.len(); + if !(MIN_PIN_LENGTH..=MAX_PIN_LENGTH).contains(&pin_length) { + pin.zeroize(); + return Err(TapSignerReaderError::InvalidPinLength); + } + + if !pin.bytes().all(|byte| byte.is_ascii_digit()) { + pin.zeroize(); + return Err(TapSignerReaderError::NonNumericPin); + } + + Ok(Self(pin)) + } + + fn as_str(&self) -> &str { + &self.0 + } +} + +impl AsRef for TapSignerPin { + fn as_ref(&self) -> &str { + self.as_str() + } +} + +impl fmt::Debug for TapSignerPin { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("") + } +} + // Main interface exposed to Swift -#[derive(Debug, uniffi::Object)] +#[derive(uniffi::Object)] pub struct TapSignerReader { id: String, reader: Mutex, - cmd: RwLock>, + cmd: RwLock>, transport: TapcardTransport, /// Last response from the setup process, has started, if the last response is `Complete` then the setup process is complete @@ -79,6 +118,18 @@ pub struct TapSignerReader { network: Network, } +impl fmt::Debug for TapSignerReader { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + let operation = self.cmd.read().as_ref().map(TapSignerOperation::kind); + + formatter + .debug_struct("TapSignerReader") + .field("id", &self.id) + .field("operation", &operation) + .finish() + } +} + #[derive(Debug, derive_more::Deref, derive_more::DerefMut)] struct VerifiedTapSigner(rust_cktap::TapSigner); @@ -107,7 +158,7 @@ impl VerifiedTapSigner { } } -#[derive(Debug, Clone, Hash, PartialEq, Eq, uniffi::Enum)] +#[derive(Clone, Hash, PartialEq, Eq, uniffi::Enum)] pub enum TapSignerCmd { Setup(Arc), Backup { pin: String }, @@ -116,13 +167,78 @@ pub enum TapSignerCmd { Sign { psbt: Arc, pin: String }, } -#[derive(Debug, Clone, Hash, PartialEq, Eq, uniffi::Object)] +impl fmt::Debug for TapSignerCmd { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + let operation = match self { + Self::Setup(_) => "setup", + Self::Backup { .. } => "backup", + Self::Derive { .. } => "derive", + Self::Change { .. } => "change", + Self::Sign { .. } => "sign", + }; + + formatter.debug_struct("TapSignerCmd").field("operation", &operation).finish() + } +} + +#[derive(Clone, Hash, PartialEq, Eq, uniffi::Object)] pub struct SetupCmd { - pub factory_pin: String, - pub new_pin: String, + factory_pin: TapSignerPin, + new_pin: TapSignerPin, pub chain_code: [u8; 32], } +impl fmt::Debug for SetupCmd { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("SetupCmd") + .field("factory_pin", &"") + .field("new_pin", &"") + .field("chain_code", &"") + .finish() + } +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +enum TapSignerOperation { + Setup(Arc), + Backup(TapSignerPin), + Derive(TapSignerPin), + Change { current_pin: TapSignerPin, new_pin: TapSignerPin }, + Sign { psbt: Arc, pin: TapSignerPin }, +} + +impl TapSignerOperation { + const fn kind(&self) -> &'static str { + match self { + Self::Setup(_) => "setup", + Self::Backup(_) => "backup", + Self::Derive(_) => "derive", + Self::Change { .. } => "change", + Self::Sign { .. } => "sign", + } + } +} + +impl TryFrom for TapSignerOperation { + type Error = TapSignerReaderError; + + fn try_from(command: TapSignerCmd) -> Result { + match command { + TapSignerCmd::Setup(command) => Ok(Self::Setup(command)), + TapSignerCmd::Backup { pin } => Ok(Self::Backup(TapSignerPin::try_new(pin)?)), + TapSignerCmd::Derive { pin } => Ok(Self::Derive(TapSignerPin::try_new(pin)?)), + TapSignerCmd::Change { current_pin, new_pin } => Ok(Self::Change { + current_pin: TapSignerPin::try_new(current_pin)?, + new_pin: TapSignerPin::try_new(new_pin)?, + }), + TapSignerCmd::Sign { psbt, pin } => { + Ok(Self::Sign { psbt, pin: TapSignerPin::try_new(pin)? }) + } + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, uniffi::Enum, derive_more::From)] pub enum TapSignerResponse { Setup(SetupCmdResponse), @@ -183,10 +299,11 @@ impl TapSignerReader { transport: Box, cmd: Option, ) -> Result { + let cmd = cmd.map(TapSignerOperation::try_from).transpose()?; let transport = TapcardTransport(Arc::new(transport)); let card = VerifiedTapSigner::connect(transport.clone()).await?; - debug!("tap_card_from_status: {:?}", card); + debug!("TapSigner card authenticated"); let id: Nanoid = Nanoid::new(); let network = Database::global().global_config.selected_network(); @@ -211,31 +328,33 @@ impl TapSignerReader { impl TapSignerReader { #[uniffi::method] pub async fn run(&self) -> Result { - let cmd = self.cmd.write().take().ok_or(TapSignerReaderError::NoCommand)?; + let operation = self.cmd.write().take().ok_or(TapSignerReaderError::NoCommand)?; + + debug!(operation = operation.kind(), "running TapSigner operation"); - match cmd { - TapSignerCmd::Setup(cmd) => { + match operation { + TapSignerOperation::Setup(cmd) => { let response = self.setup(cmd).await?; Ok(TapSignerResponse::Setup(response)) } - TapSignerCmd::Backup { pin } => { + TapSignerOperation::Backup(pin) => { let response = self.backup(&pin).await?; Ok(TapSignerResponse::Backup(response)) } - TapSignerCmd::Derive { pin } => { + TapSignerOperation::Derive(pin) => { let response = self.derive(&pin).await?; Ok(TapSignerResponse::Import(response)) } - TapSignerCmd::Change { current_pin, new_pin } => { + TapSignerOperation::Change { current_pin, new_pin } => { self.change(&new_pin, ¤t_pin).await?; Ok(TapSignerResponse::Change) } - TapSignerCmd::Sign { psbt, pin } => { - let txn = self.sign(psbt, &pin).await?; + TapSignerOperation::Sign { psbt, pin } => { + let txn = self.sign_with_pin(psbt, &pin).await?; Ok(TapSignerResponse::Sign(txn.into())) } } @@ -243,15 +362,6 @@ impl TapSignerReader { /// Start the setup process pub async fn setup(&self, cmd: Arc) -> Result { - let new_pin = cmd.new_pin.as_bytes(); - if new_pin.len() < 6 || new_pin.len() > 32 { - return Err(TapSignerReaderError::InvalidPinLength(new_pin.len() as u8)); - } - - if !cmd.new_pin.trim().chars().all(char::is_numeric) { - return Err(TapSignerReaderError::NonNumericPin(cmd.new_pin.to_string())); - } - self.init_backup_change(cmd).await } @@ -279,17 +389,8 @@ impl TapSignerReader { } pub async fn sign(&self, psbt: Arc, pin: &str) -> Result { - let psbt = Arc::unwrap_or_clone(psbt); - - let psbt: bitcoin::Psbt = self - .reader - .lock() - .await - .sign_psbt(psbt.into(), pin) - .await - .map_err_str(Error::PsbtSignError)?; - - Ok(psbt.into()) + let pin = TapSignerPin::try_new(pin.to_owned())?; + self.sign_with_pin(psbt, &pin).await } /// Get the last response from the reader @@ -302,6 +403,20 @@ impl TapSignerReader { } impl TapSignerReader { + async fn sign_with_pin(&self, psbt: Arc, pin: &TapSignerPin) -> Result { + let psbt = Arc::unwrap_or_clone(psbt); + + let psbt: bitcoin::Psbt = self + .reader + .lock() + .await + .sign_psbt(psbt.into(), pin.as_str()) + .await + .map_err_str(Error::PsbtSignError)?; + + Ok(psbt.into()) + } + async fn wait_if_needed(&self) -> Result<(), Error> { let mut auth_delay = self.reader.lock().await.auth_delay; @@ -323,7 +438,7 @@ impl TapSignerReader { .reader .lock() .await - .init(cmd.chain_code, &cmd.factory_pin) + .init(cmd.chain_code, cmd.factory_pin.as_str()) .await .map_err(TransportError::from)?; @@ -397,27 +512,31 @@ impl TapSignerReader { SetupCmdResponse::Complete(complete) } - async fn backup(&self, pin: &str) -> Result, Error> { + async fn backup(&self, pin: &TapSignerPin) -> Result, Error> { let backup_response = - self.reader.lock().await.backup(pin).await.map_err(TransportError::from)?; + self.reader.lock().await.backup(pin.as_str()).await.map_err(TransportError::from)?; Ok(backup_response.data) } - async fn change(&self, new_pin: &str, current_pin: &str) -> Result<(), Error> { + async fn change( + &self, + new_pin: &TapSignerPin, + current_pin: &TapSignerPin, + ) -> Result<(), Error> { debug!("starting pin change"); self.reader .lock() .await - .change(new_pin, current_pin) + .change(new_pin.as_str(), current_pin.as_str()) .await .map_err(TransportError::from)?; Ok(()) } - async fn derive(&self, pin: &str) -> Result { + async fn derive(&self, pin: &TapSignerPin) -> Result { debug!("starting derive"); let path: [u32; 3] = match self.network { @@ -430,7 +549,7 @@ impl TapSignerReader { let birth_height = valid_birth_height(Some( reader.birth.try_into().expect("usize birth height fits in u64"), )); - let derive_response = reader.derive(&path, pin).await?; + let derive_response = reader.derive(&path, pin.as_str()).await?; (derive_response, birth_height) }; let derive_info = @@ -459,6 +578,9 @@ impl SetupCmd { new_pin: String, chain_code: Option>, ) -> Result { + let factory_pin = TapSignerPin::try_new(factory_pin)?; + let new_pin = TapSignerPin::try_new(new_pin)?; + let chain_code = match chain_code { Some(chain_code) => { let chain_code_len = chain_code.len() as u32; @@ -473,9 +595,9 @@ impl SetupCmd { impl std::hash::Hash for TapSignerReader { fn hash(&self, state: &mut H) { - self.id.hash(state); - self.cmd.read().as_ref().hash(state); - self.last_response.lock().as_ref().hash(state); + std::hash::Hash::hash(&self.id, state); + std::hash::Hash::hash(&self.cmd.read().as_ref(), state); + std::hash::Hash::hash(&self.last_response.lock().as_ref(), state); } } @@ -719,6 +841,38 @@ mod tests { )); assert_eq!(calls.load(Ordering::SeqCst), 3); } + + #[test] + fn pin_errors_and_debug_output_do_not_include_pin_values() { + for submitted_pin in ["123", "12345a", "１２３４５６"] { + let error = TapSignerPin::try_new(submitted_pin.to_string()).unwrap_err(); + + assert!(!error.to_string().contains(submitted_pin)); + assert!(!format!("{error:?}").contains(submitted_pin)); + } + + let pin = TapSignerPin::try_new("123456".to_string()).expect("valid PIN"); + assert_eq!(format!("{pin:?}"), ""); + } + + #[test] + fn commands_and_setup_state_redact_pin_values() { + let command = TapSignerCmd::Backup { pin: "123456".to_string() }; + assert!(!format!("{command:?}").contains("123456")); + + let setup = + SetupCmd::try_new("123456".to_string(), "654321".to_string(), Some([0u8; 32].to_vec())) + .expect("valid setup command"); + assert!(!format!("{setup:?}").contains("123456")); + assert!(!format!("{setup:?}").contains("654321")); + + let response = SetupCmdResponse::ContinueFromInit(ContinueFromInit { + continue_cmd: Arc::new(setup), + error: TapSignerReaderError::NoCommand, + }); + assert!(!format!("{response:?}").contains("123456")); + assert!(!format!("{response:?}").contains("654321")); + } } mod ffi { @@ -778,11 +932,12 @@ fn _ffi_tap_signer_setup_retry_continue_cmd(preview: bool) -> SetupCmdResponse { assert!(preview); let backup = vec![0u8; 32]; - let setup_cmd = SetupCmd { - factory_pin: "123456".to_string(), - new_pin: "000000".to_string(), - chain_code: cove_util::generate_random_chain_code(), - }; + let setup_cmd = SetupCmd::try_new( + "123456".to_string(), + "000000".to_string(), + Some(cove_util::generate_random_chain_code().to_vec()), + ) + .expect("preview PINs and chain code are valid"); SetupCmdResponse::ContinueFromDerive(ContinueFromDerive { backup, From 5f7623413559eefb635a99ad80fcaa61d62fc67e Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:47:51 -0500 Subject: [PATCH 07/15] Regenerate mobile security bindings Sync the Android and iOS bindings with the restore, preview wallet, SeedQR, and TapSigner error API changes. --- .../java/org/bitcoinppl/cove_core/cove.kt | 226 +++++++++++------- .../Sources/CoveCore/generated/cove.swift | 197 ++++++++------- 2 files changed, 241 insertions(+), 182 deletions(-) diff --git a/android/app/src/main/java/org/bitcoinppl/cove_core/cove.kt b/android/app/src/main/java/org/bitcoinppl/cove_core/cove.kt index a63118f2e..162171578 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove_core/cove.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove_core/cove.kt @@ -37506,6 +37506,17 @@ sealed class BackupException: kotlin.Exception() { get() = "v1=${ v1 }" } + /** + * The wallet id is already used by local wallet state or restore artifacts + */ + class WalletIdOccupied( + + val v1: WalletId + ) : BackupException() { + override val message + get() = "v1=${ v1 }" + } + class Decompression( val v1: kotlin.String @@ -37574,7 +37585,10 @@ public object FfiConverterTypeBackupError : FfiConverterRustBuffer BackupException.Database( FfiConverterString.read(buf), ) - 15 -> BackupException.Decompression( + 15 -> BackupException.WalletIdOccupied( + FfiConverterTypeWalletId.read(buf), + ) + 16 -> BackupException.Decompression( FfiConverterString.read(buf), ) else -> throw RuntimeException("invalid error enum value, something is very wrong!!") @@ -37648,6 +37662,11 @@ public object FfiConverterTypeBackupError : FfiConverterRustBuffer ( + // Add the size for the Int that specifies the variant plus the size needed for all fields + 4UL + + FfiConverterTypeWalletId.allocationSize(value.v1) + ) is BackupException.Decompression -> ( // Add the size for the Int that specifies the variant plus the size needed for all fields 4UL @@ -37723,8 +37742,13 @@ public object FfiConverterTypeBackupError : FfiConverterRustBuffer { + is BackupException.WalletIdOccupied -> { buf.putInt(15) + FfiConverterTypeWalletId.write(value.v1, buf) + Unit + } + is BackupException.Decompression -> { + buf.putInt(16) FfiConverterString.write(value.v1, buf) Unit } @@ -53496,6 +53520,12 @@ sealed class SeedQrException: kotlin.Exception() { get() = "" } + class InvalidLength( + ) : SeedQrException() { + override val message + get() = "" + } + class IndexOutOfBounds( val v1: kotlin.UShort @@ -53549,13 +53579,14 @@ public object FfiConverterTypeSeedQrError : FfiConverterRustBuffer SeedQrException.ContainsNonNumericChars() - 2 -> SeedQrException.IndexOutOfBounds( + 2 -> SeedQrException.InvalidLength() + 3 -> SeedQrException.IndexOutOfBounds( FfiConverterUShort.read(buf), ) - 3 -> SeedQrException.IncorrectWordLength( + 4 -> SeedQrException.IncorrectWordLength( FfiConverterUShort.read(buf), ) - 4 -> SeedQrException.InvalidMnemonic( + 5 -> SeedQrException.InvalidMnemonic( FfiConverterTypeBip39Error.read(buf), ) else -> throw RuntimeException("invalid error enum value, something is very wrong!!") @@ -53568,6 +53599,10 @@ public object FfiConverterTypeSeedQrError : FfiConverterRustBuffer ( + // Add the size for the Int that specifies the variant plus the size needed for all fields + 4UL + ) is SeedQrException.IndexOutOfBounds -> ( // Add the size for the Int that specifies the variant plus the size needed for all fields 4UL @@ -53592,18 +53627,22 @@ public object FfiConverterTypeSeedQrError : FfiConverterRustBuffer { + is SeedQrException.InvalidLength -> { buf.putInt(2) + Unit + } + is SeedQrException.IndexOutOfBounds -> { + buf.putInt(3) FfiConverterUShort.write(value.v1, buf) Unit } is SeedQrException.IncorrectWordLength -> { - buf.putInt(3) + buf.putInt(4) FfiConverterUShort.write(value.v1, buf) Unit } is SeedQrException.InvalidMnemonic -> { - buf.putInt(4) + buf.putInt(5) FfiConverterTypeBip39Error.write(value.v1, buf) Unit } @@ -56939,19 +56978,15 @@ sealed class TapSignerReaderException: kotlin.Exception() { } class InvalidPinLength( - - val v1: kotlin.UByte ) : TapSignerReaderException() { override val message - get() = "v1=${ v1 }" + get() = "" } class NonNumericPin( - - val v1: kotlin.String ) : TapSignerReaderException() { override val message - get() = "v1=${ v1 }" + get() = "" } class SetupAlreadyComplete( @@ -57037,12 +57072,8 @@ public object FfiConverterTypeTapSignerReaderError : FfiConverterRustBuffer TapSignerReaderException.NoCommand() - 6 -> TapSignerReaderException.InvalidPinLength( - FfiConverterUByte.read(buf), - ) - 7 -> TapSignerReaderException.NonNumericPin( - FfiConverterString.read(buf), - ) + 6 -> TapSignerReaderException.InvalidPinLength() + 7 -> TapSignerReaderException.NonNumericPin() 8 -> TapSignerReaderException.SetupAlreadyComplete() 9 -> TapSignerReaderException.InvalidChainCodeLength( FfiConverterUInt.read(buf), @@ -57083,12 +57114,10 @@ public object FfiConverterTypeTapSignerReaderError : FfiConverterRustBuffer ( // Add the size for the Int that specifies the variant plus the size needed for all fields 4UL - + FfiConverterUByte.allocationSize(value.v1) ) is TapSignerReaderException.NonNumericPin -> ( // Add the size for the Int that specifies the variant plus the size needed for all fields 4UL - + FfiConverterString.allocationSize(value.v1) ) is TapSignerReaderException.SetupAlreadyComplete -> ( // Add the size for the Int that specifies the variant plus the size needed for all fields @@ -57135,12 +57164,10 @@ public object FfiConverterTypeTapSignerReaderError : FfiConverterRustBuffer { buf.putInt(6) - FfiConverterUByte.write(value.v1, buf) Unit } is TapSignerReaderException.NonNumericPin -> { buf.putInt(7) - FfiConverterString.write(value.v1, buf) Unit } is TapSignerReaderException.SetupAlreadyComplete -> { @@ -60465,6 +60492,12 @@ sealed class WalletManagerException: kotlin.Exception() { get() = "" } + class PreviewOperationUnavailable( + ) : WalletManagerException() { + override val message + get() = "" + } + class SecretRetrievalException( val v1: KeychainException @@ -60763,98 +60796,99 @@ public object FfiConverterTypeWalletManagerError : FfiConverterRustBuffer WalletManagerException.WalletDoesNotExist() - 3 -> WalletManagerException.SecretRetrievalException( + 3 -> WalletManagerException.PreviewOperationUnavailable() + 4 -> WalletManagerException.SecretRetrievalException( FfiConverterTypeKeychainError.read(buf), ) - 4 -> WalletManagerException.MarkWalletAsVerifiedException( + 5 -> WalletManagerException.MarkWalletAsVerifiedException( FfiConverterTypeDatabaseError.read(buf), ) - 5 -> WalletManagerException.LoadWalletException( + 6 -> WalletManagerException.LoadWalletException( FfiConverterTypeWalletError.read(buf), ) - 6 -> WalletManagerException.NodeConnectionFailed( + 7 -> WalletManagerException.NodeConnectionFailed( FfiConverterString.read(buf), ) - 7 -> WalletManagerException.WalletScanException( + 8 -> WalletManagerException.WalletScanException( FfiConverterString.read(buf), ) - 8 -> WalletManagerException.TransactionsRetrievalException( + 9 -> WalletManagerException.TransactionsRetrievalException( FfiConverterString.read(buf), ) - 9 -> WalletManagerException.WalletBalanceException( + 10 -> WalletManagerException.WalletBalanceException( FfiConverterString.read(buf), ) - 10 -> WalletManagerException.NextAddressException( + 11 -> WalletManagerException.NextAddressException( FfiConverterString.read(buf), ) - 11 -> WalletManagerException.SetWalletTypeException( + 12 -> WalletManagerException.SetWalletTypeException( FfiConverterString.read(buf), ) - 12 -> WalletManagerException.GetHeightException() - 13 -> WalletManagerException.TransactionDetailsException( + 13 -> WalletManagerException.GetHeightException() + 14 -> WalletManagerException.TransactionDetailsException( FfiConverterString.read(buf), ) - 14 -> WalletManagerException.ActorNotFound() - 15 -> WalletManagerException.UnableToSwitch( + 15 -> WalletManagerException.ActorNotFound() + 16 -> WalletManagerException.UnableToSwitch( FfiConverterTypeWalletAddressType.read(buf), FfiConverterString.read(buf), ) - 16 -> WalletManagerException.FiatException( + 17 -> WalletManagerException.FiatException( FfiConverterString.read(buf), ) - 17 -> WalletManagerException.FeesException( + 18 -> WalletManagerException.FeesException( FfiConverterString.read(buf), ) - 18 -> WalletManagerException.InitialScanIncomplete() - 19 -> WalletManagerException.BuildTxException( + 19 -> WalletManagerException.InitialScanIncomplete() + 20 -> WalletManagerException.BuildTxException( FfiConverterString.read(buf), ) - 20 -> WalletManagerException.InsufficientFunds( + 21 -> WalletManagerException.InsufficientFunds( FfiConverterString.read(buf), ) - 21 -> WalletManagerException.OutputBelowDustLimit() - 22 -> WalletManagerException.LockedOutputsSelected() - 23 -> WalletManagerException.GetConfirmDetailsException( + 22 -> WalletManagerException.OutputBelowDustLimit() + 23 -> WalletManagerException.LockedOutputsSelected() + 24 -> WalletManagerException.GetConfirmDetailsException( FfiConverterString.read(buf), ) - 24 -> WalletManagerException.SigningException( + 25 -> WalletManagerException.SigningException( FfiConverterString.read(buf), ) - 25 -> WalletManagerException.BroadcastException( + 26 -> WalletManagerException.BroadcastException( FfiConverterString.read(buf), ) - 26 -> WalletManagerException.PayjoinSessionException( + 27 -> WalletManagerException.PayjoinSessionException( FfiConverterString.read(buf), ) - 27 -> WalletManagerException.Converter( + 28 -> WalletManagerException.Converter( FfiConverterTypeConverterError.read(buf), ) - 28 -> WalletManagerException.UnknownException( + 29 -> WalletManagerException.UnknownException( FfiConverterString.read(buf), ) - 29 -> WalletManagerException.PsbtFinalizeException( + 30 -> WalletManagerException.PsbtFinalizeException( FfiConverterString.read(buf), ) - 30 -> WalletManagerException.GetHistoricalPricesException( + 31 -> WalletManagerException.GetHistoricalPricesException( FfiConverterString.read(buf), ) - 31 -> WalletManagerException.CsvCreationException( + 32 -> WalletManagerException.CsvCreationException( FfiConverterString.read(buf), ) - 32 -> WalletManagerException.AddUtxosException( + 33 -> WalletManagerException.AddUtxosException( FfiConverterString.read(buf), ) - 33 -> WalletManagerException.OutputLabelsException( + 34 -> WalletManagerException.OutputLabelsException( FfiConverterString.read(buf), ) - 34 -> WalletManagerException.DatabaseCorruption( + 35 -> WalletManagerException.DatabaseCorruption( FfiConverterTypeWalletId.read(buf), FfiConverterString.read(buf), ) - 35 -> WalletManagerException.PendingUnsignedTransactionsLoadException( + 36 -> WalletManagerException.PendingUnsignedTransactionsLoadException( FfiConverterString.read(buf), ) - 36 -> WalletManagerException.ReceiveAddressException( + 37 -> WalletManagerException.ReceiveAddressException( FfiConverterString.read(buf), ) else -> throw RuntimeException("invalid error enum value, something is very wrong!!") @@ -60872,6 +60906,10 @@ public object FfiConverterTypeWalletManagerError : FfiConverterRustBuffer ( + // Add the size for the Int that specifies the variant plus the size needed for all fields + 4UL + ) is WalletManagerException.SecretRetrievalException -> ( // Add the size for the Int that specifies the variant plus the size needed for all fields 4UL @@ -61053,170 +61091,174 @@ public object FfiConverterTypeWalletManagerError : FfiConverterRustBuffer { + is WalletManagerException.PreviewOperationUnavailable -> { buf.putInt(3) + Unit + } + is WalletManagerException.SecretRetrievalException -> { + buf.putInt(4) FfiConverterTypeKeychainError.write(value.v1, buf) Unit } is WalletManagerException.MarkWalletAsVerifiedException -> { - buf.putInt(4) + buf.putInt(5) FfiConverterTypeDatabaseError.write(value.v1, buf) Unit } is WalletManagerException.LoadWalletException -> { - buf.putInt(5) + buf.putInt(6) FfiConverterTypeWalletError.write(value.v1, buf) Unit } is WalletManagerException.NodeConnectionFailed -> { - buf.putInt(6) + buf.putInt(7) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.WalletScanException -> { - buf.putInt(7) + buf.putInt(8) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.TransactionsRetrievalException -> { - buf.putInt(8) + buf.putInt(9) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.WalletBalanceException -> { - buf.putInt(9) + buf.putInt(10) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.NextAddressException -> { - buf.putInt(10) + buf.putInt(11) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.SetWalletTypeException -> { - buf.putInt(11) + buf.putInt(12) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.GetHeightException -> { - buf.putInt(12) + buf.putInt(13) Unit } is WalletManagerException.TransactionDetailsException -> { - buf.putInt(13) + buf.putInt(14) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.ActorNotFound -> { - buf.putInt(14) + buf.putInt(15) Unit } is WalletManagerException.UnableToSwitch -> { - buf.putInt(15) + buf.putInt(16) FfiConverterTypeWalletAddressType.write(value.v1, buf) FfiConverterString.write(value.v2, buf) Unit } is WalletManagerException.FiatException -> { - buf.putInt(16) + buf.putInt(17) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.FeesException -> { - buf.putInt(17) + buf.putInt(18) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.InitialScanIncomplete -> { - buf.putInt(18) + buf.putInt(19) Unit } is WalletManagerException.BuildTxException -> { - buf.putInt(19) + buf.putInt(20) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.InsufficientFunds -> { - buf.putInt(20) + buf.putInt(21) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.OutputBelowDustLimit -> { - buf.putInt(21) + buf.putInt(22) Unit } is WalletManagerException.LockedOutputsSelected -> { - buf.putInt(22) + buf.putInt(23) Unit } is WalletManagerException.GetConfirmDetailsException -> { - buf.putInt(23) + buf.putInt(24) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.SigningException -> { - buf.putInt(24) + buf.putInt(25) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.BroadcastException -> { - buf.putInt(25) + buf.putInt(26) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.PayjoinSessionException -> { - buf.putInt(26) + buf.putInt(27) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.Converter -> { - buf.putInt(27) + buf.putInt(28) FfiConverterTypeConverterError.write(value.v1, buf) Unit } is WalletManagerException.UnknownException -> { - buf.putInt(28) + buf.putInt(29) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.PsbtFinalizeException -> { - buf.putInt(29) + buf.putInt(30) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.GetHistoricalPricesException -> { - buf.putInt(30) + buf.putInt(31) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.CsvCreationException -> { - buf.putInt(31) + buf.putInt(32) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.AddUtxosException -> { - buf.putInt(32) + buf.putInt(33) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.OutputLabelsException -> { - buf.putInt(33) + buf.putInt(34) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.DatabaseCorruption -> { - buf.putInt(34) + buf.putInt(35) FfiConverterTypeWalletId.write(value.`id`, buf) FfiConverterString.write(value.`error`, buf) Unit } is WalletManagerException.PendingUnsignedTransactionsLoadException -> { - buf.putInt(35) + buf.putInt(36) FfiConverterString.write(value.v1, buf) Unit } is WalletManagerException.ReceiveAddressException -> { - buf.putInt(36) + buf.putInt(37) FfiConverterString.write(value.v1, buf) Unit } diff --git a/ios/CoveCore/Sources/CoveCore/generated/cove.swift b/ios/CoveCore/Sources/CoveCore/generated/cove.swift index b15d69408..c59a89f1e 100644 --- a/ios/CoveCore/Sources/CoveCore/generated/cove.swift +++ b/ios/CoveCore/Sources/CoveCore/generated/cove.swift @@ -20819,6 +20819,11 @@ enum BackupError: Swift.Error, Equatable, Hashable, Foundation.LocalizedError { ) case Database(String ) + /** + * The wallet id is already used by local wallet state or restore artifacts + */ + case WalletIdOccupied(WalletId + ) case Decompression(String ) @@ -20893,7 +20898,10 @@ public struct FfiConverterTypeBackupError: FfiConverterRustBuffer { case 14: return .Database( try FfiConverterString.read(from: &buf) ) - case 15: return .Decompression( + case 15: return .WalletIdOccupied( + try FfiConverterTypeWalletId.read(from: &buf) + ) + case 16: return .Decompression( try FfiConverterString.read(from: &buf) ) @@ -20973,8 +20981,13 @@ public struct FfiConverterTypeBackupError: FfiConverterRustBuffer { FfiConverterString.write(v1, into: &buf) - case let .Decompression(v1): + case let .WalletIdOccupied(v1): writeInt(&buf, Int32(15)) + FfiConverterTypeWalletId.write(v1, into: &buf) + + + case let .Decompression(v1): + writeInt(&buf, Int32(16)) FfiConverterString.write(v1, into: &buf) } @@ -33897,6 +33910,7 @@ enum SeedQrError: Swift.Error, Equatable, Hashable, Foundation.LocalizedError { case ContainsNonNumericChars + case InvalidLength case IndexOutOfBounds(UInt16 ) case IncorrectWordLength(UInt16 @@ -33944,13 +33958,14 @@ public struct FfiConverterTypeSeedQrError: FfiConverterRustBuffer { case 1: return .ContainsNonNumericChars - case 2: return .IndexOutOfBounds( + case 2: return .InvalidLength + case 3: return .IndexOutOfBounds( try FfiConverterUInt16.read(from: &buf) ) - case 3: return .IncorrectWordLength( + case 4: return .IncorrectWordLength( try FfiConverterUInt16.read(from: &buf) ) - case 4: return .InvalidMnemonic( + case 5: return .InvalidMnemonic( try FfiConverterTypeBip39Error.read(from: &buf) ) @@ -33969,18 +33984,22 @@ public struct FfiConverterTypeSeedQrError: FfiConverterRustBuffer { writeInt(&buf, Int32(1)) - case let .IndexOutOfBounds(v1): + case .InvalidLength: writeInt(&buf, Int32(2)) + + + case let .IndexOutOfBounds(v1): + writeInt(&buf, Int32(3)) FfiConverterUInt16.write(v1, into: &buf) case let .IncorrectWordLength(v1): - writeInt(&buf, Int32(3)) + writeInt(&buf, Int32(4)) FfiConverterUInt16.write(v1, into: &buf) case let .InvalidMnemonic(v1): - writeInt(&buf, Int32(4)) + writeInt(&buf, Int32(5)) FfiConverterTypeBip39Error.write(v1, into: &buf) } @@ -36188,10 +36207,8 @@ enum TapSignerReaderError: Swift.Error, Equatable, Hashable, Foundation.Localize case UnknownCardType(String ) case NoCommand - case InvalidPinLength(UInt8 - ) - case NonNumericPin(String - ) + case InvalidPinLength + case NonNumericPin case SetupAlreadyComplete case InvalidChainCodeLength(UInt32 ) @@ -36268,12 +36285,8 @@ public struct FfiConverterTypeTapSignerReaderError: FfiConverterRustBuffer { try FfiConverterString.read(from: &buf) ) case 5: return .NoCommand - case 6: return .InvalidPinLength( - try FfiConverterUInt8.read(from: &buf) - ) - case 7: return .NonNumericPin( - try FfiConverterString.read(from: &buf) - ) + case 6: return .InvalidPinLength + case 7: return .NonNumericPin case 8: return .SetupAlreadyComplete case 9: return .InvalidChainCodeLength( try FfiConverterUInt32.read(from: &buf) @@ -36317,14 +36330,12 @@ public struct FfiConverterTypeTapSignerReaderError: FfiConverterRustBuffer { writeInt(&buf, Int32(5)) - case let .InvalidPinLength(v1): + case .InvalidPinLength: writeInt(&buf, Int32(6)) - FfiConverterUInt8.write(v1, into: &buf) - case let .NonNumericPin(v1): + case .NonNumericPin: writeInt(&buf, Int32(7)) - FfiConverterString.write(v1, into: &buf) case .SetupAlreadyComplete: @@ -38769,6 +38780,7 @@ enum WalletManagerError: Swift.Error, Equatable, Hashable, Foundation.LocalizedE case GetSelectedWalletError(String ) case WalletDoesNotExist + case PreviewOperationUnavailable case SecretRetrievalError(KeychainError ) case MarkWalletAsVerifiedError(DatabaseError @@ -38876,98 +38888,99 @@ public struct FfiConverterTypeWalletManagerError: FfiConverterRustBuffer { try FfiConverterString.read(from: &buf) ) case 2: return .WalletDoesNotExist - case 3: return .SecretRetrievalError( + case 3: return .PreviewOperationUnavailable + case 4: return .SecretRetrievalError( try FfiConverterTypeKeychainError.read(from: &buf) ) - case 4: return .MarkWalletAsVerifiedError( + case 5: return .MarkWalletAsVerifiedError( try FfiConverterTypeDatabaseError.read(from: &buf) ) - case 5: return .LoadWalletError( + case 6: return .LoadWalletError( try FfiConverterTypeWalletError.read(from: &buf) ) - case 6: return .NodeConnectionFailed( + case 7: return .NodeConnectionFailed( try FfiConverterString.read(from: &buf) ) - case 7: return .WalletScanError( + case 8: return .WalletScanError( try FfiConverterString.read(from: &buf) ) - case 8: return .TransactionsRetrievalError( + case 9: return .TransactionsRetrievalError( try FfiConverterString.read(from: &buf) ) - case 9: return .WalletBalanceError( + case 10: return .WalletBalanceError( try FfiConverterString.read(from: &buf) ) - case 10: return .NextAddressError( + case 11: return .NextAddressError( try FfiConverterString.read(from: &buf) ) - case 11: return .SetWalletTypeError( + case 12: return .SetWalletTypeError( try FfiConverterString.read(from: &buf) ) - case 12: return .GetHeightError - case 13: return .TransactionDetailsError( + case 13: return .GetHeightError + case 14: return .TransactionDetailsError( try FfiConverterString.read(from: &buf) ) - case 14: return .ActorNotFound - case 15: return .UnableToSwitch( + case 15: return .ActorNotFound + case 16: return .UnableToSwitch( try FfiConverterTypeWalletAddressType.read(from: &buf), try FfiConverterString.read(from: &buf) ) - case 16: return .FiatError( + case 17: return .FiatError( try FfiConverterString.read(from: &buf) ) - case 17: return .FeesError( + case 18: return .FeesError( try FfiConverterString.read(from: &buf) ) - case 18: return .InitialScanIncomplete - case 19: return .BuildTxError( + case 19: return .InitialScanIncomplete + case 20: return .BuildTxError( try FfiConverterString.read(from: &buf) ) - case 20: return .InsufficientFunds( + case 21: return .InsufficientFunds( try FfiConverterString.read(from: &buf) ) - case 21: return .OutputBelowDustLimit - case 22: return .LockedOutputsSelected - case 23: return .GetConfirmDetailsError( + case 22: return .OutputBelowDustLimit + case 23: return .LockedOutputsSelected + case 24: return .GetConfirmDetailsError( try FfiConverterString.read(from: &buf) ) - case 24: return .SigningError( + case 25: return .SigningError( try FfiConverterString.read(from: &buf) ) - case 25: return .BroadcastError( + case 26: return .BroadcastError( try FfiConverterString.read(from: &buf) ) - case 26: return .PayjoinSessionError( + case 27: return .PayjoinSessionError( try FfiConverterString.read(from: &buf) ) - case 27: return .Converter( + case 28: return .Converter( try FfiConverterTypeConverterError.read(from: &buf) ) - case 28: return .UnknownError( + case 29: return .UnknownError( try FfiConverterString.read(from: &buf) ) - case 29: return .PsbtFinalizeError( + case 30: return .PsbtFinalizeError( try FfiConverterString.read(from: &buf) ) - case 30: return .GetHistoricalPricesError( + case 31: return .GetHistoricalPricesError( try FfiConverterString.read(from: &buf) ) - case 31: return .CsvCreationError( + case 32: return .CsvCreationError( try FfiConverterString.read(from: &buf) ) - case 32: return .AddUtxosError( + case 33: return .AddUtxosError( try FfiConverterString.read(from: &buf) ) - case 33: return .OutputLabelsError( + case 34: return .OutputLabelsError( try FfiConverterString.read(from: &buf) ) - case 34: return .DatabaseCorruption( + case 35: return .DatabaseCorruption( id: try FfiConverterTypeWalletId.read(from: &buf), error: try FfiConverterString.read(from: &buf) ) - case 35: return .PendingUnsignedTransactionsLoadError( + case 36: return .PendingUnsignedTransactionsLoadError( try FfiConverterString.read(from: &buf) ) - case 36: return .ReceiveAddressError( + case 37: return .ReceiveAddressError( try FfiConverterString.read(from: &buf) ) @@ -38991,170 +39004,174 @@ public struct FfiConverterTypeWalletManagerError: FfiConverterRustBuffer { writeInt(&buf, Int32(2)) - case let .SecretRetrievalError(v1): + case .PreviewOperationUnavailable: writeInt(&buf, Int32(3)) + + + case let .SecretRetrievalError(v1): + writeInt(&buf, Int32(4)) FfiConverterTypeKeychainError.write(v1, into: &buf) case let .MarkWalletAsVerifiedError(v1): - writeInt(&buf, Int32(4)) + writeInt(&buf, Int32(5)) FfiConverterTypeDatabaseError.write(v1, into: &buf) case let .LoadWalletError(v1): - writeInt(&buf, Int32(5)) + writeInt(&buf, Int32(6)) FfiConverterTypeWalletError.write(v1, into: &buf) case let .NodeConnectionFailed(v1): - writeInt(&buf, Int32(6)) + writeInt(&buf, Int32(7)) FfiConverterString.write(v1, into: &buf) case let .WalletScanError(v1): - writeInt(&buf, Int32(7)) + writeInt(&buf, Int32(8)) FfiConverterString.write(v1, into: &buf) case let .TransactionsRetrievalError(v1): - writeInt(&buf, Int32(8)) + writeInt(&buf, Int32(9)) FfiConverterString.write(v1, into: &buf) case let .WalletBalanceError(v1): - writeInt(&buf, Int32(9)) + writeInt(&buf, Int32(10)) FfiConverterString.write(v1, into: &buf) case let .NextAddressError(v1): - writeInt(&buf, Int32(10)) + writeInt(&buf, Int32(11)) FfiConverterString.write(v1, into: &buf) case let .SetWalletTypeError(v1): - writeInt(&buf, Int32(11)) + writeInt(&buf, Int32(12)) FfiConverterString.write(v1, into: &buf) case .GetHeightError: - writeInt(&buf, Int32(12)) + writeInt(&buf, Int32(13)) case let .TransactionDetailsError(v1): - writeInt(&buf, Int32(13)) + writeInt(&buf, Int32(14)) FfiConverterString.write(v1, into: &buf) case .ActorNotFound: - writeInt(&buf, Int32(14)) + writeInt(&buf, Int32(15)) case let .UnableToSwitch(v1,v2): - writeInt(&buf, Int32(15)) + writeInt(&buf, Int32(16)) FfiConverterTypeWalletAddressType.write(v1, into: &buf) FfiConverterString.write(v2, into: &buf) case let .FiatError(v1): - writeInt(&buf, Int32(16)) + writeInt(&buf, Int32(17)) FfiConverterString.write(v1, into: &buf) case let .FeesError(v1): - writeInt(&buf, Int32(17)) + writeInt(&buf, Int32(18)) FfiConverterString.write(v1, into: &buf) case .InitialScanIncomplete: - writeInt(&buf, Int32(18)) + writeInt(&buf, Int32(19)) case let .BuildTxError(v1): - writeInt(&buf, Int32(19)) + writeInt(&buf, Int32(20)) FfiConverterString.write(v1, into: &buf) case let .InsufficientFunds(v1): - writeInt(&buf, Int32(20)) + writeInt(&buf, Int32(21)) FfiConverterString.write(v1, into: &buf) case .OutputBelowDustLimit: - writeInt(&buf, Int32(21)) + writeInt(&buf, Int32(22)) case .LockedOutputsSelected: - writeInt(&buf, Int32(22)) + writeInt(&buf, Int32(23)) case let .GetConfirmDetailsError(v1): - writeInt(&buf, Int32(23)) + writeInt(&buf, Int32(24)) FfiConverterString.write(v1, into: &buf) case let .SigningError(v1): - writeInt(&buf, Int32(24)) + writeInt(&buf, Int32(25)) FfiConverterString.write(v1, into: &buf) case let .BroadcastError(v1): - writeInt(&buf, Int32(25)) + writeInt(&buf, Int32(26)) FfiConverterString.write(v1, into: &buf) case let .PayjoinSessionError(v1): - writeInt(&buf, Int32(26)) + writeInt(&buf, Int32(27)) FfiConverterString.write(v1, into: &buf) case let .Converter(v1): - writeInt(&buf, Int32(27)) + writeInt(&buf, Int32(28)) FfiConverterTypeConverterError.write(v1, into: &buf) case let .UnknownError(v1): - writeInt(&buf, Int32(28)) + writeInt(&buf, Int32(29)) FfiConverterString.write(v1, into: &buf) case let .PsbtFinalizeError(v1): - writeInt(&buf, Int32(29)) + writeInt(&buf, Int32(30)) FfiConverterString.write(v1, into: &buf) case let .GetHistoricalPricesError(v1): - writeInt(&buf, Int32(30)) + writeInt(&buf, Int32(31)) FfiConverterString.write(v1, into: &buf) case let .CsvCreationError(v1): - writeInt(&buf, Int32(31)) + writeInt(&buf, Int32(32)) FfiConverterString.write(v1, into: &buf) case let .AddUtxosError(v1): - writeInt(&buf, Int32(32)) + writeInt(&buf, Int32(33)) FfiConverterString.write(v1, into: &buf) case let .OutputLabelsError(v1): - writeInt(&buf, Int32(33)) + writeInt(&buf, Int32(34)) FfiConverterString.write(v1, into: &buf) case let .DatabaseCorruption(id,error): - writeInt(&buf, Int32(34)) + writeInt(&buf, Int32(35)) FfiConverterTypeWalletId.write(id, into: &buf) FfiConverterString.write(error, into: &buf) case let .PendingUnsignedTransactionsLoadError(v1): - writeInt(&buf, Int32(35)) + writeInt(&buf, Int32(36)) FfiConverterString.write(v1, into: &buf) case let .ReceiveAddressError(v1): - writeInt(&buf, Int32(36)) + writeInt(&buf, Int32(37)) FfiConverterString.write(v1, into: &buf) } From dfc1a0413f89a93876632a7fff979f91af273403 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 08:48:03 -0500 Subject: [PATCH 08/15] Initialize logging test state directly --- rust/crates/cove-common/src/logging/capture.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/rust/crates/cove-common/src/logging/capture.rs b/rust/crates/cove-common/src/logging/capture.rs index bb438e187..a890c188c 100644 --- a/rust/crates/cove-common/src/logging/capture.rs +++ b/rust/crates/cove-common/src/logging/capture.rs @@ -906,8 +906,11 @@ mod tests { let dir = TempDir::new()?; std::fs::write(current_log_path(dir.path()), "original\n")?; - let mut state = CaptureState::default(); - state.writer = Some(writer_that_replaces_current_file_on_shutdown(dir.path())); + let mut state = CaptureState { + writer: Some(writer_that_replaces_current_file_on_shutdown(dir.path())), + ..CaptureState::default() + }; + state.attach(dir.path().to_path_buf())?; state.record_line("after reattach"); From 4b333ee2223f6241d4a48fdd7a0201a22b416d1a Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:12:48 -0500 Subject: [PATCH 09/15] Make restore rollback atomic Serialize wallet metadata updates in one redb transaction so a failed restore cannot remove a wallet that another operation added. Keep the restore reservation alive through cleanup, preserve existing artifacts, and treat unreadable wallet data paths as occupied. --- rust/src/backup/import.rs | 73 ++++++++++----------- rust/src/database/wallet.rs | 107 +++++++++++++++++++++++++++---- rust/src/database/wallet_data.rs | 21 +++++- 3 files changed, 148 insertions(+), 53 deletions(-) diff --git a/rust/src/backup/import.rs b/rust/src/backup/import.rs index 012b69489..75c36a50a 100644 --- a/rust/src/backup/import.rs +++ b/rust/src/backup/import.rs @@ -200,10 +200,8 @@ impl RestoreArtifactSnapshot { for network in Network::iter() { for mode in WalletMode::iter() { - let wallets = database - .wallets - .get_all(network, mode) - .map_err(|error| BackupError::Database(error.to_string()))?; + let wallets = + database.wallets.get_all(network, mode).map_err_str(BackupError::Database)?; if wallets.iter().any(|wallet| wallet.id == metadata.id) { metadata_present = true; @@ -273,12 +271,16 @@ impl Drop for WalletRestoreReservation { struct RestoreJournal { metadata: WalletMetadata, - initial: RestoreArtifactSnapshot, + reservation: WalletRestoreReservation, } impl RestoreJournal { - fn new(metadata: &WalletMetadata, initial: RestoreArtifactSnapshot) -> Self { - Self { metadata: metadata.clone(), initial } + fn new(metadata: &WalletMetadata, reservation: WalletRestoreReservation) -> Self { + Self { metadata: metadata.clone(), reservation } + } + + fn initial(&self) -> &RestoreArtifactSnapshot { + &self.reservation.snapshot } fn rollback(&self) -> Vec { @@ -287,13 +289,13 @@ impl RestoreJournal { self.rollback_keychain(&mut failures); self.rollback_paths( crate::bdk_store::BdkStore::wallet_store_artifact_paths(&self.metadata.id), - &self.initial.bdk_paths, + &self.initial().bdk_paths, "BDK store", &mut failures, ); self.rollback_paths( crate::database::wallet_data::wallet_data_artifact_paths(&self.metadata.id), - &self.initial.wallet_data_paths, + &self.initial().wallet_data_paths, "wallet data", &mut failures, ); @@ -303,7 +305,7 @@ impl RestoreJournal { } fn rollback_keychain(&self, failures: &mut Vec) { - if self.initial.keychain_items { + if self.initial().keychain_items { return; } @@ -349,33 +351,17 @@ impl RestoreJournal { } fn rollback_metadata(&self, failures: &mut Vec) { - if self.initial.metadata { + if self.initial().metadata { return; } let database = Database::global(); - match database.wallets.get_all(self.metadata.network, self.metadata.wallet_mode) { - Ok(mut wallets) => { - let before = wallets.len(); - wallets.retain(|wallet| wallet.id != self.metadata.id); - - if wallets.len() < before - && let Err(error) = database.wallets.save_all_wallets( - self.metadata.network, - self.metadata.wallet_mode, - wallets, - ) - { - failures.push(format!( - "{}: failed to delete metadata: {error}", - self.metadata.name - )); - } - } - Err(error) => failures.push(format!( - "{}: failed to read wallets for cleanup: {error}", - self.metadata.name - )), + if let Err(error) = database.wallets.remove_wallet_metadata( + self.metadata.network, + self.metadata.wallet_mode, + &self.metadata.id, + ) { + failures.push(format!("{}: failed to delete metadata: {error}", self.metadata.name)); } } } @@ -518,7 +504,7 @@ where { let reservation = WalletRestoreReservation::acquire(metadata).map_err(|error| (error, Vec::new()))?; - let journal = RestoreJournal::new(metadata, reservation.snapshot.clone()); + let journal = RestoreJournal::new(metadata, reservation); f().map_err(|error| (error, journal.rollback())) } @@ -1022,19 +1008,30 @@ mod tests { #[test] fn restore_journal_preserves_preexisting_bdk_artifact() { let _guard = crate::test_support::global_state_test_lock().blocking_lock(); + crate::test_support::init_test_keychain(); + crate::test_support::shared_mock_keychain().reset(); + let mut metadata = hot_metadata("Existing BDK artifact"); metadata.id = WalletId::preview_new_random(); - let artifact = - crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id)[2].clone(); + let artifact = crate::bdk_store::BdkStore::wallet_store_artifact_paths(&metadata.id) + .into_iter() + .find(|path| path.to_string_lossy().ends_with("-wal")) + .expect("wallet store artifact paths include a WAL path"); + + if let Some(parent) = artifact.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(&artifact, b"pre-existing WAL").unwrap(); let initial = RestoreArtifactSnapshot { metadata: true, - keychain_items: true, + keychain_items: false, bdk_paths: HashSet::from([artifact.clone()]), ..RestoreArtifactSnapshot::default() }; - let journal = RestoreJournal::new(&metadata, initial); + let reservation = WalletRestoreReservation { id: metadata.id.clone(), snapshot: initial }; + let journal = RestoreJournal::new(&metadata, reservation); assert!(journal.rollback().is_empty()); assert!(artifact.exists()); diff --git a/rust/src/database/wallet.rs b/rust/src/database/wallet.rs index db8428a26..877566b99 100644 --- a/rust/src/database/wallet.rs +++ b/rust/src/database/wallet.rs @@ -5,7 +5,7 @@ use std::{ time::Duration, }; -use redb::{ReadOnlyTable, ReadableTableMetadata, TableDefinition}; +use redb::{ReadOnlyTable, ReadableTable as _, ReadableTableMetadata, TableDefinition}; use tracing::{debug, warn}; use cove_util::result_ext::ResultExt as _; @@ -157,16 +157,16 @@ impl WalletsTable { ) -> Result<(), Error> { let network = wallet.network; let mode = wallet.wallet_mode; + let wallet_for_backup = should_backup_to_cloud.then(|| wallet.clone()); - let mut wallets = self.get_all(network, mode)?; - - if wallets.iter().any(|w| w.id == wallet.id) { - return Err(WalletTableError::WalletAlreadyExists.into()); - } + self.update_wallets(network, mode, |wallets| { + if wallets.iter().any(|stored| stored.id == wallet.id) { + return Err(WalletTableError::WalletAlreadyExists.into()); + } - let wallet_for_backup = should_backup_to_cloud.then(|| wallet.clone()); - wallets.push(wallet); - self.save_all_wallets(network, mode, wallets)?; + wallets.push(wallet); + Ok(()) + })?; Updater::send_update(Update::WalletsChanged); if let Some(wallet_for_backup) = wallet_for_backup { @@ -318,16 +318,27 @@ impl WalletsTable { } fn delete_inner(&self, network: Network, mode: WalletMode, id: &WalletId) -> Result<(), Error> { - let mut wallets = self.get_all(network, mode)?; - - wallets.retain(|wallet| &wallet.id != id); - self.save_all_wallets(network, mode, wallets)?; + self.remove_wallet_metadata(network, mode, id)?; Updater::send_update(Update::WalletsChanged); Ok(()) } + pub(crate) fn remove_wallet_metadata( + &self, + network: Network, + mode: WalletMode, + id: &WalletId, + ) -> Result { + self.update_wallets(network, mode, |wallets| { + let before = wallets.len(); + wallets.retain(|wallet| &wallet.id != id); + + Ok(wallets.len() < before) + }) + } + fn reorder( &self, network: Network, @@ -418,6 +429,34 @@ impl WalletsTable { Ok(()) } + fn update_wallets( + &self, + network: Network, + mode: WalletMode, + update: impl FnOnce(&mut Vec) -> Result, + ) -> Result { + let write_txn = self.db.begin_write()?; + + let result = { + let mut table = write_txn.open_table(TABLE)?; + let key = WalletKey::from((network, mode)).to_string(); + let mut wallets = table + .get(key.as_str()) + .map_err_str(WalletTableError::ReadError)? + .map(|value| value.value()) + .unwrap_or_default(); + let result = update(&mut wallets)?; + + table.insert(&*key, wallets).map_err_str(WalletTableError::SaveError)?; + result + }; + + write_txn.commit().map_err_str(WalletTableError::SaveError)?; + Updater::send_update(AppStateReconcileMessage::DatabaseUpdated); + + Ok(result) + } + pub fn find_by_tap_signer_ident( &self, ident: &str, @@ -557,6 +596,48 @@ mod tests { assert_eq!(names(&persisted), ["third", "first", "second", "fourth"]); } + #[test] + fn concurrent_restore_add_and_rollback_preserve_new_wallet() { + let (_tmp, table) = wallet_table(); + let restored = wallet("restored"); + let concurrent = wallet("concurrent"); + + for _ in 0..20 { + table + .save_all_wallets(restored.network, restored.wallet_mode, vec![restored.clone()]) + .unwrap(); + + let barrier = Arc::new(std::sync::Barrier::new(3)); + std::thread::scope(|scope| { + let remove_table = table.clone(); + let remove_barrier = barrier.clone(); + let remove_id = restored.id.clone(); + let network = restored.network; + let mode = restored.wallet_mode; + scope.spawn(move || { + remove_barrier.wait(); + remove_table.remove_wallet_metadata(network, mode, &remove_id).unwrap(); + }); + + let add_table = table.clone(); + let add_barrier = barrier.clone(); + let concurrent = concurrent.clone(); + scope.spawn(move || { + add_barrier.wait(); + add_table + .save_new_wallet_metadata_with_backup_behavior(concurrent, false) + .unwrap(); + }); + + barrier.wait(); + }); + + let persisted = table.get_all(restored.network, restored.wallet_mode).unwrap(); + assert_eq!(persisted.len(), 1); + assert_eq!(persisted[0].id, concurrent.id); + } + } + #[test] fn delete_and_metadata_update_preserve_order() { let (_tmp, table) = wallet_table(); diff --git a/rust/src/database/wallet_data.rs b/rust/src/database/wallet_data.rs index 87e5e008f..b8cd4a82b 100644 --- a/rust/src/database/wallet_data.rs +++ b/rust/src/database/wallet_data.rs @@ -394,11 +394,18 @@ pub(crate) fn wallet_data_artifact_paths(id: &WalletId) -> Vec { pub(crate) fn wallet_data_artifacts_exist(id: &WalletId) -> bool { let directory = WALLET_DATA_DIR.join(id.as_str()); + directory_contains_wallet_data(&directory) +} + +fn directory_contains_wallet_data(directory: &Path) -> bool { if directory.is_file() { return true; } - std::fs::read_dir(directory).is_ok_and(|mut entries| entries.next().is_some()) + match std::fs::read_dir(directory) { + Ok(mut entries) => entries.next().is_some(), + Err(error) => error.kind() != std::io::ErrorKind::NotFound, + } } /// Drop all cached wallet data connections and open locks @@ -459,9 +466,19 @@ pub(crate) mod test_support { #[cfg(test)] mod tests { - use super::*; use std::sync::Barrier; + use super::*; + + #[test] + fn unreadable_wallet_data_path_is_treated_as_occupied() { + let tmp = tempfile::tempdir().expect("failed to create temp dir"); + let parent_file = tmp.path().join("not-a-directory"); + std::fs::write(&parent_file, b"occupied").expect("failed to create parent file"); + + assert!(directory_contains_wallet_data(&parent_file.join("wallet"))); + } + #[test] fn concurrent_new_or_existing_calls_share_one_database_handle() { crate::database::encrypted_backend::tests::set_test_encryption_key(); From 0f282beeb93a91a93335ea0b438a656526d16a32 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:12:58 -0500 Subject: [PATCH 10/15] Keep preview transactions isolated Read preview transaction labels from the actor-owned in-memory database instead of opening persistent wallet storage. Batch label reads for transaction lists and calculate the spend policy once per fee-options request to avoid repeated database work. --- rust/src/database/wallet_data/label.rs | 48 +++++++++++++- rust/src/label_manager.rs | 2 +- rust/src/manager/wallet_manager.rs | 9 ++- rust/src/manager/wallet_manager/actor.rs | 25 +++++-- .../wallet_manager/actor/transactions.rs | 66 +++++++++++++++++-- rust/src/transaction.rs | 11 +--- 6 files changed, 135 insertions(+), 26 deletions(-) diff --git a/rust/src/database/wallet_data/label.rs b/rust/src/database/wallet_data/label.rs index 8f86fe699..3bfd4f9ab 100644 --- a/rust/src/database/wallet_data/label.rs +++ b/rust/src/database/wallet_data/label.rs @@ -1,4 +1,9 @@ -use std::{borrow::Borrow, collections::HashSet, fmt::Debug, sync::Arc}; +use std::{ + borrow::Borrow, + collections::{HashMap, HashSet}, + fmt::Debug, + sync::Arc, +}; use crate::database::{Record, error::DatabaseError, record::Timestamps}; use bip329::{ @@ -142,6 +147,45 @@ impl LabelsTable { Ok(labels) } + /// Loads all labels for several transactions in one database read transaction + pub fn all_labels_for_txns( + &self, + txids: impl IntoIterator, + ) -> Result>, Error> { + let read_txn = self.db.begin_read().map_err_str(DatabaseError::DatabaseAccess)?; + let txn_table = read_txn.open_table(TXN_TABLE).map_err_str(DatabaseError::TableAccess)?; + let input_table = + read_txn.open_table(INPUT_TABLE).map_err_str(DatabaseError::TableAccess)?; + let output_table = + read_txn.open_table(OUTPUT_TABLE).map_err_str(DatabaseError::TableAccess)?; + let mut labels_by_txid = HashMap::new(); + + for txid in txids { + let Some(txn) = txn_table.get(&txid)?.map(|record| record.value().item) else { + continue; + }; + + let txid_bytes = *AsRef::<[u8; 32]>::as_ref(&txid); + let start = OutPointKey { id: txid_bytes, index: 0 }; + let inputs = input_table + .range(start.clone()..)? + .filter_map(Result::ok) + .take_while(move |(key, _record)| key.value().id == txid_bytes) + .map(|(_key, record)| Label::Input(record.value().item)); + let outputs = output_table + .range(start..)? + .filter_map(Result::ok) + .take_while(move |(key, _record)| key.value().id == txid_bytes) + .map(|(_key, record)| Label::Output(record.value().item)); + let labels = + std::iter::once(Label::Transaction(txn)).chain(inputs).chain(outputs).collect(); + + labels_by_txid.insert(txid, labels); + } + + Ok(labels_by_txid) + } + pub fn txn_input_records_iter( &self, txid: impl AsRef<[u8; 32]>, @@ -667,8 +711,10 @@ mod tests { ) .expect("failed to parse txid"); let labels = db.all_labels_for_txn(txid).expect("failed to get labels"); + let labels_by_txid = db.all_labels_for_txns([txid]).expect("failed to batch get labels"); assert_eq!(labels.len(), 5); + assert_eq!(labels_by_txid.get(&txid).map(Vec::len), Some(5)); } #[test] diff --git a/rust/src/label_manager.rs b/rust/src/label_manager.rs index 265e831e4..da8675c7a 100644 --- a/rust/src/label_manager.rs +++ b/rust/src/label_manager.rs @@ -262,7 +262,7 @@ impl LabelManager { } impl LabelManager { - pub(crate) fn try_new_with_db(db: WalletDataDb) -> Self { + pub(crate) fn new_with_db(db: WalletDataDb) -> Self { Self { db } } diff --git a/rust/src/manager/wallet_manager.rs b/rust/src/manager/wallet_manager.rs index feb6bf0d8..5039a8d77 100644 --- a/rust/src/manager/wallet_manager.rs +++ b/rust/src/manager/wallet_manager.rs @@ -654,7 +654,7 @@ impl RustWalletManager { let fee_client = &FEE_CLIENT; let fees = fee_client.fetch_and_get_fees().await.map_err(WalletManagerFeesError::from)?; - fees.fee_rate_options().map_err(|error| Error::FeesError(error.to_string())) + fees.fee_rate_options().map_err_str(Error::FeesError) } #[uniffi::method] @@ -982,7 +982,7 @@ impl RustWalletManager { let fee_client = &FEE_CLIENT; let fees = fee_client.fetch_and_get_fees().await.map_err(WalletManagerFeesError::from)?; - fees.fee_rate_options().map_err(|error| Error::FeesError(error.to_string())) + fees.fee_rate_options().map_err_str(Error::FeesError) } #[uniffi::method] @@ -1295,7 +1295,7 @@ impl RustWalletManager { let wallet = Wallet::preview_new_wallet_with_metadata(metadata.clone()); let wallet_data_db = WalletDataDb::new_in_memory(wallet.metadata.id.clone()) .expect("failed to open in-memory wallet data database for preview wallet"); - let label_manager = LabelManager::try_new_with_db(wallet_data_db.clone()).into(); + let label_manager = LabelManager::new_with_db(wallet_data_db.clone()).into(); let wallet_snapshot = Arc::new(RwLock::new(WalletSnapshot::from_wallet(&wallet))); let unsigned_transactions = WalletBootstrapUnsignedTransactions::in_memory(Vec::new()); let scan_status = Arc::new(RwLock::new(WalletScanStatus::Idle)); @@ -1305,8 +1305,7 @@ impl RustWalletManager { scan_status.clone(), wallet_snapshot.clone(), wallet_data_db, - ) - .expect("failed to open in-memory wallet data database for preview wallet"); + ); let actor = task::spawn_actor(wallet_actor); Self { diff --git a/rust/src/manager/wallet_manager/actor.rs b/rust/src/manager/wallet_manager/actor.rs index 8b378965a..63116827f 100644 --- a/rust/src/manager/wallet_manager/actor.rs +++ b/rust/src/manager/wallet_manager/actor.rs @@ -158,7 +158,7 @@ impl WalletActor { ) -> Result { let db = WalletDataDb::new_or_existing(wallet.id.clone())?; - Self::new_with_db(wallet, reconciler, scan_status, wallet_snapshot, db) + Ok(Self::new_with_db(wallet, reconciler, scan_status, wallet_snapshot, db)) } pub(crate) fn new_with_db( @@ -167,10 +167,10 @@ impl WalletActor { scan_status: Arc>, wallet_snapshot: Arc>, db: WalletDataDb, - ) -> Result { + ) -> Self { let seed = rand::rng().random(); - Ok(Self { + Self { addr: Default::default(), reconciler, seed, @@ -191,7 +191,7 @@ impl WalletActor { scan_generation: WalletScanGeneration::INITIAL, payjoin_actor: None, db, - }) + } } pub async fn balance(&mut self) -> ActorResult { @@ -246,7 +246,7 @@ impl WalletActor { pub async fn transactions(&mut self) -> Vec { let zero = Amount::ZERO.into(); - let mut transactions = self + let transaction_data = self .wallet .bdk .transactions() @@ -254,8 +254,21 @@ impl WalletActor { let sent_and_received = self.wallet.bdk.sent_and_received(&tx.tx_node.tx).into(); (tx, sent_and_received) }) + .collect::>(); + + let mut labels_by_txid = self + .db + .labels + .all_labels_for_txns(transaction_data.iter().map(|(tx, _)| tx.tx_node.txid)) + .unwrap_or_else(|error| { + warn!("failed to batch load transaction labels: {error}"); + Default::default() + }); + let mut transactions = transaction_data + .into_iter() .map(|(tx, sent_and_received)| { - Transaction::new_with_labels(sent_and_received, tx, &self.db.labels) + let labels = labels_by_txid.remove(&tx.tx_node.txid).unwrap_or_default().into(); + Transaction::new_with_labels(sent_and_received, tx, labels) }) .filter(|tx| tx.sent_and_received().amount() > zero) .inspect(|tx| { diff --git a/rust/src/manager/wallet_manager/actor/transactions.rs b/rust/src/manager/wallet_manager/actor/transactions.rs index dd51c3120..dc0a7a50b 100644 --- a/rust/src/manager/wallet_manager/actor/transactions.rs +++ b/rust/src/manager/wallet_manager/actor/transactions.rs @@ -28,7 +28,7 @@ use crate::{ manager::wallet_manager::{ Error, SendFlowErrorAlert, WalletManagerBuildTxError, WalletManagerError, WalletManagerFeesError, WalletManagerReconcileMessage, - actor::{WalletActor, current_wallet_unspent_outpoints_for_txid}, + actor::{SpendPolicy, WalletActor, current_wallet_unspent_outpoints_for_txid}, payjoin::{PayjoinActor, PayjoinSessionPersister, build_sender}, }, node::client::NodeClient, @@ -67,9 +67,9 @@ impl WalletActor { option: FeeRateOption, amount: Amount, address: Address, + spend_policy: &SpendPolicy, ) -> Result { let coin_selection = CoveDefaultCoinSelection::new(self.seed); - let spend_policy = self.automatic_spend_policy()?; let mut tx_builder = self.wallet.bdk.build_tx().coin_selection(coin_selection); spend_policy.apply(&mut tx_builder); @@ -241,15 +241,27 @@ impl WalletActor { address: Address, ) -> Result { self.ensure_ledger_ready_for_spend()?; + let spend_policy = self.automatic_spend_policy()?; let options = FeeRateOptionsWithTotalFee { - fast: self.fee_option_with_total_fee(fee_rate_options.fast, amount, address.clone())?, + fast: self.fee_option_with_total_fee( + fee_rate_options.fast, + amount, + address.clone(), + &spend_policy, + )?, medium: self.fee_option_with_total_fee( fee_rate_options.medium, amount, address.clone(), + &spend_policy, + )?, + slow: self.fee_option_with_total_fee( + fee_rate_options.slow, + amount, + address.clone(), + &spend_policy, )?, - slow: self.fee_option_with_total_fee(fee_rate_options.slow, amount, address.clone())?, custom: None, }; @@ -1067,7 +1079,14 @@ impl WalletActor { }; let sent_and_received = self.wallet.bdk.sent_and_received(&tx.tx_node.tx).into(); - Ok(Some(Transaction::new(&self.wallet.id, sent_and_received, tx))) + let labels = self + .db + .labels + .all_labels_for_txn(tx.tx_node.txid) + .map_err_str(Error::TransactionDetailsError)? + .into(); + + Ok(Some(Transaction::new_with_labels(sent_and_received, tx, labels))) } pub(crate) fn transaction_details_for_tx_id( @@ -1193,9 +1212,46 @@ async fn broadcast_to_node_with_connection( #[cfg(test)] mod tests { + use std::sync::Arc; + + use bdk_wallet::test_utils::{ReceiveTo, receive_output}; use bitcoin::Amount; + use parking_lot::RwLock; use super::{WalletActor, WalletManagerError}; + use crate::{ + database::wallet_data::{WalletDataDb, wallet_data_artifact_paths}, + manager::wallet_manager::{WalletScanStatus, WalletSnapshot}, + wallet::{Wallet, metadata::WalletMetadata}, + }; + + #[test] + fn preview_transaction_lookup_uses_in_memory_labels() { + let _guard = crate::test_support::global_state_test_lock().blocking_lock(); + crate::test_support::ensure_tokio_runtime(); + crate::database::test_support::init_test_database(); + crate::test_support::init_test_keychain(); + let metadata = WalletMetadata::preview_new(); + let wallet_data_paths = wallet_data_artifact_paths(&metadata.id); + let mut wallet = Wallet::preview_new_wallet_with_metadata(metadata); + let outpoint = + receive_output(&mut wallet.bdk, Amount::from_sat(50_000), ReceiveTo::Mempool(1)); + let wallet_snapshot = Arc::new(RwLock::new(WalletSnapshot::from_wallet(&wallet))); + let scan_status = Arc::new(RwLock::new(WalletScanStatus::Idle)); + let wallet_data_db = + WalletDataDb::new_in_memory(wallet.id.clone()).expect("in-memory label database opens"); + let (reconciler, _) = flume::bounded(1); + let actor = WalletActor::new_with_db( + wallet, + reconciler, + scan_status, + wallet_snapshot, + wallet_data_db, + ); + + assert!(actor.transaction_for_tx_id(outpoint.txid).unwrap().is_some()); + assert!(wallet_data_paths.iter().all(|path| !path.exists())); + } #[test] fn insufficient_funds_needed_amount_derives_fee() { diff --git a/rust/src/transaction.rs b/rust/src/transaction.rs index 362aae6d3..702abc040 100644 --- a/rust/src/transaction.rs +++ b/rust/src/transaction.rs @@ -11,10 +11,7 @@ use bdk_wallet::chain::{ use bip329::Labels; use crate::{ - database::{ - Database, - wallet_data::{WalletDataDb, label::LabelsTable}, - }, + database::{Database, wallet_data::WalletDataDb}, fiat::FiatAmount, wallet::metadata::WalletId, }; @@ -79,16 +76,14 @@ impl Transaction { .and_then(|db| db.labels.all_labels_for_txn(tx.tx_node.txid).ok()) .unwrap_or_default(); - Self::new_with_label_values(sent_and_received, tx, labels.into()) + Self::new_with_labels(sent_and_received, tx, labels.into()) } pub(crate) fn new_with_labels( sent_and_received: SentAndReceived, tx: CanonicalTx, ConfirmationBlockTime>, - labels_table: &LabelsTable, + labels: Labels, ) -> Self { - let labels = labels_table.all_labels_for_txn(tx.tx_node.txid).unwrap_or_default().into(); - Self::new_with_label_values(sent_and_received, tx, labels) } From 1a57a6a01dc586160e6f2e710e00f2ba845010b0 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:13:06 -0500 Subject: [PATCH 11/15] Keep high fee estimates usable Accept plausible remote fee responses and cap automatic transaction fees at 500 sat/vB so the send flow stays available during fee spikes. Track persisted cache age with wall-clock time, normalize cached values, and log invalid fee responses instead of silently dropping them. --- rust/src/fee_client.rs | 126 ++++++++++++------ rust/src/manager/send_flow_manager.rs | 47 ++++--- .../send_flow_manager/fee_selection.rs | 6 +- 3 files changed, 114 insertions(+), 65 deletions(-) diff --git a/rust/src/fee_client.rs b/rust/src/fee_client.rs index 57a5a514d..d1b5fb907 100644 --- a/rust/src/fee_client.rs +++ b/rust/src/fee_client.rs @@ -32,8 +32,11 @@ const HARD_LIMIT: u64 = 30; /// Do not use a persisted fee snapshot as a fallback after this amount of time const STALE_FALLBACK_MAX_AGE: Duration = Duration::from_secs(60 * 60); -/// Maximum fee rate accepted from the remote fee service, in sat/vB -const MAX_REMOTE_FEE_RATE: f32 = 500.0; +/// Maximum fee rate accepted as plausible remote input, in sat/vB +const MAX_REMOTE_FEE_RATE: f32 = 100_000.0; + +/// Maximum remote fee rate used for automatic transaction building, in sat/vB +const MAX_AUTOMATIC_FEE_RATE: f32 = 500.0; // Global client for getting fees pub static FEE_CLIENT: LazyLock = LazyLock::new(FeeClient::new); @@ -129,7 +132,9 @@ impl FeeClient { pub fn fees(&self) -> Option { if let Some(cached) = self.cached_fees() { if cached.snapshot().is_usable_fallback() { - if cached.last_fetched.elapsed() <= Duration::from_secs(BACKGROUND_REFRESH_INTERVAL) + if cached + .age() + .is_some_and(|age| age <= Duration::from_secs(BACKGROUND_REFRESH_INTERVAL)) { return Some(cached.fees); } @@ -151,7 +156,7 @@ impl FeeClient { let cached = self.cached_fees(); if let Some(cached) = cached && cached.snapshot().is_usable_fallback() - && cached.last_fetched.elapsed() < Duration::from_secs(HARD_LIMIT) + && cached.age().is_some_and(|age| age < Duration::from_secs(HARD_LIMIT)) { return Ok(cached.fees); } @@ -190,8 +195,13 @@ impl FeeClient { fn cached_fees(&self) -> Option { if let Some(cached) = FEES.load().as_ref() { let cached = *cached; - if ValidatedFeeResponse::try_from(cached.fees).is_ok() { - return Some(cached); + if let Ok(validated) = ValidatedFeeResponse::try_from(cached.fees) { + let normalized = cached.with_fees(validated.0); + if normalized.fees != cached.fees { + FEES.swap(Arc::new(Some(normalized))); + } + + return Some(normalized); } warn!("ignoring invalid fee snapshot from memory"); @@ -207,10 +217,14 @@ impl FeeClient { } }; - if let Err(error) = ValidatedFeeResponse::try_from(snapshot.fees) { - warn!("ignoring invalid fee snapshot from database: {error}"); - return None; - } + let validated = match ValidatedFeeResponse::try_from(snapshot.fees) { + Ok(validated) => validated, + Err(error) => { + warn!("ignoring invalid fee snapshot from database: {error}"); + return None; + } + }; + let snapshot = FeeSnapshot { fees: validated.0, ..snapshot }; let cached = CachedFeeResponse::from_persisted_snapshot(snapshot); debug!("loaded cached fees from database"); @@ -261,18 +275,7 @@ impl TryFrom for ValidatedFeeResponse { } } - let options = derive_fee_rate_options(fees); - for (field, value) in [ - ("slow", options.slow.fee_rate.sat_per_vb()), - ("medium", options.medium.fee_rate.sat_per_vb()), - ("fast", options.fast.fee_rate.sat_per_vb()), - ] { - if !value.is_finite() || value <= 0.0 || value > MAX_REMOTE_FEE_RATE { - return Err(FeeValidationError::InvalidRate { field, value }); - } - } - - Ok(Self(fees)) + Ok(Self(fees.clamped_for_automatic_selection())) } } @@ -285,28 +288,59 @@ impl ValidatedFeeResponse { impl FeeResponse { /// Convert a validated remote fee response into display and builder fee tiers pub fn fee_rate_options(self) -> Result { - Ok(ValidatedFeeResponse::try_from(self)?.fee_rate_options()) + self.try_into() + } + + fn clamped_for_automatic_selection(self) -> Self { + Self { + fastest_fee: self.fastest_fee.min(MAX_AUTOMATIC_FEE_RATE), + half_hour_fee: self.half_hour_fee.min(MAX_AUTOMATIC_FEE_RATE), + hour_fee: self.hour_fee.min(MAX_AUTOMATIC_FEE_RATE), + economy_fee: self.economy_fee.min(MAX_AUTOMATIC_FEE_RATE), + minimum_fee: self.minimum_fee.min(MAX_AUTOMATIC_FEE_RATE), + } } } +#[derive(Debug, Clone, Copy)] +enum FeeCacheOrigin { + FetchedInProcess(Instant), + Persisted, +} + #[derive(Debug, Clone, Copy)] pub struct CachedFeeResponse { pub fees: FeeResponse, - pub last_fetched: Instant, pub fetched_at: FeeFetchedAt, + origin: FeeCacheOrigin, } impl CachedFeeResponse { fn from_fresh_snapshot(snapshot: FeeSnapshot) -> Self { - Self { fees: snapshot.fees, last_fetched: Instant::now(), fetched_at: snapshot.fetched_at } + Self { + fees: snapshot.fees, + fetched_at: snapshot.fetched_at, + origin: FeeCacheOrigin::FetchedInProcess(Instant::now()), + } } fn from_persisted_snapshot(snapshot: FeeSnapshot) -> Self { - let now = Instant::now(); - let last_fetched = - snapshot.fetched_at.age().and_then(|age| now.checked_sub(age)).unwrap_or(now); + Self { + fees: snapshot.fees, + fetched_at: snapshot.fetched_at, + origin: FeeCacheOrigin::Persisted, + } + } - Self { fees: snapshot.fees, last_fetched, fetched_at: snapshot.fetched_at } + fn with_fees(self, fees: FeeResponse) -> Self { + Self { fees, ..self } + } + + fn age(self) -> Option { + match self.origin { + FeeCacheOrigin::FetchedInProcess(fetched_at) => Some(fetched_at.elapsed()), + FeeCacheOrigin::Persisted => self.fetched_at.age(), + } } fn snapshot(self) -> FeeSnapshot { @@ -321,13 +355,15 @@ fn derive_fee_rate_options(fees: FeeResponse) -> FeeRateOptions { /// Minimum gap between fee tiers to ensure they're visually distinct const TIER_GAP: f32 = 0.1; - let min_relay_rate = fees.minimum_fee.max(POLICY_MIN_FEE_RATE); + let min_relay_rate = fees.minimum_fee.clamp(POLICY_MIN_FEE_RATE, MAX_AUTOMATIC_FEE_RATE); - let slow_rate = - f32::midpoint(fees.economy_fee, fees.hour_fee).min(fees.hour_fee).max(min_relay_rate); + let slow_rate = f32::midpoint(fees.economy_fee, fees.hour_fee) + .min(fees.hour_fee) + .max(min_relay_rate) + .min(MAX_AUTOMATIC_FEE_RATE); - let medium_rate = fees.half_hour_fee.max(slow_rate + TIER_GAP); - let fast_rate = fees.fastest_fee.max(medium_rate + TIER_GAP); + let medium_rate = fees.half_hour_fee.max(slow_rate + TIER_GAP).min(MAX_AUTOMATIC_FEE_RATE); + let fast_rate = fees.fastest_fee.max(medium_rate + TIER_GAP).min(MAX_AUTOMATIC_FEE_RATE); let slow = FeeRateOption { fee_speed: FeeSpeed::Slow, fee_rate: FeeRate::from_sat_per_vb(slow_rate) }; @@ -408,7 +444,7 @@ pub async fn init_and_update_fees() { pub async fn fetch_and_update_fees_if_needed() -> Result<()> { if let Some(cached) = FEE_CLIENT.cached_fees() && cached.snapshot().is_usable_fallback() - && cached.last_fetched.elapsed() < Duration::from_secs(HARD_LIMIT) + && cached.age().is_some_and(|age| age < Duration::from_secs(HARD_LIMIT)) { return Ok(()); } @@ -441,20 +477,23 @@ mod tests { fee_response(0.0, 1.0, 1.0, 1.0), fee_response(f32::NAN, 1.0, 1.0, 1.0), fee_response(1.0, f32::INFINITY, 1.0, 1.0), - fee_response(1.0, 1.0, 501.0, 1.0), + fee_response(1.0, 1.0, MAX_REMOTE_FEE_RATE + 1.0, 1.0), ] { assert!(ValidatedFeeResponse::try_from(invalid).is_err()); } } #[test] - fn derived_fee_rates_are_bounded_after_tier_separation() { - let fees = fee_response(499.9, 499.9, 499.9, 499.9); + fn high_remote_fee_rates_are_clamped_for_automatic_selection() { + let fees = fee_response(600.0, 700.0, 800.0, 900.0); + let validated = ValidatedFeeResponse::try_from(fees).expect("high fees remain usable"); + let options = validated.fee_rate_options(); - assert!(matches!( - ValidatedFeeResponse::try_from(fees), - Err(FeeValidationError::InvalidRate { field: "medium" | "fast", .. }) - )); + assert_eq!(validated.0.fastest_fee, MAX_AUTOMATIC_FEE_RATE); + assert_eq!(validated.0.minimum_fee, MAX_AUTOMATIC_FEE_RATE); + assert_eq!(options.slow.fee_rate.sat_per_vb(), MAX_AUTOMATIC_FEE_RATE); + assert_eq!(options.medium.fee_rate.sat_per_vb(), MAX_AUTOMATIC_FEE_RATE); + assert_eq!(options.fast.fee_rate.sat_per_vb(), MAX_AUTOMATIC_FEE_RATE); } #[test] @@ -478,11 +517,12 @@ mod tests { let cached = CachedFeeResponse::from_persisted_snapshot(snapshot); - assert!(cached.last_fetched.elapsed() >= Duration::from_secs(30)); + assert!(cached.age().is_some_and(|age| age >= Duration::from_secs(30))); } #[tokio::test] async fn fee_client_rejects_http_error_status() { + let _ = rustls::crypto::ring::default_provider().install_default(); let listener = TcpListener::bind("127.0.0.1:0").await.expect("listener binds"); let address = listener.local_addr().expect("listener has an address"); let server = tokio::spawn(async move { diff --git a/rust/src/manager/send_flow_manager.rs b/rust/src/manager/send_flow_manager.rs index c78b7ef58..a543c189a 100644 --- a/rust/src/manager/send_flow_manager.rs +++ b/rust/src/manager/send_flow_manager.rs @@ -20,7 +20,7 @@ use cove_tokio::DebouncedTask; use crate::{ app::App, - fee_client::FEE_CLIENT, + fee_client::{FEE_CLIENT, FeeResponse}, fiat::client::PriceResponse, wallet::{ Address, @@ -36,7 +36,7 @@ use btc_on_change::BtcOnChangeHandler; use cove_common::consts::LOW_SEND_WARNING_SATS; use cove_types::{ amount::Amount, - fees::{FeeRateOptionWithTotalFee, FeeRateOptionsWithTotalFee, FeeSpeed}, + fees::{FeeRateOptionWithTotalFee, FeeRateOptions, FeeRateOptionsWithTotalFee, FeeSpeed}, unit::BitcoinUnit, utxo::Utxo, }; @@ -44,7 +44,7 @@ use error::SendFlowError; use fiat_on_change::FiatOnChangeHandler; use parking_lot::Mutex; use state::{CoinControlMode, EnterMode, FeeSelection, SendFlowManagerState, State}; -use tracing::{debug, error, trace}; +use tracing::{debug, error, trace, warn}; use super::{ deferred_sender, @@ -53,6 +53,16 @@ use super::{ }; pub type Error = error::SendFlowError; + +fn remote_fee_rate_options(fees: FeeResponse) -> Option { + match fees.fee_rate_options() { + Ok(options) => Some(options), + Err(error) => { + warn!("ignoring invalid remote fee rates: {error}"); + None + } + } +} type Result = std::result::Result; type Action = SendFlowManagerAction; @@ -167,21 +177,20 @@ impl RustSendFlowManager { let state = State::new(metadata, balance); // immediately populate cached values if available - let has_base_fees = if let Some(base_options) = - FEE_CLIENT.fees().and_then(|fees| fees.fee_rate_options().ok()) - { - let fee_options = FeeRateOptionsWithTotalFee::without_totals(base_options); - let selected = Arc::new(fee_options.medium); - let fee_selection = FeeSelection::new(Arc::new(fee_options), selected); - - let mut state_guard = state.lock(); - state_guard.fee_rate_options_base = Some(Arc::new(base_options)); - state_guard.fee_selection = Some(fee_selection); - state_guard.has_base_fees = true; - true - } else { - false - }; + let has_base_fees = + if let Some(base_options) = FEE_CLIENT.fees().and_then(remote_fee_rate_options) { + let fee_options = FeeRateOptionsWithTotalFee::without_totals(base_options); + let selected = Arc::new(fee_options.medium); + let fee_selection = FeeSelection::new(Arc::new(fee_options), selected); + + let mut state_guard = state.lock(); + state_guard.fee_rate_options_base = Some(Arc::new(base_options)); + state_guard.fee_selection = Some(fee_selection); + state_guard.has_base_fees = true; + true + } else { + false + }; debug!( "SendFlowManager::new - has_base_fees: {}, balance: {:?}", @@ -674,7 +683,7 @@ impl RustSendFlowManager { return; }; - let Ok(base_options) = fee_response.fee_rate_options() else { + let Some(base_options) = remote_fee_rate_options(fee_response) else { return; }; let fee_options = FeeRateOptionsWithTotalFee::without_totals(base_options); diff --git a/rust/src/manager/send_flow_manager/fee_selection.rs b/rust/src/manager/send_flow_manager/fee_selection.rs index ac0ff211a..290ced5f6 100644 --- a/rust/src/manager/send_flow_manager/fee_selection.rs +++ b/rust/src/manager/send_flow_manager/fee_selection.rs @@ -12,8 +12,8 @@ use cove_types::{ }; use super::{ - Error, Message, Result, RustSendFlowManager, SendFlowError, state::EnterMode, - state::FeeSelection, + Error, Message, Result, RustSendFlowManager, SendFlowError, remote_fee_rate_options, + state::EnterMode, state::FeeSelection, }; fn selected_fee_rate_for_options( @@ -77,7 +77,7 @@ impl RustSendFlowManager { self: &Arc, ) -> Option> { let fee_response = FEE_CLIENT.fetch_and_get_fees().await.ok()?; - let fees = Arc::new(fee_response.fee_rate_options().ok()?); + let fees = Arc::new(remote_fee_rate_options(fee_response)?); { let mut state = self.state.lock(); From de73657e50301534018724edbfb6ed6b0b8d43a6 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:13:19 -0500 Subject: [PATCH 12/15] Preserve TapSigner retry feedback Show the wrong-PIN recovery path after an iOS import retry and use consistent setup failure copy. Use cancellation-safe Android retry handling and remove redundant broad exception wrappers so detekt can verify the NFC flow. --- .../TapSignerFlow/TapSignerImportRetryView.kt | 42 +++++++------ .../flows/TapSignerFlow/TapSignerNfcHelper.kt | 27 +++------ .../TapSignerFlow/TapSignerSetupRetryView.kt | 60 +++++++++++-------- .../TapSignerImportRetryView.swift | 17 ++++-- .../TapSignerSetupRetryView.swift | 4 +- 5 files changed, 79 insertions(+), 71 deletions(-) diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt index a9c5536cb..025b94cc6 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerImportRetryView.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.launch import org.bitcoinppl.cove.AppManager import org.bitcoinppl.cove.TaggedItem import org.bitcoinppl.cove.nfc.TapCardNfcManager +import org.bitcoinppl.cove.runCatchingCancellable import org.bitcoinppl.cove_core.AppAlertState import org.bitcoinppl.cove_core.TapSignerRoute @@ -126,27 +127,30 @@ fun TapSignerImportRetryView( manager.isTagDetected = false manager.isScanning = true - try { - val deriveInfo = nfc.derive(pin) - manager.isScanning = false - manager.isTagDetected = false - nfcManager.onMessageUpdate = null - nfcManager.onTagDetected = null + val result = + runCatchingCancellable( + "TapSignerImportRetryView", + "TapSigner import retry failed", + ) { + nfc.derive(pin) + } - manager.resetRoute(TapSignerRoute.ImportSuccess(tapSigner, deriveInfo)) - } catch (e: Exception) { - manager.isScanning = false - manager.isTagDetected = false - nfcManager.onMessageUpdate = null - nfcManager.onTagDetected = null + manager.isScanning = false + manager.isTagDetected = false + nfcManager.onMessageUpdate = null + nfcManager.onTagDetected = null - app.alertState = - TaggedItem( - AppAlertState.TapSignerDeriveFailed( - "TapSigner import failed. Please try again.", - ), - ) - } + result + .onSuccess { deriveInfo -> + manager.resetRoute(TapSignerRoute.ImportSuccess(tapSigner, deriveInfo)) + }.onFailure { + app.alertState = + TaggedItem( + AppAlertState.TapSignerDeriveFailed( + "TapSigner import failed. Please try again.", + ), + ) + } } }, modifier = Modifier.fillMaxWidth().padding(bottom = 30.dp), diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt index e4fa94e5d..a75b155ba 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerNfcHelper.kt @@ -1,6 +1,5 @@ package org.bitcoinppl.cove.flows.TapSignerFlow -import org.bitcoinppl.cove.Log import org.bitcoinppl.cove.nfc.TapCardNfcManager import org.bitcoinppl.cove_core.* import org.bitcoinppl.cove_core.tapcard.TapSigner @@ -12,7 +11,6 @@ import org.bitcoinppl.cove_core.types.Psbt class TapSignerNfcHelper( private val tapSigner: TapSigner, ) { - private val tag = "TapSignerNfcHelper" private val nfcManager = TapCardNfcManager.getInstance() private var lastResponse: TapSignerResponse? = null @@ -20,13 +18,7 @@ class TapSignerNfcHelper( factoryPin: String, newPin: String, chainCode: ByteArray? = null, - ): SetupCmdResponse = - try { - doSetupTapSigner(factoryPin, newPin, chainCode) - } catch (error: Exception) { - Log.e(tag, "TapSigner setup failed") - throw error - } + ): SetupCmdResponse = doSetupTapSigner(factoryPin, newPin, chainCode) suspend fun derive(pin: String): DeriveInfo = performTapSignerCmd(TapSignerCmd.Derive(pin)) { response -> @@ -87,17 +79,12 @@ class TapSignerNfcHelper( cmd: TapSignerCmd, successResult: (TapSignerResponse?) -> T?, ): T { - try { - val (result, response) = nfcManager.performTapSignerCmd(cmd, successResult) - // store last response for retry scenarios (matches iOS behavior) - // clean up previous response before storing new one - lastResponse?.destroy() - lastResponse = response - return result - } catch (error: Exception) { - Log.e(tag, "TapSigner operation failed") - throw error - } + val (result, response) = nfcManager.performTapSignerCmd(cmd, successResult) + // store last response for retry scenarios (matches iOS behavior) + // clean up previous response before storing new one + lastResponse?.destroy() + lastResponse = response + return result } private suspend fun doSetupTapSigner( diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt index 45199938b..08c387dbd 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt @@ -30,6 +30,7 @@ import org.bitcoinppl.cove.AppManager import org.bitcoinppl.cove.TaggedItem import org.bitcoinppl.cove.findActivity import org.bitcoinppl.cove.nfc.TapCardNfcManager +import org.bitcoinppl.cove.runCatchingCancellable import org.bitcoinppl.cove_core.AppAlertState import org.bitcoinppl.cove_core.SetupCmdResponse import org.bitcoinppl.cove_core.TapSignerRoute @@ -136,35 +137,42 @@ fun TapSignerSetupRetryView( manager.isTagDetected = false manager.isScanning = true - try { - val result = nfc.continueSetup(response) - manager.isScanning = false - manager.isTagDetected = false - nfcManager.onMessageUpdate = null - nfcManager.onTagDetected = null + val result = + runCatchingCancellable( + "TapSignerSetupRetryView", + "TapSigner setup retry failed", + ) { + nfc.continueSetup(response) + } - when (result) { - is SetupCmdResponse.Complete -> { - manager.resetRoute(TapSignerRoute.SetupSuccess(tapSigner, result.v1)) - } - else -> { - manager.resetRoute(TapSignerRoute.SetupRetry(tapSigner, result)) + manager.isScanning = false + manager.isTagDetected = false + nfcManager.onMessageUpdate = null + nfcManager.onTagDetected = null + + result + .onSuccess { setupResponse -> + when (setupResponse) { + is SetupCmdResponse.Complete -> { + manager.resetRoute( + TapSignerRoute.SetupSuccess(tapSigner, setupResponse.v1), + ) + } + else -> { + manager.resetRoute( + TapSignerRoute.SetupRetry(tapSigner, setupResponse), + ) + } } + }.onFailure { + app.sheetState = null + app.alertState = + TaggedItem( + AppAlertState.TapSignerSetupFailed( + "TapSigner setup failed. Please try again.", + ), + ) } - } catch (e: Exception) { - manager.isScanning = false - manager.isTagDetected = false - nfcManager.onMessageUpdate = null - nfcManager.onTagDetected = null - - app.sheetState = null - app.alertState = - TaggedItem( - AppAlertState.TapSignerSetupFailed( - "TapSigner setup failed. Please try again.", - ), - ) - } } }, modifier = Modifier.fillMaxWidth().padding(bottom = 30.dp), diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift index 64ac2c730..d69e56f66 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerImportRetryView.swift @@ -47,12 +47,19 @@ struct TapSignerImportRetry: View { switch await nfc.derive(pin: pin) { case let .success(deriveInfo): manager.resetRoute(to: .importSuccess(tapSigner, deriveInfo)) - case .failure: - app.alertState = .init( - .tapSignerDeriveFailed( - message: "TapSigner import failed. Please try again." + case let .failure(error): + if error.isAuthError() { + app.sheetState = nil + app.alertState = .init( + .tapSignerWrongPin(tapSigner: tapSigner, action: .derive) ) - ) + } else { + app.alertState = .init( + .tapSignerDeriveFailed( + message: "TapSigner import failed. Please try again." + ) + ) + } } } } diff --git a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift index 7ac199565..961951936 100644 --- a/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift +++ b/ios/Cove/Flows/TapSignerFlow/TapSignerSetupRetryView.swift @@ -47,7 +47,9 @@ struct TapSignerSetupRetry: View { Log.error("TapSigner setup retry returned an incomplete response") app.sheetState = nil app.alertState = .init( - .tapSignerSetupFailed(message: "Failed to setup TapSigner") + .tapSignerSetupFailed( + message: "TapSigner setup failed. Please try again." + ) ) case .failure: app.sheetState = nil From 54b6f88c179ea160b78205884eb6751799a37339 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:13:23 -0500 Subject: [PATCH 13/15] Restrict CI token permissions Grant the workflow token read-only repository content access so jobs do not inherit broader default permissions. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1e8f0b663..e0a7571c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,9 @@ on: branches: - master +permissions: + contents: read + env: CARGO_HTTP_MULTIPLEXING: "false" CARGO_NET_RETRY: "10" From 37148f2cc0814229afd9b4c4a7685387b53daea0 Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Tue, 11 Aug 2026 10:13:27 -0500 Subject: [PATCH 14/15] Use wallet error context helpers Build wallet storage errors with the shared result helper to keep context handling consistent and preserve the original failure text. --- rust/src/wallet/addressing.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/rust/src/wallet/addressing.rs b/rust/src/wallet/addressing.rs index 6df724eb3..d2b806fc8 100644 --- a/rust/src/wallet/addressing.rs +++ b/rust/src/wallet/addressing.rs @@ -36,9 +36,8 @@ impl Wallet { if is_persistent { // delete the bdk wallet filestore - BdkStore::delete_sqlite_store(&self.id).map_err(|error| { - WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) - })?; + BdkStore::delete_sqlite_store(&self.id) + .map_err_prefix("failed to delete wallet filestore", WalletError::PersistError)?; } let store = if is_persistent { @@ -78,9 +77,8 @@ impl Wallet { if self.uses_persistent_storage() { // delete the bdk wallet filestore - BdkStore::delete_sqlite_store(&self.id).map_err(|error| { - WalletError::PersistError(format!("failed to delete wallet filestore: {error}")) - })?; + BdkStore::delete_sqlite_store(&self.id) + .map_err_prefix("failed to delete wallet filestore", WalletError::PersistError)?; } let secret = Keychain::global() From 726a051fc39f32de4289e052c3bf4b2c1319169c Mon Sep 17 00:00:00 2001 From: Praveen Perera Date: Thu, 13 Aug 2026 15:34:59 -0500 Subject: [PATCH 15/15] Clean up TapSigner NFC after cancellation --- .../TapSignerFlow/TapSignerSetupRetryView.kt | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt index 08c387dbd..b7574d7de 100644 --- a/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt +++ b/android/app/src/main/java/org/bitcoinppl/cove/flows/TapSignerFlow/TapSignerSetupRetryView.kt @@ -138,18 +138,20 @@ fun TapSignerSetupRetryView( manager.isScanning = true val result = - runCatchingCancellable( - "TapSignerSetupRetryView", - "TapSigner setup retry failed", - ) { - nfc.continueSetup(response) + try { + runCatchingCancellable( + "TapSignerSetupRetryView", + "TapSigner setup retry failed", + ) { + nfc.continueSetup(response) + } + } finally { + manager.isScanning = false + manager.isTagDetected = false + nfcManager.onMessageUpdate = null + nfcManager.onTagDetected = null } - manager.isScanning = false - manager.isTagDetected = false - nfcManager.onMessageUpdate = null - nfcManager.onTagDetected = null - result .onSuccess { setupResponse -> when (setupResponse) {