Skip to content

fix:adjust default paths and gap limits to match android/iOS behavior #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: v0.40-dev
Choose a base branch
from
+159 −47
Open

fix:adjust default paths and gap limits to match android/iOS behavior #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9f845aa
fix: add a BIP32 path to Default wallet creation options
HashEngineering Oct 3, 2025
0f0978e
fix: increase gap limits to match Android and iOS defaults
HashEngineering Oct 3, 2025
cc8d49c
chore: fix special gap limit comment
HashEngineering Oct 4, 2025
bbf43a9
fix: add BIP32 account 0 to create_accounts_with_passphrase_from_options
HashEngineering Oct 4, 2025
971be53
style: format to pass fmt
HashEngineering Oct 4, 2025
7fc5df7
tests: update tests for adding BIP32 to Default wallet
HashEngineering Oct 4, 2025
430da5b
style: add missing ,
HashEngineering Oct 4, 2025
3cdd496
chore: remove unused import
HashEngineering Oct 4, 2025
7bec305
chore: fix comment in test
HashEngineering Oct 4, 2025
a282394
fix: lower gap limits based on testing with one old wallet from 2019
HashEngineering Oct 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions key-wallet-manager/tests/integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ fn test_account_management() {
);
assert!(result.is_ok());

// 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
let accounts = manager.get_accounts(&wallet_id);
assert!(accounts.is_ok());
assert_eq!(accounts.unwrap().len(), 8); // 7 from Default + 1 we added
assert_eq!(accounts.unwrap().len(), 9); // 8 from Default + 1 we added
}

#[test]
Expand Down
8 changes: 5 additions & 3 deletions key-wallet/src/gap_limit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@ use serde::{Deserialize, Serialize};
use std::collections::HashSet;

/// Standard gap limit for external addresses (BIP44 recommendation)
pub const DEFAULT_EXTERNAL_GAP_LIMIT: u32 = 20;
pub const DEFAULT_EXTERNAL_GAP_LIMIT: u32 = 30;
Comment on lines 13 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update the doc comment to clarify deviation from BIP44.

The doc comment states this follows the "BIP44 recommendation," but BIP44 actually recommends a gap limit of 20. The value of 30 was chosen based on testing with iOS/Android wallet compatibility (as discussed in the PR comments).

Consider updating the comment to reflect this:

-/// Standard gap limit for external addresses (BIP44 recommendation)
+/// Standard gap limit for external addresses (increased from BIP44's recommended 20 for mobile wallet compatibility)
 pub const DEFAULT_EXTERNAL_GAP_LIMIT: u32 = 30;
🤖 Prompt for AI Agents 
In key-wallet/src/gap_limit.rs around lines 13 to 14, the doc comment
incorrectly states this value follows the "BIP44 recommendation" while BIP44
recommends a gap limit of 20; update the comment to explain that
DEFAULT_EXTERNAL_GAP_LIMIT is set to 30 to support iOS/Android wallet
compatibility based on testing and explicitly note it deviates from the BIP44
recommendation of 20.


/// Standard gap limit for internal (change) addresses
pub const DEFAULT_INTERNAL_GAP_LIMIT: u32 = 10;
pub const DEFAULT_INTERNAL_GAP_LIMIT: u32 = 30;

/// Standard gap limit for CoinJoin addresses
pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 10;
pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 30;

/// Standard gap limit for special purpose keys (identity, provider keys)
pub const DEFAULT_SPECIAL_GAP_LIMIT: u32 = 5;
/// Maximum gap limit to prevent excessive address generation
pub const MAX_GAP_LIMIT: u32 = 1000;

Expand Down
3 changes: 2 additions & 1 deletion key-wallet/src/managed_account/address_pool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::bip32::{ChildNumber, DerivationPath, ExtendedPrivKey, ExtendedPubKey}
use crate::error::{Error, Result};
use crate::Network;
use dashcore::{Address, AddressType, ScriptBuf};
use crate::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT;

/// Types of public keys used in the address pool
#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -1059,7 +1060,7 @@ impl AddressPoolBuilder {
Self {
base_path: None,
pool_type: AddressPoolType::External,
gap_limit: 20,
gap_limit: DEFAULT_EXTERNAL_GAP_LIMIT,
network: Network::Dash,
lookahead_size: 40,
address_type: AddressType::P2pkh,
Expand Down
82 changes: 63 additions & 19 deletions key-wallet/src/managed_account/managed_account_collection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
//! across different networks in a hierarchical manner.

use crate::account::account_type::AccountType;
use crate::gap_limit::{
DEFAULT_COINJOIN_GAP_LIMIT, DEFAULT_EXTERNAL_GAP_LIMIT, DEFAULT_INTERNAL_GAP_LIMIT,
DEFAULT_SPECIAL_GAP_LIMIT,
};
use crate::managed_account::address_pool::{AddressPool, AddressPoolType};
use crate::managed_account::managed_account_type::ManagedAccountType;
use crate::managed_account::ManagedAccount;
Expand Down Expand Up @@ -329,7 +333,7 @@ impl ManagedAccountCollection {
let external_pool = AddressPool::new(
external_path,
AddressPoolType::External,
20,
DEFAULT_EXTERNAL_GAP_LIMIT,
network,
key_source,
)?;
Expand All @@ -339,7 +343,7 @@ impl ManagedAccountCollection {
let internal_pool = AddressPool::new(
internal_path,
AddressPoolType::Internal,
20,
DEFAULT_INTERNAL_GAP_LIMIT,
network,
key_source,
)?;
Expand All @@ -356,61 +360,101 @@ impl ManagedAccountCollection {
AccountType::CoinJoin {
index,
} => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_COINJOIN_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::CoinJoin {
index,
addresses,
}
}
AccountType::IdentityRegistration => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::IdentityRegistration {
addresses,
}
}
AccountType::IdentityTopUp {
registration_index,
} => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::IdentityTopUp {
registration_index,
addresses,
}
}
AccountType::IdentityTopUpNotBoundToIdentity => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::IdentityTopUpNotBoundToIdentity {
addresses,
}
}
AccountType::IdentityInvitation => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::IdentityInvitation {
addresses,
}
}
AccountType::ProviderVotingKeys => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::ProviderVotingKeys {
addresses,
}
}
AccountType::ProviderOwnerKeys => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::ProviderOwnerKeys {
addresses,
}
}
AccountType::ProviderOperatorKeys => {
let addresses =
AddressPool::new(base_path, AddressPoolType::Absent, 20, network, key_source)?;
let addresses = AddressPool::new(
base_path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
ManagedAccountType::ProviderOperatorKeys {
addresses,
}
Expand All @@ -419,7 +463,7 @@ impl ManagedAccountCollection {
let addresses = AddressPool::new(
base_path,
AddressPoolType::AbsentHardened,
20,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
Expand Down
83 changes: 64 additions & 19 deletions key-wallet/src/managed_account/managed_account_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
use crate::account::StandardAccountType;
use crate::gap_limit::{
DEFAULT_COINJOIN_GAP_LIMIT, DEFAULT_EXTERNAL_GAP_LIMIT, DEFAULT_INTERNAL_GAP_LIMIT,
DEFAULT_SPECIAL_GAP_LIMIT,
};

use crate::{AccountType, AddressPool, DerivationPath};
#[cfg(feature = "bincode")]
use bincode_derive::{Decode, Encode};
Expand Down Expand Up @@ -356,7 +361,7 @@ impl ManagedAccountType {
let external_pool = AddressPool::new(
external_path,
AddressPoolType::External,
20,
DEFAULT_EXTERNAL_GAP_LIMIT,
network,
key_source,
)?;
Expand All @@ -366,7 +371,7 @@ impl ManagedAccountType {
let internal_pool = AddressPool::new(
internal_path,
AddressPoolType::Internal,
20,
DEFAULT_INTERNAL_GAP_LIMIT,
network,
key_source,
)?;
Expand All @@ -384,8 +389,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_COINJOIN_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::CoinJoin {
index,
Expand All @@ -396,8 +406,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::IdentityRegistration {
addresses: pool,
Expand All @@ -409,8 +424,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::IdentityTopUp {
registration_index,
Expand All @@ -421,8 +441,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::IdentityTopUpNotBoundToIdentity {
addresses: pool,
Expand All @@ -432,8 +457,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::IdentityInvitation {
addresses: pool,
Expand All @@ -443,8 +473,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::ProviderVotingKeys {
addresses: pool,
Expand All @@ -454,8 +489,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::ProviderOwnerKeys {
addresses: pool,
Expand All @@ -465,8 +505,13 @@ impl ManagedAccountType {
let path = account_type
.derivation_path(network)
.unwrap_or_else(|_| DerivationPath::master());
let pool =
AddressPool::new(path, AddressPoolType::Absent, 20, network, key_source)?;
let pool = AddressPool::new(
path,
AddressPoolType::Absent,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;

Ok(Self::ProviderOperatorKeys {
addresses: pool,
Expand All @@ -479,7 +524,7 @@ impl ManagedAccountType {
let pool = AddressPool::new(
path,
AddressPoolType::AbsentHardened,
20,
DEFAULT_SPECIAL_GAP_LIMIT,
network,
key_source,
)?;
Expand Down
4 changes: 2 additions & 2 deletions key-wallet/src/transaction_checking/transaction_router/tests/routing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,9 @@ fn test_transaction_affects_multiple_accounts() {
};
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

Copy link
Author

Choose a reason for hiding this comment

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

To maintain compatibility with old wallets, BIP32 was added to Default. That is why these tests were updated.

However, if we wanted Default to not have BIP32 (for new wallets), but have a another enum item that does contain BIP32 for restoring a wallet (as in the test wallet we are using), then this test could be returned to its original code. And I could add a RestoreDefault item for the case of restoring a wallet.

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.

Copy link
Author

Choose a reason for hiding this comment

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

It matters because the Default wallet already contains a BIP32 account 0, based on my changes. See above comment.

standard_account_type: StandardAccountType::BIP32Account,
};
wallet.add_account(account_type, network, None).expect("Failed to add account to wallet");
Expand Down
Loading
Loading