Skip to content

Comments

refactor(wallet): replace Option<&AppContext> with &AppContext in register_addresses params#649

Merged
lklimek merged 1 commit intov1.0-devfrom
refactor/remove-option-appcontext-from-register-addresses
Feb 24, 2026
Merged

refactor(wallet): replace Option<&AppContext> with &AppContext in register_addresses params#649
lklimek merged 1 commit intov1.0-devfrom
refactor/remove-option-appcontext-from-register-addresses

Conversation

@lklimek
Copy link
Contributor

@lklimek lklimek commented Feb 24, 2026

Summary

All 12 wallet functions that accepted register_addresses: Option<&AppContext> now require &AppContext unconditionally, placed first after &mut self per project convention.

Details

  • Every caller already passed Some(...) except one search-loop optimization in load_identity.rs
  • The Option wrapper masked silently skipped UTXO removal and address registration
  • For identity_authentication_ecdsa_public_keys_data_map, a separate register_addresses: bool flag preserves the search-loop optimization (pass false to skip DB registration)
  • if let Some(app_context) = register_addresses guards removed — inner code now runs unconditionally
  • 10 files changed, net -13 lines

Files modified

Signatures + bodies:

  • src/model/wallet/mod.rs — 6 functions
  • src/model/wallet/asset_lock_transaction.rs — 6 functions

Callers:

  • src/backend_task/core/create_asset_lock.rs
  • src/backend_task/core/mod.rs
  • src/backend_task/identity/register_identity.rs
  • src/backend_task/identity/top_up_identity.rs
  • src/backend_task/identity/load_identity.rs
  • src/backend_task/identity/load_identity_from_wallet.rs
  • src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs
  • src/ui/identities/add_new_identity_screen/mod.rs

Test plan

  • cargo build — clean
  • cargo test --all-features --workspace — 42 passed, 0 failed
  • cargo clippy --all-features --all-targets -- -D warnings — clean
  • cargo +nightly fmt --all — clean

🤖 Co-authored by Claudius the Magnificent AI Agent

Summary by CodeRabbit

  • Refactor
    • Improved internal API design for wallet transaction functions and identity operations by restructuring how application context is passed throughout the system. These changes enhance code clarity and maintainability with no user-facing impact.

…ister_addresses params

All 12 wallet functions that took `register_addresses: Option<&AppContext>`
now require `&AppContext` unconditionally (placed first after `&mut self`
per project convention). This eliminates silently skipped UTXO removal and
address registration when callers forget to pass Some(...).

For `identity_authentication_ecdsa_public_keys_data_map`, a separate
`register_addresses: bool` flag controls whether addresses are registered
in the DB, preserving the search-loop optimization in load_identity.rs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79e1479 and d72f029.

📒 Files selected for processing (10)
  • src/backend_task/core/create_asset_lock.rs
  • src/backend_task/core/mod.rs
  • src/backend_task/identity/load_identity.rs
  • src/backend_task/identity/load_identity_from_wallet.rs
  • src/backend_task/identity/register_identity.rs
  • src/backend_task/identity/top_up_identity.rs
  • src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs
  • src/model/wallet/asset_lock_transaction.rs
  • src/model/wallet/mod.rs
  • src/ui/identities/add_new_identity_screen/mod.rs

📝 Walkthrough

Walkthrough

This pull request refactors wallet method signatures to replace optional AppContext parameters with explicit required ones. Methods previously accepting register_addresses: Option<&AppContext> now take app_context: &AppContext with an optional boolean flag for registration control. All corresponding call sites across the backend and UI layers are updated to pass the context directly.

Changes

Cohort / File(s) Summary
Wallet Method Definitions
src/model/wallet/asset_lock_transaction.rs, src/model/wallet/mod.rs
Refactored public method signatures to require explicit app_context: &AppContext parameter instead of optional register_addresses: Option<&AppContext>. Affected methods include asset lock transactions, identity key derivation, and payment transaction builders. Internal logic updated to propagate app_context through downstream calls and private key retrieval.
Asset Lock Transaction Call Sites
src/backend_task/core/create_asset_lock.rs, src/backend_task/identity/register_identity.rs, src/backend_task/identity/top_up_identity.rs, src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs
Updated all calls to asset lock methods (registration_asset_lock_transaction, top_up_asset_lock_transaction, generic_asset_lock_transaction) to pass self as the first argument instead of Some(self), matching the new method signatures.
Identity and Payment Method Call Sites
src/backend_task/core/mod.rs, src/backend_task/identity/load_identity.rs, src/backend_task/identity/load_identity_from_wallet.rs, src/ui/identities/add_new_identity_screen/mod.rs
Updated calls to identity key derivation and payment methods to pass app_context as explicit first parameter with appropriate registration flag (boolean true/false), replacing previous optional parameter approach.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop, hop, the context flows so clear,
No more Options wrapping here,
Explicit paths through wallet's door,
Refactored code we all adore! 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main refactoring: replacing Option<&AppContext> with &AppContext in wallet function parameters, which is the primary change across 12 functions and all their call sites.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/remove-option-appcontext-from-register-addresses

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

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

@lklimek lklimek merged commit 5a13ded into v1.0-dev Feb 24, 2026
4 checks passed
@lklimek lklimek deleted the refactor/remove-option-appcontext-from-register-addresses branch February 24, 2026 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant