Skip to content

Commit 44fb0b1

Browse files
ZocoLiniclaude
andauthored
fix(key-wallet): extend DIP-15 contact pools as their payments arrive (#1035)
* fix(key-wallet): extend DIP-15 contact pools as their payments arrive A DashPay contact chain monitored only the addresses built at construction, so once a contact had used them all, its next payment landed on an address the wallet never derived and was never seen: invisible to balances, and unrecoverable by rescan since the address does not exist client-side. check_core_transaction marks the matched address used, then asks the wallet for a key source to run maintain_gap_limit with. It asked via AccountTypeToCheck, whose Dashpay variants carry no data: a contact account is keyed by its index plus the friendship's two identity ids, so no lookup by (variant, index) can find one. The helper returned None for both Dashpay variants, that became KeySource::NoKeySource, and the NoKeySource guard skipped pool maintenance altogether. Resolve the key source from the fully-keyed AccountType, which the matched account already carries, via AccountCollection::account_of_type. That is the same per-variant `.map(|a| a.account_xpub)` lookup extended_public_key_for_account_type performed, against a table that does hold the contact chains, so every other account type resolves exactly as before: the BLS operator arm is untouched, and ProviderPlatformKeys still yields NoKeySource because account_of_type returns None for the BLS/EdDSA accounts by design. The test pays 21 consecutive addresses of one friendship and asserts the monitored window advances with each. It reads pool.gap_limit rather than a literal, so it holds at whatever the contact gap limit is set to. Closes #1032 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(key-wallet): name the DashPay contact gap limit, per DIP-15 The contact pool size was the literal 20, repeated at both Dashpay arms of ManagedAccountType::from_account_type with nothing to say where it came from. No DIP mandates 20 for contact chains: DIP-15 recommends 10 ("load 10 addresses past the last used address"), and the 20 that DIP-17 recommends covers Platform payment keys, a different chain already served by DIP17_GAP_LIMIT. Name it DEFAULT_DASHPAY_GAP_LIMIT and set it to what DIP-15 recommends. This narrows each friendship's initial pool from 20 addresses to 10, in both the receiving and the external direction; raising it again is a policy call that deserves its own discussion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(key-wallet): cover both directions of a DIP-15 friendship The regression test exercised only DashpayReceivingFunds. The external direction reaches gap maintenance by its own route — account_of_type reads it from dashpay_external_accounts, a separate collection — so a regression confined to that arm would have gone unnoticed (#1035 review). Run both chains of one friendship, and assert up front that reversing the identity ids gives the two directions distinct address chains, since DIP-15 derives them from the same pair in opposite order. Address the window by gap limit rather than by a literal index. The first address past the construction window is the one a contact's payment goes missing on, and it moved from 20 to 10 when the pool size became DEFAULT_DASHPAY_GAP_LIMIT; indices relative to pool.gap_limit keep the test aimed at that boundary whatever the limit is set to. Two payments per chain now cover it: one to the last address built at construction, which must widen the window, and one to the address that only exists because it did. Verified by mutation: returning NoKeySource from key_source_for_account for both Dashpay variants fails the test on the receiving chain, and doing it for DashpayExternalAccount alone fails it on the external chain, where it previously passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(key-wallet): assert a contact pool keeps sliding, not widens once The payment past the construction window was only checked for relevance, so gap maintenance that ran a single time would have satisfied the test (#1035 review). Assert the window moved again after that payment: an address derived by maintenance must, once used, widen the window in its turn. Verified by mutation: capping maintain_gap_limit's target at one widening fails this assertion alone, with every other assertion in the file still passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9bc580d commit 44fb0b1

6 files changed

Lines changed: 140 additions & 26 deletions

File tree

key-wallet/src/gap_limit.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ pub const DEFAULT_COINJOIN_GAP_LIMIT: u32 = 100;
4545
/// Standard gap limit for special purpose keys (identity, provider keys)
4646
pub const DEFAULT_SPECIAL_GAP_LIMIT: u32 = 5;
4747

48+
/// Gap limit for DashPay contact chains, as recommended by DIP-15.
49+
pub const DEFAULT_DASHPAY_GAP_LIMIT: u32 = 10;
50+
4851
/// Gap limit for DIP-17 platform payment addresses
4952
pub const DIP17_GAP_LIMIT: u32 = 20;
5053

key-wallet/src/managed_account/managed_account_type.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::account::account_collection::{DashpayContactIdentityId, DashpayOurUserIdentityId};
22
use crate::account::StandardAccountType;
33
use crate::gap_limit::{
4-
DEFAULT_COINJOIN_GAP_LIMIT, DEFAULT_EXTERNAL_GAP_LIMIT, DEFAULT_INTERNAL_GAP_LIMIT,
5-
DEFAULT_SPECIAL_GAP_LIMIT, DIP17_GAP_LIMIT,
4+
DEFAULT_COINJOIN_GAP_LIMIT, DEFAULT_DASHPAY_GAP_LIMIT, DEFAULT_EXTERNAL_GAP_LIMIT,
5+
DEFAULT_INTERNAL_GAP_LIMIT, DEFAULT_SPECIAL_GAP_LIMIT, DIP17_GAP_LIMIT,
66
};
77

88
use crate::managed_account::address_pool::AddressPoolType;
@@ -731,7 +731,7 @@ impl ManagedAccountType {
731731
let pool = Self::single_pool(
732732
account_type,
733733
AddressPoolType::Absent,
734-
20,
734+
DEFAULT_DASHPAY_GAP_LIMIT,
735735
network,
736736
key_source,
737737
)?;
@@ -750,7 +750,7 @@ impl ManagedAccountType {
750750
let pool = Self::single_pool(
751751
account_type,
752752
AddressPoolType::Absent,
753-
20,
753+
DEFAULT_DASHPAY_GAP_LIMIT,
754754
network,
755755
key_source,
756756
)?;
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
//! Regression test for DIP-15 contact address pool maintenance (issue #1032).
2+
3+
use dashcore::hashes::Hash;
4+
use dashcore::{Address, BlockHash, Transaction};
5+
6+
use crate::account::AccountType;
7+
use crate::managed_account::address_pool::AddressPool;
8+
use crate::managed_account::managed_account_trait::ManagedAccountTrait;
9+
use crate::test_utils::TestWalletContext;
10+
use crate::transaction_checking::{BlockInfo, TransactionContext};
11+
use crate::wallet::managed_wallet_info::managed_account_operations::ManagedAccountOperations;
12+
13+
const US: [u8; 32] = [0xaa; 32];
14+
const THEM: [u8; 32] = [0xbb; 32];
15+
16+
/// The two chains of one friendship. DIP-15 derives them from the same pair of
17+
/// identity ids in opposite order, and the wallet files them in separate
18+
/// collections, so each reaches gap maintenance by its own route.
19+
const CONTACT_CHAINS: [AccountType; 2] = [
20+
AccountType::DashpayReceivingFunds {
21+
index: 0,
22+
user_identity_id: US,
23+
friend_identity_id: THEM,
24+
},
25+
AccountType::DashpayExternalAccount {
26+
index: 0,
27+
user_identity_id: US,
28+
friend_identity_id: THEM,
29+
},
30+
];
31+
32+
/// The pool a contact chain monitors, looked up by its fully-keyed account type.
33+
fn contact_pool(ctx: &TestWalletContext, contact: AccountType) -> &AddressPool {
34+
ctx.managed_wallet
35+
.accounts
36+
.dashpay_receival_accounts
37+
.values()
38+
.chain(ctx.managed_wallet.accounts.dashpay_external_accounts.values())
39+
.find(|account| account.managed_account_type().to_account_type() == contact)
40+
.expect("contact account")
41+
.managed_account_type()
42+
.address_pools()[0]
43+
}
44+
45+
/// Pays `address` in a block, and reports whether the wallet saw it.
46+
async fn pay(ctx: &mut TestWalletContext, address: &Address, height: u32) -> bool {
47+
let tx = Transaction::dummy(address, 0..1, &[29_000]);
48+
let context = TransactionContext::InBlock(BlockInfo::new(
49+
height,
50+
BlockHash::from_byte_array([height as u8; 32]),
51+
1_700_000_000,
52+
));
53+
ctx.check_transaction(&tx, context).await.is_relevant
54+
}
55+
56+
/// A contact chain must extend as its payments arrive. Without that, the first
57+
/// payment past the addresses built at construction lands on an address the
58+
/// wallet never derived, and is never seen.
59+
#[tokio::test]
60+
async fn contact_payments_extend_the_contact_pool() {
61+
let mut ctx = TestWalletContext::new_random();
62+
for contact in CONTACT_CHAINS {
63+
ctx.wallet.add_account(contact, None).expect("contact account is addable");
64+
ctx.managed_wallet
65+
.add_managed_account(&ctx.wallet, contact)
66+
.expect("contact account is manageable");
67+
}
68+
assert_ne!(
69+
contact_pool(&ctx, CONTACT_CHAINS[0]).address_at_index(0),
70+
contact_pool(&ctx, CONTACT_CHAINS[1]).address_at_index(0),
71+
"reversing the identity ids must give the two directions distinct chains"
72+
);
73+
74+
for (chain, contact) in CONTACT_CHAINS.into_iter().enumerate() {
75+
let height = 1_555_196 + chain as u32 * 2;
76+
let direction = if chain == 0 {
77+
"receiving"
78+
} else {
79+
"external"
80+
};
81+
let pool = contact_pool(&ctx, contact);
82+
let gap = pool.gap_limit;
83+
let last_built = pool.address_at_index(gap - 1).expect("construction fills the window");
84+
assert!(
85+
pool.address_at_index(gap).is_none(),
86+
"{direction}: nothing is derived past that window yet"
87+
);
88+
89+
assert!(
90+
pay(&mut ctx, &last_built, height).await,
91+
"{direction}: payment to a monitored address is seen"
92+
);
93+
assert_eq!(
94+
contact_pool(&ctx, contact).highest_generated,
95+
Some(gap - 1 + gap),
96+
"{direction}: using an address must leave a full window monitored ahead of it"
97+
);
98+
99+
let past_the_window =
100+
contact_pool(&ctx, contact).address_at_index(gap).expect("the window moved");
101+
assert!(
102+
pay(&mut ctx, &past_the_window, height + 1).await,
103+
"{direction}: the payment past the construction window must be seen too"
104+
);
105+
assert_eq!(
106+
contact_pool(&ctx, contact).highest_generated,
107+
Some(gap * 2),
108+
"{direction}: the window must keep moving, not widen once"
109+
);
110+
}
111+
}

key-wallet/src/tests/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ mod advanced_transaction_tests;
1414

1515
mod backup_restore_tests;
1616

17+
mod dashpay_contact_gap_limit_tests;
18+
1719
mod edge_case_tests;
1820

1921
mod integration_tests;

key-wallet/src/transaction_checking/wallet_checker.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,15 +298,12 @@ impl WalletTransactionChecker for ManagedWalletInfo {
298298
account.mark_address_used(&address_info.address);
299299
}
300300

301-
let key_source = wallet.key_source_for_account_type(
302-
&account_match.account_type_match.to_account_type_to_check(),
303-
account_match.account_type_match.account_index(),
304-
);
301+
let owning_account_type = account.managed_account_type().to_account_type();
302+
let key_source = wallet.key_source_for_account(owning_account_type);
305303
if matches!(key_source, KeySource::NoKeySource) {
306304
continue;
307305
}
308306
let rev_before = result.new_addresses.len();
309-
let owning_account_type = account.managed_account_type().to_account_type();
310307
for pool in account.managed_account_type_mut().address_pools_mut() {
311308
let pool_type = pool.pool_type;
312309
match pool.maintain_gap_limit(&key_source) {

key-wallet/src/wallet/helper.rs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -618,35 +618,36 @@ impl Wallet {
618618
}
619619

620620
/// Get a [`crate::KeySource`] capable of public (watch-side) derivation
621-
/// for a specific account type.
621+
/// for a specific account.
622622
///
623-
/// This is the key-source counterpart of
624-
/// [`Self::extended_public_key_for_account_type`]: ECDSA accounts yield
625-
/// [`crate::KeySource::Public`], while the BLS provider operator account
626-
/// yields [`crate::KeySource::BLSPublic`] (legacy-scheme non-hardened
627-
/// derivation) so gap-limit maintenance can extend the operator key pool
628-
/// just like the owner/voting pools.
623+
/// Takes the fully-keyed [`crate::account::AccountType`] rather than an
624+
/// [`crate::transaction_checking::transaction_router::AccountTypeToCheck`],
625+
/// so accounts identified by more than an index — the Dashpay contact
626+
/// chains, keyed by the friendship's two identity ids — resolve as well.
627+
/// ECDSA accounts yield [`crate::KeySource::Public`], while the BLS
628+
/// provider operator account yields [`crate::KeySource::BLSPublic`]
629+
/// (legacy-scheme non-hardened derivation) so gap-limit maintenance can
630+
/// extend the operator key pool just like the owner/voting pools.
629631
///
630-
/// Returns [`crate::KeySource::NoKeySource`] for account types that
631-
/// cannot derive publicly: the Ed25519 platform node account (SLIP-0010
632-
/// supports hardened derivation only) and Dashpay accounts (not
633-
/// retrieved via this helper).
634-
pub fn key_source_for_account_type(
632+
/// Returns [`crate::KeySource::NoKeySource`] for the Ed25519 platform node
633+
/// account, which cannot derive publicly (SLIP-0010 supports hardened
634+
/// derivation only), and for accounts the wallet does not hold.
635+
pub fn key_source_for_account(
635636
&self,
636-
account_type: &crate::transaction_checking::transaction_router::AccountTypeToCheck,
637-
account_index: Option<u32>,
637+
account_type: crate::account::AccountType,
638638
) -> crate::KeySource {
639639
match account_type {
640640
#[cfg(feature = "bls")]
641-
crate::transaction_checking::transaction_router::AccountTypeToCheck::ProviderOperatorKeys => self
641+
crate::account::AccountType::ProviderOperatorKeys => self
642642
.accounts
643643
.provider_operator_keys
644644
.as_ref()
645645
.map(|a| crate::KeySource::BLSPublic(a.bls_public_key.clone()))
646646
.unwrap_or(crate::KeySource::NoKeySource),
647647
_ => self
648-
.extended_public_key_for_account_type(account_type, account_index)
649-
.map(crate::KeySource::Public)
648+
.accounts
649+
.account_of_type(account_type)
650+
.map(|a| crate::KeySource::Public(a.account_xpub))
650651
.unwrap_or(crate::KeySource::NoKeySource),
651652
}
652653
}

0 commit comments

Comments
 (0)