-
Notifications
You must be signed in to change notification settings - Fork 7
feat(key-wallet): Implement DIP17/DIP18 Platform Payment Addresses #229
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.
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
base: v0.41-dev
Are you sure you want to change the base?
Conversation
WalkthroughAdds DIP-17/DIP-18 Platform Payment support across crates: new platform address types and prefixes, address encoding/decoding, derivation paths, account types and storage, managed address pools, transaction-checking hooks, FFI bindings, and tests for mainnet/testnet vectors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (9)**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/tests/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
key-wallet/**/tests/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/{account,managed_account}/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/{bip32,derivation,account}/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/{wallet,account,managed_account,address_pool}/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/{wallet,account,managed_account}/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
key-wallet/**/transaction_checking/**/*.rs📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Files:
🧠 Learnings (22)📓 Common learnings📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:17.379ZApplied to files:
📚 Learning: 2025-12-01T08:00:17.379ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-06-15T16:42:18.187ZApplied to files:
📚 Learning: 2025-12-01T08:00:17.379ZApplied to files:
📚 Learning: 2025-12-01T07:59:46.015ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
📚 Learning: 2025-12-01T08:00:37.919ZApplied to files:
🧬 Code graph analysis (2)key-wallet/tests/derivation_tests.rs (2)
key-wallet/src/account/account_type.rs (1)
🪛 GitHub Actions: Pre-commit Checkskey-wallet/tests/derivation_tests.rs[error] 129-129: [error] 176-176: 🪛 Gitleaks (8.30.0)key-wallet/tests/derivation_tests.rs[high] 126-126: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 150-150: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 218-218: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 242-242: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 294-294: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) [high] 318-318: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key) ⏰ 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). (4)
🔇 Additional comments (26)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
key-wallet/src/account/account_collection.rs (2)
553-576:is_empty()missing check forplatform_payment_accounts.The
is_empty()method doesn't include a check forplatform_payment_accounts.is_empty(), which means it will incorrectly returntrueif only PlatformPayment accounts exist.&& self.provider_owner_keys.is_none(); #[cfg(feature = "bls")] { is_empty = is_empty && self.provider_operator_keys.is_none(); } #[cfg(feature = "eddsa")] { is_empty = is_empty && self.provider_platform_keys.is_none(); } + is_empty = is_empty + && self.dashpay_receival_accounts.is_empty() + && self.dashpay_external_accounts.is_empty() + && self.platform_payment_accounts.is_empty(); + is_emptyNote:
dashpay_receival_accountsanddashpay_external_accountsalso appear to be missing from theis_empty()check.
579-597:clear()missing cleanup for several account maps.The
clear()method doesn't cleardashpay_receival_accounts,dashpay_external_accounts, or the newplatform_payment_accounts, which could leave stale data after clearing.pub fn clear(&mut self) { self.standard_bip44_accounts.clear(); self.standard_bip32_accounts.clear(); self.coinjoin_accounts.clear(); self.identity_registration = None; self.identity_topup.clear(); self.identity_topup_not_bound = None; self.identity_invitation = None; self.provider_voting_keys = None; self.provider_owner_keys = None; #[cfg(feature = "bls")] { self.provider_operator_keys = None; } #[cfg(feature = "eddsa")] { self.provider_platform_keys = None; } + self.dashpay_receival_accounts.clear(); + self.dashpay_external_accounts.clear(); + self.platform_payment_accounts.clear(); }
🧹 Nitpick comments (4)
key-wallet-ffi/src/transaction_checking.rs (1)
458-478: PlatformPayment mapping into FFITransactionCheckResult is correctThe
AccountTypeMatch::PlatformPaymentarm correctly:
- propagates
account_index,received, andsent,- derives
external_addresses_count/has_external_addressesfrominvolved_addresses,- sets
account_typeto 13 in line withFFIAccountType::PLATFORM_PAYMENT.This mirrors DashPay handling and keeps FFI in sync with the new account type, while remaining defensive about platform addresses not being used on Core chain.
If you ever touch this area again, consider defining a shared Rust
FFIAccountTypeenum or constants instead of raw integers (0–13) to avoid accidental drift with the generated C header.key-wallet/src/managed_account/address_pool.rs (1)
397-418: Platform address pool support is correct; consider tightening PlatformP2sh handling
new_platformcleanly reusesnew_without_generation, flipsaddress_typetoPlatformP2pkh, and optionally pre‑generates up to the gap limit when a key source is available, matching the existing pool semantics but with DIP‑18 address types.is_platformgives a simple, type‑level way to gate Platform pools (e.g., exclude them from Core‑chain transaction checking), which aligns with the DIP‑17 requirement that Platform addresses not be used on Core.- Extending
generate_address_at_indexto callAddress::platform_p2pkhforAddressType::PlatformP2pkhis exactly what we want for Platform payment addresses.The only nuance is the
AddressType::PlatformP2shbranch currently defaulting toplatform_p2pkh, which is safe but potentially surprising if anyone later assumes genuine P2SH semantics.Consider, when you need stricter behavior, returning a dedicated error for
AddressType::PlatformP2shin this context (e.g., “PlatformP2sh generation not supported in AddressPool; script required”) instead of silently mapping it to P2PKH. That keeps library code non‑panicking per guidelines while making any misuse explicit.Also applies to: 425-428, 513-519
key-wallet/src/managed_account/managed_account_collection.rs (1)
730-777:all_accounts()does not includeplatform_payment_accounts.The
all_accounts()method returns all accounts but does not includeplatform_payment_accounts. Similarly,all_accounts_mut()(lines 781-828),is_empty()(lines 856-870), andclear()(lines 873-887) also omit this field.This may be intentional to exclude Platform accounts from Core wallet operations, but could cause issues if these methods are used for wallet-wide operations like serialization or total balance calculation.
If Platform accounts should be accessible via these methods, add them:
// In all_accounts() accounts.extend(self.dashpay_receival_accounts.values()); accounts.extend(self.dashpay_external_accounts.values()); +accounts.extend(self.platform_payment_accounts.values()); accounts // In is_empty() && self.dashpay_receival_accounts.is_empty() && self.dashpay_external_accounts.is_empty() +&& self.platform_payment_accounts.is_empty() // In clear() self.dashpay_receival_accounts.clear(); self.dashpay_external_accounts.clear(); +self.platform_payment_accounts.clear();dash/src/address.rs (1)
535-548: Potential risk: Platform addresses can generate Core-compatible scripts.The
script_pubkey()method generates equivalent P2PKH/P2SH scripts for Platform addresses. While documented with a warning, this could lead to accidental fund loss if a developer callsscript_pubkey()on a Platform address and uses it in a Core transaction.Consider returning an error or a distinct script type for Platform addresses to prevent accidental misuse:
/// Generates a script pubkey spending to this [Payload]. /// /// Note: For Platform addresses, this generates the equivalent Core chain script, /// but Platform addresses should NOT be used in Core chain transactions. /// Consider using `script_pubkey_checked()` which returns an error for Platform addresses. pub fn script_pubkey(&self) -> ScriptBuf { // ... existing implementation } /// Generates a script pubkey, returning an error for Platform addresses. pub fn script_pubkey_checked(&self) -> Result<ScriptBuf, Error> { if self.is_platform() { return Err(Error::PlatformAddressNotForCore); } Ok(self.script_pubkey()) }Alternatively, add a runtime check or logging when
script_pubkey()is called on Platform addresses to help catch misuse during development.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
dash/src/address.rs(17 hunks)dash/src/blockdata/constants.rs(1 hunks)key-wallet-ffi/include/key_wallet_ffi.h(1 hunks)key-wallet-ffi/src/address_pool.rs(2 hunks)key-wallet-ffi/src/managed_account.rs(3 hunks)key-wallet-ffi/src/transaction_checking.rs(1 hunks)key-wallet-ffi/src/types.rs(3 hunks)key-wallet/src/account/account_collection.rs(9 hunks)key-wallet/src/account/account_type.rs(5 hunks)key-wallet/src/dip9.rs(3 hunks)key-wallet/src/managed_account/address_pool.rs(2 hunks)key-wallet/src/managed_account/managed_account_collection.rs(7 hunks)key-wallet/src/managed_account/managed_account_type.rs(6 hunks)key-wallet/src/managed_account/mod.rs(4 hunks)key-wallet/src/transaction_checking/account_checker.rs(6 hunks)key-wallet/src/transaction_checking/transaction_router/mod.rs(3 hunks)key-wallet/src/transaction_checking/wallet_checker.rs(1 hunks)key-wallet/src/wallet/helper.rs(1 hunks)key-wallet/tests/derivation_tests.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rsdash/src/blockdata/constants.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rsdash/src/address.rs
key-wallet/**/transaction_checking/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/transaction_checking/**/*.rs: UseTransactionRouter::classify_transaction()to determine transaction type andget_relevant_account_types()to avoid checking all accounts for every transaction; route CoinJoin and other specialized transactions only to relevant account types
In SPV sync integration, usewallet_info.check_transaction()with appropriateTransactionContextand update wallet state atomically only whenis_relevantflag is true
Files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/transaction_checking/account_checker.rs
key-wallet/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/*.rs: Never panic in library code; useResult<T>for all fallible operations with customErrortype variants and provide context in error messages
Use?operator for error propagation in Rust code to maintain clean error handling patterns
Files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
key-wallet/**/{wallet,account,managed_account,address_pool}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Always validate network consistency: use the account's network type when creating addresses and assert that operations use the expected network to prevent mainnet/testnet mixing
Files:
key-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/{wallet,account,managed_account}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/{wallet,account,managed_account}/**/*.rs: Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Clearly separate watch-only wallets from full wallets using the wallet type system; never attempt signing operations on watch-only accounts and validate external signatures match expected pubkeys
Files:
key-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/{account,managed_account}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/{account,managed_account}/**/*.rs: Separate immutable and mutable concerns: UseAccountfor immutable identity information (keys, derivation paths) andManagedAccountfor mutable state (address pools, metadata, balances)
Use strongly typed enum-based system forAccountTypewith specific variants (Standard, Identity, Masternode, etc.) to provide compile-time safety for different account purposes
Files:
key-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rs
key-wallet/**/{bip32,derivation,account}/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Use
ExtendedPubKeyandExtendedPrivKeyfrom the BIP32 implementation for key derivation; avoid direct private key exposure or serialization in logs
Files:
key-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rs
**/*-ffi/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Careful handling of memory safety at FFI boundaries in Rust code
Files:
key-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet-ffi/src/managed_account.rskey-wallet-ffi/src/transaction_checking.rs
key-wallet/**/managed_account/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Files:
key-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rs
**/tests/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/tests/**/*.rs: Write integration tests for network operations in Rust
Test both mainnet and testnet configurations
Use proptest for property-based testing where appropriate in RustIntegration tests use
tests/directory (e.g.,rpc-integration-test)
Files:
key-wallet/tests/derivation_tests.rs
key-wallet/**/tests/**/*.rs
📄 CodeRabbit inference engine (key-wallet/CLAUDE.md)
key-wallet/**/tests/**/*.rs: Use deterministic test vectors with fixed seeds and known test vectors in unit tests for key derivation, mnemonic generation, and address generation to ensure reproducibility
Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Files:
key-wallet/tests/derivation_tests.rs
🧠 Learnings (32)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Support future enhancements: prepare architecture for Schnorr/Taproot support, descriptor wallets, native multisig account types, and Lightning Network payment channel key management
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation,account}/**/*.rs : Use `ExtendedPubKey` and `ExtendedPrivKey` from the BIP32 implementation for key derivation; avoid direct private key exposure or serialization in logs
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) in address pool initialization to enable both full wallets and watch-only wallets with the same interface
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Use strongly typed enum-based system for `AccountType` with specific variants (Standard, Identity, Masternode, etc.) to provide compile-time safety for different account purposes
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Clearly separate watch-only wallets from full wallets using the wallet type system; never attempt signing operations on watch-only accounts and validate external signatures match expected pubkeys
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : Use `TransactionRouter::classify_transaction()` to determine transaction type and `get_relevant_account_types()` to avoid checking all accounts for every transaction; route CoinJoin and other specialized transactions only to relevant account types
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Clearly separate watch-only wallets from full wallets using the wallet type system; never attempt signing operations on watch-only accounts and validate external signatures match expected pubkeys
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Use strongly typed enum-based system for `AccountType` with specific variants (Standard, Identity, Masternode, etc.) to provide compile-time safety for different account purposes
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet-ffi/include/key_wallet_ffi.hkey-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account,address_pool}/**/*.rs : Always validate network consistency: use the account's network type when creating addresses and assert that operations use the expected network to prevent mainnet/testnet mixing
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/transaction_checking/**/*.rs : In SPV sync integration, use `wallet_info.check_transaction()` with appropriate `TransactionContext` and update wallet state atomically only when `is_relevant` flag is true
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/managed_account/**/*.rs : Implement atomic state updates in ManagedAccount: when processing transactions, update address usage state, UTXO set, transaction history, and balance calculations together to maintain consistency
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Separate immutable and mutable concerns: Use `Account` for immutable identity information (keys, derivation paths) and `ManagedAccount` for mutable state (address pools, metadata, balances)
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/wallet/**/*.rs : Implement wallet functionality with UTXO tracking, balance calculation with confirmation states, and transaction processing
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/transaction_checking.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation,account}/**/*.rs : Use `ExtendedPubKey` and `ExtendedPrivKey` from the BIP32 implementation for key derivation; avoid direct private key exposure or serialization in logs
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{wallet,account,managed_account}/**/*.rs : Use BTreeMap for ordered data structures (accounts, transactions) and HashMap for quick lookups (address to index mapping) to optimize data structure performance
Applied to files:
key-wallet/src/transaction_checking/wallet_checker.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet-ffi/src/transaction_checking.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
Applied to files:
key-wallet/src/dip9.rsdash/src/blockdata/constants.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{bip32,derivation}/**/*.rs : Cache intermediate derivation results and use hardened derivation only when required; batch derive child keys when possible to optimize derivation performance
Applied to files:
key-wallet/src/dip9.rskey-wallet/src/account/account_type.rskey-wallet/tests/derivation_tests.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Support multiple `KeySource` variants (Private, Public, NoKeySource) in address pool initialization to enable both full wallets and watch-only wallets with the same interface
Applied to files:
key-wallet/src/dip9.rskey-wallet/src/wallet/helper.rskey-wallet/src/account/account_type.rskey-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/types.rskey-wallet/src/account/account_collection.rskey-wallet/src/transaction_checking/transaction_router/mod.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/transaction_checking/account_checker.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Organize tests by functionality: BIP32 key derivation, mnemonic generation/validation, address generation/validation, path derivation, and PSBT serialization in separate test files
Applied to files:
key-wallet/src/dip9.rskey-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-06-15T15:31:44.136Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet/src/address.rs:30-40
Timestamp: 2025-06-15T15:31:44.136Z
Learning: In the key-wallet crate, QuantumExplorer prefers keeping wildcard arms that default unknown Network variants to testnet prefixes rather than using exhaustive matches or panics. This fallback behavior is intentional.
Applied to files:
key-wallet/src/wallet/helper.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/*.rs : Never panic in library code; use `Result<T>` for all fallible operations with custom `Error` type variants and provide context in error messages
Applied to files:
key-wallet/src/wallet/helper.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Support future enhancements: prepare architecture for Schnorr/Taproot support, descriptor wallets, native multisig account types, and Lightning Network payment channel key management
Applied to files:
key-wallet/src/account/account_type.rskey-wallet/src/account/account_collection.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Implement staged gap limit management using `GapLimitStage` structure with tracking of last_used_index and used_indices HashSet to enable efficient address discovery without loading entire address chains into memory
Applied to files:
key-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/address_pool/**/*.rs : Pre-generate addresses in batches (typically 20-100) and store in address pools; only derive on-demand when pool is exhausted to optimize performance and memory usage
Applied to files:
key-wallet-ffi/src/address_pool.rskey-wallet-ffi/src/managed_account.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/managed_account_type.rskey-wallet/src/managed_account/address_pool.rskey-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-06-15T16:42:18.187Z
Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 74
File: key-wallet-ffi/src/lib_tests.rs:41-48
Timestamp: 2025-06-15T16:42:18.187Z
Learning: In key-wallet-ffi, the HDWallet::derive_xpriv method returns Result<String, KeyWalletError>, not an ExtPrivKey wrapper. When unwrapped, it yields a String that can have .is_empty() called on it.
Applied to files:
key-wallet-ffi/src/types.rskey-wallet/tests/derivation_tests.rs
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash/src/blockdata/constants.rsdash/src/address.rs
📚 Learning: 2025-06-26T15:47:37.438Z
Learnt from: DCG-Claude
Repo: dashpay/rust-dashcore PR: 0
File: :0-0
Timestamp: 2025-06-26T15:47:37.438Z
Learning: Transaction IDs (txids) and block hashes in Dash are always 32 bytes and should be passed as fixed-size byte arrays in FFI interfaces, not as C strings, to avoid inefficiency and encoding issues.
Applied to files:
dash/src/blockdata/constants.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Maintain Minimum Supported Rust Version (MSRV) of 1.89; ensure compatibility with Dash Core versions 0.18.0 - 0.21.0
Applied to files:
dash/src/blockdata/constants.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/tests/**/*.rs : Use deterministic test vectors with fixed seeds and known test vectors in unit tests for key derivation, mnemonic generation, and address generation to ensure reproducibility
Applied to files:
key-wallet/tests/derivation_tests.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/tests/**/*.rs : Add integration tests in the `tests/` directory for comprehensive test suites
Applied to files:
key-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/src/**/*.rs : Add comprehensive unit tests in-module for individual components
Applied to files:
key-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Applies to dash-spv/tests/**/*.rs : Add real network tests with live Dash Core node integration that gracefully handle node unavailability
Applied to files:
key-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-12-01T07:59:46.015Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:46.015Z
Learning: Applies to **/tests/**/*.rs : Test both mainnet and testnet configurations
Applied to files:
key-wallet/tests/derivation_tests.rsdash/src/address.rs
📚 Learning: 2025-12-01T08:00:17.379Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:17.379Z
Learning: Follow a layered architecture with clear separation of concerns: `client/`, `network/`, `storage/`, `sync/`, `validation/`, `wallet/`, `types.rs`, and `error.rs`
Applied to files:
key-wallet/src/managed_account/managed_account_collection.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash/src/address.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Applies to **/*.rs : Unit tests should be placed near code with descriptive names (e.g., `test_parse_address_mainnet`)
Applied to files:
dash/src/address.rs
🧬 Code graph analysis (7)
key-wallet/src/account/account_type.rs (1)
key-wallet/src/transaction_checking/transaction_router/mod.rs (2)
from(187-237)from(241-291)
key-wallet-ffi/src/types.rs (1)
key-wallet/src/derivation.rs (1)
account(224-227)
key-wallet/src/managed_account/managed_account_type.rs (2)
key-wallet/src/managed_account/address_pool.rs (3)
network(1135-1138)new_platform(402-418)key_source(1153-1156)key-wallet/src/bip32.rs (1)
master(1302-1304)
key-wallet/tests/derivation_tests.rs (2)
key-wallet/src/derivation.rs (5)
from_seed(72-75)new(64-69)new(161-166)new(200-209)new(366-372)dash/src/address.rs (7)
from_str(225-236)from_str(294-297)from_str(1493-1572)encode(949-954)encode(985-990)new(479-499)new(1136-1144)
key-wallet-ffi/src/transaction_checking.rs (1)
key-wallet/src/transaction_checking/account_checker.rs (1)
account_index(178-210)
key-wallet/src/managed_account/managed_account_collection.rs (2)
key-wallet/src/account/account_collection.rs (1)
new(81-100)key-wallet/src/managed_account/address_pool.rs (6)
new(357-372)new(1094-1104)new_platform(402-418)base_path(1107-1110)network(1135-1138)key_source(1153-1156)
key-wallet/src/transaction_checking/account_checker.rs (2)
key-wallet/src/managed_account/managed_account_collection.rs (1)
new(58-75)key-wallet/src/managed_account/mod.rs (1)
new(63-73)
🪛 Gitleaks (8.30.0)
key-wallet/tests/derivation_tests.rs
[high] 126-126: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 227-227: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 310-310: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (20)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: Pre-commit (ubuntu-latest)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: Pre-commit (macos-latest)
🔇 Additional comments (34)
key-wallet/src/wallet/helper.rs (1)
876-880: LGTM! Correct handling of PlatformPayment account type.The implementation correctly returns
Nonefor PlatformPayment accounts, consistent with DIP-17/DIP-18 requirements that Platform Payment addresses are not used for Core chain transactions. This prevents xpub retrieval through this helper for platform-side-only account types.key-wallet/src/managed_account/mod.rs (1)
247-251: LGTM! Consistent integration of PlatformPayment across address pool methods.PlatformPayment is correctly grouped with DashpayExternalAccount in all address pool management methods (get_next_address_index, next_address, next_address_with_info, and gap_limit). This maintains consistent behavior for single-pool account types and properly integrates the new variant into the existing address generation and gap limit management flows.
Also applies to: 499-503, 588-592, 817-821
key-wallet/src/transaction_checking/wallet_checker.rs (1)
144-150: LGTM! Appropriate defensive handling for PlatformPayment.The implementation correctly returns
Nonefor PlatformPayment accounts with clear documentation that this branch should never be reached by design. Per DIP-17/DIP-18, Platform Payment addresses are not used in Core chain transactions, so preventing any state updates here is correct. This represents good defensive programming for exhaustive pattern matching.key-wallet-ffi/src/address_pool.rs (1)
62-67: LGTM! Correct FFI handling for PlatformPayment accounts.Both helper functions properly handle PlatformPayment by returning
None, consistent with the architecture where Platform Payment accounts are not persisted in ManagedAccountCollection. This aligns with DashPay account handling and correctly prevents retrieval of platform-side-only accounts through the FFI boundary.Also applies to: 111-116
key-wallet-ffi/include/key_wallet_ffi.h (1)
95-99: FFIAccountType extension for PlatformPayment looks consistent
PLATFORM_PAYMENT = 13cleanly extends the existing 0–12 range and matches the Rust-side mapping intransaction_checking.rs. No behavioral concerns as long as cbindgen and all FFI consumers (Swift, etc.) are regenerated/updated together.Please confirm that all language bindings consuming
FFIAccountTypehave been re-generated or updated so they see the newPLATFORM_PAYMENTcase.key-wallet/tests/derivation_tests.rs (1)
114-388: DIP‑17/DIP‑18 platform payment test vectors are thorough and deterministicThe new tests validate full paths (including non‑default
key_class'), private keys, compressed pubkeys, HASH160, and final PlatformP2PKH encodings on both mainnet and testnet, all derived from a fixed mnemonic. This gives strong regression coverage for the DIP‑17 derivation scheme and DIP‑18 Base58 encodings and matches the deterministic‑vector guidance forkey-wallettests.dash/src/blockdata/constants.rs (1)
58-71: DIP‑18 platform prefix constants are well‑placed and consistentDefining
PLATFORM_P2PKH_PREFIX_*andPLATFORM_P2SH_PREFIX_*here, alongside the existing pubkey/script prefixes, keeps all Base58 version bytes centralized. The values and doc comments align with the DIP‑18 handling indash::addressand the new platform address tests.key-wallet/src/transaction_checking/transaction_router/mod.rs (2)
181-183: Correct exclusion from Core chain transaction checking.The
PlatformPaymentvariant is properly documented as not being checked for Core chain transactions, aligning with the DIP-17 security requirement. Theget_relevant_account_types()method correctly excludesPlatformPaymentfrom all transaction type checks (lines 72-128), which is the intended behavior per the PR objectives.
233-235: Consistent conversion implementations for PlatformPayment.Both
From<ManagedAccountType>andFrom<&ManagedAccountType>implementations correctly mapPlatformPaymenttoAccountTypeToCheck::PlatformPayment, maintaining consistency with other account type conversions.Also applies to: 287-289
key-wallet/src/dip9.rs (3)
30-30: PlatformPayment derivation reference correctly positioned.The
PlatformPayment = 16variant is properly sequenced within theDerivationPathReferenceenum, maintaining the established pattern for new references.
382-419: Platform Payment root paths correctly implement DIP-17 structure.The root paths (
m/9'/5'/17'for mainnet,m/9'/1'/17'for testnet) correctly use:
- Hardened
FEATURE_PURPOSE(9')- Hardened coin type (5' for mainnet, 1' for testnet)
- Hardened
FEATURE_PURPOSE_PLATFORM_PAYMENT(17')The
CLEAR_FUNDSpath type is appropriate for payment addresses, consistent with other payment-related paths like DashPay.
135-136: DIP-17 feature index constant is correctly defined.The
FEATURE_PURPOSE_PLATFORM_PAYMENT = 17constant aligns with DIP-17 HD derivation specifications. Test vectors inkey-wallet/tests/derivation_tests.rsconfirm the constant's correctness with validated derivation paths (m/9'/5'/17'/0'/0'/0) and expected addresses.key-wallet-ffi/src/types.rs (2)
269-270: FFI enum variant correctly added for PlatformPayment.The
PlatformPayment = 13variant is properly sequenced and the doc comment correctly references DIP-17/DIP-18 with the derivation path structure.
424-427: Correct extraction of PlatformPayment fields for FFI.The
from_account_typeimplementation correctly mapsaccountto the primary index andkey_classto the optional secondary index, following the established tuple return pattern.key-wallet-ffi/src/managed_account.rs (3)
533-535: Correct FFI type mapping for PlatformPayment.The
managed_account_get_account_typefunction correctly mapsAccountType::PlatformPaymenttoFFIAccountType::PlatformPayment, maintaining consistency with other account type conversions.
1019-1022: PlatformPayment correctly exposed via Single address pool.The
managed_account_get_address_poolfunction correctly handlesPlatformPaymentby returning itsaddressesfield whenFFIAddressPoolType::Singleis requested, consistent with other non-standard account types like CoinJoin and Identity accounts.
181-183: PlatformPayment correctly excluded from generic account lookup.Returning
NoneforPlatformPaymentis consistent with howDashpayReceivingFundsandDashpayExternalAccountare handled. This follows the established pattern where account types requiring dedicated handling returnNonefrom the generic lookup function.Currently, no dedicated retrieval function exists for
PlatformPaymentaccounts (unlike the dedicatedmanaged_wallet_get_dashpay_receiving_accountandmanaged_wallet_get_dashpay_external_accountfunctions). If FFI callers need direct access toPlatformPaymentaccounts, consider adding a similar dedicated function for consistency and completeness.key-wallet/src/account/account_type.rs (3)
86-95: Well-documented PlatformPayment variant with security warning.The
PlatformPaymentvariant is properly documented with:
- DIP-17 reference and path structure
- DIP-18 encoding format note ("D"/"d" prefix)
- Critical warning that Platform addresses must not be used in Core chain transactions
The
accountandkey_classfields with their documentation align with DIP-17 path structure.
156-159: Correct primary index extraction for PlatformPayment.Returning
Some(*account)as the primary index is appropriate sinceaccountrepresents the account-level index in the derivation path, similar to how other account types expose their primary index.
413-437: DIP-17 Platform Payment derivation path implementation is correct.The code correctly implements the DIP-17 path structure (
m/9'/coin_type'/17'/account'/key_class') using network-appropriate root path constants and appending hardened indices. Restricting support to Mainnet and Testnet is intentional—no constants are defined indip9.rsfor Regtest/Devnet. This aligns with the design of other Platform-related variants (Masternode, Platform), which similarly support only Mainnet and Testnet.key-wallet/src/account/account_collection.rs (4)
31-40: Clean key structure for PlatformPayment accounts.The
PlatformPaymentAccountKeystruct correctly models the composite key withaccountandkey_classfields, properly deriving necessary traits includingOrdfor BTreeMap compatibility. The doc comment references DIP-17.
75-76: Consistent storage pattern for PlatformPayment accounts.Using
BTreeMap<PlatformPaymentAccountKey, Account>follows the established pattern for account types with composite keys (like DashPay accounts). Initialization innew()is correct.Also applies to: 98-98
174-183: Complete CRUD implementation for PlatformPayment.The
insert(),contains_account_type(),account_of_type(), andaccount_of_type_mut()methods all correctly handlePlatformPaymentby constructing the composite key and delegating to theplatform_payment_accountsmap.Also applies to: 267-276, 330-336, 390-396
433-435: Aggregation methods include PlatformPayment accounts.Both
all_accounts()andall_accounts_mut()correctly extend their results withplatform_payment_accounts.values(), ensuring PlatformPayment accounts are included in collection-wide operations.Also applies to: 473-475
key-wallet/src/transaction_checking/account_checker.rs (4)
106-112: Well-structured PlatformPayment variant with clear documentation.The
PlatformPaymentvariant correctly capturesaccount_index,key_class, andinvolved_addresses, matching the DIP-17 derivation path structure. The doc comment clearly warns that Platform addresses are not used in Core chain transactions.
387-392: Correct enforcement of DIP-17 security requirement.Returning an empty
VecforAccountTypeToCheck::PlatformPaymentcorrectly implements the DIP-17 mandate that Platform Payment addresses must not be used in Core chain transactions. The comment clearly explains this design decision.
648-654: PlatformPayment match arm correctly extracts account and key_class.The match arm properly destructures
ManagedAccountType::PlatformPaymentand constructs the correspondingAccountTypeMatch::PlatformPaymentwith the correct field mappings.
914-994:find_address_accountdoes not checkplatform_payment_accounts.This method searches for addresses across account types but does not include
platform_payment_accounts. This is likely intentional per DIP-17 (Platform addresses shouldn't appear in Core transactions), but if Platform addresses should be discoverable for other purposes (e.g., wallet display), this method may need updating in the future.Confirm this omission is intentional. If Platform addresses should be findable via this method for non-transaction purposes, consider adding:
// Check Platform Payment accounts for ((account, key_class), account_obj) in &collection.platform_payment_accounts { if account_obj.contains_address(address) { return Some((AccountTypeToCheck::PlatformPayment, Some(*account))); } }key-wallet/src/managed_account/managed_account_type.rs (1)
107-118: Clear and well-documented PlatformPayment variant.The variant definition includes:
- DIP-17 derivation path in the doc comment
- DIP-18 encoding format note
- Critical warning about not using in Core chain transactions
- Appropriate fields matching the derivation path structure
key-wallet/src/managed_account/managed_account_collection.rs (2)
52-53: Platform payment accounts field follows established patterns.The
platform_payment_accountsfield usesBTreeMap<PlatformPaymentAccountKey, ManagedAccount>consistent with other keyed account collections likedashpay_receival_accounts. This follows the coding guideline to use BTreeMap for ordered data structures.Also applies to: 73-73
607-624: Consistent Platform account creation in factory method.The
create_managed_account_from_account_typecorrectly usesAddressPool::new_platform()for Platform Payment accounts, ensuring addresses are generated with the correctPlatformP2pkhaddress type.dash/src/address.rs (3)
201-206: Platform address types clearly documented with usage warning.The
PlatformP2pkhandPlatformP2shvariants include clear doc comments explaining they are for Dash Platform only and MUST NOT be used in Core chain transactions. This is critical for preventing fund loss.
2296-2422: Comprehensive DIP-18 test coverage.The tests cover:
- Mainnet and testnet P2PKH Platform addresses with DIP-18 vectors
- Multiple test vectors for different HASH160 values
- P2SH Platform addresses
- Round-trip parsing verification
- Distinction between Platform and Core addresses
is_platform()flag verificationThis provides strong confidence in the DIP-18 implementation correctness.
1005-1006: bincode serialization uses sequential integer values.Values 5 and 6 are assigned for
PlatformP2pkhandPlatformP2shrespectively, continuing the sequence fromP2tr = 4. This maintains backward compatibility for existing serialized data.Also applies to: 1024-1025
| // DIP-18: Platform address prefixes | ||
| PLATFORM_P2PKH_PREFIX_MAIN => ( | ||
| Network::Dash, | ||
| Payload::PlatformPubkeyHash(PubkeyHash::from_slice(&data[1..]).unwrap()), | ||
| ), | ||
| PLATFORM_P2SH_PREFIX_MAIN => ( | ||
| Network::Dash, | ||
| Payload::PlatformScriptHash(ScriptHash::from_slice(&data[1..]).unwrap()), | ||
| ), | ||
| PLATFORM_P2PKH_PREFIX_TEST => ( | ||
| Network::Testnet, | ||
| Payload::PlatformPubkeyHash(PubkeyHash::from_slice(&data[1..]).unwrap()), | ||
| ), | ||
| PLATFORM_P2SH_PREFIX_TEST => ( | ||
| Network::Testnet, | ||
| Payload::PlatformScriptHash(ScriptHash::from_slice(&data[1..]).unwrap()), | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check platform prefix constant values in constants.rs
rg -n "PLATFORM_P2PKH_PREFIX_MAIN|PLATFORM_P2PKH_PREFIX_TEST|PLATFORM_P2SH_PREFIX_MAIN|PLATFORM_P2SH_PREFIX_TEST" dash/src/blockdata/constants.rsRepository: dashpay/rust-dashcore
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Check the context around the parsing code and verify it's in library code
# Also check if from_slice can fail
head -20 dash/src/address.rs | head -5
echo "---"
# Check the from_slice implementations
rg -A 5 "impl.*from_slice" dash/src/ | head -50Repository: dashpay/rust-dashcore
Length of output: 219
🏁 Script executed:
#!/bin/bash
# Check if from_slice implementations can fail (return Result/Option)
rg -B 2 -A 8 "fn from_slice" dash/src/address.rs | head -40Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Verify the exact code lines to confirm unwrap() usage
sed -n '1551,1567p' dash/src/address.rsRepository: dashpay/rust-dashcore
Length of output: 837
Replace unwrap() calls with proper error handling in library code.
The DIP-18 prefix constant mappings are correct (0x1e, 0x38, 0x5a, 0x75 match their respective payload types). However, all four from_slice() calls use .unwrap(), which violates the coding guideline prohibiting unwrap()/expect() in library code. Replace with a proper error type propagation using ? operator or explicit error handling.
🤖 Prompt for AI Agents
In dash/src/address.rs around lines 1551–1567, the match arms call
PubkeyHash::from_slice(...) and ScriptHash::from_slice(...) and then .unwrap(),
which is prohibited in library code; replace each .unwrap() with proper error
propagation (use ? or map_err to convert the slice failure into the function's
error type), update the surrounding function signature/return type if needed to
return a Result, and ensure you construct/propagate a meaningful Address error
variant (or existing error type) so invalid slice parsing returns Err instead of
panicking.
| FFIAccountType::PlatformPayment => { | ||
| panic!( | ||
| "FFIAccountType::PlatformPayment cannot be converted to AccountType \ | ||
| without account and key_class indices. The FFI API does not yet \ | ||
| support passing these values. This is a programming error - \ | ||
| Platform Payment account creation must use a different API path." | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual context around the lines mentioned
cd key-wallet-ffi && head -n 350 src/types.rs | tail -n 50Repository: dashpay/rust-dashcore
Length of output: 3078
🏁 Script executed:
# Check for the claimed pattern at lines 316-331
cd key-wallet-ffi && sed -n '316,339p' src/types.rsRepository: dashpay/rust-dashcore
Length of output: 1479
🏁 Script executed:
# Search for existing tests related to PlatformPayment or panic behavior in to_account_type
rg -n "PlatformPayment|to_account_type" key-wallet-ffi/src/ --type rs -A 3 -B 1Repository: dashpay/rust-dashcore
Length of output: 91
🏁 Script executed:
# Check if there's a test directory and what tests exist
find key-wallet-ffi -type f -name "*.rs" | head -20Repository: dashpay/rust-dashcore
Length of output: 755
🏁 Script executed:
# Search for tests related to to_account_type or PlatformPayment
rg -n "to_account_type|PlatformPayment" key-wallet-ffi/src --type rust -A 2 -B 1Repository: dashpay/rust-dashcore
Length of output: 8832
🏁 Script executed:
# Check if there's a types_tests.rs file
find key-wallet-ffi -name "*types*" -o -name "*test*"Repository: dashpay/rust-dashcore
Length of output: 1144
🏁 Script executed:
# Look at the lib.rs or lib_tests.rs to understand test organization
head -n 50 key-wallet-ffi/src/lib_tests.rsRepository: dashpay/rust-dashcore
Length of output: 1889
🏁 Script executed:
# Check if there's an error module in this FFI crate
cat key-wallet-ffi/src/error.rs | head -n 100Repository: dashpay/rust-dashcore
Length of output: 2627
🏁 Script executed:
# Look at the full context of the to_account_type function
rg -B 20 "fn to_account_type" key-wallet-ffi/src/types.rsRepository: dashpay/rust-dashcore
Length of output: 1053
🏁 Script executed:
# Search for any tests related to PlatformPayment panic
rg -n "test.*platform.*payment|platform.*payment.*panic" key-wallet-ffi/src/types.rs -iRepository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
# Check the full test section in types.rs to see all tests
sed -n '436,495p' key-wallet-ffi/src/types.rsRepository: dashpay/rust-dashcore
Length of output: 2497
🏁 Script executed:
# Check if to_account_type is documented anywhere about when it panics
rg -B 10 "pub fn to_account_type" key-wallet-ffi/src/types.rs | head -30Repository: dashpay/rust-dashcore
Length of output: 563
🏁 Script executed:
# Verify the usage pattern - does PlatformPayment ever get passed to to_account_type in normal code flow?
rg "FFIAccountType::PlatformPayment.*to_account_type|to_account_type.*PlatformPayment" key-wallet-ffi/srcRepository: dashpay/rust-dashcore
Length of output: 47
Add test for PlatformPayment panic behavior to match existing DashPay tests.
The panic for PlatformPayment in to_account_type() follows the established pattern for account types that cannot be converted due to FFI API limitations (DashpayReceivingFunds and DashpayExternalAccount both panic for similar reasons). However, unlike those types which have explicit panic tests (test_dashpay_receiving_funds_to_account_type_panics, test_dashpay_external_account_to_account_type_panics), there is no corresponding test for PlatformPayment. Add a #[should_panic] test at lines 449-452 to verify the panic occurs when attempting to convert FFIAccountType::PlatformPayment to AccountType, similar to the DashPay test pattern.
🤖 Prompt for AI Agents
In key-wallet-ffi/src/types.rs around lines 332 and add test at lines 449-452:
the FFIAccountType::PlatformPayment arm panics when converting to AccountType
but there's no unit test asserting this behavior; add a new #[test] with
#[should_panic] at the indicated lines that constructs
FFIAccountType::PlatformPayment and calls to_account_type() (mirroring the
existing DashPay panic tests) to verify the panic occurs.
| AccountType::PlatformPayment { | ||
| account, | ||
| key_class, | ||
| } => { | ||
| // DIP-17: m/9'/coin_type'/17'/account'/key_class'/index | ||
| // The leaf index is non-hardened | ||
| let path = account_type | ||
| .derivation_path(network) | ||
| .unwrap_or_else(|_| DerivationPath::master()); | ||
| let pool = AddressPool::new_platform( | ||
| path, | ||
| crate::managed_account::address_pool::AddressPoolType::Absent, | ||
| DEFAULT_SPECIAL_GAP_LIMIT, // DIP-17 recommends gap limit of 20 | ||
| network, | ||
| key_source, | ||
| )?; | ||
| Ok(Self::PlatformPayment { | ||
| account, | ||
| key_class, | ||
| addresses: pool, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "DEFAULT_SPECIAL_GAP_LIMIT" key-wallet/src/Repository: dashpay/rust-dashcore
Length of output: 2323
🏁 Script executed:
rg -n "fn new_platform" key-wallet/src/managed_account/address_pool/Repository: dashpay/rust-dashcore
Length of output: 149
🏁 Script executed:
rg -n "AddressType::PlatformP2pkh" key-wallet/src/managed_account/address_pool/Repository: dashpay/rust-dashcore
Length of output: 149
🏁 Script executed:
fd "address_pool" key-wallet/src/ -type fRepository: dashpay/rust-dashcore
Length of output: 235
🏁 Script executed:
rg -n "pub const DEFAULT_SPECIAL_GAP_LIMIT" key-wallet/src/gap_limit.rs -A 2Repository: dashpay/rust-dashcore
Length of output: 218
🏁 Script executed:
rg -n "impl AddressPool" key-wallet/src/ -A 5Repository: dashpay/rust-dashcore
Length of output: 1068
🏁 Script executed:
rg -n "fn new_platform" key-wallet/src/managed_account/address_pool.rs -A 15Repository: dashpay/rust-dashcore
Length of output: 710
🏁 Script executed:
rg -n "DIP-17\|DIP17" key-wallet/src/ -B 2 -A 2Repository: dashpay/rust-dashcore
Length of output: 47
🏁 Script executed:
rg -n "PlatformP2pkh" key-wallet/src/managed_account/address_pool.rsRepository: dashpay/rust-dashcore
Length of output: 322
Incorrect gap limit for Platform addresses.
While the implementation correctly uses AddressPool::new_platform() and sets AddressType::PlatformP2pkh, there is a discrepancy: the code uses DEFAULT_SPECIAL_GAP_LIMIT (which is set to 5 in key-wallet/src/gap_limit.rs:23), but the inline comment states "DIP-17 recommends gap limit of 20". The actual gap limit used is 5, not 20.
Either DEFAULT_SPECIAL_GAP_LIMIT should be updated to 20 to match the DIP-17 recommendation, or the comment should be corrected to reflect the actual intended value.
🤖 Prompt for AI Agents
In key-wallet/src/managed_account/managed_account_type.rs around lines 679 to
700, the code uses DEFAULT_SPECIAL_GAP_LIMIT (currently 5) while the comment
claims DIP-17 recommends 20; fix by making the implementation and comment
consistent: either change the call to AddressPool::new_platform to pass 20
explicitly or (preferable) update DEFAULT_SPECIAL_GAP_LIMIT in
key-wallet/src/gap_limit.rs to 20 and keep the comment as-is, then run
tests/formatting to ensure no other code relies on the old value.
|
Just reopening to trigger CI after pre-commit fix. |
Summary
Changes
dashcore crate
PlatformP2pkhandPlatformP2shvariants toAddressTypeenumPlatformPubkeyHashandPlatformScriptHashvariants toPayloadenumis_platform()method toAddresstypekey-wallet crate
FEATURE_PURPOSE_PLATFORM_PAYMENT = 17constant (DIP-9 feature index)PlatformPaymenttoDerivationPathReferenceenumPlatformPaymentvariant toAccountTypeandManagedAccountTypeenumsnew_platform()constructor toAddressPoolfor Platform address generationkey-wallet-ffi crate
PlatformPaymenttoFFIAccountTypeenumSummary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.