Skip to content

Conversation

HashEngineering
Copy link

@HashEngineering HashEngineering commented Oct 3, 2025

Summary by CodeRabbit

  • New Features
    • Wallet now creates a default BIP32 account alongside existing BIP44 and CoinJoin accounts during setup.
    • Increased default address gap limits (external, internal, and CoinJoin) to improve address discovery and syncing.
  • Documentation
    • Updated setup documentation to include the new default BIP32 account.
  • Tests
    • Adjusted integration and routing tests to reflect the added default account and updated gap limits.

Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Increased default gap limit constants and introduced DEFAULT_SPECIAL_GAP_LIMIT. Replaced hard-coded gap values with constants across managed account modules. Added default creation of BIP32 account 0 during wallet initialization. Updated related documentation and tests to reflect the new account and gap limits. Adjusted a routing test to use BIP32 index 1.

Changes

Cohort / File(s) Summary of changes
Gap limit constants
key-wallet/src/gap_limit.rs
Raised DEFAULT_EXTERNAL_GAP_LIMIT to 100, DEFAULT_INTERNAL_GAP_LIMIT to 100, DEFAULT_COINJOIN_GAP_LIMIT to 500; added DEFAULT_SPECIAL_GAP_LIMIT = 20; MAX_GAP_LIMIT unchanged.
Managed account gap limit wiring
key-wallet/src/managed_account/managed_account_collection.rs, key-wallet/src/managed_account/managed_account_type.rs
Replaced literal gap values with imported constants for external, internal, CoinJoin, and special pools; added corresponding imports; no API changes.
Wallet default account creation
key-wallet/src/wallet/helper.rs, key-wallet/src/wallet/initialization.rs
Inserted creation of default BIP32 account 0 in Default flow; updated docs for Default option to include BIP32.
Tests updated for new defaults
key-wallet-manager/tests/integration_test.rs, key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
Adjusted expected account count from 8 to 9; changed test to create/use BIP32 account at index 1 instead of 0.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant W as Wallet
  participant AM as AccountManager

  U->>W: create_accounts_from_options(Default)
  activate W
  W->>AM: add_account(BIP32, index=0)
  AM-->>W: ok
  W->>AM: add_account(BIP44, index=0)
  AM-->>W: ok
  W->>AM: add_account(CoinJoin, index=0)
  AM-->>W: ok
  W-->>U: success
  deactivate W

  note over AM: Address pools use new gap limits:<br/>External=100, Internal=100,<br/>CoinJoin=500, Special=20
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • ogabrielides

Poem

I hop through keys where gaps once lay,
Stretching paths from 20 to a brighter day.
A fresh BIP32 sprouts at zero’s gate,
Tests count one more—how they celebrate!
With ears up high and pools aligned,
This wallet’s warren is well-defined. 🐇✨

Pre-merge checks and finishing touches

✅ 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 clearly and concisely summarizes the primary change by indicating that default derivation paths and gap limits are being adjusted to align with Android and iOS behavior, which directly matches the modifications in the pull request. It avoids vague wording and focuses on the core update performed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 7fc5df7 and 3cdd496.

📒 Files selected for processing (1)
  • key-wallet/src/managed_account/managed_account_collection.rs (5 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use secure random number generation for key material

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
key-wallet/src/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/src/**/*.rs: Never serialize, print, or log private keys; avoid exposing secret key material in any form
Prefer using public keys or key fingerprints for identification in logs and messages instead of private keys
Always validate network consistency; never mix mainnet and testnet operations
Use the custom Error type; propagate errors with the ? operator and provide contextual messages
Never panic in library code; return Result<T, Error> for all fallible operations

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
key-wallet/src/managed_account/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Never attempt signing operations for watch-only accounts

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
key-wallet/src/{managed_account,transaction_checking}/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
🧠 Learnings (2)
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools

Applied to files:

  • key-wallet/src/managed_account/managed_account_collection.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools

Applied to files:

  • key-wallet/src/managed_account/managed_account_collection.rs
🧬 Code graph analysis (1)
key-wallet/src/managed_account/managed_account_collection.rs (3)
key-wallet/src/managed_account/address_pool.rs (6)
  • gap_limit (1093-1096)
  • new (356-371)
  • new (1058-1068)
  • base_path (1071-1074)
  • network (1099-1102)
  • key_source (1117-1120)
key-wallet/src/managed_account/mod.rs (4)
  • gap_limit (733-775)
  • new (63-73)
  • network (787-789)
  • index (138-140)
key-wallet/src/gap_limit.rs (2)
  • new (65-76)
  • new (267-273)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: SPV Components Tests
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: Strict Warnings and Clippy Checks
  • GitHub Check: Core Components Tests
🔇 Additional comments (1)
key-wallet/src/managed_account/managed_account_collection.rs (1)

334-349: Good call on centralized gap limits

Wiring the standard external/internal pools through DEFAULT_*_GAP_LIMIT keeps every branch consistent with the mobile defaults while still honoring the bounded generation contract for address pools. Based on learnings


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e44b1fb and 0f0978e.

📒 Files selected for processing (5)
  • key-wallet/src/gap_limit.rs (1 hunks)
  • key-wallet/src/managed_account/managed_account_collection.rs (6 hunks)
  • key-wallet/src/managed_account/managed_account_type.rs (12 hunks)
  • key-wallet/src/wallet/helper.rs (1 hunks)
  • key-wallet/src/wallet/initialization.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • key-wallet/src/wallet/initialization.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • key-wallet/src/wallet/initialization.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
key-wallet/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use secure random number generation for key material

Files:

  • key-wallet/src/wallet/initialization.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
key-wallet/src/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/src/**/*.rs: Never serialize, print, or log private keys; avoid exposing secret key material in any form
Prefer using public keys or key fingerprints for identification in logs and messages instead of private keys
Always validate network consistency; never mix mainnet and testnet operations
Use the custom Error type; propagate errors with the ? operator and provide contextual messages
Never panic in library code; return Result<T, Error> for all fallible operations

Files:

  • key-wallet/src/wallet/initialization.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • key-wallet/src/wallet/initialization.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
key-wallet/src/managed_account/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Never attempt signing operations for watch-only accounts

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/managed_account/managed_account_type.rs
key-wallet/src/{managed_account,transaction_checking}/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Files:

  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/managed_account/managed_account_type.rs
🧠 Learnings (4)
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools

Applied to files:

  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools

Applied to files:

  • key-wallet/src/managed_account/managed_account_collection.rs
  • key-wallet/src/managed_account/managed_account_type.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,derivation/**/*.rs} : Optimize derivation: cache intermediate results, use hardened derivation only when required, and batch-derive child keys where possible

Applied to files:

  • key-wallet/src/wallet/helper.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs

Applied to files:

  • key-wallet/src/wallet/helper.rs
🧬 Code graph analysis (2)
key-wallet/src/managed_account/managed_account_collection.rs (2)
key-wallet/src/managed_account/mod.rs (3)
  • gap_limit (733-775)
  • new (63-73)
  • network (787-789)
key-wallet/src/managed_account/address_pool.rs (6)
  • gap_limit (1093-1096)
  • new (356-371)
  • new (1058-1068)
  • base_path (1071-1074)
  • network (1099-1102)
  • key_source (1117-1120)
key-wallet/src/managed_account/managed_account_type.rs (2)
key-wallet/src/managed_account/mod.rs (3)
  • gap_limit (733-775)
  • new (63-73)
  • network (787-789)
key-wallet/src/managed_account/address_pool.rs (5)
  • gap_limit (1093-1096)
  • new (356-371)
  • new (1058-1068)
  • network (1099-1102)
  • key_source (1117-1120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: Clippy (Non-strict)
  • GitHub Check: RPC Tests (stable, true)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Core Components Tests
  • GitHub Check: SPV Components Tests
🔇 Additional comments (4)
key-wallet/src/wallet/initialization.rs (1)

33-33: LGTM! Documentation updated to reflect BIP32 account 0 creation.

The doc comment now accurately describes that the Default option creates account 0 for BIP32 in addition to BIP44 and CoinJoin.

key-wallet/src/managed_account/managed_account_collection.rs (1)

16-16: LGTM! Gap limits replaced with predefined constants.

All AddressPool constructions now use the appropriate constants from crate::gap_limit instead of hard-coded literals, ensuring consistency and maintainability.

Also applies to: 333-333, 343-343, 361-361, 369-369, 378-378, 386-386, 393-393, 400-400, 407-407, 414-414, 423-423

key-wallet/src/gap_limit.rs (1)

14-20: LGTM! Gap limit constants updated to match Android/iOS behavior.

The increases in default gap limits (external: 20→100, internal: 10→100, coinjoin: 10→500) align with the PR objective to match Android/iOS behavior.

key-wallet/src/managed_account/managed_account_type.rs (1)

8-8: LGTM! Gap limits replaced with predefined constants.

All AddressPool constructions now use the appropriate constants from crate::gap_limit instead of hard-coded literals, ensuring consistency and alignment with the updated defaults.

Also applies to: 360-360, 370-370, 389-389, 401-401, 414-414, 426-426, 437-437, 448-448, 459-459, 470-470, 483-483

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
key-wallet-manager/tests/integration_test.rs (1)

65-69: Update stale comment about default account count.

The inline explanation still claims the default flow creates 7 accounts, but the new expectation shows there are 8 defaults plus the extra one added in the test. Please bump the comment so it doesn’t contradict the assertion.

-    // Get accounts from wallet - Default creates 7 accounts, plus the one we added
+    // Get accounts from wallet - Default creates 8 accounts, plus the one we added
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f0978e and 7fc5df7.

📒 Files selected for processing (7)
  • key-wallet-manager/tests/integration_test.rs (1 hunks)
  • key-wallet/src/gap_limit.rs (1 hunks)
  • key-wallet/src/managed_account/address_pool.rs (1 hunks)
  • key-wallet/src/managed_account/managed_account_collection.rs (5 hunks)
  • key-wallet/src/managed_account/managed_account_type.rs (12 hunks)
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs (1 hunks)
  • key-wallet/src/wallet/helper.rs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • key-wallet/src/wallet/helper.rs
  • key-wallet/src/managed_account/managed_account_collection.rs
🧰 Additional context used
📓 Path-based instructions (9)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)

**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable

Files:

  • key-wallet-manager/tests/integration_test.rs
  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
  • key-wallet/src/gap_limit.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use proptest for property-based testing where appropriate

**/tests/**/*.rs: Integration tests must live under tests/ (e.g., rpc-integration-test)
Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Mark network-dependent or long-running tests with #[ignore] and run with -- --ignored
Add regression tests for fixes; consider property tests (e.g., proptest) where valuable
Keep test vectors deterministic

Files:

  • key-wallet-manager/tests/integration_test.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
{key-wallet,swift-dash-core-sdk}/**/*.{rs,swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Never log or expose private keys

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
  • key-wallet/src/gap_limit.rs
key-wallet/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Use secure random number generation for key material

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
  • key-wallet/src/gap_limit.rs
key-wallet/src/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

key-wallet/src/**/*.rs: Never serialize, print, or log private keys; avoid exposing secret key material in any form
Prefer using public keys or key fingerprints for identification in logs and messages instead of private keys
Always validate network consistency; never mix mainnet and testnet operations
Use the custom Error type; propagate errors with the ? operator and provide contextual messages
Never panic in library code; return Result<T, Error> for all fallible operations

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
  • key-wallet/src/gap_limit.rs
key-wallet/src/managed_account/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Never attempt signing operations for watch-only accounts

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
key-wallet/src/{managed_account,transaction_checking}/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Update wallet state atomically: add transaction, update UTXOs, recalculate balance, and mark addresses used in a single operation

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
**/src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]

Files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
  • key-wallet/src/gap_limit.rs
key-wallet/src/transaction_checking/**/*.rs

📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)

Use transaction type routing to check only relevant accounts

Files:

  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
🧠 Learnings (8)
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Do not generate unbounded addresses; enforce gap limit and staged address generation in address pools

Applied to files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/managed_account/managed_account_type.rs
  • key-wallet/src/gap_limit.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs

Applied to files:

  • key-wallet/src/managed_account/address_pool.rs
  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,derivation/**/*.rs} : Optimize derivation: cache intermediate results, use hardened derivation only when required, and batch-derive child keys where possible

Applied to files:

  • key-wallet/src/managed_account/address_pool.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/address_pool/**/*.rs : Pre-generate addresses in bounded batches (typically 20–100) and store them in pools

Applied to files:

  • key-wallet/src/managed_account/managed_account_type.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/transaction_checking/**/*.rs : Use transaction type routing to check only relevant accounts

Applied to files:

  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/bip32_tests.rs : Organize unit tests so key-derivation correctness lives in tests/bip32_tests.rs

Applied to files:

  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/derivation_tests.rs : Place path derivation tests in tests/derivation_tests.rs

Applied to files:

  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs

Applied to files:

  • key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
🧬 Code graph analysis (3)
key-wallet-manager/tests/integration_test.rs (1)
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs (2)
  • accounts (81-81)
  • accounts (241-243)
key-wallet/src/managed_account/address_pool.rs (1)
key-wallet/src/managed_account/mod.rs (1)
  • gap_limit (733-775)
key-wallet/src/managed_account/managed_account_type.rs (3)
key-wallet/src/managed_account/address_pool.rs (5)
  • gap_limit (1094-1097)
  • new (357-372)
  • new (1059-1069)
  • network (1100-1103)
  • key_source (1118-1121)
key-wallet/src/managed_account/mod.rs (3)
  • gap_limit (733-775)
  • new (63-73)
  • network (787-789)
key-wallet/src/gap_limit.rs (2)
  • new (65-76)
  • new (267-273)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: fuzz (hashes_sha512)
  • GitHub Check: fuzz (dash_deserialize_amount)
  • GitHub Check: fuzz (hashes_sha1)
  • GitHub Check: fuzz (hashes_ripemd160)
  • GitHub Check: fuzz (hashes_sha256)
  • GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
  • GitHub Check: fuzz (hashes_sha512_256)
  • GitHub Check: fuzz (hashes_cbor)
  • GitHub Check: fuzz (hashes_json)
  • GitHub Check: fuzz (dash_deserialize_script)
  • GitHub Check: fuzz (dash_deserialize_address)
  • GitHub Check: fuzz (dash_deserialize_witness)
  • GitHub Check: fuzz (dash_deserialize_block)
  • GitHub Check: fuzz (dash_outpoint_string)
  • GitHub Check: Core Components Tests
  • GitHub Check: fuzz (dash_deser_net_msg)
  • GitHub Check: RPC Tests (stable, true)


/// Standard gap limit for external addresses (BIP44 recommendation)
pub const DEFAULT_EXTERNAL_GAP_LIMIT: u32 = 20;
pub const DEFAULT_EXTERNAL_GAP_LIMIT: u32 = 100;
Copy link
Member

Choose a reason for hiding this comment

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

This would represent a significant additional computation, and I'm not sure the benefit. I Think iOS used to use 30.

Copy link
Member

Choose a reason for hiding this comment

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

I believe GC filters are checked sequentially as well... so the added benefit we got with bloom filters no longer applies.

Copy link
Member

Choose a reason for hiding this comment

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

I think he said that android uses 100; and so to confirm compatibility; and to get correct balances in the mobile test wallet we needed to increase. It's possible 30 work would work; @HashEngineering please advise.


/// Standard gap limit for internal (change) addresses
pub const DEFAULT_INTERNAL_GAP_LIMIT: u32 = 10;
pub const DEFAULT_INTERNAL_GAP_LIMIT: u32 = 100;
Copy link
Member

Choose a reason for hiding this comment

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

There is no point in having 100 here, it would mean we skipped 99 internal addresses. I'm not even sure how you can miss 1.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we should have it at 25, because that's how many transactions you can have in a block from one ancestor. But honestly with some good coding we could have this at 5.


/// Standard gap limit for CoinJoin addresses
pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 10;
pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 500;
Copy link
Member

Choose a reason for hiding this comment

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

I really don't think we need more than 30 here.

pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 500;

/// Standard gap limit for special purpose keys (identity, provider keys)
pub const DEFAULT_SPECIAL_GAP_LIMIT: u32 = 20;
Copy link
Member

Choose a reason for hiding this comment

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

We should probably have this less.

Copy link
Member

Choose a reason for hiding this comment

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

The benefit to having high gap limits is significantly lower with bip157/158 compared to bloom filters.

wallet.add_account(account_type, network, None).expect("Failed to add account to wallet");

// Add a BIP32 account
// Add another BIP32 account
Copy link
Member

Choose a reason for hiding this comment

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

the first one was BIP44

// Add another BIP32 account
let account_type = AccountType::Standard {
index: 0,
index: 1,
Copy link
Member

Choose a reason for hiding this comment

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

okay, but I don't think this matters.

@QuantumExplorer
Copy link
Member

One thing I wanted to add, is that with an idea sync the limiting factor should be your CPU. And filter checking will represent a non negligeable amount of that CPU. Which is why less scripts to check is better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants