fix: update testnet nodes#37
Merged
HashEngineering merged 25 commits intomainfrom Apr 20, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBumped package versions, expanded and replaced testnet address lists and strengthened/renamed tests, updated CI to fetch external repos and install protoc/cbindgen/cargo-ndk plus Android Rust targets, and made small README and .gitignore edits. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (1)
platform-mobile/src/fetch_document.rs (1)
450-480: Avoid duplicating the testnet node source of truth.This list mirrors
platform-mobile/src/config.rsand will be easy to desynchronize. Prefer reusingTESTNET_ADDRESS_LISTdirectly in the test loop.♻️ Proposed refactor
-use crate::config::{Config, EntryPoint}; +use crate::config::{Config, EntryPoint, TESTNET_ADDRESS_LIST}; @@ -pub const ADDRESS_LIST: [&str; 29] = [ - "68.67.122.1", - ... - "68.67.122.29" -]; @@ - for address in ADDRESS_LIST { + for address in TESTNET_ADDRESS_LIST {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platform-mobile/src/fetch_document.rs` around lines 450 - 480, The ADDRESS_LIST constant duplicates the testnet node list from config.rs; remove ADDRESS_LIST and update the test loop(s) in fetch_document.rs to reference the single source of truth TESTNET_ADDRESS_LIST (e.g., import or use config::TESTNET_ADDRESS_LIST or crate::config::TESTNET_ADDRESS_LIST as appropriate) so tests iterate over that constant instead of the duplicated array; ensure any local references to ADDRESS_LIST are replaced and imports adjusted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@platform-mobile/src/fetch_document.rs`:
- Around line 450-480: The ADDRESS_LIST constant duplicates the testnet node
list from config.rs; remove ADDRESS_LIST and update the test loop(s) in
fetch_document.rs to reference the single source of truth TESTNET_ADDRESS_LIST
(e.g., import or use config::TESTNET_ADDRESS_LIST or
crate::config::TESTNET_ADDRESS_LIST as appropriate) so tests iterate over that
constant instead of the duplicated array; ensure any local references to
ADDRESS_LIST are replaced and imports adjusted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34d565b6-0bbe-4531-a1e7-eed40ff955f6
⛔ Files ignored due to path filters (2)
dash-sdk-bindings/Cargo.lockis excluded by!**/*.lockplatform-mobile/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
README.mdbuild.gradledash-sdk-android/src/main/rust/Cargo.tomldash-sdk-bindings/Cargo.tomlplatform-mobile/Cargo.tomlplatform-mobile/src/config.rsplatform-mobile/src/fetch_document.rs
abaranouski
reviewed
Apr 16, 2026
| @@ -1,5 +1,5 @@ | |||
| buildscript { | |||
| ext.version = '2.0.4' | |||
| ext.version = '2.0.5-SNAPSHOT' | |||
There was a problem hiding this comment.
@HashEngineering, do we really use the SNAPSHOT version?
Collaborator
Author
There was a problem hiding this comment.
yes, until 2.0.5 is finalized.
abaranouski
reviewed
Apr 16, 2026
| ### Use in other projects | ||
| ```groovy | ||
| dppVersion = "2.0.4" | ||
| dppVersion = "2.0.5-SNAPSHOT" |
Collaborator
Author
There was a problem hiding this comment.
yes, until 2.0.5 is finalized.
abaranouski
approved these changes
Apr 16, 2026
|
Though the build failed... |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/build-and-verify-alignment.yml (1)
59-60: Pincbindgenandcargo-ndkversions and cache the binaries.
cargo install cbindgenandcargo install cargo-ndkwithout--versioninstall whatever is latest on crates.io at build time, harming reproducibility and risking unexpected breakage when new versions ship. These installs also recompile from source on every run, adding significant overhead.Recommendations:
- Pin versions:
cargo install cbindgen --version 0.29.2 --lockedandcargo install cargo-ndk --version 4.1.2 --locked(update versions as needed).- Cache binaries using
Swatinem/rust-cache@v2,taiki-e/cache-cargo-install-action, orbaptiste0928/cargo-install.- Optionally group the two Rust tool installs together before
generate the header filefor clarity (sincecargo-ndkis only needed by./gradlew build).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-verify-alignment.yml around lines 59 - 60, Pin the cbindgen and cargo-ndk installs and add caching: replace the unpinned "cargo install cbindgen" step by installing with explicit versions (e.g., cbindgen --version 0.29.2 --locked and cargo-ndk --version 4.1.2 --locked) and use a Rust/cargo-install cache action (such as Swatinem/rust-cache@v2, taiki-e/cache-cargo-install-action, or baptiste0928/cargo-install) to cache the installed binaries, and optionally group the two cargo install commands together before the "generate the header file" step so cargo-ndk is available for ./gradlew build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-verify-alignment.yml:
- Around line 44-46: The CI currently clones
https://github.com/hashengineering/platform.git on branch ferment-2.0 which is a
personal fork and not pinned; change the workflow step that runs the git clone
so it either clones the upstream dashpay/platform (replace the remote URL and
branch name in the clone step) or, if you must use that branch, make the clone
immutable by cloning then checking out a specific commit SHA (run git clone ...
then git -C ../platform checkout <sha>) and update the workflow to use that SHA
instead of the branch name to ensure reproducible, non-force-pushable builds;
update any references to the branch/remote in the workflow step that contains
the git clone command.
- Around line 50-57: The protoc download step currently uses curl to fetch the
zip without checksum verification and unzips directly into $HOME/.local which
can silently overwrite; update the job to download the corresponding .sha256 (or
.sha256sum) for PROTOC_VERSION, compute and verify the SHA256 of the downloaded
protoc-${PROTOC_VERSION}-linux-x86_64.zip before unzipping, and change the
extraction to avoid overwriting (e.g., extract to a temporary directory and move
binaries or use a non-overwrite flag). Specifically modify the commands that
reference PROTOC_VERSION, the curl download of protoc, and the unzip step so
they perform checksum verification and safe extraction.
---
Nitpick comments:
In @.github/workflows/build-and-verify-alignment.yml:
- Around line 59-60: Pin the cbindgen and cargo-ndk installs and add caching:
replace the unpinned "cargo install cbindgen" step by installing with explicit
versions (e.g., cbindgen --version 0.29.2 --locked and cargo-ndk --version 4.1.2
--locked) and use a Rust/cargo-install cache action (such as
Swatinem/rust-cache@v2, taiki-e/cache-cargo-install-action, or
baptiste0928/cargo-install) to cache the installed binaries, and optionally
group the two cargo install commands together before the "generate the header
file" step so cargo-ndk is available for ./gradlew build.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e86eec9-1b88-4e05-b3ec-af80121d7002
⛔ Files ignored due to path filters (1)
dash-sdk-android/src/main/rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/build-and-verify-alignment.ymldash-sdk-android/.gitignoredash-sdk-android/src/main/rust/src/fermented.rsdash-sdk-bindings/.gitignoredash-sdk-bindings/README.mddash-sdk-bindings/src/fermented.rs
💤 Files with no reviewable changes (1)
- dash-sdk-android/.gitignore
✅ Files skipped from review due to trivial changes (2)
- dash-sdk-bindings/.gitignore
- dash-sdk-bindings/README.md
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/build-and-verify-alignment.yml (1)
74-75: Pincbindgenversion for reproducible CI.
cargo install cbindgenalways pulls the latest version from crates.io, so a new upstream release can silently change generated bindings and break the build. Pin it the same waycargo-ndkis pinned below.♻️ Suggested change
- - name: Install cbindgen - run: cargo install cbindgen + - name: Install cbindgen + run: cargo install cbindgen --version "^0.28" # pick the version you've validated🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build-and-verify-alignment.yml around lines 74 - 75, Replace the unpinned `cargo install cbindgen` invocation with a pinned install so CI is reproducible: change it to the same style used for `cargo-ndk` (e.g., `cargo install cbindgen --version "<SPECIFIC_VERSION>"`) and pick the concrete version your repo currently expects; ensure the workflow uses that explicit version string instead of the floating latest.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build-and-verify-alignment.yml:
- Around line 89-93: The workflow step "Install Cargo NDK and Rust targets"
fails if the nightly toolchain isn't present; before running rustup component
add --toolchain nightly and rustup target add ... --toolchain nightly,
explicitly install the nightly toolchain by running rustup toolchain install
nightly so subsequent calls (rustup component add and rustup target add)
succeed.
- Around line 98-99: The workflow currently skips tests by using "./gradlew
build -x test" in the "Build with Gradle" step; change this so CI actually runs
unit tests by removing the "-x test" flag (use "./gradlew build") or add an
additional explicit step named "Run tests" that executes "./gradlew test" after
the build step; ensure the new/modified step uses the same Gradle wrapper
invocation and fails the job on test failures so
platform-mobile/src/fetch_document.rs tests run in CI.
- Around line 48-49: The clone step named "clone rs-x11-hash repo" currently
clones into the Kotlin repo root and uses a mutable branch; change it to clone
the repo into the parent directory expected by the build (destination
../rs-x11-hash) and pin to a specific commit by checking out <pinned-commit-sha>
after cloning (replace <pinned-commit-sha> with the actual commit SHA) so the
path dependency ../rs-x11-hash resolves correctly and builds are reproducible.
---
Nitpick comments:
In @.github/workflows/build-and-verify-alignment.yml:
- Around line 74-75: Replace the unpinned `cargo install cbindgen` invocation
with a pinned install so CI is reproducible: change it to the same style used
for `cargo-ndk` (e.g., `cargo install cbindgen --version "<SPECIFIC_VERSION>"`)
and pick the concrete version your repo currently expects; ensure the workflow
uses that explicit version string instead of the floating latest.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d716eea-2bfc-40e5-a27a-8522f5ac72d4
📒 Files selected for processing (1)
.github/workflows/build-and-verify-alignment.yml
abaranouski
approved these changes
Apr 19, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Chores
Documentation
Tests
CI