chore(wallet/sdk): minor cleanup

[sdbondi]

Description

chore: move traits into modules

Motivation and Context

Investigating improvements to api with better atomicity, but unfortunately, was not able to figure it out without large changes. However part of that was moving traits into smaller modules, so why not PR it?

Summary by CodeRabbit

• New Features

  • Enhanced lock management with new timeout-based creation, release, vault fund locking, and stale lock cleanup capabilities.

• Bug Fixes

  • Corrected storage type naming inconsistency across wallet components.

• Refactor

  • Reorganized wallet storage layer with improved transaction handling and modularity.
  • Removed deprecated SDK context management methods; use new storage transaction APIs instead.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

This PR consolidates the wallet storage abstraction by restructuring a monolithic storage module into separate error, reader, and writer components, fixes the typo CommitableStore → CommittableStore across the codebase, refactors LocksApi with trait-bounded impl blocks, removes context management methods from WalletSdk, and updates the accounts API to use transactional helpers.

Changes

Cohort / File(s)	Summary
Typo Fix: CommitableStore → CommittableStore applications/tari_walletd/src/handlers/auth/jwt.rs, applications/tari_walletd/src/services/webauthn.rs, crates/wallet/sdk/src/apis/confidential_outputs.rs, crates/wallet/sdk/src/apis/config.rs, crates/wallet/sdk/src/apis/key_manager.rs, crates/wallet/sdk/src/apis/non_fungible_tokens.rs, crates/wallet/sdk/src/apis/stealth_outputs.rs, crates/wallet/sdk/src/models/lock_guard.rs, crates/wallet/storage_sqlite/src/writer.rs, crates/wallet/storage_sqlite/tests/*	Import spelling corrected from CommitableStore to CommittableStore across multiple modules.
Storage Module Restructuring crates/wallet/sdk/src/storage.rs (removed), crates/wallet/sdk/src/storage/error.rs, crates/wallet/sdk/src/storage/mod.rs, crates/wallet/sdk/src/storage/reader.rs, crates/wallet/sdk/src/storage/writer.rs	Monolithic storage.rs file removed and refactored into modular components: error.rs defines WalletStorageError and CommittableStore trait; mod.rs defines ReadableWalletStore, WriteableWalletStore, WalletStore with blanket impls and with_read_tx/with_write_tx helpers; reader.rs defines comprehensive WalletStoreReader trait; writer.rs defines WalletStoreWriter trait extending CommittableStore.
LocksApi Refactoring crates/wallet/sdk/src/apis/locks.rs	Split impl blocks by trait bounds: general impl with no bounds, WriteableWalletStore-bound impl containing mutating operations (create_lock_with_timeout, release_lock, finalize_lock, lock_funds_in_vault, clear_stale_locks), and ReadableWalletStore-bound impl containing read operation (get_lock_by_transaction_id).
Accounts API Update crates/wallet/sdk/src/apis/accounts.rs	Changed count() method to use `store.with_read_tx(
WalletSdk Context Methods Removal crates/wallet/sdk/src/sdk.rs	Removed four public methods: create_read_context(), with_read_context(), create_write_context(), and with_write_context().

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant SDK as WalletSdk
    participant Store as ReadableWalletStore/<br/>WriteableWalletStore
    participant Tx as Transaction

    rect rgb(200, 230, 255)
    Note over Client,Tx: Read Flow (with_read_tx)
    Client->>SDK: api.with_read_tx(closure)
    SDK->>Store: create_read_tx()
    Store->>Tx: create transaction
    Tx-->>SDK: ReadTransaction
    SDK->>Tx: closure(&mut tx)
    Tx->>Tx: read operations
    Tx-->>SDK: Result<R>
    SDK-->>Client: Result<R>
    end

    rect rgb(230, 200, 255)
    Note over Client,Tx: Write Flow (with_write_tx)
    Client->>SDK: api.with_write_tx(closure)
    SDK->>Store: create_write_tx()
    Store->>Tx: create transaction
    Tx-->>SDK: WriteTransaction
    SDK->>Tx: closure(&mut tx)
    Tx->>Tx: write operations
    alt Success
        Tx->>Tx: commit()
        Tx-->>SDK: Ok(Result<R>)
    else Error in Closure
        Tx->>Tx: rollback()
        Tx-->>SDK: Err(WalletStorageError)
    end
    SDK-->>Client: Result<R>
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Storage module restructuring: Verify that trait definitions, blanket implementations, and with_read_tx/with_write_tx logic are correctly implemented across error.rs, mod.rs, reader.rs, and writer.rs files
• LocksApi trait-bounded impl blocks: Ensure all methods are correctly placed in the appropriate impl block (WriteableWalletStore vs. ReadableWalletStore vs. unbounded) and that trait bounds are properly enforced
• Removed public APIs: Confirm that removal of context management methods from WalletSdk doesn't break existing callers and validate the migration path to with_read_tx/with_write_tx patterns
• Repetitive import updates: While most are straightforward spelling corrections, verify that all CommitableStore → CommittableStore renames are complete and consistent across all files
• Transaction logic update in accounts.rs: Validate that the closure-based pattern in count() correctly replaces the previous transaction handling

Possibly related PRs

• PR fix: timeout fund locks after a while #1621: Restructures wallet storage abstraction with CommittableStore trait and modular storage components (error, reader, writer modules) — directly related at code level
• PR feat(wallet): add stealth_utxos.list rpc call #1576: Extends WalletStoreReader trait with stealth_outputs_get_many and related reader methods — shares storage reader API modifications
• PR fix(wallet)!: support multiple vault locks, handle partial commit case #1611: Modifies storage abstraction and LocksApi trait implementations — overlapping changes to storage traits and lock handling

Suggested labels

P-acks_required, P-reviews_required

Suggested reviewers

• stringhandler

Poem

🐰 A typo fixed, storage neat,
From one big file to modules sweet,
With read and write txns flow,
Through branching bounds, the traits now glow,
Context gone, but closures greet!

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab4ae0 and 1b5e0a6.

📒 Files selected for processing (22)

• applications/tari_walletd/src/handlers/auth/jwt.rs (1 hunks)
• applications/tari_walletd/src/services/webauthn.rs (1 hunks)
• crates/wallet/sdk/src/apis/accounts.rs (2 hunks)
• crates/wallet/sdk/src/apis/confidential_outputs.rs (1 hunks)
• crates/wallet/sdk/src/apis/config.rs (1 hunks)
• crates/wallet/sdk/src/apis/key_manager.rs (1 hunks)
• crates/wallet/sdk/src/apis/locks.rs (2 hunks)
• crates/wallet/sdk/src/apis/non_fungible_tokens.rs (1 hunks)
• crates/wallet/sdk/src/apis/stealth_outputs.rs (1 hunks)
• crates/wallet/sdk/src/models/lock_guard.rs (1 hunks)
• crates/wallet/sdk/src/sdk.rs (0 hunks)
• crates/wallet/sdk/src/storage.rs (0 hunks)
• crates/wallet/sdk/src/storage/error.rs (1 hunks)
• crates/wallet/sdk/src/storage/mod.rs (1 hunks)
• crates/wallet/sdk/src/storage/reader.rs (1 hunks)
• crates/wallet/sdk/src/storage/writer.rs (1 hunks)
• crates/wallet/storage_sqlite/src/writer.rs (2 hunks)
• crates/wallet/storage_sqlite/tests/accounts.rs (1 hunks)
• crates/wallet/storage_sqlite/tests/config.rs (1 hunks)
• crates/wallet/storage_sqlite/tests/key_manager_state.rs (1 hunks)
• crates/wallet/storage_sqlite/tests/substates.rs (1 hunks)
• crates/wallet/storage_sqlite/tests/transaction.rs (1 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}