fix: integration tests, continued dashificiation #137
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughReplaces a two-node Bitcoin regtest with a single Dash Core regtest node on port 19898, consolidates RPC URL/auth fallbacks, gates Evo/ChainLocks tests behind RUN_EVO_TESTS, updates test funding/signing flow for deterministic confirmed UTXOs, renames GetNetworkInfo connection-count fields, and enables core-block-hash-use-x11 in the dashcore dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Run as run.sh
participant Dashd as dashd (regtest)
participant CLI as dash-cli
participant Test as cargo run (integration tests)
participant RPCJSON as rpc-json deserializer
Run->>Dashd: start -regtest -datadir dash -rpcport 19898 -server -txindex
Run-->>Run: sleep 5
Run->>CLI: createwallet "main"
Run->>CLI: getnewaddress (wallet "main")
CLI-->>Run: faucet address
Run->>CLI: generatetoaddress 110, faucet address
Note over Run,Test: export WALLET/EVO RPC_URL=http://127.0.0.1:19898 and cookie paths
Run->>Test: cargo run
Test->>Test: resolve RPC URLs (WALLET/EVO → RPC_URL → default)
Test->>Test: resolve auth (node cookie → node user/pass → RPC_COOKIE → none)
alt RUN_EVO_TESTS=true/1
Test->>Dashd: call Evo endpoints (masternode, ChainLocks)
else
Test-->>Test: skip Evo tests (trace)
end
Test->>Dashd: call getnetworkinfo
Dashd-->>Test: JSON response
Test->>RPCJSON: deserialize GetNetworkInfoResult (fields: connections_in/out/mn...)
Run->>Dashd: kill (cleanup)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ 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). (19)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rpc-integration-test/src/main.rs (1)
1162-1167: Fix: get_peer_info will always panic on a single-node setup.With one node there are no peers by design; this test now fails deterministically. Skip gracefully when empty (consistent with prior guidance to skip when external dependencies aren’t available).
fn test_get_peer_info(cl: &Client) { - let info = cl.get_peer_info().unwrap(); - if info.is_empty() { - panic!("No peers are connected so we can't test get_peer_info"); - } + let info = cl.get_peer_info().unwrap(); + if info.is_empty() { + trace!(target: "integration_test", "Skipping get_peer_info: no peers connected"); + return; + } }
🧹 Nitpick comments (4)
rpc-integration-test/run.sh (1)
21-21: Consider enabling console logs during failures.
-printtoconsole=0hides node logs. For debugging CI flakes, consider-printtoconsole=1(or tail${TESTDIR}/dash/regtest/debug.logon failures).rpc-integration-test/src/main.rs (3)
139-191: Avoid silently using empty passwords when only USER is set.If
*_RPC_USER(orRPC_USER) is present but no corresponding PASS, the code buildsAuth::UserPass(user, ""), which will 401 instead of falling back to cookie/none. Only use UserPass when a non-empty PASS exists.Apply this minimal change for both wallet and evo blocks:
- .map(|user| { - let pass = std::env::var("WALLET_NODE_RPC_PASS") - .or_else(|_| std::env::var("RPC_PASS")) - .unwrap_or_default(); - Auth::UserPass(user, pass) - }) + .and_then(|user| { + std::env::var("WALLET_NODE_RPC_PASS") + .or_else(|_| std::env::var("RPC_PASS")) + .ok() + .filter(|s| !s.is_empty()) + .map(|pass| Auth::UserPass(user, pass)) + }) .unwrap_or_else(|| {- .map(|user| { - let pass = std::env::var("EVO_NODE_RPC_PASS") - .or_else(|_| std::env::var("RPC_PASS")) - .unwrap_or_default(); - Auth::UserPass(user, pass) - }) + .and_then(|user| { + std::env::var("EVO_NODE_RPC_PASS") + .or_else(|_| std::env::var("RPC_PASS")) + .ok() + .filter(|s| !s.is_empty()) + .map(|pass| Auth::UserPass(user, pass)) + }) .unwrap_or_else(|| {
226-229: Remove redundant clone.
clone().clone()is unnecessary.-let faucet_client = Client::new(&faucet_rpc_url, wallet_node_auth.clone().clone()).unwrap(); +let faucet_client = Client::new(&faucet_rpc_url, wallet_node_auth.clone()).unwrap();
313-315: Update stale comment (no longer a 3-node setup).The “network of three nodes” note is outdated after moving to a single regtest node. Update to avoid confusion.
- // TODO: fix - looks like there's a consensus delay when things are running on network of three nodes. + // Note: some balance-related checks may need additional blocks due to confirmation/maturity rules on a single-node regtest.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
rpc-integration-test/run.sh(1 hunks)rpc-integration-test/src/main.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
rpc-integration-test/src/main.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
rpc-integration-test/src/main.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
rpc-integration-test/run.shrpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
rpc-integration-test/run.shrpc-integration-test/src/main.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Applied to files:
rpc-integration-test/run.shrpc-integration-test/src/main.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
Applied to files:
rpc-integration-test/run.shrpc-integration-test/src/main.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Applied to files:
rpc-integration-test/run.sh
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Include integration tests that cover full wallet workflows
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/bip32_tests.rs : Organize unit tests so key-derivation correctness lives in tests/bip32_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/mnemonic_tests.rs : Place mnemonic generation and validation tests in tests/mnemonic_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
⏰ 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). (12)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: Core Components Tests
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: SPV Components Tests
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deserialize_address)
🔇 Additional comments (4)
rpc-integration-test/src/main.rs (4)
43-46: Port defaults update looks good.
123-136: Generic RPC_URL fallback is a sensible simplification.
283-293: Evo/masternode test gating is appropriate.
317-324: ChainLocks gated behind RUN_EVO_TESTS is correct for single-node regtest.
Comment on lines
+3
to
46
| set -e | ||
|
|
||
| rm -rf ${TESTDIR} | ||
| mkdir -p ${TESTDIR}/1 ${TESTDIR}/2 | ||
| TESTDIR=/tmp/rust_dashcore_rpc_test | ||
|
|
||
| # To kill any remaining open bitcoind. | ||
| killall -9 bitcoind | ||
| rm -rf "${TESTDIR}" | ||
| mkdir -p "${TESTDIR}/dash" | ||
|
|
||
| bitcoind -regtest \ | ||
| -datadir=${TESTDIR}/1 \ | ||
| -port=12348 \ | ||
| -server=0 \ | ||
| -printtoconsole=0 & | ||
| PID1=$! | ||
| # Kill any remaining dashd to avoid port conflicts | ||
| if command -v killall >/dev/null 2>&1; then | ||
| killall -9 dashd 2>/dev/null || true | ||
| fi | ||
|
|
||
| # Make sure it's listening on its p2p port. | ||
| sleep 3 | ||
| # Start Dash Core on regtest using standard Dash RPC port 19898 | ||
| dashd -regtest \ | ||
| -datadir="${TESTDIR}/dash" \ | ||
| -rpcport=19898 \ | ||
| -server=1 \ | ||
| -txindex=1 \ | ||
| -printtoconsole=0 & | ||
| PID=$! | ||
|
|
||
| BLOCKFILTERARG="" | ||
| if bitcoind -version | grep -q "v0\.\(19\|2\)"; then | ||
| BLOCKFILTERARG="-blockfilterindex=1" | ||
| fi | ||
| # Allow time for startup | ||
| sleep 5 | ||
|
|
||
| FALLBACKFEEARG="" | ||
| if bitcoind -version | grep -q "v0\.2"; then | ||
| FALLBACKFEEARG="-fallbackfee=0.00001000" | ||
| fi | ||
| # Pre-create faucet wallet "main" so the test can fund addresses | ||
| dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -named createwallet wallet_name=main descriptors=false >/dev/null 2>&1 || true | ||
|
|
||
| bitcoind -regtest $BLOCKFILTERARG $FALLBACKFEEARG \ | ||
| -datadir=${TESTDIR}/2 \ | ||
| -connect=127.0.0.1:12348 \ | ||
| -rpcport=12349 \ | ||
| -server=1 \ | ||
| -txindex=1 \ | ||
| -printtoconsole=0 & | ||
| PID2=$! | ||
| # Fund the faucet wallet with mature coins | ||
| FAUCET_ADDR=$(dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -rpcwallet=main getnewaddress) | ||
| dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 generatetoaddress 110 "$FAUCET_ADDR" >/dev/null | ||
|
|
||
| # Let it connect to the other node. | ||
| sleep 5 | ||
| # Export per-node env vars expected by the test (both point to same node) | ||
| export WALLET_NODE_RPC_URL="http://127.0.0.1:19898" | ||
| export EVO_NODE_RPC_URL="http://127.0.0.1:19898" | ||
| export WALLET_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | ||
| export EVO_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | ||
|
|
||
| RPC_URL=http://localhost:12349 \ | ||
| RPC_COOKIE=${TESTDIR}/2/regtest/.cookie \ | ||
| cargo run | ||
| cargo run | ||
|
|
||
| RESULT=$? | ||
|
|
||
| kill -9 $PID1 $PID2 | ||
| kill -9 $PID 2>/dev/null || true | ||
|
|
||
| exit $RESULT |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the harness deterministic and safe: trap-based cleanup, readiness wait, and configurable RPC port.
As written, set -e will abort before cleanup if cargo run fails, leaving dashd running and causing flaky port conflicts. Also, sleep 5 is racy on CI, and hard-coding port 19898 risks collisions. Apply this consolidated patch:
-#!/bin/sh
+#!/bin/sh
-set -e
+set -eu
TESTDIR=/tmp/rust_dashcore_rpc_test
+
+# Allow overriding the RPC port; default to Dash regtest typical port
+RPC_PORT="${RPC_PORT:-19898}"
+
+cleanup() {
+ if [ -n "${PID:-}" ] && kill -0 "$PID" 2>/dev/null; then
+ # Prefer graceful stop; fall back to TERM
+ dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport="${RPC_PORT}" stop >/dev/null 2>&1 \
+ || kill -TERM "$PID" 2>/dev/null || true
+ wait "$PID" 2>/dev/null || true
+ fi
+}
+trap 'cleanup' EXIT INT TERM
rm -rf "${TESTDIR}"
mkdir -p "${TESTDIR}/dash"
-# Kill any remaining dashd to avoid port conflicts
-if command -v killall >/dev/null 2>&1; then
- killall -9 dashd 2>/dev/null || true
-fi
+# Remove any stray dashd that uses our test datadir (avoid nuking user daemons)
+if command -v pgrep >/dev/null 2>&1; then
+ PIDS="$(pgrep -f "dashd .* -datadir=${TESTDIR}/dash" || true)"
+ if [ -n "$PIDS" ]; then kill -9 $PIDS 2>/dev/null || true; fi
+fi
-# Start Dash Core on regtest using standard Dash RPC port 19898
+# Start Dash Core on regtest
dashd -regtest \
-datadir="${TESTDIR}/dash" \
- -rpcport=19898 \
+ -rpcport="${RPC_PORT}" \
-server=1 \
-txindex=1 \
-printtoconsole=0 &
PID=$!
-# Allow time for startup
-sleep 5
+# Wait for RPC to come up
+rpc() { dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport="${RPC_PORT}" "$@"; }
+i=0
+until rpc getblockchaininfo >/dev/null 2>&1; do
+ i=$((i+1))
+ if [ "$i" -ge 120 ]; then
+ echo "dashd not responding on 127.0.0.1:${RPC_PORT} after 60s" >&2
+ exit 1
+ fi
+ sleep 0.5
+done
# Pre-create faucet wallet "main" so the test can fund addresses
-dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -named createwallet wallet_name=main descriptors=false >/dev/null 2>&1 || true
+rpc -named createwallet wallet_name=main descriptors=false >/dev/null 2>&1 || true
# Fund the faucet wallet with mature coins
-FAUCET_ADDR=$(dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -rpcwallet=main getnewaddress)
-dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 generatetoaddress 110 "$FAUCET_ADDR" >/dev/null
+FAUCET_ADDR="$(rpc -rpcwallet=main getnewaddress)"
+rpc generatetoaddress 110 "$FAUCET_ADDR" >/dev/null
# Export per-node env vars expected by the test (both point to same node)
-export WALLET_NODE_RPC_URL="http://127.0.0.1:19898"
-export EVO_NODE_RPC_URL="http://127.0.0.1:19898"
+export WALLET_NODE_RPC_URL="http://127.0.0.1:${RPC_PORT}"
+export EVO_NODE_RPC_URL="http://127.0.0.1:${RPC_PORT}"
export WALLET_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie"
export EVO_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie"
-cargo run
+set +e
+cargo run
+RESULT=$?
+set -e
-
-RESULT=$?
-
-kill -9 $PID 2>/dev/null || true
-
-exit $RESULT
+exit $RESULT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| set -e | |
| rm -rf ${TESTDIR} | |
| mkdir -p ${TESTDIR}/1 ${TESTDIR}/2 | |
| TESTDIR=/tmp/rust_dashcore_rpc_test | |
| # To kill any remaining open bitcoind. | |
| killall -9 bitcoind | |
| rm -rf "${TESTDIR}" | |
| mkdir -p "${TESTDIR}/dash" | |
| bitcoind -regtest \ | |
| -datadir=${TESTDIR}/1 \ | |
| -port=12348 \ | |
| -server=0 \ | |
| -printtoconsole=0 & | |
| PID1=$! | |
| # Kill any remaining dashd to avoid port conflicts | |
| if command -v killall >/dev/null 2>&1; then | |
| killall -9 dashd 2>/dev/null || true | |
| fi | |
| # Make sure it's listening on its p2p port. | |
| sleep 3 | |
| # Start Dash Core on regtest using standard Dash RPC port 19898 | |
| dashd -regtest \ | |
| -datadir="${TESTDIR}/dash" \ | |
| -rpcport=19898 \ | |
| -server=1 \ | |
| -txindex=1 \ | |
| -printtoconsole=0 & | |
| PID=$! | |
| BLOCKFILTERARG="" | |
| if bitcoind -version | grep -q "v0\.\(19\|2\)"; then | |
| BLOCKFILTERARG="-blockfilterindex=1" | |
| fi | |
| # Allow time for startup | |
| sleep 5 | |
| FALLBACKFEEARG="" | |
| if bitcoind -version | grep -q "v0\.2"; then | |
| FALLBACKFEEARG="-fallbackfee=0.00001000" | |
| fi | |
| # Pre-create faucet wallet "main" so the test can fund addresses | |
| dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -named createwallet wallet_name=main descriptors=false >/dev/null 2>&1 || true | |
| bitcoind -regtest $BLOCKFILTERARG $FALLBACKFEEARG \ | |
| -datadir=${TESTDIR}/2 \ | |
| -connect=127.0.0.1:12348 \ | |
| -rpcport=12349 \ | |
| -server=1 \ | |
| -txindex=1 \ | |
| -printtoconsole=0 & | |
| PID2=$! | |
| # Fund the faucet wallet with mature coins | |
| FAUCET_ADDR=$(dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 -rpcwallet=main getnewaddress) | |
| dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport=19898 generatetoaddress 110 "$FAUCET_ADDR" >/dev/null | |
| # Let it connect to the other node. | |
| sleep 5 | |
| # Export per-node env vars expected by the test (both point to same node) | |
| export WALLET_NODE_RPC_URL="http://127.0.0.1:19898" | |
| export EVO_NODE_RPC_URL="http://127.0.0.1:19898" | |
| export WALLET_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | |
| export EVO_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | |
| RPC_URL=http://localhost:12349 \ | |
| RPC_COOKIE=${TESTDIR}/2/regtest/.cookie \ | |
| cargo run | |
| cargo run | |
| RESULT=$? | |
| kill -9 $PID1 $PID2 | |
| kill -9 $PID 2>/dev/null || true | |
| exit $RESULT | |
| #!/bin/sh | |
| set -eu | |
| TESTDIR=/tmp/rust_dashcore_rpc_test | |
| # Allow overriding the RPC port; default to Dash regtest typical port | |
| RPC_PORT="${RPC_PORT:-19898}" | |
| cleanup() { | |
| if [ -n "${PID:-}" ] && kill -0 "$PID" 2>/dev/null; then | |
| # Prefer graceful stop; fall back to TERM | |
| dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport="${RPC_PORT}" stop >/dev/null 2>&1 \ | |
| || kill -TERM "$PID" 2>/dev/null || true | |
| wait "$PID" 2>/dev/null || true | |
| fi | |
| } | |
| trap 'cleanup' EXIT INT TERM | |
| rm -rf "${TESTDIR}" | |
| mkdir -p "${TESTDIR}/dash" | |
| # Remove any stray dashd that uses our test datadir (avoid nuking user daemons) | |
| if command -v pgrep >/dev/null 2>&1; then | |
| PIDS="$(pgrep -f "dashd .* -datadir=${TESTDIR}/dash" || true)" | |
| if [ -n "$PIDS" ]; then kill -9 $PIDS 2>/dev/null || true; fi | |
| fi | |
| # Start Dash Core on regtest | |
| dashd -regtest \ | |
| -datadir="${TESTDIR}/dash" \ | |
| -rpcport="${RPC_PORT}" \ | |
| -server=1 \ | |
| -txindex=1 \ | |
| -printtoconsole=0 & | |
| PID=$! | |
| # Wait for RPC to come up | |
| rpc() { dash-cli -regtest -datadir="${TESTDIR}/dash" -rpcport="${RPC_PORT}" "$@"; } | |
| i=0 | |
| until rpc getblockchaininfo >/dev/null 2>&1; do | |
| i=$((i+1)) | |
| if [ "$i" -ge 120 ]; then | |
| echo "dashd not responding on 127.0.0.1:${RPC_PORT} after 60s" >&2 | |
| exit 1 | |
| fi | |
| sleep 0.5 | |
| done | |
| # Pre-create faucet wallet "main" so the test can fund addresses | |
| rpc -named createwallet wallet_name=main descriptors=false >/dev/null 2>&1 || true | |
| # Fund the faucet wallet with mature coins | |
| FAUCET_ADDR="$(rpc -rpcwallet=main getnewaddress)" | |
| rpc generatetoaddress 110 "$FAUCET_ADDR" >/dev/null | |
| # Export per-node env vars expected by the test (both point to same node) | |
| export WALLET_NODE_RPC_URL="http://127.0.0.1:${RPC_PORT}" | |
| export EVO_NODE_RPC_URL="http://127.0.0.1:${RPC_PORT}" | |
| export WALLET_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | |
| export EVO_NODE_RPC_COOKIE="${TESTDIR}/dash/regtest/.cookie" | |
| set +e | |
| cargo run | |
| RESULT=$? | |
| set -e | |
| exit $RESULT |
🤖 Prompt for AI Agents
In rpc-integration-test/run.sh around lines 3 to 46, the test harness currently
can leave dashd running on failures, uses a racy fixed sleep to wait for
readiness, and hard-codes RPC port 19898; change it to (1) set a TRAP-based
cleanup that kills the dashd PID on EXIT (and on INT/TERM) so dashd is always
cleaned up even on failures or set -e aborts, (2) replace the fixed sleep with a
readiness loop that polls dash-cli (or tests TCP socket) until RPC responds or a
short timeout elapses, failing if not ready, and (3) make the RPC port
configurable via an environment variable like RPC_PORT with a default (e.g.,
19898) and use that variable everywhere (dashd, dash-cli URLs, cookie paths) to
avoid hard-coded collisions; ensure kill commands tolerate missing PID and that
the script exits with the cargo run exit code after cleanup.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rpc-json/src/lib.rs (1)
67-101: Document API change and verify downstream compatibility
No occurrences of the legacy field names or JSON keys remain in this repo. Update any downstream crates to use the renamed fields and record in the changelog the minimum Dash Core version that emits these new JSON keys.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rpc-json/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
rpc-json/src/lib.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
rpc-json/src/lib.rs
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
⏰ 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). (19)
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: SPV Components Tests
- GitHub Check: Key Wallet Components Tests
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_script)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_outpoint_string)
Comment on lines
+86
to
+90
| pub connections_in: usize, | ||
| pub connections_out: usize, | ||
| pub connections_mn: usize, | ||
| pub connections_mn_in: usize, | ||
| pub connections_mn_out: usize, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve back-compat in GetNetworkInfoResult: add serde aliases + defaults to avoid deserialization breakage across Core versions
Nodes that still return the legacy keys (e.g., inboundconnections/outboundconnections/mnconnections/inboundmnconnections/outboundmnconnections) will fail to deserialize since these fields are required and not optional. Add aliases and defaults so both old and new schemas work.
Apply this diff:
pub connections: usize,
- pub connections_in: usize,
- pub connections_out: usize,
- pub connections_mn: usize,
- pub connections_mn_in: usize,
- pub connections_mn_out: usize,
+ #[serde(default, alias = "inboundconnections")]
+ pub connections_in: usize,
+ #[serde(default, alias = "outboundconnections")]
+ pub connections_out: usize,
+ #[serde(default, alias = "mnconnections")]
+ pub connections_mn: usize,
+ #[serde(default, alias = "inboundmnconnections")]
+ pub connections_mn_in: usize,
+ #[serde(default, alias = "outboundmnconnections")]
+ pub connections_mn_out: usize,Optionally, I can add a small serde test to verify both old and new JSON shapes deserialize correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub connections_in: usize, | |
| pub connections_out: usize, | |
| pub connections_mn: usize, | |
| pub connections_mn_in: usize, | |
| pub connections_mn_out: usize, | |
| pub connections: usize, | |
| #[serde(default, alias = "inboundconnections")] | |
| pub connections_in: usize, | |
| #[serde(default, alias = "outboundconnections")] | |
| pub connections_out: usize, | |
| #[serde(default, alias = "mnconnections")] | |
| pub connections_mn: usize, | |
| #[serde(default, alias = "inboundmnconnections")] | |
| pub connections_mn_in: usize, | |
| #[serde(default, alias = "outboundmnconnections")] | |
| pub connections_mn_out: usize, |
🤖 Prompt for AI Agents
In rpc-json/src/lib.rs around lines 86 to 90, the GetNetworkInfoResult struct
currently requires new field names and will fail to deserialize JSON from older
Core versions that use legacy keys; add serde aliases for each field mapping the
legacy keys to the new names and add serde(default) so missing legacy or new
keys default to 0. Concretely, annotate connections_in with #[serde(alias =
"inboundconnections", default)], connections_out with #[serde(alias =
"outboundconnections", default)], connections_mn with #[serde(alias =
"mnconnections", default)], connections_mn_in with #[serde(alias =
"inboundmnconnections", default)], and connections_mn_out with #[serde(alias =
"outboundmnconnections", default)] so both old and new JSON shapes deserialize
successfully.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rpc-json/Cargo.toml (1)
1-16: Declare minimum supported Rust version
No workspace-levelrust-versionis defined, so add it under[package]inrpc-json/Cargo.toml:[package] edition = "2024" +rust-version = "1.89"
🧹 Nitpick comments (1)
rpc-json/Cargo.toml (1)
29-29: core-block-hash-use-x11 feature exists and is plumbed correctly; no conflictingdashcorefeature usage detected. To make X11 opt-in, forward the feature:[dependencies] -dashcore = { path = "../dash", features=["std","secp-recovery","rand-std","signer","serde","core-block-hash-use-x11"], default-features = false } +dashcore = { path = "../dash", features=["std","secp-recovery","rand-std","signer","serde"], default-features = false } +[features] +core-block-hash-use-x11 = ["dashcore/core-block-hash-use-x11"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rpc-json/Cargo.toml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/Cargo.toml
📄 CodeRabbit inference engine (AGENTS.md)
MSRV is 1.89; set and respect rust-version = "1.89" in Cargo.toml
Files:
rpc-json/Cargo.toml
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
rpc-json/Cargo.toml
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
rpc-json/Cargo.toml
📚 Learning: 2025-08-21T04:45:50.436Z
Learnt from: QuantumExplorer
PR: dashpay/rust-dashcore#108
File: key-wallet/src/gap_limit.rs:291-295
Timestamp: 2025-08-21T04:45:50.436Z
Learning: The rust-dashcore project uses Rust 1.89 as evidenced by rust-version = "1.89" in dash-spv/Cargo.toml. Modern Rust features like Option::is_none_or (stabilized in 1.82) can be used safely.
Applied to files:
rpc-json/Cargo.toml
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/**/*.rs : All code must be clippy-clean: run `cargo clippy --all-targets --all-features -- -D warnings`
Applied to files:
rpc-json/Cargo.toml
⏰ 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). (13)
- GitHub Check: fuzz (hashes_sha512_256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: fuzz (dash_deserialize_witness)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (dash_outpoint_string)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
rpc-integration-test/src/main.rs (6)
123-136: Env-based RPC URL fallbacks look good; trim trailing slashes to prevent accidental//wallet/paths.Functionally fine. Minor hardening: normalize URLs once here.
let wallet_node_url = std::env::var("WALLET_NODE_RPC_URL") .ok() .filter(|s| !s.is_empty()) - .or_else(|| generic_url.clone()); + .or_else(|| generic_url.clone()) + .map(|u| u.trim_end_matches('/').to_string()); let evo_node_rpc_url = std::env::var("EVO_NODE_RPC_URL") .ok() .filter(|s| !s.is_empty()) - .or_else(|| generic_url.clone()); + .or_else(|| generic_url.clone()) + .map(|u| u.trim_end_matches('/').to_string());
139-191: Don’t constructAuth::UserPasswith an empty password; prefer falling back toRPC_COOKIEorAuth::None.Empty-pass basic auth generates noisy failures. Use user/pass only if both are present; otherwise try
RPC_COOKIE, thenNone. Mirror for EVO.- std::env::var("WALLET_NODE_RPC_USER") + std::env::var("WALLET_NODE_RPC_USER") .or_else(|_| std::env::var("RPC_USER")) .ok() .filter(|s| !s.is_empty()) - .map(|user| { - let pass = std::env::var("WALLET_NODE_RPC_PASS") - .or_else(|_| std::env::var("RPC_PASS")) - .unwrap_or_default(); - Auth::UserPass(user, pass) - }) + .and_then(|user| { + std::env::var("WALLET_NODE_RPC_PASS") + .ok() + .filter(|s| !s.is_empty()) + .or_else(|| std::env::var("RPC_PASS").ok().filter(|s| !s.is_empty())) + .map(|pass| Auth::UserPass(user, pass)) + }) .unwrap_or_else(|| { // Generic cookie as last resort std::env::var("RPC_COOKIE") .ok() .filter(|s| !s.is_empty()) .map(|cookie| Auth::CookieFile(cookie.into())) .unwrap_or(Auth::None) }) }); @@ - std::env::var("EVO_NODE_RPC_USER") + std::env::var("EVO_NODE_RPC_USER") .or_else(|_| std::env::var("RPC_USER")) .ok() .filter(|s| !s.is_empty()) - .map(|user| { - let pass = std::env::var("EVO_NODE_RPC_PASS") - .or_else(|_| std::env::var("RPC_PASS")) - .unwrap_or_default(); - Auth::UserPass(user, pass) - }) + .and_then(|user| { + std::env::var("EVO_NODE_RPC_PASS") + .ok() + .filter(|s| !s.is_empty()) + .or_else(|| std::env::var("RPC_PASS").ok().filter(|s| !s.is_empty())) + .map(|pass| Auth::UserPass(user, pass)) + }) .unwrap_or_else(|| { std::env::var("RPC_COOKIE") .ok() .filter(|s| !s.is_empty()) .map(|cookie| Auth::CookieFile(cookie.into())) .unwrap_or(Auth::None) })
223-229: Remove redundant clones and the temporaryevo_rpc_urlbinding.Minor cleanup; avoids an extra
clone()and an unnecessary String copy.- // Evo/masternode RPCs are non-wallet; use base RPC URL - let evo_rpc_url = evo_node_rpc_url.clone(); - - let faucet_client = Client::new(&faucet_rpc_url, wallet_node_auth.clone().clone()).unwrap(); + // Evo/masternode RPCs are non-wallet; use base RPC URL + let faucet_client = Client::new(&faucet_rpc_url, wallet_node_auth.clone()).unwrap(); let wallet_client = Client::new(&wallet_rpc_url, wallet_node_auth).unwrap(); - let evo_client = Client::new(&evo_rpc_url, evo_node_auth).unwrap(); + let evo_client = Client::new(&evo_node_rpc_url, evo_node_auth).unwrap();
283-293: DRY the RUN_EVO_TESTS gate with a tiny helper.You compute the same flag twice. Centralize to reduce drift.
Diff within this block:
- let run_evo = std::env::var("RUN_EVO_TESTS") - .ok() - .map(|v| v.eq_ignore_ascii_case("1") || v.eq_ignore_ascii_case("true")) - .unwrap_or(false); - if run_evo { + if is_evo_enabled() { test_evo_node_endpoints(&evo_client, &wallet_client); } else { trace!(target: "integration_test", "Skipping evo/masternode RPC tests (set RUN_EVO_TESTS=true to enable)"); }Add this helper near the top (outside this hunk):
fn is_evo_enabled() -> bool { std::env::var("RUN_EVO_TESTS") .ok() .map(|v| v == "1" || v.eq_ignore_ascii_case("true")) .unwrap_or(false) }Want me to push a follow-up commit wiring this in both places?
317-325: Use the sameis_evo_enabled()helper here for consistency.Keeps the gating logic single-sourced.
- let run_evo = std::env::var("RUN_EVO_TESTS") - .ok() - .map(|v| v.eq_ignore_ascii_case("1") || v.eq_ignore_ascii_case("true")) - .unwrap_or(false); - if run_evo { + if is_evo_enabled() { test_get_best_chain_lock(wallet_client); }
783-787: Remove the unusedoptionsvariable (shadowed later).This triggers
unused_variablesand may fail CI if warnings are denied.- let options = json::ListUnspentQueryOptions { - minimum_amount: Some(btc(2)), - ..Default::default() - };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
rpc-integration-test/src/main.rs(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Use proper error types with thiserror and propagate errors appropriately
Use the tokio runtime for async operations in Rust
Use conditional compilation feature flags for optional features (#[cfg(feature = ...)])
Format Rust code with cargo fmt (and enforce via cargo fmt --check)
Run clippy with -D warnings and fix all lints
Adhere to MSRV Rust 1.89 (avoid features requiring newer compiler)
**/*.rs: Format Rust code with rustfmt (per rustfmt.toml); run cargo fmt --all before commits
Lint with clippy; treat warnings as errors in CI
Follow Rust naming: snake_case for functions/variables, UpperCamelCase for types/traits, SCREAMING_SNAKE_CASE for consts
Prefer async using tokio where applicable
Files:
rpc-integration-test/src/main.rs
**/src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.rs: Each crate keeps sources in src/
Avoid unwrap()/expect() in library code; use proper error types (e.g., thiserror)
Place unit tests alongside code with #[cfg(test)]
Files:
rpc-integration-test/src/main.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/**/*.rs : Place integration tests under the `tests/` directory
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Keep test vectors in sync with Dash Core releases
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:00:14.154Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-29T04:00:14.154Z
Learning: Applies to {dash-spv,rpc-integration-test}/tests/**/*.rs : Provide integration tests for network operations
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-09-01T14:44:53.683Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: AGENTS.md:0-0
Timestamp: 2025-09-01T14:44:53.683Z
Learning: Applies to **/tests/**/*.rs : Use descriptive test names (e.g., test_parse_address_mainnet) for integration tests
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/**/*.rs : Include integration tests that cover full wallet workflows
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/src/{bip32.rs,dip9.rs} : Keep Dash-specific derivation paths up to date (e.g., DIP-9) in bip32.rs and dip9.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-25T17:15:59.361Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-08-25T17:15:59.361Z
Learning: Applies to dash-spv/tests/integration_real_node_test.rs : Integration tests that connect to a live Dash Core node must gracefully skip when the node at 127.0.0.1:9999 is unavailable
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/address_tests.rs : Place address generation and validation tests in tests/address_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/bip32_tests.rs : Organize unit tests so key-derivation correctness lives in tests/bip32_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
📚 Learning: 2025-08-29T04:01:18.111Z
Learnt from: CR
PR: dashpay/rust-dashcore#0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-08-29T04:01:18.111Z
Learning: Applies to key-wallet/tests/mnemonic_tests.rs : Place mnemonic generation and validation tests in tests/mnemonic_tests.rs
Applied to files:
rpc-integration-test/src/main.rs
⏰ 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). (16)
- GitHub Check: fuzz (hashes_sha1)
- GitHub Check: fuzz (hashes_sha256)
- GitHub Check: fuzz (hashes_sha512)
- GitHub Check: fuzz (hashes_cbor)
- GitHub Check: fuzz (dash_deserialize_block)
- GitHub Check: fuzz (dash_deserialize_address)
- GitHub Check: fuzz (dash_script_bytes_to_asm_fmt)
- GitHub Check: fuzz (hashes_json)
- GitHub Check: fuzz (dash_deserialize_amount)
- GitHub Check: fuzz (hashes_ripemd160)
- GitHub Check: fuzz (dash_outpoint_string)
- GitHub Check: fuzz (dash_deser_net_msg)
- GitHub Check: Strict Warnings and Clippy Checks
- GitHub Check: RPC Tests (stable, true)
- GitHub Check: Core Components Tests
- GitHub Check: SPV Components Tests
🔇 Additional comments (1)
rpc-integration-test/src/main.rs (1)
43-46: Clarify regtest RPC port comment-// Dash regtest default RPC port is 19898. For mainnet/testnet use 9998/19998. +// In this integration harness we default to RPC port 19898 (see run.sh). +// Dash Core defaults remain 9998 (mainnet) and 19998 (testnet).
- Start only dashd on regtest (19898); remove bitcoind usage - Pre-create faucet wallet and mine 110 blocks for mature funds - Default RPC URLs to Dash regtest port; honor RPC_URL/RPC_COOKIE and per-node vars - Use base RPC URL for evo client (no wallet path) - Gate evo/masternode and ChainLock tests behind RUN_EVO_TESTS Ensures CI integration tests connect to Dash ports and pass reliably.
Use connections_in/out and connections_mn[_in/_out] directly to match RPC output and remove backward-compat renames/aliases to keep it simple for CI target.
Enable dash so BlockHash and header.block_hash() match Dash RPC hits command 1 /usr/bin/git fields; fixes header/hash assertions in integration tests.
Mine 6 blocks before selecting a UTXO with minconf=6 to ensure there is a confirmed, spendable input (>=2 DASH) after prior mempool activity. Prevents None unwrap when the wallet only has unconfirmed outputs or immature coinbases.
Send 3 DASH to a fresh wallet address and mine 6 blocks, then select that UTXO (minconf=6, min amount=2). Avoids reliance on earlier test ordering and coinbase maturity, eliminating intermittent empty listunspent results.
Attempt funding with 3/1/0.5 DASH before mining and selecting the UTXO, so the test does not fail on temporary low balance; adjusts minimum_amount to match the funded output and filters by address for deterministic selection.
PastaPastaPasta
force-pushed
the
fix/integration-tests
branch
from
bf85872 to
4a66bc5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Chores
Breaking Changes