-
Notifications
You must be signed in to change notification settings - Fork 3
feat: coinjoin derivation path support #63
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
Conversation
WalkthroughThe pull request updates the workspace version in the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant D as DerivationPath
participant DIP9 as DIP9 Constants
C->>D: call coinjoin_path(network, account)
D->>D: Check network type
alt Network is Dash
D->>D: Use COINJOIN_PATH_MAINNET (from DIP9)
else Other Network
D->>D: Use COINJOIN_PATH_TESTNET (from DIP9)
end
D->>D: Extend path with hardened account index
D-->>C: Return updated derivation path
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (1)
dash/src/bip32.rs (1)
492-503: New coinjoin_path method looks good but lacks documentation.The implementation correctly handles different networks and follows the same design pattern as other derivation path methods. It builds a path based on the network type and appends the hardened account index.
However, adding documentation comments to describe the purpose and usage of this method would improve maintainability.
+ /// Returns a derivation path for CoinJoin operations + /// + /// The path follows the pattern m/9'/5'/4'/account' for mainnet and + /// m/9'/1'/4'/account' for testnet networks where account is hardened. pub fn coinjoin_path( network: Network, account: u32, ) -> Self { let mut root_derivation_path: DerivationPath = match network { Network::Dash => COINJOIN_PATH_MAINNET, _ => COINJOIN_PATH_TESTNET, } .into(); root_derivation_path.0.extend(&[ChildNumber::Hardened { index: account }]); root_derivation_path }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
dash/src/bip32.rs(3 hunks)dash/src/dip9.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- dash/src/dip9.rs
🔇 Additional comments (2)
dash/src/bip32.rs (2)
39-39: Import statement updated to include coinjoin path constants.The import statement has been modified to include the new COINJOIN_PATH_MAINNET and COINJOIN_PATH_TESTNET constants from dip9.rs. This is aligned with the PR objective to support CoinJoin derivation paths.
2069-2076: Test coverage for coinjoin_path is sufficient.The test validates the generation of CoinJoin paths for both mainnet and testnet networks with different account indices, ensuring that the paths are correctly constructed. It follows the same pattern as other derivation path tests in this file.
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.
Approved, seems to match the DIP here https://github.com/dashpay/dips/blob/master/dip-0009/assignments.md
This pull request introduces a new feature for supporting CoinJoin paths in the Dash project. The major changes include adding new constants, modifying existing enums, and adding new methods and tests to handle the CoinJoin functionality.
New feature for CoinJoin paths:
dash/src/dip9.rs: Added new constantsFEATURE_PURPOSE_COINJOIN,COINJOIN_PATH_MAINNET, andCOINJOIN_PATH_TESTNETto define CoinJoin paths for both mainnet and testnet. [1] [2]dash/src/dip9.rs: Updated theDerivationPathReferenceenum to include a new variantCoinJoin.Modifications to existing files:
dash/src/bip32.rs: Updated the imports to include the new CoinJoin paths and added a new methodcoinjoin_pathin theDerivationPathimplementation to generate CoinJoin paths based on the network and account. [1] [2]Testing:
dash/src/bip32.rs: Added a new testtest_coinjoin_pathto verify the correctness of the CoinJoin path generation.Version bump:
Cargo.toml: Updated the package version from0.38.0to0.38.1to reflect the new feature addition.Summary by CodeRabbit
Chores
New Features