feat: support platform 4#39
Conversation
📝 WalkthroughWalkthroughBumps crate and Gradle build versions to 4.0.0. Adds ChangesKotlin/Android DPP model and FFI updates
platform-mobile Rust SDK upgrade
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Actionable comments posted: 5
🧹 Nitpick comments (2)
examples/src/main/kotlin/dashj/org/platform/PlatformExplorer.kt (1)
104-104: Consider gating verbose logging behind an opt-in flag.At line 104,
BriefLogFormatter.initVerbose()is always enabled without user control. Sinceargs[0]is already parsed for the network parameter, consider adding an optional flag (e.g.,--verbose) to gate verbose logging, allowing users to opt-in to verbose output when needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/src/main/kotlin/dashj/org/platform/PlatformExplorer.kt` at line 104, The BriefLogFormatter.initVerbose() call at line 104 is unconditionally enabled without user control. Modify the command-line argument parsing logic to check for an optional --verbose flag (similar to how args[0] is already parsed for the network parameter), and only invoke BriefLogFormatter.initVerbose() when this flag is explicitly provided by the user, allowing verbose logging to be opt-in rather than always enabled.dash-sdk-java/src/main/swig/document.i (1)
14-26: ⚡ Quick winStale comment does not document the new parameter.
The comment documenting the
dpp_document_v0_DocumentV0_ctorsignature (lines 14-26) is now outdated. The actual constructor now takes an additional parameter (likelycreator_idbased on related changes), but this comment still shows only 13 parameters. Consider updating the comment to reflect the current signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dash-sdk-java/src/main/swig/document.i` around lines 14 - 26, The comment documenting the dpp_document_v0_DocumentV0_ctor function signature in the SWIG interface file is stale and does not include a newly added parameter. Update the commented function signature to match the actual current constructor signature by adding the missing parameter (likely creator_id based on related changes) in the appropriate position within the parameter list. Ensure the comment accurately reflects all parameters that the current constructor accepts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dpp/src/main/java/org/dashj/platform/wallet/TxMetadataItem.kt`:
- Line 109: The current code at line 109 checks if protoTxMetadata.index is not
equal to zero to detect unset optional fields, but this approach fails when zero
is a valid value for the index field. Replace the conditional check `if
(protoTxMetadata.index != 0)` with a call to the generated hasIndex() method on
protoTxMetadata, which properly detects whether the optional field was actually
set or left unset, correctly distinguishing between an unset field and an
explicitly set field with value zero.
In `@platform-mobile/src/fetch_document.rs`:
- Line 365: The hardcoded IP address "68.67.122.29" in the
create_dash_sdk_using_single_evonode function call makes the test flaky when
that node is unavailable or rotates. Replace the hardcoded IP string with a
value read from an environment variable (for example, something like
std::env::var) that has a sensible default fallback, allowing CI and local test
runs to override the endpoint when needed for better test stability.
In `@platform-mobile/src/put.rs`:
- Around line 244-246: The sign_create_witness method in the file contains a
todo!() placeholder that will cause a panic if invoked by the SDK dynamically
during identity operations. Replace the todo!() with a proper error return
statement that returns a Result type with an appropriate ProtocolError variant
instead of panicking. This ensures the method safely handles calls from the
SDK's internal logic like put_to_platform_with_private_key without crashing the
application.
In `@platform-mobile/src/sdk.rs`:
- Line 185: The max_decoding_message_size configuration is inconsistent between
different SDK factory outputs in the codebase. Locate the two occurrences of
max_decoding_message_size: None in platform-mobile/src/sdk.rs and replace both
instances with max_decoding_message_size: Some(16 * 1024 * 1024) to match the
consistent behavior defined in the callback-backed setup in
platform-mobile/src/config.rs, which uses the same value. This ensures uniform
request handling across all SDK factory creation paths and prevents
payload-size-dependent decode failures.
In `@README.md`:
- Line 55: The dppVersion variable in the README.md documentation is set to
"4.0.0-SNAPSHOT" but the project's actual version has been bumped to
"4.0.0-RC2-SNAPSHOT". Update the dppVersion value in the README to match the
current project version to prevent users from pulling mismatched artifacts.
Locate the dppVersion assignment and change it to reflect the correct bumped
version from the root build configuration.
---
Nitpick comments:
In `@dash-sdk-java/src/main/swig/document.i`:
- Around line 14-26: The comment documenting the dpp_document_v0_DocumentV0_ctor
function signature in the SWIG interface file is stale and does not include a
newly added parameter. Update the commented function signature to match the
actual current constructor signature by adding the missing parameter (likely
creator_id based on related changes) in the appropriate position within the
parameter list. Ensure the comment accurately reflects all parameters that the
current constructor accepts.
In `@examples/src/main/kotlin/dashj/org/platform/PlatformExplorer.kt`:
- Line 104: The BriefLogFormatter.initVerbose() call at line 104 is
unconditionally enabled without user control. Modify the command-line argument
parsing logic to check for an optional --verbose flag (similar to how args[0] is
already parsed for the network parameter), and only invoke
BriefLogFormatter.initVerbose() when this flag is explicitly provided by the
user, allowing verbose logging to be opt-in rather than always enabled.
🪄 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: 7d5844e7-1b5d-4c5c-812a-bcfc48e66b8f
⛔ 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 (22)
README.mdbuild.gradledash-sdk-android/src/main/rust/Cargo.tomldash-sdk-android/src/main/rust/src/fermented.rsdash-sdk-bindings/Cargo.tomldash-sdk-bindings/contested-resource-value-fqn.patchdash-sdk-bindings/src/fermented.rsdash-sdk-java/src/main/cpp/config.hdash-sdk-java/src/main/swig/document.idpp/src/main/java/org/dashj/platform/dashpay/BlockchainIdentity.ktdpp/src/main/java/org/dashj/platform/wallet/TxMetadataItem.ktdpp/src/main/proto/wallet-utils.protoexamples/src/main/kotlin/dashj/org/platform/PlatformExplorer.ktplatform-mobile/Cargo.tomlplatform-mobile/rust-toolchain.tomlplatform-mobile/src/config.rsplatform-mobile/src/fetch_document.rsplatform-mobile/src/provider.rsplatform-mobile/src/put.rsplatform-mobile/src/put_test.rsplatform-mobile/src/sdk.rsplatform-mobile/src/voting.rs
| protoTxMetadata.otherDataMap.ifEmpty { null }, | ||
| if (protoTxMetadata.order != "") protoTxMetadata.order else null, | ||
| if (protoTxMetadata.giftCardChallenge != "") protoTxMetadata.giftCardChallenge else null, | ||
| if (protoTxMetadata.index != 0) protoTxMetadata.index else null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the protobuf generated code has a hasIndex() method
fd -e java -i txmetadataitem | xargs grep -l "hasIndex" 2>/dev/null || echo "No hasIndex method found"
# Check how index field is used elsewhere to understand if 0 is valid
rg -n "\.index\s*=" --type kt -C2 | head -40Repository: dashpay/kotlin-platform
Length of output: 137
🏁 Script executed:
# Find the protobuf definition file
fd -type f -name "*.proto" | xargs grep -l "TxMetadataItem" 2>/dev/null | head -5Repository: dashpay/kotlin-platform
Length of output: 238
🏁 Script executed:
# Search for index field usage in Kotlin files (fixing the file type issue)
rg -n "\.index\s*=" -g "*.kt" -C 2 | head -50Repository: dashpay/kotlin-platform
Length of output: 1145
🏁 Script executed:
# Check the TxMetadataItem.kt file to understand the index property definition
cat -n dpp/src/main/java/org/dashj/platform/wallet/TxMetadataItem.kt | sed -n '50,120p'Repository: dashpay/kotlin-platform
Length of output: 3844
🏁 Script executed:
# Search for how index is being used in tests or other parts of the code
rg -n "index\s*=" -g "*.kt" -C 1 | grep -i "index\|txmetadata" | head -30Repository: dashpay/kotlin-platform
Length of output: 1108
🏁 Script executed:
# Find protobuf files
fd -type f -name "*.proto" -o -name "*.pb.kt"Repository: dashpay/kotlin-platform
Length of output: 238
🏁 Script executed:
# Look for WalletUtils.TxMetadataItem to understand the proto source
rg "WalletUtils.*TxMetadataItem" -g "*.kt" -A 3 | head -20Repository: dashpay/kotlin-platform
Length of output: 1621
🏁 Script executed:
# Check if there are any tests that might clarify valid index values
fd -type f -name "*Test*.kt" | xargs grep -l "index" 2>/dev/null | head -5Repository: dashpay/kotlin-platform
Length of output: 238
🏁 Script executed:
# Find all proto files without using -type
find . -name "*.proto" 2>/dev/null | head -10Repository: dashpay/kotlin-platform
Length of output: 174
🏁 Script executed:
# Look for proto files generated in Java
find . -path "*/generated*" -name "*.kt" | grep -i wallet | head -5Repository: dashpay/kotlin-platform
Length of output: 49
🏁 Script executed:
# Search for TxMetadataItem definition in proto files
find . -name "*.proto" -exec grep -l "TxMetadataItem" {} \;Repository: dashpay/kotlin-platform
Length of output: 107
🏁 Script executed:
# Examine test file for TxMetadataItem to see if 0 index is ever used
cat dpp/src/test/kotlin/org/dashj/platform/contracts/wallet/TxMetaDataTests.ktRepository: dashpay/kotlin-platform
Length of output: 8910
🏁 Script executed:
# Check the wallet-utils.proto definition for TxMetadataItem
cat dpp/src/main/proto/wallet-utils.proto | grep -A 50 "message TxMetadataItem"Repository: dashpay/kotlin-platform
Length of output: 814
Use hasIndex() to properly detect unset optional fields.
Protobuf's optional uint32 index generates a hasIndex() method. The current code at line 109 treats index = 0 as unset, but with proto3 optional semantics, 0 could be a valid value. Replace the default-value check with:
if (protoTxMetadata.hasIndex()) protoTxMetadata.index else nullThis correctly distinguishes between unset (hasIndex() = false) and explicitly set to 0 (hasIndex() = true, index = 0).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dpp/src/main/java/org/dashj/platform/wallet/TxMetadataItem.kt` at line 109,
The current code at line 109 checks if protoTxMetadata.index is not equal to
zero to detect unset optional fields, but this approach fails when zero is a
valid value for the index field. Replace the conditional check `if
(protoTxMetadata.index != 0)` with a call to the generated hasIndex() method on
protoTxMetadata, which properly detects whether the optional field was actually
set or left unset, correctly distinguishing between an unset field and an
explicitly set field with value zero.
| #[test] | ||
| fn docs_startswith_query_sdk_using_single_node_test() { | ||
| let mut sdk = create_dash_sdk_using_single_evonode("35.163.144.230".into(), 0, 0, true); | ||
| let mut sdk = create_dash_sdk_using_single_evonode("68.67.122.29".into(), 0, 0, true); |
There was a problem hiding this comment.
Avoid hardcoded single-node endpoints in integration tests
Line [365] hardcodes a live evonode IP, which can make this test flaky when that node rotates or is unavailable. Prefer reading the endpoint from an env var (with a default) so CI and local runs can control stability.
Suggested change
- let mut sdk = create_dash_sdk_using_single_evonode("68.67.122.29".into(), 0, 0, true);
+ let evonode = std::env::var("DASH_TEST_EVO_NODE")
+ .unwrap_or_else(|_| "68.67.122.29".to_string());
+ let mut sdk = create_dash_sdk_using_single_evonode(evonode, 0, 0, true);📝 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.
| let mut sdk = create_dash_sdk_using_single_evonode("68.67.122.29".into(), 0, 0, true); | |
| let evonode = std::env::var("DASH_TEST_EVO_NODE") | |
| .unwrap_or_else(|_| "68.67.122.29".to_string()); | |
| let mut sdk = create_dash_sdk_using_single_evonode(evonode, 0, 0, true); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-mobile/src/fetch_document.rs` at line 365, The hardcoded IP address
"68.67.122.29" in the create_dash_sdk_using_single_evonode function call makes
the test flaky when that node is unavailable or rotates. Replace the hardcoded
IP string with a value read from an environment variable (for example, something
like std::env::var) that has a sensible default fallback, allowing CI and local
test runs to override the endpoint when needed for better test stability.
| async fn sign_create_witness(&self, key: &IdentityPublicKey, data: &[u8]) -> Result<AddressWitness, ProtocolError> { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify potential reachability of sign_create_witness and signer wiring paths
rg -nP --type=rust -C3 'sign_create_witness\s*\('
rg -nP --type=rust -C5 'impl\s+Signer<IdentityPublicKey>\s+for\s+CallbackSigner' platform-mobile/src/put.rs
rg -nP --type=rust -C4 'put_to_platform_with_private_key|top_up_identity_with_private_key' platform-mobile/src/put.rsRepository: dashpay/kotlin-platform
Length of output: 1438
🏁 Script executed:
#!/bin/bash
# Find the Signer trait definition
rg -nP --type=rust 'trait\s+Signer' -A 20
# Find all usages/calls to sign_create_witness
rg -nP --type=rust 'sign_create_witness'
# Find other implementations of Signer
rg -nP --type=rust 'impl\s+(.*\s+)?Signer<IdentityPublicKey>' -A 5Repository: dashpay/kotlin-platform
Length of output: 631
🏁 Script executed:
#!/bin/bash
# Find imports of Signer trait
rg -nP --type=rust '^use\s+.*Signer' platform-mobile/src/put.rs
# Search more broadly for sign_create_witness calls
rg -nP --type=rust 'sign_create_witness' -A 2 -B 2
# Check the file header/imports
head -50 platform-mobile/src/put.rs | grep -E 'use |mod 'Repository: dashpay/kotlin-platform
Length of output: 2936
🏁 Script executed:
#!/bin/bash
# Find trait definition of Signer from dpp
rg -nP --type=rust 'trait Signer' -A 30
# Find SingleKeySigner implementation to compare
rg -nP --type=rust 'impl.*Signer.*SingleKeySigner' -A 30
# Search for any trait object usage with Signer
rg -nP --type=rust 'Box<dyn.*Signer|&dyn.*Signer|impl.*Signer'Repository: dashpay/kotlin-platform
Length of output: 1030
🏁 Script executed:
#!/bin/bash
# Get full CallbackSigner implementation
rg -nP --type=rust 'impl.*CallbackSigner|impl Signer.*CallbackSigner' -A 50
# Look at where CallbackSigner is created/used
rg -nP --type=rust 'CallbackSigner\s*\{|CallbackSigner::' -B 2 -A 5
# Check Cargo.toml for dpp version
grep -A 5 '^\[dependencies\]' Cargo.toml | head -20Repository: dashpay/kotlin-platform
Length of output: 9827
🏁 Script executed:
#!/bin/bash
# Find AddressWitness usage and creation
rg -nP --type=rust 'AddressWitness' -B 2 -A 2
# Search for witness-related operations in identity transitions
rg -nP --type=rust 'witness|Witness' -i -C 2 platform-mobile/src/put.rs | head -60Repository: dashpay/kotlin-platform
Length of output: 1029
🏁 Script executed:
#!/bin/bash
# Search for dynamic dispatch patterns or explicit trait method calls
rg -nP --type=rust '\.sign_create_witness|Signer.*sign_create_witness' -C 3
# Search for asset lock or witness in context of the operations
rg -nP --type=rust 'asset_lock|witness' -i -C 2 platform-mobile/src/put.rs | grep -A 5 -B 5 'put_to_platform\|top_up_identity'
# Check if there's any reference documentation or comments about this method
rg -nP --type=rust 'sign_create_witness' -B 5 | head -30Repository: dashpay/kotlin-platform
Length of output: 1887
🏁 Script executed:
#!/bin/bash
# Search for documentation or TODOs related to witness signing
rg -nP --type=rust '(TODO|FIXME|XXX|NOTE).*witness' -i
# Search for any test or documentation about CallbackSigner
rg -nP --type=rust 'CallbackSigner' -B 3 -A 3
# Check if there's any indication of whether witness signing is optional
rg -nP --type=rust 'optional|required|must|should.*witness' -i -C 2Repository: dashpay/kotlin-platform
Length of output: 22566
Replace todo!() with error return in sign_create_witness before merge.
The method at line 244 will panic if witness signing is requested during identity operations. Since sign_create_witness is a required trait method for Signer<IdentityPublicKey> and CallbackSigner is passed to SDK methods like put_to_platform_with_private_key, the SDK may invoke this dynamically even if not called explicitly in visible code paths.
Suggested safe fallback (until full implementation lands)
- async fn sign_create_witness(&self, key: &IdentityPublicKey, data: &[u8]) -> Result<AddressWitness, ProtocolError> {
- todo!()
- }
+ async fn sign_create_witness(&self, _key: &IdentityPublicKey, _data: &[u8]) -> Result<AddressWitness, ProtocolError> {
+ Err(ProtocolError::InvalidSigningKeyTypeError(
+ "sign_create_witness is not implemented for CallbackSigner".to_string(),
+ ))
+ }📝 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.
| async fn sign_create_witness(&self, key: &IdentityPublicKey, data: &[u8]) -> Result<AddressWitness, ProtocolError> { | |
| todo!() | |
| } | |
| async fn sign_create_witness(&self, _key: &IdentityPublicKey, _data: &[u8]) -> Result<AddressWitness, ProtocolError> { | |
| Err(ProtocolError::InvalidSigningKeyTypeError( | |
| "sign_create_witness is not implemented for CallbackSigner".to_string(), | |
| )) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-mobile/src/put.rs` around lines 244 - 246, The sign_create_witness
method in the file contains a todo!() placeholder that will cause a panic if
invoked by the SDK dynamically during identity operations. Replace the todo!()
with a proper error return statement that returns a Result type with an
appropriate ProtocolError variant instead of panicking. This ensures the method
safely handles calls from the SDK's internal logic like
put_to_platform_with_private_key without crashing the application.
| timeout: Some(Duration::from_secs(timeout as u64)), | ||
| retries: Some(retries), | ||
| ban_failed_address: Some(true), | ||
| max_decoding_message_size: None, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all RequestSettings initializations and max_decoding_message_size usage
rg -nP --type=rust -C3 'RequestSettings\s*\{' platform-mobile/src/sdk.rs platform-mobile/src/config.rs platform-mobile/src/put.rs
rg -nP --type=rust -C3 'max_decoding_message_size' platform-mobile/src/sdk.rs platform-mobile/src/config.rsRepository: dashpay/kotlin-platform
Length of output: 3694
Make decoding-size policy consistent across SDK factory outputs.
Lines 185 and 238 in platform-mobile/src/sdk.rs set max_decoding_message_size: None, but the callback-backed setup in platform-mobile/src/config.rs line 369 uses Some(16 * 1024 * 1024). This inconsistency creates path-dependent request behavior and can cause hard-to-reproduce decode failures under larger payloads.
Update both lines in sdk.rs to use Some(16 * 1024 * 1024) to match the config.rs behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-mobile/src/sdk.rs` at line 185, The max_decoding_message_size
configuration is inconsistent between different SDK factory outputs in the
codebase. Locate the two occurrences of max_decoding_message_size: None in
platform-mobile/src/sdk.rs and replace both instances with
max_decoding_message_size: Some(16 * 1024 * 1024) to match the consistent
behavior defined in the callback-backed setup in platform-mobile/src/config.rs,
which uses the same value. This ensures uniform request handling across all SDK
factory creation paths and prevents payload-size-dependent decode failures.
| ### Use in other projects | ||
| ```groovy | ||
| dppVersion = "2.0.5" | ||
| dppVersion = "4.0.0-SNAPSHOT" |
There was a problem hiding this comment.
Align the documented dependency version with the project’s bumped version.
At Line 55, README uses 4.0.0-SNAPSHOT, while the root build version is now 4.0.0-RC2-SNAPSHOT. This can mislead consumers into pulling a non-matching artifact.
Suggested doc fix
-dppVersion = "4.0.0-SNAPSHOT"
+dppVersion = "4.0.0-RC2-SNAPSHOT"📝 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.
| dppVersion = "4.0.0-SNAPSHOT" | |
| dppVersion = "4.0.0-RC2-SNAPSHOT" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 55, The dppVersion variable in the README.md documentation
is set to "4.0.0-SNAPSHOT" but the project's actual version has been bumped to
"4.0.0-RC2-SNAPSHOT". Update the dppVersion value in the README to match the
current project version to prevent users from pulling mismatched artifacts.
Locate the dppVersion assignment and change it to reflect the correct bumped
version from the root build configuration.
1 participant
V12 for testnet
V11 for mainnet
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores