feat: wasm sdk build proof-of-concept #2405
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request introduces configuration and dependency updates across multiple packages to support the WebAssembly target and improve architecture‐specific behavior. New Rust flags have been added for the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as Web UI (index.html)
participant B as WasmSdkBuilder
participant S as WasmSdk
participant CP as WasmContext
U->>UI: Enter Identity/DataContract ID
UI->>B: Initialize SDK (new_mainnet/new_testnet)
B-->>S: Build WasmSdk instance
U->>S: Set context provider (set_context_provider)
U->>S: Call identity_fetch/data_contract_fetch
S->>S: Process request asynchronously
S-->>U: Return IdentityWasm/DataContractWasm result
Possibly related PRs
Suggested reviewers
Poem
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 (
|
# Conflicts: # .github/workflows/release.yml # .github/workflows/tests-build-js.yml # Cargo.lock
…feat/wasm-dapi-sdk-client # Conflicts: # Cargo.lock
…feat/wasm-dapi-sdk-client
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/wasm-sdk/src/verify.rs (1)
71-126:⚠️ Potential issueReplace placeholder values with actual verification data.
Similar to
verify_identity_response, this function uses empty vectors which won't provide actual verification.
🧹 Nitpick comments (7)
packages/wasm-sdk/src/dpp.rs (4)
225-227: Consider returning aUint8Arrayinstead ofVec<u8>In the
hashmethod, returning aUint8Arraywould be more compatible with JavaScript environments and WebAssembly, providing better interoperability.Apply this diff to modify the return type:
- pub fn hash(&self) -> Result<Vec<u8>, JsError> { + pub fn hash(&self) -> Result<js_sys::Uint8Array, JsError> { self.inner.hash().map_err(to_js_error) + .map(|hash| js_sys::Uint8Array::from(hash.as_slice())) }
262-266: Validate the buffer before deserializationIn the
from_buffermethod, it's good practice to validate the input buffer to ensure it's not empty or malformed before attempting deserialization. This can prevent potential panics or undesired behavior.Apply this diff to add buffer validation:
pub fn from_buffer(buffer: Vec<u8>) -> Result<IdentityWasm, JsError> { + if buffer.is_empty() { + return Err(JsError::from("Buffer is empty")); + } let identity: Identity = PlatformDeserializable::deserialize_from_bytes(buffer.as_slice()) .map_err(to_js_error)?; Ok(identity.into())
41-47: Handle invalid platform versions gracefullyIn the
newconstructor, when retrieving the platform version usingPlatformVersion::get(platform_version), consider providing a user-friendly error message or defaulting to a supported version if an invalid version is supplied.Apply this diff to enhance error handling:
pub fn new(platform_version: u32) -> Result<IdentityWasm, JsError> { - let platform_version = &PlatformVersion::get(platform_version).map_err(to_js_error)?; + let platform_version = PlatformVersion::get(platform_version) + .map_err(|_| JsError::from("Invalid platform version supplied"))?; Identity::default_versioned(&platform_version) .map(Into::into) .map_err(to_js_error) }
296-299: Expose additionalDataContractmethods for completenessCurrently, the
DataContractWasmstruct provides theidandto_jsonmethods. Consider exposing additional methods such asget_document_typeorget_schemato provide more functionality to the Wasm SDK users.packages/rs-dapi-client/src/transport/wasm_channel.rs (2)
77-82: Enhance error handling inwasm_client_error_to_statusThe
wasm_client_error_to_statusfunction currently handlesTonicStatusErrorand maps all other errors toStatus::internal. To provide more detailed error information, consider handling additional specific variants oftonic_web_wasm_client::Error.Would you like assistance in expanding the error handling to cover more error cases?
90-91: Consider relocatingWasmBackonSleeperto a dedicated moduleThere's a TODO comment suggesting moving
WasmBackonSleeperto a different module. Organizing code by placing it in an appropriate module can improve maintainability and readability.Would you like help in refactoring to move
WasmBackonSleeperto a suitable module?packages/rs-dapi-client/tests/local_platform_connectivity.rs (1)
Line range hint
9-13: Consider documenting the test identity bytes.The
OWNER_ID_BYTESconstant contains hardcoded test data. Adding a doc comment explaining what this identity represents and why these specific bytes were chosen would improve maintainability.+ /// Test identity bytes representing [add context here] + /// Source: [explain where these bytes came from] pub const OWNER_ID_BYTES: [u8; 32] = [
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Cargo.toml(1 hunks)packages/rs-dapi-client/Cargo.toml(2 hunks)packages/rs-dapi-client/src/transport/wasm_channel.rs(1 hunks)packages/rs-dapi-client/tests/local_platform_connectivity.rs(1 hunks)packages/rs-sdk/Cargo.toml(5 hunks)packages/rs-sdk/src/sdk.rs(10 hunks)packages/wasm-sdk/index.html(1 hunks)packages/wasm-sdk/src/dpp.rs(1 hunks)packages/wasm-sdk/src/lib.rs(1 hunks)packages/wasm-sdk/src/sdk.rs(1 hunks)packages/wasm-sdk/src/state_transitions/documents.rs(1 hunks)packages/wasm-sdk/src/verify.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Cargo.toml
- packages/wasm-sdk/index.html
- packages/rs-sdk/src/sdk.rs
- packages/rs-sdk/Cargo.toml
🧰 Additional context used
📓 Learnings (1)
packages/wasm-sdk/src/lib.rs (1)
Learnt from: lklimek
PR: dashpay/platform#2405
File: packages/wasm-sdk/src/lib.rs:13-17
Timestamp: 2025-01-23T09:43:25.080Z
Learning: The codebase uses tracing_wasm for WebAssembly tracing as it provides sufficient functionality for the project's needs.
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (18)
packages/rs-dapi-client/Cargo.toml (6)
8-8: Great improvement on default features!Moving to empty default features is a good practice that aligns with Rust conventions and reduces unnecessary dependencies in production builds.
23-28: Well-structured target-specific dependency split!Good practice to separate non-wasm dependencies. The
backonconfiguration withtokio-sleepis appropriate for native targets.
46-50: Well-coordinated dependency updates for WASM support.Good addition of the
getrandomfeature torandwhile disabling default features. This aligns well with the WASM target requirements.
43-43: Verify the http dependency version.Let's check if version 1.1.0 is the latest stable version and if there are any known security issues.
✅ Verification successful
The http dependency version 1.1.0 is secure and properly configured.
The version is recent and well above any known vulnerable versions. While version 1.2.0 is available, the current version is secure and stable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for latest version and security advisories for http echo "Latest version on crates.io:" curl -s https://crates.io/api/v1/crates/http | jq '.crate.max_stable_version' echo -e "\nChecking for security advisories..." gh api graphql -f query='{ securityVulnerabilities(first: 5, ecosystem: RUST, package: "http") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 1230
29-33: Verify security advisories for WASM dependencies.Let's check for any security advisories on the new WASM dependencies.
✅ Verification successful
No security advisories found for WASM dependencies
All WASM dependencies (
gloo-timers,tonic-web-wasm-client,wasm-bindgen-futures, andgetrandom) are clear of known security vulnerabilities in the GitHub Security Advisory Database.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for security advisories for the new WASM dependencies for pkg in "gloo-timers" "tonic-web-wasm-client" "wasm-bindgen-futures" "getrandom"; do echo "Checking $pkg..." gh api graphql -f query='{ securityVulnerabilities(first: 5, ecosystem: RUST, package: "'$pkg'") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' doneLength of output: 2111
21-21: Verify the impact of renaming 'offline-testing' to 'online-testing'.This appears to be a significant semantic change that could affect existing code. Let's verify all usages to ensure nothing breaks.
packages/wasm-sdk/src/dpp.rs (1)
102-103: Handle potential precision loss when converting betweenu64andf64Converting between
u64andf64can lead to precision loss for large integer values. Since JavaScript numbers are floating-point, they cannot accurately represent integers beyond2^53 - 1. Consider using string representations or JavaScriptBigIntto handle large integer values safely.Also applies to: 107-108, 112-113, 117-118, 122-123, 127-128, 132-133
packages/wasm-sdk/src/lib.rs (3)
3-8: LGTM!The module organization is clean and follows a logical structure.
10-11: LGTM!Using
wee_allocis a good choice for WebAssembly as it's a size-optimized allocator.
13-24: LGTM!The start function correctly sets up error handling with
console_error_panic_hookand tracing withtracing_wasm. Based on the learnings,tracing_wasmprovides sufficient functionality for the project's needs.packages/wasm-sdk/src/state_transitions/documents.rs (1)
42-75: LGTM!The implementation includes:
- Comprehensive boundary checks for signature_public_key_id
- Proper error handling and conversion
- Correct serialization to bytes
packages/wasm-sdk/src/sdk.rs (4)
26-83: LGTM!The implementations are well-structured with:
- Proper wrapping of SDK functionality
- Correct implementation of Deref traits
- Consistent builder methods
85-112: LGTM!The functions include:
- Proper base58 identifier handling
- Comprehensive error handling with descriptive messages
114-145:⚠️ Potential issueReplace zero-initialized values with secure implementations.
While this is marked as a mock implementation, using zero-initialized values for identifiers and private keys is unsafe and should not be used as an example.
Apply this diff to improve security:
-let id = Identifier::from_bytes(&[0; 32]).expect("create identifier"); +let id = Identifier::random(); -let asset_lock_proof_private_key = - PrivateKey::from_slice(&[0; 32], Network::Testnet).expect("create private key"); +// TODO: Implement secure private key handling +let asset_lock_proof_private_key = /* secure key generation */;Likely invalid or redundant comment.
195-208:⚠️ Potential issueImplement mock signing for testing purposes.
While the comment explains why actual signing is not implemented, the
todo!()macro will panic if called. For testing purposes, implement a mock that returns empty bytes.Apply this diff to provide a mock implementation:
- todo!("signature creation is not implemented due to lack of dash platform wallet support in wasm") + // Mock implementation for testing + Ok(dash_sdk::dpp::platform_value::BinaryData::new(vec![0; 32]))Likely invalid or redundant comment.
packages/wasm-sdk/src/verify.rs (1)
182-189: LGTM!The DocumentWasm implementation correctly wraps Document and provides proper base58 string conversion for IDs.
packages/rs-dapi-client/tests/local_platform_connectivity.rs (2)
1-2: LGTM! The feature flag change improves clarity.The change from
#[cfg(not(feature = "offline-testing"))]to#[cfg(feature = "online-testing")]makes the test configuration more explicit and easier to understand. This aligns well with the feature restructuring needed for WASM compatibility.
Line range hint
14-53: Verify local server dependency in CI environment.The test requires a local DAPI server running on port 2443. Ensure that the CI pipeline is configured to handle this dependency appropriately when the "online-testing" feature is enabled.
✅ Verification successful
Local DAPI server dependency is properly handled in CI
The test is correctly configured with the
online-testingfeature flag, and the CI environment properly handles the DAPI server dependency through the local network setup action which builds and deploys DAPI as part of the test infrastructure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if CI configuration handles local DAPI server setup # Look for CI configuration files echo "Checking CI configuration files..." fd -t f '\.ya?ml$' .github/workflows/ # Look for Docker compose or similar setup files echo "Checking for Docker setup files..." fd -t f 'docker-compose.*\.ya?ml$' . # Look for documentation about test setup echo "Checking for test setup documentation..." fd -t f 'README\.md$'Length of output: 2526
Script:
#!/bin/bash # Check CI configuration for Rust package tests echo "Content of tests-rs-package.yml:" cat .github/workflows/tests-rs-package.yml echo -e "\nChecking rs-dapi-client documentation:" cat packages/rs-dapi-client/README.md 2>/dev/null || echo "README not found" echo -e "\nChecking for online-testing feature usage:" rg "online-testing|test.*feature" packages/rs-dapi-client/ -g '!target' echo -e "\nChecking for test setup in Cargo.toml:" cat packages/rs-dapi-client/Cargo.tomlLength of output: 11620
Script:
#!/bin/bash # Check main test workflow and search for DAPI setup echo "Content of main tests workflow:" cat .github/workflows/tests.yml echo -e "\nSearching for DAPI setup in workflows:" rg "dapi.*setup|setup.*dapi|start.*dapi" .github/workflows/Length of output: 7089
Script:
#!/bin/bash # Check functional test workflow and dashmate setup echo "Content of functional test workflow:" cat .github/workflows/tests-packges-functional.yml echo -e "\nChecking dashmate docker compose for DAPI setup:" cat packages/dashmate/docker-compose.build.dapi_api.ymlLength of output: 2379
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/dapi-grpc/build.rs (2)
256-263: Consider using constants for directory names.The directory name strings could be defined as constants to avoid duplication and make updates easier.
enum ImplType { Server, Client, Wasm, } +const SERVER_DIR: &str = "server"; +const CLIENT_DIR: &str = "client"; +const WASM_DIR: &str = "wasm"; + impl ImplType { // ... configure method ... fn dirname(&self) -> String { match self { - Self::Server => "server", - Self::Client => "client", - Self::Wasm => "wasm", + Self::Server => SERVER_DIR, + Self::Client => CLIENT_DIR, + Self::Wasm => WASM_DIR, } .to_string() } }
268-276: Update documentation to include the new parameter.The function documentation needs to be updated to include information about the
typparameter./// Create a new MappingConfig instance. /// /// ## Arguments /// /// * `protobuf_file` - Path to the protobuf file to use as input. /// * `out_dir` - Output directory where subdirectories for generated files will be created. +/// * `typ` - Implementation type that determines the build configuration and output directory. /// -/// Depending on the features, either `client`, `server` or `client_server` subdirectory +/// Depending on the implementation type, either `client`, `server` or `wasm` subdirectory /// will be created inside `out_dir`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dapi-grpc/build.rs(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (3)
packages/dapi-grpc/build.rs (3)
15-27: LGTM! Well-structured implementation for handling different targets.The main function effectively handles Server, Client, and Wasm implementations using feature flags and target architecture checks.
29-53: LGTM! Clean implementation of the code generation logic.The function effectively handles code generation for both core and platform protos while maintaining proper error handling and build dependencies.
229-264: LGTM! Well-designed enum for handling different implementation types.The
ImplTypeenum and its methods provide a clean and type-safe way to handle different implementation configurations.
packages/rs-dpp/src/schema/data_contract/v0/stateTransition/dataContractCreate.json
Outdated
Show resolved
Hide resolved
lklimek
force-pushed
the
feat/wasm-sdk-poc
branch
from
6fc1c38 to
00239e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs (3)
725-725: Consider breaking down this large function.The TODO comment correctly identifies that this function is quite large. Consider breaking it down into smaller, more focused functions:
- Schema reference resolution
- Type parsing and validation
- Nested property handling
749-795: Document the planned integer type refinements.The commented-out code suggests future plans for more granular integer type handling. Consider:
- Adding a TODO with a tracking issue for this enhancement.
- Documenting the version in which this refinement is planned.
587-921: Reduce code duplication between property insertion functions.The
insert_valuesandinsert_values_nestedfunctions share similar logic for type parsing and validation. Consider:
- Extracting common type parsing and validation logic into shared utility functions.
- Creating a trait or enum to abstract the property storage strategy (flat vs. nested).
Example approach:
// Shared type parsing fn parse_property_type( inner_properties: &BTreeMap<&str, &Value>, root_schema: &Value, ) -> Result<DocumentPropertyType, DataContractError> { // Common type parsing logic } // Strategy trait trait PropertyStorage { fn store_property( &mut self, key: String, property: DocumentProperty, ) -> Result<(), DataContractError>; } // Implementations for flat and nested storage struct FlatStorage(IndexMap<String, DocumentProperty>); struct NestedStorage(IndexMap<String, DocumentProperty>);packages/dapi-grpc/build.rs (3)
21-24: Improve error handling for target architecture check.The current error handling using
unwrap_or_default()could be more explicit about the fallback case.Consider this alternative implementation:
- if std::env::var("CARGO_CFG_TARGET_ARCH") - .unwrap_or_default() - .eq("wasm32") + if std::env::var("CARGO_CFG_TARGET_ARCH") + .map(|arch| arch == "wasm32") + .unwrap_or(false)
237-242: Consider removing unnecessary #[allow(unused)] attribute.The
ImplTypeenum is actively used in the code, so the#[allow(unused)]attribute appears to be unnecessary.-#[allow(unused)] enum ImplType { Server, Client, Wasm, }
276-284: Update documentation to include the new typ parameter.The function documentation needs to be updated to include information about the new
typparameter./// Create a new MappingConfig instance. /// /// ## Arguments /// /// * `protobuf_file` - Path to the protobuf file to use as input. /// * `out_dir` - Output directory where subdirectories for generated files will be created. +/// * `typ` - The implementation type that determines how the code is generated. /// /// Depending on the features, either `client`, `server` or `client_server` subdirectory /// will be created inside `out_dir`.packages/rs-dpp/Cargo.toml (2)
125-148: Dash-SDK Features Configuration ReviewWithin the
dash-sdk-featureslist, several features (e.g.,"json-object","platform-value","system_contracts", etc.) remain commented out, while others (such as"identity-hashing","data-contract-json-conversion", and"state-transition-value-conversion") are active. Please double-check that the removal (especially of"platform-value") and commenting-out of features are intentional and consistently reflected across the codebase. Clear documentation or a follow-up cleanup might also help avoid confusion later on.
212-213: Typo and New Feature Flag Review
- The comment on line 212 contains a typo ("Tring" should be "Trying").
- Additionally, the feature
create-contested-documentis introduced. Verify that its naming accurately reflects its intended functionality (i.e., confirm that “contested” is correct and not meant to be “consented” or another term).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.toml(1 hunks)packages/dapi-grpc/build.rs(5 hunks)packages/rs-dpp/Cargo.toml(5 hunks)packages/rs-dpp/src/balances/credits.rs(0 hunks)packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs(1 hunks)packages/rs-dpp/src/data_contract/mod.rs(1 hunks)packages/rs-dpp/src/lib.rs(0 hunks)
💤 Files with no reviewable changes (2)
- packages/rs-dpp/src/lib.rs
- packages/rs-dpp/src/balances/credits.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- packages/rs-dpp/src/data_contract/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (7)
packages/dapi-grpc/build.rs (1)
29-53: LGTM!The generate_code function is well-structured and properly handles both core and platform proto generation with appropriate error messages and cargo directives.
packages/rs-dpp/Cargo.toml (6)
59-60: Optional Decimal Dependencies Update
rust_decimalandrust_decimal_macroshave been marked as optional. This change enhances flexibility by allowing users to enable these dependencies only when needed. Please ensure that all code which requires them is properly gated behind feature flags.
63-63: Marking JSON Schema Compatibility Validator as OptionalThe
json-schema-compatibility-validatordependency is now optional. Verify that any functionality relying on it activates the respective feature flag so that builds without this dependency don’t break.
73-74: Dev Dependencies Version Update for LoggingThe versions for
env_loggerandlogin the dev-dependencies have been updated (withenv_loggerexplicitly added andlogupdated to version0.4.22). Please confirm that the versions are aligned with production usage if applicable, or if any conflicts might arise in test runs.
77-77: Default Feature ModificationThe default feature has been updated to only include
"state-transitions", thereby removing"platform-value". This change aligns with the overall objective to deprecateplatform-valuein defaults. Ensure that consumers who previously relied on the broader default set are updated accordingly or are aware of this behavioral change.
216-220: JSON Schema-Validation Feature ExpansionThe
json-schema-validationfeature now includes"jsonschema","platform-value-json", and"dep:json-schema-compatibility-validator". This appears consistent with the updated dependency configuration. Please ensure that the inclusion of the optionaljson-schema-compatibility-validatoris accounted for in any conditional builds or feature tests.
291-291: Fee Distribution Feature Dependency MappingThe
fee-distributionfeature now depends on"dep:rust_decimal"and"dep:rust_decimal_macros", which aligns with making those dependencies optional earlier. Confirm that any modules using fee-distribution logic correctly handle the scenario when these features are enabled.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-dpp/src/tokens/token_event.rs (1)
33-52: Consider adding documentation for the Transfer variant.The TokenEvent enum is well-structured with appropriate derive macros. The Transfer variant's parameter ordering is logical, placing required fields before optional ones.
Add documentation for the Transfer variant to clarify the purpose of each parameter:
pub enum TokenEvent { + /// Transfers tokens from one identity to another + /// * `recipient_id` - The identifier of the recipient + /// * `public_note` - Optional public note visible to all + /// * `shared_encrypted_note` - Optional encrypted note shared between sender and recipient + /// * `personal_encrypted_note` - Optional personal encrypted note + /// * `amount` - The amount of tokens to transfer Transfer( RecipientIdentifier, TokenEventPublicNote, TokenEventSharedEncryptedNote, TokenEventPersonalEncryptedNote, TokenAmount, ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rs-dpp/src/tokens/token_event.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Formatting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/rs-dpp/src/tokens/token_event.rs (2)
128-134: LGTM! Encrypted note handling is well-implemented.The implementation correctly maps the shared encrypted note fields to document properties, maintaining consistency with the existing pattern.
17-18:✅ Verification successful
LGTM! Verify feature flag usage across the codebase.
The conditional compilation for WASM support is well-structured. The new type and import are correctly gated behind the
state-transitionsfeature flag.Let's verify the consistent usage of this feature flag:
Also applies to: 25-26
🏁 Script executed:
#!/bin/bash # Description: Check for consistent usage of state-transitions feature flag # across the codebase, particularly for SharedEncryptedNote. # Search for SharedEncryptedNote usage rg "SharedEncryptedNote" -A 3 -B 3 # Search for state-transitions feature usage rg "#\[cfg\(feature\s*=\s*\"state-transitions\"\)\]" -A 3 -B 3Length of output: 76484
Conditional Compilation for WASM Support – LGTM!
The new type definition
TokenEventSharedEncryptedNoteinpackages/rs-dpp/src/tokens/token_event.rsis properly guarded by thestate-transitionsfeature flag. The grep results show consistent usage of bothSharedEncryptedNoteand the feature flag across the codebase (with relevant occurrences in other modules confirming that this gating strategy is maintained). No issues were found with conditional compilation or integration with dependent code.
This reverts commit 70d9192.
commit 6776651 Author: QuantumExplorer <quantum@dash.org> Date: Sat Mar 1 22:23:41 2025 +0700 chore: update to latest dash core 37 (#2483) commit 1501103 Merge: a7c7a0f da17fc5 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 27 14:21:41 2025 +0700 chore: merge master and resolve conflicts (#2481) commit da17fc5 Author: pshenmic <pshenmic@gmail.com> Date: Thu Feb 27 13:31:51 2025 +0700 feat(js-dash-sdk): fix tests after merge commit c7e40cb Merge: c57e8b2 f9eb069 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 27 09:35:02 2025 +0700 Merge remote-tracking branch 'origin/chore/merge-master' into chore/merge-master commit c57e8b2 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 27 09:34:40 2025 +0700 test(dpp): fix assertion with the same value commit 045b6fa Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 27 09:32:33 2025 +0700 chore(dpp): remove unnecessary type conversion commit 8160ccd Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 27 09:31:32 2025 +0700 chore: remove duplicated commented code commit f9eb069 Merge: 05d0085 a7c7a0f Author: pshenmic <pshenmic@gmail.com> Date: Wed Feb 26 20:03:00 2025 +0700 Merge branch 'v2.0-dev' into chore/merge-master commit a7c7a0f Author: pshenmic <pshenmic@gmail.com> Date: Wed Feb 26 19:52:02 2025 +0700 build: bump rust version to 1.85 (#2480) commit 05d0085 Merge: bcf1785 196976c Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Feb 26 18:03:38 2025 +0700 Merge branch 'master' into v2.0-dev commit bcf1785 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Fri Feb 21 08:43:35 2025 +0100 feat: wasm sdk build proof-of-concept (#2405) Co-authored-by: Ivan Shumkov <ivan@shumkov.ru> commit 5e32426 Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Thu Feb 20 19:22:52 2025 +0700 fix: token already paused unpaused and frozen validation (#2466) commit 374a036 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 20 17:46:57 2025 +0700 test: fix slowdown of JS SDK unit tests (#2475) commit 1fed09b Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 20 13:46:36 2025 +0700 fix(dpp): invalid feature flag usage (#2477) commit 33507bb Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Thu Feb 20 13:18:55 2025 +0700 fix: destroy frozen funds used wrong identity and proof verification (#2467) commit 91a9766 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Feb 19 16:57:32 2025 +0700 feat(sdk): return state transition execution error (#2454) commit cb915a7 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Feb 19 16:46:54 2025 +0700 test: fix token history contract tests (#2470) commit 04276d5 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Tue Feb 18 21:00:05 2025 +0700 fix: xss vulnerability in mocha (#2469) commit 196976c Author: pshenmic <pshenmic@gmail.com> Date: Fri Feb 14 18:50:08 2025 +0700 fix(sdk)!: bigint for uint64 values (#2443) commit 0bd29a6 Author: pshenmic <pshenmic@gmail.com> Date: Fri Feb 14 17:29:35 2025 +0700 feat(dpp): extra methods for state transitions in wasm (#2462) commit 1eae781 Author: pshenmic <pshenmic@gmail.com> Date: Fri Feb 14 15:29:17 2025 +0700 chore(platform): npm audit fix (#2463) commit ddf4e67 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Feb 14 11:28:08 2025 +0700 test: fix `fetchProofForStateTransition` tests and warnings (#2460) commit d88ea46 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Feb 14 09:52:53 2025 +0700 fix(dpp): invalid imports and tests (#2459) commit 82e4d4c Merge: 125cfe7 4becf5f Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Thu Feb 13 19:05:51 2025 +0700 fix: check if token is paused on token transfers (#2458) commit 4becf5f Author: pauldelucia <pauldelucia2@gmail.com> Date: Thu Feb 13 18:34:24 2025 +0700 add costs commit 907971d Merge: 9026669 125cfe7 Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Thu Feb 13 18:05:06 2025 +0700 Merge branch 'v2.0-dev' into feat/token-paused-validation commit 125cfe7 Merge: 91f65c6 c286ec0 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Feb 13 15:51:46 2025 +0700 Merge branch 'v2.0-dev' into v2.0-tokens-dev commit 9026669 Author: pauldelucia <pauldelucia2@gmail.com> Date: Thu Feb 13 13:41:19 2025 +0700 feat: check if token is paused on token transfers commit c286ec0 Author: pshenmic <pshenmic@gmail.com> Date: Wed Feb 12 15:41:21 2025 +0700 feat(sdk): add option to request all keys (#2445) commit 91f65c6 Merge: d6b40e6 1a1c50b Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Wed Feb 12 12:04:58 2025 +0700 fix: wrong order of parameters in UnauthorizedTokenActionError (#2456) commit 1a1c50b Author: pauldelucia <pauldelucia2@gmail.com> Date: Wed Feb 12 11:51:31 2025 +0700 fix: wrong order of parameters in UnauthorizedTokenActionError commit 26aff36 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Tue Feb 11 13:06:54 2025 +0100 build: bump Alpine version to 3.21 (#2074) commit 9daa195 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Tue Feb 11 14:38:55 2025 +0700 ci: use github-hosted arm runner for release workflow (#2452) commit 2b1c252 Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Tue Feb 4 16:40:34 2025 +0700 fix: proof result error for credit transfers in sdk (#2451) commit d6b40e6 Author: QuantumExplorer <quantum@dash.org> Date: Tue Feb 4 06:49:03 2025 +0700 feat(platform): token distribution part two (#2450) commit 93f7d44 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Jan 29 14:07:55 2025 +0700 fix(dpp): invalid feature flag instructions (#2448) commit 6d5af88 Author: QuantumExplorer <quantum@dash.org> Date: Mon Jan 27 16:59:39 2025 +0700 feat(dpp): token distribution model (#2447) commit e735313 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Mon Jan 27 14:24:26 2025 +0700 feat: add token transitions to SDK and DAPI (#2434) commit 0743be2 Author: pshenmic <pshenmic@gmail.com> Date: Sun Jan 26 22:00:40 2025 +0700 feat(dpp): extra methods for state transitions in wasm (#2401) commit f609bcf Merge: 3733f56 cbddb8d Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Jan 24 18:16:38 2025 +0700 Merge branch 'v2.0-dev' into v2.0-tokens-dev commit cbddb8d Author: QuantumExplorer <quantum@dash.org> Date: Fri Jan 24 17:59:16 2025 +0700 chore(platform): make bls sig compatibility an optional feature (#2440) Co-authored-by: Ivan Shumkov <ivan@shumkov.ru> commit 764684b Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Jan 24 17:57:27 2025 +0700 chore: ignore deprecated `lodash.get` (#2441) commit 3733f56 Author: QuantumExplorer <quantum@dash.org> Date: Thu Jan 23 09:16:12 2025 +0700 feat(platform)!: enhance token configuration and validation mechanisms (#2439) commit 2480ceb Author: QuantumExplorer <quantum@dash.org> Date: Wed Jan 22 16:33:13 2025 +0700 chore: dapi grpc queries (#2437) commit c9ab154 Author: QuantumExplorer <quantum@dash.org> Date: Wed Jan 22 15:50:25 2025 +0700 feat(platform)!: improved token validation and token config update transition (#2435) commit d9647cc Author: QuantumExplorer <quantum@dash.org> Date: Tue Jan 21 10:28:58 2025 +0700 feat: get proofs for tokens (#2433) commit e5964b8 Author: QuantumExplorer <quantum@dash.org> Date: Mon Jan 20 23:31:50 2025 +0700 feat: group queries (#2432) commit 0220302 Author: QuantumExplorer <quantum@dash.org> Date: Sun Jan 19 14:43:51 2025 +0700 feat(platform): proof verification for many queries and a few more queries (#2431) commit cd1527d Author: QuantumExplorer <quantum@dash.org> Date: Fri Jan 17 19:39:37 2025 +0700 fix(dpp)!: wrapping overflow issue (#2430) commit fd7ee85 Merge: d7143cc e4e156c Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Jan 16 21:45:47 2025 +0700 Merge branch 'master' into v1.9-dev commit e4e156c Author: QuantumExplorer <quantum@dash.org> Date: Thu Jan 16 18:11:57 2025 +0700 chore(release): update change log and release v1.8.0 (#2427) Co-authored-by: Ivan Shumkov <ivan@shumkov.ru> commit 55a1e03 Author: QuantumExplorer <quantum@dash.org> Date: Thu Jan 16 15:30:42 2025 +0700 feat(platform)!: token base support (#2383) commit 59bf0af Author: QuantumExplorer <quantum@dash.org> Date: Thu Jan 16 13:10:39 2025 +0700 chore(release): bump to v1.8.0-rc.2 (#2426) commit 410eb09 Author: QuantumExplorer <quantum@dash.org> Date: Thu Jan 16 06:31:26 2025 +0700 fix(drive-abci): rebroadcasting should not only take first 2 quorums too (#2425) commit 2abce8e Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Jan 15 22:51:58 2025 +0700 chore(release): update changelog and bump version to 1.8.0-rc.1 (#2423) commit ad5f604 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Jan 15 22:14:13 2025 +0700 chore: update bls library (#2424) commit c6feb5b Author: QuantumExplorer <quantum@dash.org> Date: Wed Jan 15 18:57:49 2025 +0700 feat(platform)!: distribute prefunded specialized balances after vote (#2422) Co-authored-by: Ivan Shumkov <ivan@shumkov.ru> commit 94dcbb2 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Jan 15 05:51:45 2025 +0700 chore(drive): increase withdrawal limits to 2000 Dash per day (#2287) commit 6a0aede Author: Ivan Shumkov <ivan@shumkov.ru> Date: Tue Jan 14 21:42:59 2025 +0700 chore: fix test suite configuration script (#2402) commit e94b7bb Author: QuantumExplorer <quantum@dash.org> Date: Tue Jan 14 19:23:46 2025 +0700 fix(drive-abci): document purchase on mutable document from different epoch had issue (#2420) commit 4ee57a6 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Tue Jan 14 19:12:20 2025 +0700 fix(drive): more than one key was returned when expecting only one result (#2421) commit be5cd6d Author: Ivan Shumkov <ivan@shumkov.ru> Date: Mon Jan 13 15:12:33 2025 +0700 fix(sdk): failed to deserialize consensus error (#2410) commit e07271e Author: Ivan Shumkov <ivan@shumkov.ru> Date: Mon Jan 13 14:57:08 2025 +0700 chore: resolve NPM audit warnings (#2417) commit a809df7 Author: QuantumExplorer <quantum@dash.org> Date: Sun Jan 12 09:21:48 2025 +0700 test: unify identity versioned cost coverage (#2416) commit 6d637fe Author: Paul DeLucia <69597248+pauldelucia@users.noreply.github.com> Date: Fri Dec 27 09:42:04 2024 -0500 fix: try DriveDocumentQuery from DocumentQuery start field (#2407) commit cfd9c4d Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Dec 19 18:30:06 2024 +0700 chore(release): update changelog and bump version to 1.8.0-dev.2 (#2404) commit fecda31 Merge: 37d5732 fc7d994 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Dec 19 15:33:45 2024 +0700 Merge branch 'master' into v1.8-dev commit fc7d994 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Dec 19 14:40:44 2024 +0700 chore(release): update changelog and bump version to 1.7.1 (#2403) commit adcd3b8 Author: QuantumExplorer <quantum@dash.org> Date: Thu Dec 19 09:54:07 2024 +0300 fix!: emergency hard fork to fix masternode voting (#2397) commit 37d5732 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Dec 18 22:24:37 2024 +0700 fix(dashmate): some group commands fail with mtime not found (#2400) commit 01a5b7a Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Dec 18 20:44:44 2024 +0700 refactor(dpp): using deprecated param to init wasm module (#2399) commit c5f5878 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Dec 18 18:04:14 2024 +0700 fix(dashmate): local network starting issues (#2394) commit 71c41ff Author: Ivan Shumkov <ivan@shumkov.ru> Date: Wed Dec 18 18:03:55 2024 +0700 perf(dpp): reduce JS binding size by 3x (#2396) commit 21ec393 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Wed Dec 18 10:47:58 2024 +0100 build!: update rust to 1.83 - backport #2393 to v1.7 (#2398) commit d7143cc Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Wed Dec 18 08:53:53 2024 +0100 build!: optimize for x86-64-v3 cpu microarchitecture (Haswell+) (#2374) commit d318b1c Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Tue Dec 17 14:56:15 2024 +0100 build: bump wasm-bindgen to 0.2.99 (#2395) commit 889d192 Author: Ivan Shumkov <ivan@shumkov.ru> Date: Tue Dec 17 19:25:58 2024 +0700 chore(release): update changelog and bump version to 1.8.0-dev.1 (#2391) commit 8185d21 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Tue Dec 17 10:47:53 2024 +0100 feat(sdk)!: allow setting CA cert (#1924) commit 82a6217 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Tue Dec 17 02:51:18 2024 +0100 build!: update rust to 1.83 (#2393) commit 494054a Author: QuantumExplorer <quantum@dash.org> Date: Mon Dec 16 13:47:58 2024 +0300 refactor(platform): replace bls library (#2257) Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> commit 4c203e4 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Mon Dec 16 10:38:34 2024 +0100 test(sdk): generate test vectors using testnet (#2381) commit 0ff6b27 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Mon Dec 16 10:37:35 2024 +0100 chore: remove deprecated check_network_version.sh (#2084) commit b265bb8 Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Fri Dec 13 13:25:40 2024 +0100 ci: fix artifact upload issue on release build (#2389) commit 40ae73f Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Dec 13 17:35:40 2024 +0700 chore(release): update changelog and bump version to 1.7.0 (#2387) commit 257e3da Author: Ivan Shumkov <ivan@shumkov.ru> Date: Fri Dec 13 15:44:10 2024 +0700 chore(dashmate)!: update Core to version 22 (#2384) commit 19a4c6d Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Dec 12 18:30:14 2024 +0700 chore(dashmate): set tenderdash version to 1 (#2385) commit 0e9d4dc Author: lklimek <842586+lklimek@users.noreply.github.com> Date: Thu Dec 12 11:39:35 2024 +0100 chore: address vulnerabilty GHSA-mwcw-c2x4-8c55 (#2382) Co-authored-by: Ivan Shumkov <ivan@shumkov.ru> commit bdae90c Author: Ivan Shumkov <ivan@shumkov.ru> Date: Thu Dec 12 13:36:04 2024 +0700 chore(dashmate): increase subsidy for devnet (#2353)
Issue being fixed or feature implemented
To make some decisions about direction in which we should move,
we need to validate if it's possible and reasonable to create WASM
bindings for Rust Dash SDK.
What was done?
How Has This Been Tested?
Created simple test web page in
packages/wasm-sdk/index.htmlthat tests SDK.Note: It requires hardcoding quorums in
packages/wasm-sdk/src/context_provider.rs.Breaking Changes
TODO
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Refactor
Tests