Upgrade to Platform 2.0 #27
Conversation
WalkthroughThis update introduces broad dependency and build system upgrades across Android, Java, and Rust modules, including version bumps, migration to JReleaser for artifact publishing, and modernization of build scripts. It also refactors FFI and SDK initialization logic to support explicit platform versioning, adjusts test coverage, and adds new example and logging functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI/Gradle
participant JReleaser
participant MavenCentral
User->>CLI/Gradle: Initiate build and publish
CLI/Gradle->>JReleaser: Run JReleaser tasks (deploy, release)
JReleaser->>MavenCentral: Upload artifacts (staging/snapshot)
MavenCentral-->>JReleaser: Confirm upload/release
JReleaser-->>CLI/Gradle: Report status
CLI/Gradle-->>User: Build and publish complete
sequenceDiagram
participant App
participant SDK
participant PlatformVersion
App->>SDK: Initialize with network and options
SDK->>PlatformVersion: Select version (e.g., PLATFORM_V7/PLATFORM_V9)
SDK->>SDK: Setup API with version parameter
SDK-->>App: Ready for use
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 8
🔭 Outside diff range comments (1)
dpp/build.gradle (1)
158-198: Review JReleaser Sonatype endpoint and activation logicThe JReleaser URLs in your configuration are correct and up-to-date with the new Sonatype Central Publisher API (OSS RH EOL June 30, 2025). You’re already using:
https://central.sonatype.com/api/v1/publisherfor bundle uploadshttps://central.sonatype.com/repository/maven-snapshots/for snapshotsLegacy OSSRH endpoints (e.g. s01.oss.sonatype.org) are now deprecated and will no longer accept uploads.
Please address the following in dpp/build.gradle (lines 158–198):
• Signing scope
- Currently:
signing { active = 'ALWAYS' armored = true }- Suggestion: restrict signing to releases only, for example:
signing { active = project.version.endsWith('-SNAPSHOT') ? 'NEVER' : 'ALWAYS' armored = true }• Snapshot activation
- Ensure
nexus2.app.active = 'SNAPSHOT'aligns with your version detection (e.g.version.endsWith("-SNAPSHOT")).• Plugin defaults
- If you adopt the central-publishing-maven-plugin, you can drop manual URLs and rely on its default
centralBaseUrl = 'https://central.sonatype.com'.
🧹 Nitpick comments (5)
examples/src/main/kotlin/dashj/org/platform/DisplayIdentityKeys.kt (1)
27-29: Security consideration: Recovery phrase input is not masked.The recovery phrase is read as plain text from the console, making it visible to anyone who can see the terminal.
Consider using
System.console()for masked input:- println("Enter a recovery phrase: ") - val scanner = Scanner(System.`in`) - val phrase = scanner.nextLine() + val console = System.console() + val phrase = if (console != null) { + String(console.readPassword("Enter recovery phrase (hidden): ")) + } else { + println("Enter a recovery phrase: ") + Scanner(System.`in`).use { it.nextLine() } + }dash-sdk-java/src/test/java/org/dashj/platform/sdk/BaseTest.java (1)
29-32: Fix the naming inconsistency in the field name.The field name
walletUtilsContracIdis missing a 't' compared to other contract ID fields in the class (dpnsContractId,dashPayContractId,contractIdentifier).Apply this diff to maintain consistency:
- static byte[] walletUtilsContracId = new byte[]{ + static byte[] walletUtilsContractId = new byte[]{ 92, 20, 14, 101, 92, 2, 101, (byte)187, (byte)194, (byte)168, 8, 113, 109, (byte)225, (byte)132, 121, (byte)133, 19, 89, 24, (byte)173, 81, (byte)205, (byte)253, 11, 118, 102, 75, (byte)169, 91, (byte)163, 124 };README.md (1)
32-52: Improve formatting and clarify deployment instructions.The expanded build instructions are much more comprehensive, but there are some formatting and clarity issues:
- Step numbering is inconsistent (numbered list vs bullet points)
- Step 4 mentions "Stage all artifacts to a local directory" but doesn't clarify this is local staging for deployment
- The transition from step 4 to 5 could be clearer
Apply this diff to improve consistency and clarity:
-Build the android libraries to publish to Maven Central: -1) Verify release & deploy configuration +### Build the android libraries to publish to Maven Central: + +1. Verify release & deploy configuration ```bash ./gradlew jreleaserConfig-2) Ensure a clean deployment
+
+2. Ensure a clean deployment./gradlew clean-3) Generate all needed code for SWIG and Protobuf
+
+3. Generate all needed code for SWIG and Protobuf./gradlew generateSWIG && ./gradlew generateProto-4) Stage all artifacts to a local directory
+
+4. Stage all artifacts to local staging directory./gradlew publish-5) Deploy and release
+
+5. Deploy and release to Maven Central./gradlew jreleaserDeploy</blockquote></details> <details> <summary>dpp/src/test/kotlin/org/dashj/platform/dapiclient/DapiGrpcClientTest.kt (1)</summary><blockquote> `298-302`: **Clarify the purpose of the repeated transaction retrieval loop.** The loop calling `getTransactionBytesKotlin` 101 times appears to be a stress test or consistency check. Consider adding a comment explaining the purpose, or removing it if it was added for debugging. ```diff + // Stress test: verify consistent transaction retrieval across multiple calls for (i in 0..100) { val resultOfI: ByteArray? = client.getTransactionBytesKotlin(txid) println("$i. ${Utils.HEX.encode(resultOfI!!)}") }platform-mobile/src/config.rs (1)
275-290: LGTM: Platform version integration in setup_api.The addition of the
versionparameter and.with_version(version)calls on theSdkBuildercorrectly integrates platform versioning. Note that there are two.with_version(version)calls (lines 279 and 288) - while this is redundant, it shouldn't cause issues as the second call will overwrite the first.Consider removing the duplicate
.with_version(version)call:- let builder = dash_sdk::SdkBuilder::new(self.address_list()) - .with_version(version) - .with_core( + let builder = dash_sdk::SdkBuilder::new(self.address_list()) + .with_core( &self.core_ip, self.core_port, &self.core_user, &self.core_password, ); builder .with_version(version) .build().expect("cannot initialize api")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dash-sdk-android/src/main/rust/Cargo.lockis excluded by!**/*.lockdash-sdk-bindings/Cargo.lockis excluded by!**/*.lockplatform-mobile/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
README.md(1 hunks)build.gradle(1 hunks)dash-sdk-android/build.gradle(5 hunks)dash-sdk-android/src/main/AndroidManifest.xml(1 hunks)dash-sdk-android/src/main/rust/Cargo.toml(2 hunks)dash-sdk-android/src/main/rust/build.rs(1 hunks)dash-sdk-bindings/Cargo.toml(2 hunks)dash-sdk-bindings/build.rs(1 hunks)dash-sdk-bindings/rust-toolchain.toml(1 hunks)dash-sdk-java/build.gradle(3 hunks)dash-sdk-java/ignore.py(1 hunks)dash-sdk-java/src/main/cpp/context_provider.cpp(1 hunks)dash-sdk-java/src/main/cpp/conversions.h(1 hunks)dash-sdk-java/src/main/swig/platform_value.i(3 hunks)dash-sdk-java/src/main/swig/root.i(2 hunks)dash-sdk-java/src/test/java/org/dashj/platform/sdk/BaseTest.java(1 hunks)dash-sdk-java/src/test/java/org/dashj/platform/sdk/BinaryDataTest.java(0 hunks)dash-sdk-java/src/test/java/org/dashj/platform/sdk/DataContractTest.java(1 hunks)dash-sdk-java/src/test/java/org/dashj/platform/sdk/ValueTest.java(2 hunks)dpp/build.gradle(2 hunks)dpp/src/main/java/org/dashj/platform/dashpay/Profiles.kt(2 hunks)dpp/src/main/java/org/dashj/platform/dpp/util/PlatformValueFunctions.kt(2 hunks)dpp/src/test/kotlin/org/dashj/platform/dapiclient/DapiGrpcClientTest.kt(1 hunks)examples/src/main/kotlin/dashj/org/platform/DisplayIdentityKeys.kt(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)platform-mobile/Cargo.toml(2 hunks)platform-mobile/src/config.rs(8 hunks)platform-mobile/src/core.rs(2 hunks)platform-mobile/src/custom.rs(4 hunks)platform-mobile/src/data_contracts.rs(5 hunks)platform-mobile/src/fetch_identity.rs(1 hunks)platform-mobile/src/lib.rs(0 hunks)platform-mobile/src/provider.rs(4 hunks)platform-mobile/src/put.rs(5 hunks)platform-mobile/src/put_test.rs(6 hunks)platform-mobile/src/sdk.rs(7 hunks)platform-mobile/src/tests/documents.rs(0 hunks)platform-mobile/src/tests/mod.rs(0 hunks)platform-mobile/src/voting.rs(1 hunks)
💤 Files with no reviewable changes (4)
- platform-mobile/src/lib.rs
- platform-mobile/src/tests/documents.rs
- dash-sdk-java/src/test/java/org/dashj/platform/sdk/BinaryDataTest.java
- platform-mobile/src/tests/mod.rs
🧰 Additional context used
🧬 Code Graph Analysis (4)
platform-mobile/src/fetch_identity.rs (1)
platform-mobile/src/sdk.rs (3)
create_dash_sdk(102-116)create_dash_sdk_using_core_mainnet(98-100)create_dash_sdk_using_core_testnet(94-96)
platform-mobile/src/put.rs (1)
dpp/src/main/java/org/dashj/platform/dpp/contract/DataContractCreateTransition.kt (1)
entropy(9-57)
dash-sdk-android/src/main/rust/build.rs (1)
dash-sdk-bindings/build.rs (1)
main(4-23)
platform-mobile/src/sdk.rs (2)
platform-mobile/src/config.rs (2)
address_list(223-252)new(136-157)platform-mobile/src/provider.rs (2)
new(76-98)new(200-205)
🔇 Additional comments (88)
gradle/wrapper/gradle-wrapper.properties (1)
3-3: ✅ Gradle 8.7 Compatibility VerifiedAll build scripts were scanned and no deprecated configurations were found.
• Seven build.gradle files checked (root, tools, dpp, dash-sdk-java, dashj-platform-android-simple, examples, dash-sdk-android)
• NocompileortestCompileusages detected
• Publishing is done via themaven-publishplugin (fully supported in Gradle 8.x)No further changes are needed before upgrading the wrapper to Gradle 8.7.
examples/src/main/kotlin/dashj/org/platform/DisplayIdentityKeys.kt (1)
33-33: No import needed for DefaultIdentity
Both DefaultIdentity.kt and DisplayIdentityKeys.kt declare
package dashj.org.platform, so DefaultIdentity is in the same package and doesn’t require an import.You can ignore the suggested import.
Likely an incorrect or invalid review comment.
dash-sdk-bindings/rust-toolchain.toml (1)
3-3: No Rust version reference in README.md; no update required.The verification script confirms that README.md contains no Rust version mentions, so there’s nothing to synchronize.
dash-sdk-android/src/main/AndroidManifest.xml (1)
2-2: Confirm package configuration in GradleVerified that
dash-sdk-android/build.gradledeclares the namespace:
- Line 9:
namespace "org.dashj.platform"Removal of the
packageattribute inAndroidManifest.xmlis safe.dash-sdk-java/src/main/cpp/conversions.h (1)
10-10: LGTM: Header path update is consistent with build system restructuring.The include path update from
target/dash_sdk_bindings.htotarget/include/dash_sdk_bindings.haligns with the Platform 2.0 upgrade and build system restructuring mentioned in the PR objectives.dash-sdk-java/src/main/cpp/context_provider.cpp (1)
4-4: LGTM: Header path update matches restructured build output.The include path update is consistent with the build system changes across the codebase and aligns with the similar change in
conversions.h.dash-sdk-java/ignore.py (1)
4-4: LGTM: Build script path update aligns with header restructuring.The header file path update in this Python build script is consistent with the restructured output directory and matches the changes in the C++ files.
platform-mobile/src/core.rs (2)
4-4: LGTM: Platform version import supports explicit versioning.The import of
LATEST_PLATFORM_VERSIONaligns with the Platform 2.0 upgrade strategy to introduce explicit platform versioning across SDK components.
23-23: LGTM: SDK setup now uses explicit platform versioning.Passing
LATEST_PLATFORM_VERSIONtocfg.setup_api()ensures the SDK is initialized with the correct platform version, which is essential for the Platform 2.0 upgrade.platform-mobile/src/fetch_identity.rs (1)
16-16: LGTM: Mainnet SDK creation function import prepares for network flexibility.Adding
create_dash_sdk_using_core_mainnetto the imports aligns with the Platform 2.0 upgrade and provides consistency across network-specific SDK creation functions, even though it's not used in this file yet.platform-mobile/src/voting.rs (1)
66-66: LGTM! Backward-compatible configuration enhancement.The addition of
state_transition_creation_options: Noneto thePutSettingsstruct follows the established pattern across the codebase and maintains backward compatibility by defaulting toNone. This enhances configurability for state transition creation as part of the Platform 2.0 upgrade.dash-sdk-java/src/main/swig/root.i (1)
5-5: LGTM! Necessary header path updates for build system refactoring.The header file path updates from
/target/dash_sdk_bindings.hto/target/include/dash_sdk_bindings.hare consistent between the C++ preprocessor directive and SWIG include directive. This aligns with the broader build system reorganization as part of the Platform 2.0 upgrade.Also applies to: 94-94
README.md (1)
55-55: LGTM! Version updated correctly for Platform 2.0.The version update from "1.7.5" to "2.0.0-SNAPSHOT" correctly reflects the Platform 2.0 upgrade mentioned in the PR objectives.
build.gradle (4)
2-2: LGTM! Version bump aligns with Platform 2.0 upgrade.The major version bump to 2.0.0-SNAPSHOT is appropriate for the platform upgrade mentioned in the PR objectives.
8-8: LGTM! Repository addition supports JReleaser plugin.The gradlePluginPortal() repository is correctly added to support the new JReleaser plugin dependency.
15-15: LGTM! JReleaser plugin supports modern artifact publishing.The JReleaser plugin addition at version 1.17.0 aligns with the migration to modern artifact publishing mentioned in the PR summary.
13-13: Confirm Android modules build and run under AGP 8.1.4We’ve located two Android modules that rely on the Gradle plugin:
- dash-sdk-android/build.gradle
- dashj-platform-android-simple/build.gradle
Please manually run a full build and smoke-test on each module (e.g.
./gradlew :dash-sdk-android:assembleDebugand:dashj-platform-android-simple:assembleDebug, plus any unit/instrumentation tests) to verify there are no breaking-change issues with AGP 8.1.4. If you encounter errors, update your module configurations (compileSdk, dependencies, AndroidX migrations, etc.) accordingly.dash-sdk-java/src/test/java/org/dashj/platform/sdk/DataContractTest.java (1)
25-36: LGTM! Test extends coverage for wallet utils data contract.The new test method follows the established pattern and correctly tests fetching the wallet utils data contract. The assertion for exactly 1 document type is appropriate for this contract type.
dash-sdk-android/src/main/rust/Cargo.toml (4)
3-3: LGTM! Version bump consistent with Platform 2.0 upgrade.The version change from 1.0.0 to 2.0.0 aligns with the major platform upgrade objectives.
25-25: LGTM! Improved stability with tagged dependency.Moving from branch "master" to tag "v0.39.6" for dashcore dependency improves build reproducibility and stability.
31-31: LGTM! Ferment dependency supports FFI refactoring.The ferment dependency addition at tag "v0.2.3" aligns with the FFI refactoring mentioned in the PR summary.
35-37: LGTM! Build dependency transition to ferment-sys with appropriate feature selection.The transition from ferment to ferment-sys with the "cbindgen_only" feature is appropriate for C binding generation. The commented alternatives provide good documentation of configuration options.
platform-mobile/src/put.rs (5)
411-411: LGTM! Consistent PutSettings field addition.The addition of
state_transition_creation_options: Noneto PutSettings maintains backward compatibility while enabling future extensibility for state transition creation options.
483-483: LGTM! Consistent field addition in topup_identity_sdk.The PutSettings field addition is consistent with the pattern established in other functions.
515-517: LGTM! Method call updates match evolved API signature.The changes to wrap entropy in
Some()and add an additionalNoneargument indicate theput_to_platformmethod signature has evolved to accept optional parameters.
610-610: LGTM! Consistent PutSettings field addition in put_document_sdk.The field addition maintains consistency with other functions in the file.
736-736: LGTM! Consistent PutSettings field addition in replace_document_sdk.The field addition maintains consistency with other functions in the file.
platform-mobile/src/put_test.rs (6)
29-29: LGTM! Import addition supports platform versioning.The addition of
LATEST_PLATFORM_VERSIONto the import supports the platform versioning integration in SDK setup.
198-198: LGTM! Test PutSettings updated for compatibility.The PutSettings field addition maintains test compatibility with the updated struct definition.
326-326: LGTM! Consistent PutSettings update in second test.The field addition ensures consistency across test functions.
215-217: LGTM! Method calls updated for API compatibility.The method call updates wrap entropy in
Some()and addNonearguments to match the evolved API signature.
251-253: LGTM! Consistent method call updates in second test.The method call changes maintain consistency with the API updates.
305-305: LGTM! SDK setup updated with platform versioning.The SDK setup call now correctly passes
LATEST_PLATFORM_VERSION, aligning with the platform versioning integration.dash-sdk-bindings/Cargo.toml (3)
3-3: LGTM! Version bump aligns with Platform 2.0 upgrade.The major version bump from 1.0.0 to 2.0.0 is appropriate for this platform upgrade.
7-8: Good practice: Moving to tagged dependencies for stability.The migration from branch-based dependencies to fixed tag versions (v0.2.3) improves build reproducibility and stability.
33-34: Build dependency configuration looks correct.The ferment-sys dependency with
cbindgen_onlyfeature is properly configured for the build process. The commented alternative without features provides flexibility if needed.platform-mobile/Cargo.toml (3)
3-3: LGTM! Consistent version bump across platform modules.The major version bump to 2.0.0 is consistent with the platform-wide upgrade.
7-8: Consistent dependency management across modules.The ferment dependencies are updated consistently with dash-sdk-bindings, ensuring compatibility across the platform.
48-51: dashcore-rpc consolidation safe—no compatibility issues detectedOur searches in
platform-mobileshow no code or docs importing or invoking thedashcore-rpccrate:• No
dashcore_rpc::ordashcore-rpcusages in Rust source files
• Only a single entry inplatform-mobile/Cargo.toml(lines 50–51)
• No mentions in Markdown or example codeSince neither crate is actually referenced in the codebase, pointing both
dashcoreanddashcore-rpcat[email protected]introduces no runtime or build‐time conflicts. You may leave the existing TODO to removedashcore-rpconce the core RPC implementation is in place.dash-sdk-java/src/test/java/org/dashj/platform/sdk/ValueTest.java (3)
143-143: Consistent with platform-wide removal of I128 support.The commenting out of the I128 test case aligns with the broader removal of 128-bit integer support across the platform.
181-181: I128 assertion removal is consistent.The removal of the I128 exception test is appropriate given the removal of I128 support from the platform.
1-198: Ignore outdated test removal concern.
BothcreatePlatformValueTestandcreatePlatformValueMapTestare still defined inValueTest.java; no PlatformValue tests were removed and coverage remains intact.Likely an incorrect or invalid review comment.
dpp/src/test/kotlin/org/dashj/platform/dapiclient/DapiGrpcClientTest.kt (1)
288-292: Good separation of concerns for debugging.Separating the response retrieval from byte extraction and adding response printing improves debugging capabilities.
dash-sdk-java/src/main/swig/platform_value.i (3)
28-29: Systematic removal of 128-bit integer constructors.The commenting out of U128 and I128 constructors is consistent with the platform-wide removal of 128-bit integer support.
116-122: Consistent removal of 128-bit integer getters.The removal of I128 and U128 getter methods aligns with the constructor removals and platform changes.
152-152: Complete removal of I128 support in SWIG interface.The commenting out of the I128 ignore directive completes the systematic removal of 128-bit integer support from the SWIG interface.
dash-sdk-android/src/main/rust/build.rs (2)
1-23: LGTM! Clean migration to ferment_sys with consistent API usage.The migration from
fermenttoferment_sysfollows the same pattern asdash-sdk-bindings/build.rs, using the fluent API approach which is cleaner than manualcbindgencommand invocation. The error handling with match/panic is appropriate for build scripts.
5-5: Confirm crate name constant aligns with Cargo.toml
- Cargo.toml declares
name = "dash-sdk-bindings".- Rust converts hyphens to underscores for the crate identifier, so
dash_sdk_bindingsis correct.- A search for
rs_sdk_bindingsreturned no matches.No changes required.
dpp/build.gradle (2)
8-8: LGTM! Consistent JReleaser plugin application.The addition of the JReleaser plugin aligns with the modernization effort described in the PR objectives.
152-156: LGTM! Simplified Maven repository configuration.The change to use a local staging directory (
build/staging-deploy) instead of remote repository URLs with credentials simplifies the publishing configuration and delegates deployment management to JReleaser.dash-sdk-bindings/build.rs (1)
1-23: LGTM! Consistent migration pattern with dash-sdk-android.This build script follows the exact same migration pattern as
dash-sdk-android/src/main/rust/build.rs, demonstrating good consistency across the codebase. The fluent API approach withferment_sysis cleaner and more maintainable than the previous manual approach.platform-mobile/src/provider.rs (4)
11-11: LGTM! Appropriate import addition for new functionality.The
TokenConfigurationimport supports the newget_token_configurationmethod.
21-21: LGTM! Platform version import for version-aware functionality.The
PlatformVersionimport supports the updatedget_data_contractmethod signature.
180-183: LGTM! Default implementation for new trait method.The
get_token_configurationmethod provides a sensible default implementation returningOk(None), which allows for graceful handling when token configurations are not available.
142-146: Double-check allget_data_contractsignatures after addingplatform_versionI see the impl for
ContextProvider::get_data_contractin
platform-mobile/src/provider.rsnow takes
(&self, data_contract_id: &Identifier, platform_version: &PlatformVersion),
but I wasn’t able to locate a localtrait ContextProviderdefinition (it’s likely imported from the upstreamdppcrate). Please verify:
- The trait signature in the
dppcrate has been updated to includeplatform_version: &PlatformVersion.- The FFI wrapper in
platform-mobile/src/sdk.rs(and any other callers ofget_data_contract) now passes the newplatform_versionargument.- Any additional
ContextProviderimplementations (if they exist) have been updated to match the new signature.Run a full build and test cycle to ensure no mismatches remain.
dash-sdk-android/build.gradle (5)
6-6: LGTM! Consistent JReleaser plugin application.The JReleaser plugin application follows the same pattern as other modules in this upgrade.
106-106: LGTM! Modern Gradle API usage for archive classifiers.The update to use
archiveClassifier.set()instead of the deprecated assignment approach follows current Gradle best practices.Also applies to: 121-121
134-135: LGTM! Consistent version management.Extracting the version from the root project ensures consistent versioning across all modules.
172-175: LGTM! Consistent staging directory approach.The local staging directory approach matches the pattern used in other modules and simplifies the publishing configuration.
176-215: LGTM! Android-optimized JReleaser configuration.The JReleaser configuration is well-structured with Android-specific optimizations:
applyMavenCentralRules = falseis correctly set for AAR files (line 206)- MIT license specification aligns with the pom configuration
- Staging directory references are consistent
The configuration properly handles both release and snapshot deployments.
platform-mobile/src/data_contracts.rs (6)
17-22: LGTM: SDK import expansion aligns with platform versioning updates.The expanded imports for
create_dash_sdk_using_core_mainnetandcreate_dash_sdk_using_single_evonodesupport the new network configuration testing added in this file.
50-57: LGTM: DataContract V1 variant handling implemented correctly.The implementation correctly mirrors the V0 variant handling pattern, accessing the same contract methods (
id(),owner_id(),document_types,version()) through the appropriate trait methods.
118-124: Enhanced debugging with structured logging.The addition of tracing logs for both the fetched
DataContractand its FFI representation will help with debugging data contract fetching issues.
151-152: Consistent logging approach in tests.The change from
tracing::info!toprintln!maintains consistency with the other test functions in this file.
154-200: Comprehensive test coverage for wallet utils contract.The new test functions provide good coverage for testing wallet utils contract fetching across different network configurations:
- Mainnet testing
- All testnet evonodes iteration
- All mainnet evonodes iteration with failure tracking
The test implementations properly handle different result cases and provide meaningful output.
221-221: Correct keyword conflict resolution.Using
rng.r#gen()instead ofrng.gen()properly resolves the Rust keyword conflict with thegenkeyword.dash-sdk-java/build.gradle (4)
9-9: LGTM: Explicit JReleaser plugin application.The explicit application of the
org.jreleaserplugin is correct and follows Gradle best practices for plugin configuration.
89-89: LGTM: Modern Gradle API usage.The change from
archiveClassifier = 'tests'toarchiveClassifier.set("tests")uses the modern Gradle Property API, which is the recommended approach for Gradle 5.0+.
136-141: LGTM: Simplified repository configuration for JReleaser.The change to use a local staging directory (
layout.buildDirectory.dir('staging-deploy')) simplifies the configuration and aligns with JReleaser's workflow where it handles the actual deployment to remote repositories.
143-183: Well-configured JReleaser deployment setup.The JReleaser configuration properly defines:
- Project metadata with correct license and author information
- Signing configuration set to always active with armored output
- Maven Central deployment via Sonatype with appropriate staging repository
- Nexus2 configuration for snapshot handling with proper URL and settings
The configuration follows JReleaser best practices for Maven Central deployment.
platform-mobile/src/custom.rs (7)
12-23: LGTM: Correct trait split implementation for OutPoint.The refactoring properly splits the original
FFIConversiontrait intoFFIConversionFromandFFIConversionTo, with theffi_from_constmethod moved toFFIConversionFromandffi_to_constmoved toFFIConversionTo. The logic and error handling remain unchanged.
28-28: LGTM: Updated to use ferment crate.The change from
ferment_interfaces::unbox_anytoferment::unbox_anyaligns with the dependency migration fromferment-interfacestoferment.
45-57: LGTM: Consistent trait split for InstantLock.The trait split implementation for
InstantLockfollows the same correct pattern asOutPoint, maintaining the existing logic while properly separating the conversion directions.
62-62: LGTM: Consistent ferment crate usage in Drop implementation.The update to use
ferment::unbox_anymaintains consistency with the dependency migration.
79-91: LGTM: Consistent trait split for Transaction.The
Transactionstruct's trait split implementation follows the same correct pattern, properly separatingFFIConversionFromandFFIConversionTowhile maintaining existing functionality.
96-96: LGTM: Updated Drop implementation for Transaction.The change to
ferment::unbox_anymaintains consistency with the dependency migration throughout the file.
106-111: LGTM: Updated commented code for consistency.Even the commented code has been updated to use the
ferment::prefix, maintaining consistency should this code be uncommented in the future.platform-mobile/src/sdk.rs (6)
8-12: LGTM: Updated imports for dependency migration and platform versioning.The import changes correctly reflect:
- Migration from
ferment_interfacestoferment- Addition of platform versioning imports (
PlatformVersion,PLATFORM_V7,PLATFORM_V9)
67-85: LGTM: Platform version parameter added to address list update function.The addition of the
versionparameter toupdate_sdk_with_address_listmaintains consistency with other SDK setup functions and properly propagates the version to the underlying setup call.
167-173: LGTM: Enhanced logging and proper version propagation.The addition of platform version and SDK version logging improves debugging capabilities. The version parameter is correctly passed to the setup functions.
212-224: LGTM: Platform version handling in single evonode setup.The platform version selection logic (V9 for testnet, V7 for mainnet) and proper propagation to setup functions is implemented correctly, though it differs from the other SDK creation method as noted above.
232-234: LGTM: Increased timeout values for better reliability.The increase from 5 to 10 seconds for both
connect_timeoutandtimeoutin the single evonode configuration should improve reliability when connecting to individual evonodes, which may have more variable response times than load-balanced endpoints.
149-156: Inconsistent platform version selection across SDK constructorsI noticed two different version‐selection patterns in
platform-mobile/src/sdk.rs:
create_dash_sdk_with_context(lines ~149–156):
Both testnet and mainnet unconditionally usePLATFORM_V9.
create_dash_sdk_using_single_evonode(lines ~212–216):
Testnet usesPLATFORM_V9but mainnet falls back toPLATFORM_V7.Please confirm the intended strategy:
- Should mainnet always use the same version as testnet (i.e.
PLATFORM_V9), or should it remain onPLATFORM_V7in some contexts?- If the goal is to keep both constructors aligned, should we update one of these methods so they follow identical logic?
Once clarified, we can adjust the code accordingly.
platform-mobile/src/config.rs (5)
21-25: LGTM: Updated imports for dependency migration and platform versioning.The import changes correctly reflect the migration from
ferment_interfacestofermentand the addition of platform versioning support withPlatformVersionandPLATFORM_V7imports.
295-312: LGTM: Consistent platform version integration in setup_api_list.The platform version parameter addition and
.with_version(version)call are correctly implemented, maintaining consistency with the other setup methods.
342-382: LGTM: Platform version integration in callback-based setup.The addition of the
versionparameter and proper integration with theSdkBuildervia.with_version(&version)(note the reference operator) is correctly implemented.
390-413: LGTM: Consistent platform version handling in list-based callback setup.The platform version parameter and
.with_version(version)integration are correctly implemented, maintaining consistency across all setup methods.
67-69: Verify mainnet address list reductionThe
MAINNET_ADDRESS_LISTsize changed from 128 to 92 entries. Please confirm that:
- The 36 removed addresses were intentionally deprecated or found unreliable
- The remaining addresses still deliver sufficient geographic and network redundancy
- Any related documentation (README, deployment guides, etc.) is updated to reflect the new list size
Snippet for reference (platform-mobile/src/config.rs):
- pub const MAINNET_ADDRESS_LIST: [&str; 128] = [ … ]; + pub const MAINNET_ADDRESS_LIST: [&str; 92] = [ … ];
| dashcore = { git = "https://github.com/dashpay/rust-dashcore", features = [ | ||
| "std", | ||
| "secp-recovery", | ||
| "rand", | ||
| "signer", | ||
| "serde", | ||
| ], default-features = false, tag = "v0.39.6" } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify dashcore dependency features and compatibility.
The new dashcore dependency includes several features (std, secp-recovery, rand, signer, serde) with default features disabled. Ensure these features are necessary and compatible with the current platform version.
🏁 Script executed:
#!/bin/bash
# Verify dashcore dependency usage and feature requirements
echo "Searching for dashcore usage in the codebase..."
rg -A 3 -B 3 "dashcore::" --type rust
echo "Checking for secp-recovery feature usage..."
rg -A 2 -B 2 "secp.*recovery" --type rustLength of output: 11691
🏁 Script executed:
#!/bin/bash
# Verify dashcore feature usage specifically within the dash-sdk-bindings crate
echo "Scanning dash-sdk-bindings/src for direct dashcore imports..."
rg -A2 -B2 "dashcore::" dash-sdk-bindings/src --type rust || true
echo -e "\nChecking for secp-recovery (recovery API) usage..."
rg -A2 -B2 "recover" dash-sdk-bindings/src --type rust || true
echo -e "\nLooking for rand usage in dash-sdk-bindings..."
rg -A2 -B2 "rand::" dash-sdk-bindings/src --type rust || true
echo -e "\nLooking for serde usage in dash-sdk-bindings..."
rg -A2 -B2 "serde::" dash-sdk-bindings/src --type rust || trueLength of output: 719
Remove or Justify Unused dashcore Features in dash-sdk-bindings/Cargo.toml
We scanned dash-sdk-bindings/src and didn’t find any direct dashcore:: imports, nor usage of secp-recovery, rand, signer or serde APIs in that crate. Please trim these feature flags down to only what’s actually needed, or add comments explaining why each is required (e.g., pulled in transitively by dash-sdk). Also verify that the remaining features work correctly on your target platforms.
Files to update:
- dash-sdk-bindings/Cargo.toml (lines 24–30)
Suggested diff:
dashcore = {
git = "https://github.com/dashpay/rust-dashcore",
- features = [
- "std",
- "secp-recovery",
- "rand",
- "signer",
- "serde",
- ],
+ // enable only the features actually used by this crate
+ features = [
+ // e.g. "std",
+ ],
default-features = false,
tag = "v0.39.6"
}📝 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.
| dashcore = { git = "https://github.com/dashpay/rust-dashcore", features = [ | |
| "std", | |
| "secp-recovery", | |
| "rand", | |
| "signer", | |
| "serde", | |
| ], default-features = false, tag = "v0.39.6" } | |
| dashcore = { | |
| git = "https://github.com/dashpay/rust-dashcore", | |
| // enable only the features actually used by this crate | |
| features = [ | |
| // e.g. "std", | |
| ], | |
| default-features = false, | |
| tag = "v0.39.6" | |
| } |
🤖 Prompt for AI Agents
In dash-sdk-bindings/Cargo.toml between lines 24 and 30, the dashcore dependency
includes multiple feature flags that appear unused in the dash-sdk-bindings/src
code. Review the codebase to confirm which dashcore features are actually
required, then remove any unused features from the list to minimize unnecessary
dependencies. If some features are needed transitively or for platform
compatibility, add clear comments next to each feature flag explaining their
purpose. Finally, test the crate on target platforms to ensure the trimmed
feature set works correctly.
| //CTOR(U128, uint128_t) | ||
| //CTOR(I128, int128_t) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify complete removal of 128-bit integer references.
Ensure that all references to 128-bit integer types have been consistently removed or commented out across the codebase to prevent compilation issues.
Also applies to: 116-122, 152-152
🏁 Script executed:
#!/bin/bash
# Search for any remaining 128-bit integer references
echo "Searching for remaining I128/U128 references..."
rg -i "i128|u128" --type java --type cpp --type c
echo "Checking SWIG files for 128-bit integer references..."
fd -e i -x rg -H "i128|u128" {} \;
echo "Verifying no active BigInteger conversions for 128-bit types..."
rg -A 3 -B 3 "BigInteger.*128|128.*BigInteger"Length of output: 4429
Remove all remaining 128-bit integer references
Our search shows that 128-bit types and their Java/BigInteger mappings are still present in several places. Please remove or comment out every residual definition, SWIG mapping, and test reference to i128/u128:
• dash-sdk-java/src/main/cpp/int128.h
– typedefs for int128_t i128, __int128 i128, uint128_t u128, unsigned __int128 u128
• SWIG interface files:
– src/main/swig/i128.i – delete or fully comment out this file (all %typemap mappings)
– src/main/swig/ignore.i – remove lines
swig %ignore platform_value_Value_U128_ctor(u128 o_o_0); %ignore platform_value_Value_I128_ctor(i128 o_o_0);
– src/main/swig/platform_value.i – ensure the VALUE_GET(I128,…), VALUE_GET(U128,…), and VALUE_IGNORE entries are all removed or commented out
– src/main/swig/root.i – remove %include "i128.i" and %ignore i128;
• Unit tests:
– dash-sdk-java/src/test/java/org/dashj/platform/sdk/ValueTest.java – delete or comment out any lines referencing BigInteger i128Value, PlatformValue.Tag.I128/U128, and related assertions
Fix these locations so that no 128-bit integer types or their Java conversions remain.
🤖 Prompt for AI Agents
In dash-sdk-java/src/main/swig/platform_value.i around lines 28 to 29, remove or
comment out all references to 128-bit integer types, specifically the VALUE_GET
and VALUE_IGNORE entries related to I128 and U128. This is part of a broader
cleanup to eliminate all 128-bit integer definitions and mappings across the
codebase, including typedefs in int128.h, SWIG interface files like i128.i and
ignore.i, and unit tests referencing these types.
| signer: WalletSignerCallback | ||
| ): Document { | ||
| val profileDocument = createProfileDocument(displayName, publicMessage, avatarUrl, avatarHash, avatarFingerprint, identity) | ||
| log.info("creating profile document: {}", profileDocument.toJSON()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider PII implications of logging profile document JSON.
The profile document JSON may contain personally identifiable information like display names, public messages, and avatar URLs. Logging this at INFO level could expose sensitive data in production logs.
Consider one of these approaches:
- log.info("creating profile document: {}", profileDocument.toJSON())
+ log.debug("creating profile document: {}", profileDocument.toJSON())Or filter sensitive fields:
- log.info("creating profile document: {}", profileDocument.toJSON())
+ log.info("creating profile document with type: {}, revision: {}", profileDocument.type, profileDocument.revision)🤖 Prompt for AI Agents
In dpp/src/main/java/org/dashj/platform/dashpay/Profiles.kt at line 46, the
current logging statement outputs the entire profile document JSON, which may
contain sensitive personally identifiable information. To fix this, modify the
logging to either exclude or mask sensitive fields such as display names, public
messages, and avatar URLs before logging, or reduce the log level to DEBUG or
TRACE to avoid exposing this data in production logs.
| profileDocument.updatedAt = Date().time | ||
| profileDocument.revision += 1 | ||
|
|
||
| log.info("replacing profile document: {}", profileDocument.toJSON()) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider PII implications of logging profile document JSON.
Similar to the create method, this logs potentially sensitive profile data at INFO level.
Apply the same PII protection as suggested for the create method:
- log.info("replacing profile document: {}", profileDocument.toJSON())
+ log.debug("replacing profile document: {}", profileDocument.toJSON())Or use filtered logging:
- log.info("replacing profile document: {}", profileDocument.toJSON())
+ log.info("replacing profile document with type: {}, revision: {}", profileDocument.type, profileDocument.revision)📝 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.
| log.info("replacing profile document: {}", profileDocument.toJSON()) | |
| log.debug("replacing profile document: {}", profileDocument.toJSON()) |
| log.info("replacing profile document: {}", profileDocument.toJSON()) | |
| log.info( | |
| "replacing profile document with type: {}, revision: {}", | |
| profileDocument.type, | |
| profileDocument.revision | |
| ) |
🤖 Prompt for AI Agents
In dpp/src/main/java/org/dashj/platform/dashpay/Profiles.kt at line 103, the log
statement outputs the full profile document JSON at INFO level, which may expose
personally identifiable information (PII). Modify the logging to either redact
sensitive fields from the profileDocument before logging or implement a filtered
logging method that excludes PII, similar to the approach used in the create
method. Ensure the log message does not contain raw sensitive data to protect
user privacy.
| //PlatformValue.Tag.U128 -> value.u128 | ||
| //PlatformValue.Tag.I128 -> value.i128 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented code or add TODO explaining temporary removal.
The 128-bit integer handling code is commented out but not removed. If this is a permanent removal as part of disabling 128-bit support, the commented lines should be deleted completely to avoid maintenance overhead.
If the removal is permanent, apply this diff:
- //PlatformValue.Tag.U128 -> value.u128
- //PlatformValue.Tag.I128 -> value.i128If the removal is temporary, add a TODO comment explaining when it will be re-enabled:
- //PlatformValue.Tag.U128 -> value.u128
- //PlatformValue.Tag.I128 -> value.i128
+ // TODO: Re-enable 128-bit integer support when Platform X.Y is released
+ //PlatformValue.Tag.U128 -> value.u128
+ //PlatformValue.Tag.I128 -> value.i128📝 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.
| //PlatformValue.Tag.U128 -> value.u128 | |
| //PlatformValue.Tag.I128 -> value.i128 |
| //PlatformValue.Tag.U128 -> value.u128 | |
| //PlatformValue.Tag.I128 -> value.i128 | |
| // TODO: Re-enable 128-bit integer support when Platform X.Y is released | |
| //PlatformValue.Tag.U128 -> value.u128 | |
| //PlatformValue.Tag.I128 -> value.i128 |
🤖 Prompt for AI Agents
In dpp/src/main/java/org/dashj/platform/dpp/util/PlatformValueFunctions.kt
around lines 85 to 86, the 128-bit integer handling code is commented out
without explanation. If this removal is permanent, delete these commented lines
entirely to reduce maintenance overhead. If it is temporary, add a TODO comment
clarifying when or under what conditions this code will be re-enabled.
| is Int -> PlatformValue(value) | ||
| is Long -> PlatformValue(value) | ||
| is BigInteger -> PlatformValue(value) | ||
| //is BigInteger -> PlatformValue(value) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented BigInteger conversion or add TODO.
Similar to the 128-bit integer handling above, this commented BigInteger conversion should either be removed completely or documented with a TODO if temporary.
Apply the same approach as suggested for the 128-bit integer handling above.
🤖 Prompt for AI Agents
In dpp/src/main/java/org/dashj/platform/dpp/util/PlatformValueFunctions.kt at
line 151, there is a commented-out BigInteger conversion. To maintain code
clarity and consistency with the 128-bit integer handling, either remove this
commented line entirely if it is no longer needed, or add a clear TODO comment
explaining why it is commented out and what future action is required.
| val scanner = Scanner(System.`in`) | ||
| val phrase = scanner.nextLine() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve resource management and null safety.
Two issues: Scanner resource leak and unsafe null assertion on wallet.
- val scanner = Scanner(System.`in`)
- val phrase = scanner.nextLine()
+ Scanner(System.`in`).use { scanner ->
+ val phrase = scanner.nextLine()
+ // ... rest of logic inside use block
+ }And for the wallet access:
- val authGroup = client.wallet!!.getKeyChainExtension(AuthenticationGroupExtension.EXTENSION_ID) as AuthenticationGroupExtension
+ val wallet = client.wallet ?: run {
+ println("Error: Wallet not initialized")
+ return
+ }
+ val authGroup = wallet.getKeyChainExtension(AuthenticationGroupExtension.EXTENSION_ID) as AuthenticationGroupExtensionAlso applies to: 40-40
🤖 Prompt for AI Agents
In examples/src/main/kotlin/dashj/org/platform/DisplayIdentityKeys.kt at lines
28-29 and 40, fix the Scanner resource leak by using a Kotlin 'use' block to
automatically close the Scanner after reading input. For the wallet access,
replace unsafe null assertions with safe calls or proper null checks to avoid
potential NullPointerExceptions. This ensures better resource management and
null safety.
| val scanner = Scanner(System.`in`) | ||
| val phrase = scanner.nextLine() | ||
|
|
||
| MnemonicCode().check(phrase.split(" ", "\n")) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add exception handling for mnemonic validation.
MnemonicCode().check() can throw MnemonicException if the phrase is invalid, which will crash the application.
- MnemonicCode().check(phrase.split(" ", "\n"))
+ try {
+ MnemonicCode().check(phrase.split(" ", "\n"))
+ } catch (e: Exception) {
+ println("Invalid recovery phrase: ${e.message}")
+ return
+ }🤖 Prompt for AI Agents
In examples/src/main/kotlin/dashj/org/platform/DisplayIdentityKeys.kt at line
31, the call to MnemonicCode().check() can throw a MnemonicException if the
phrase is invalid, which currently is not handled and may crash the application.
Wrap this call in a try-catch block to catch MnemonicException and handle it
gracefully, such as logging the error or notifying the user, to prevent the
application from crashing.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Chores
Tests
Style