Implement Dogecoin #130
Implement Dogecoin #130
Conversation
WalkthroughThis pull request introduces comprehensive support for the Dogecoin cryptocurrency across multiple components of the codebase. The changes span network type definitions, address prefix handling, transaction building utilities, constants, and predefined assets. A new Dogecoin network type is added with specific configurations, including address prefixes, BIP standards, and network-specific parameters. The modifications enable the system to recognize, process, and interact with Dogecoin transactions and addresses, expanding the existing cryptocurrency support infrastructure. Changes
Sequence DiagramsequenceDiagram
participant Client
participant NetworkType
participant TransactionBuilder
participant AddressHandler
Client->>NetworkType: Select DOGECOIN_NETWORK
NetworkType-->>Client: Network Configuration
Client->>TransactionBuilder: Build Transaction
TransactionBuilder->>AddressHandler: Validate Address Prefixes
AddressHandler-->>TransactionBuilder: Address Validation Result
TransactionBuilder-->>Client: Transaction Ready
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
test/ci/fetching/assets/doge_fetch_test.dart (2)
8-8: Test name could be more descriptiveThe test name "No Structure Transaction with Xpub reject wallet" doesn't clearly indicate that this is a Dogecoin-specific test.
Consider renaming to "Dogecoin - Fetch transactions for wallet with no structure".
9-9: Remove or document the hardcoded addressThe commented address
D65DuQST2rnXaSeyEZQrerjSFCQjd8ZQ5Tappears to be a test address but its purpose is unclear.Either remove it if unused or document its purpose and whether it's a testnet/mainnet address.
test/ci/fetching/endpoint_test.dart (1)
23-35: Align test configuration with other cryptocurrenciesThe test uses different parameters compared to other cryptocurrency tests:
- Uses
min: 1while Zeniq usesmin: 3- Placement between Zeniq and Bitcoin tests seems arbitrary
Consider:
- Increasing minimum endpoints for better reliability
- Moving the test to maintain alphabetical order with other cryptocurrency tests
- min: 1, + min: 3,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
lib/src/crypto/network_type.dart(2 hunks)lib/src/crypto/utxo/entities/payments/p2h.dart(1 hunks)lib/src/crypto/utxo/utils/send.dart(2 hunks)lib/src/domain/constants.dart(2 hunks)lib/src/domain/predefined_assets.dart(1 hunks)test/ci/fetching/assets/doge_fetch_test.dart(1 hunks)test/ci/fetching/endpoint_test.dart(1 hunks)test/no_ci/input_simulation_test.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Static code analysis and run unit tests
🔇 Additional comments (8)
lib/src/domain/constants.dart (2)
37-39: LGTM! Address prefixes are correctly definedThe Dogecoin address prefixes are accurate:
- 'D' for legacy addresses (P2PKH)
- 'A' for compatibility addresses (P2SH)
- 'dc1' for native SegWit addresses (P2WPKH)
56-65: Verify BIP prefixes for DogecoinThe implementation uses the same prefix for BIP49 and BIP84, which is unusual. While the comments acknowledge this, we should verify this is intentional.
lib/src/domain/predefined_assets.dart (1)
31-35: LGTM! Dogecoin asset is correctly definedThe Dogecoin asset definition follows the established pattern for UTXO coins and uses the correct parameters:
- Standard 8 decimal places
- Correct symbol 'DOGE'
- Proper placement in UTXO assets section
lib/src/crypto/utxo/entities/payments/p2h.dart (1)
22-23: LGTM! P2PKH prefix check for Dogecoin added correctly.The Dogecoin P2PKH prefix check is properly integrated alongside other network prefixes.
test/no_ci/input_simulation_test.dart (1)
305-305: LGTM! Dogecoin network handling added to simulation tests.The Dogecoin network case is correctly added to use BTCOutput, consistent with other UTXO networks like Bitcoin and Litecoin.
lib/src/crypto/network_type.dart (2)
370-395: LGTM! Dogecoin network configuration looks correct.The network parameters are properly configured:
- Message prefix: '\x19Dogecoin Signed Message:\n'
- Bech32 prefix: 'dc'
- Address prefixes: 'D' (legacy), '9' (compatibility), 'dc1' (segwit)
- Block time: 60 seconds (1 minute)
174-177: Verify the dust threshold value for Dogecoin.The dust threshold is set to 100000000 (1 DOGE), which is significantly higher than Bitcoin's threshold (546 satoshis). Please verify if this is the correct value according to Dogecoin's specifications.
✅ Verification successful
The Dogecoin dust threshold of 1 DOGE is correct
The higher dust threshold compared to Bitcoin (546 satoshis) is appropriate given Dogecoin's lower unit value and transaction fees. Setting it to 1 DOGE (100000000) helps prevent spam transactions while maintaining the network's accessibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for dust threshold references in Dogecoin Core rg "dust|dustthreshold|minrelaytxfee" -g '!node_modules'Length of output: 1183
Script:
#!/bin/bash # Find BTC dust threshold and Dogecoin configuration echo "=== BTC Dust Threshold ===" rg "BTC_DUSTTRESHOLD.*=.*\(" -A 2 echo -e "\n=== Dogecoin Documentation ===" fd -e md -e txt | grep -i "doge\|coin\|readme" | xargs cat echo -e "\n=== Network Constants ===" rg "const.*=.*\(" lib/src/crypto/network_type.dartLength of output: 8466
Script:
#!/bin/bash # Search for Dogecoin-specific documentation or comments echo "=== Dogecoin Implementation Details ===" rg -B 2 -A 2 "DOGE" lib/src/crypto/ echo -e "\n=== Comments about dust threshold ===" rg -B 2 -A 2 "dust.*threshold" -i lib/src/crypto/Length of output: 4115
lib/src/crypto/utxo/utils/send.dart (1)
470-470: LGTM! Dogecoin transaction building support added correctly.The Dogecoin network case is properly added to both
buildInputandbuildOutputfunctions, using the appropriate BTC transaction types consistent with other UTXO networks.Also applies to: 494-494
| print(adr); | ||
| } | ||
|
|
||
| expect(txList.length, greaterThanOrEqualTo(0)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen the test assertion
The test only verifies that txList.length >= 0, which is a weak assertion that would pass even if the fetching fails.
Consider adding more specific assertions:
- expect(txList.length, greaterThanOrEqualTo(0));
+ expect(txList, isNotNull);
+ expect(nodes, isNotNull);
+ expect(nodes.addresses, isNotEmpty, "Should have generated at least one address");
+ for (final tx in txList) {
+ expect(tx.networkType, equals(DogecoinNetwork));
+ }Committable suggestion skipped: line range outside the PR's diff.
| address.startsWith(P2SH_PREFIX_LTC) || | ||
| address.startsWith(P2SH_PREFIX_DOGE)) { |
There was a problem hiding this comment.
💡 Codebase verification
Remove Dogecoin SegWit (P2WPKH) validation
Dogecoin has not activated SegWit on mainnet, therefore the P2WPKH address validation with dc1 prefix should be removed. The P2SH validation is correct and can remain unchanged.
- Remove
P2WPKH_PREFIX_DOGEconstant fromlib/src/domain/constants.dart - Remove Dogecoin check from P2WPKH validation in
lib/src/crypto/utxo/entities/payments/p2h.dart
🔗 Analysis chain
Verify Dogecoin SegWit support.
While the P2SH prefix check is correct, we should verify if Dogecoin has activated SegWit and supports P2WPKH addresses with the dc1 prefix.
Run this script to check for SegWit activation status:
Also applies to: 34-35
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any Dogecoin addresses in the codebase use SegWit format
# and verify SegWit support in Dogecoin documentation/specifications
# Search for Dogecoin SegWit addresses
rg "dc1[a-zA-HJ-NP-Z0-9]+" -g '!node_modules'
# Search for SegWit-related constants or configurations for Dogecoin
rg "P2WPKH.*DOGE|SegWit.*Doge|Dogecoin.*SegWit" -g '!node_modules'
Length of output: 4574
| endpoints: const { | ||
| ("doge.aftrek.org", 50001), | ||
| // ("electrum1.cipig.net", 10060), | ||
| // ("electrum2.cipig.net", 10060), | ||
| // ("electrum3.cipig.net", 10060), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add more reliable Dogecoin ElectrumX endpoints.
Currently, only one active endpoint is configured (doge.aftrek.org). Consider:
- Uncomment and verify the cipig.net endpoints
- Add more reliable endpoints for redundancy
The limited number of endpoints could affect network reliability and transaction broadcasting.
Summary by CodeRabbit
New Features
Tests
Documentation