test(sdk): generate test vectors using testnet #2381
Conversation
WalkthroughThe pull request introduces enhancements to the Dash Platform Rust SDK documentation and scripts. Key changes include updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Learnings (1)packages/rs-sdk/scripts/connect_to_remote.sh (1)🔇 Additional comments (2)packages/rs-sdk/scripts/connect_to_remote.sh (1)
The packages/rs-sdk/tests/fetch/config.rs (1)
The addition of the optional Also applies to: 188-191 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: 5
🧹 Outside diff range and nitpick comments (5)
packages/rs-sdk/README.md (1)
80-84: Consider adding more context about the helper script.The documentation clearly explains the new remote connection functionality. Consider adding:
- The full path to the helper script for easier discovery
- A note mentioning this is particularly useful for testnet vector generation, aligning with the PR's main objective
-To generate test vectors against a testnet node (or other remote node), you can use helper script -`packages/rs-sdk/scripts/connect_to_remote.sh` which will generate `.env` file for you and tunnel connection to Dash -Core RPC on the remote host. +To generate test vectors against a testnet node (or other remote node), you can use the helper script +`packages/rs-sdk/scripts/connect_to_remote.sh`. This script is particularly useful for testnet vector generation as it will: +- Generate the `.env` file with appropriate configuration +- Establish a secure tunnel to access the remote Dash Core RPCpackages/rs-sdk/tests/fetch/config.rs (2)
39-43: LGTM! Consider enhancing documentation.The new
core_hostfield is well-structured and properly integrated. The implementation correctly handles optional values and feature flags.Consider adding an example value in the documentation to make it clearer for users:
/// Host of the Dash Core RPC interface running on the Dash Platform node. /// Defaults to the same as [platform_host](Config::platform_host). +/// Example: "127.0.0.1" or "testnet.example.com"
188-191: Consider enhancing robustness and idiomaticity.While the implementation is functionally correct, consider these improvements:
- Use
as_deref()for more idiomatic Option handling- Add basic validation for the host value
- let core_host = self.core_host.as_ref().unwrap_or(&self.platform_host); + let core_host = self.core_host.as_deref().unwrap_or(&self.platform_host); + // Basic validation to ensure host is not empty and has valid format + if core_host.trim().is_empty() { + panic!("core_host cannot be empty"); + } // Dump all traffic to disk let builder = dash_sdk::SdkBuilder::new(self.address_list()).with_core(packages/rs-sdk/scripts/connect_to_remote.sh (2)
1-6: Add shellcheck directive and consider making CORE_USER configurable.Good error handling setup, but a few improvements could make the script more robust:
#! /bin/bash +# shellcheck disable=SC2029 set -e -o pipefail -CORE_USER=dashmate +CORE_USER="${DASH_CORE_USER:-dashmate}"The shellcheck directive is needed because we're intentionally allowing command expansion on the remote side for SSH commands.
50-66: Add safety measures for .env file generation.Consider adding backup functionality and configurable ports:
+LOCAL_PORT="${DASH_LOCAL_PORT:-12367}" + ENV_FILE="$(realpath "$0" | xargs dirname)/../tests/.env" + +# Backup existing .env file +if [ -f "${ENV_FILE}" ]; then + mv "${ENV_FILE}" "${ENV_FILE}.backup" + echo "Backed up existing .env file to ${ENV_FILE}.backup" +fi + echo -n Generating "{$ENV_FILE}" file for the SDK tests... cat >"${ENV_FILE}" <<EOF # Configuration of tests and examples, generated by $(realpath "$0") DASH_SDK_PLATFORM_HOST="${REMOTE_NODE}" DASH_SDK_PLATFORM_PORT=${DAPI_PORT} DASH_SDK_PLATFORM_SSL=${DAPI_SSL_ENABLED} DASH_SDK_CORE_HOST=127.0.0.1 -DASH_SDK_CORE_PORT=12367 +DASH_SDK_CORE_PORT=${LOCAL_PORT} DASH_SDK_CORE_USER="${CORE_USER}" DASH_SDK_CORE_PASSWORD="${CORE_PASSWORD}" EOF
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/rs-sdk/README.md(1 hunks)packages/rs-sdk/scripts/connect_to_remote.sh(1 hunks)packages/rs-sdk/scripts/generate_test_vectors.sh(1 hunks)packages/rs-sdk/tests/.env.example(1 hunks)packages/rs-sdk/tests/fetch/config.rs(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/rs-sdk/tests/.env.example
- packages/rs-sdk/scripts/generate_test_vectors.sh
🔇 Additional comments (3)
packages/rs-sdk/README.md (1)
77-79: LGTM! Clear and accurate documentation.
The explanation of the test vector generation script's functionality is well-written and provides the necessary information for users.
packages/rs-sdk/tests/fetch/config.rs (1)
188-194: Verify integration with test vector generation.
The implementation looks correct, but let's verify the integration with test vector generation functionality.
✅ Verification successful
Let me gather more information about the test vector generation implementation to ensure proper integration.
Integration with test vector generation is properly implemented
The implementation correctly integrates with test vector generation:
- Test vectors are generated in
tests/vectorsdirectory using thegenerate-test-vectorsfeature - The
core_hostconfiguration is properly used in the SDK builder, defaulting toplatform_hostwhen not specified - Test vector generation can be configured to use different core hosts (local/testnet) via environment variables
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test vector generation configuration and integration
# Check if test vector generation is properly configured
echo "Checking test vector generation configuration..."
# Look for test vector generation related files
fd -t f "generate.*test.*vectors" -x echo "Found test vector file: {}"
# Check for environment variable usage in test files
echo "Checking for DASH_SDK_CORE_HOST usage..."
rg "DASH_SDK_CORE_HOST" -A 3
# Check for test vector output paths
echo "Checking test vector output paths..."
fd -t d "vectors" -x echo "Found vectors directory: {}"
Length of output: 880
Script:
#!/bin/bash
# Check the test vector generation script content
echo "Checking test vector generation script..."
cat ./packages/rs-sdk/scripts/generate_test_vectors.sh
# Check test vector related test files
echo -e "\nChecking test files using test vectors..."
rg -l "vectors" packages/rs-sdk/tests/
# Check if core_host is properly used in test vector generation
echo -e "\nChecking core_host usage in tests..."
rg "core_host" packages/rs-sdk/tests/ -A 2
Length of output: 2137
packages/rs-sdk/scripts/connect_to_remote.sh (1)
7-22: LGTM! Well-documented usage function.
The usage instructions are clear, comprehensive, and include helpful examples.
Issue being fixed or feature implemented
We want to easily generate test vectors for Rust SDK using testnet. Unfortunately, it requires access to Dash Core RPC which only listens on 127.0.0.1 on the server.
What was done?
How Has This Been Tested?
Regenerate test vectors against testnet.
Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Documentation
Tests