feat(dashmate)!: add quroum list service #2868
Conversation
📝 WalkthroughWalkthroughAdds local-network support and trusted local contexts for WASM and JS SDKs, two new gRPC RPCs (getAddressesTrunkState, getAddressesBranchState), a Quorum List service and config, numerous protobuf client bindings, many functional test changes (migrate to local WASM tests, remove JS testnet suites), and several API type/signature updates for identity/wasm surfaces. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application / Tests
participant WasmSDK as WasmSdk
participant Prefetch as Prefetcher
participant Context as WasmTrustedContext
participant Builder as WasmSdkBuilder
participant Client as SDK Client
App->>WasmSDK: prefetchTrustedQuorumsLocal()
alt no cached context
WasmSDK->>Context: new_local_with_url(DEFAULT_LOCAL_QUORUM_URL)
Context-->>WasmSDK: Trusted context
end
WasmSDK->>Prefetch: fetch quorum data
Prefetch-->>WasmSDK: quorum addresses
Note over WasmSDK: cache LOCAL_TRUSTED_CONTEXT\nand LOCAL_DISCOVERED_ADDRESSES
App->>WasmSDK: localTrusted() / local()
WasmSDK->>Builder: create builder (Regtest/local)
Builder->>Context: use cached LOCAL_TRUSTED_CONTEXT
Builder-->>App: configured builder
App->>Builder: build()
Builder->>Client: instantiate client (uses cached addresses)
Client-->>App: client ready
App->>Client: getAddressInfo(addr)
Client->>Server: (gRPC) getAddressesTrunkState/getAddressesBranchState
Server-->>Client: address state response
Client-->>App: return address info
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
❌ gRPC Query Coverage Report |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/dashmate/src/config/generateEnvsFactory.js (1)
26-35: Update JSDoc typedef to include DASH_CORE_NETWORK.The JSDoc typedef for the
generateEnvsreturn type is missing the newly addedDASH_CORE_NETWORKproperty that's set at line 102. This creates a documentation inconsistency.🔧 Suggested fix
/** * @typedef {function} generateEnvs * @param {Config} config * @returns {{ * COMPOSE_DOCKER_CLI_BUILD: number, * CONFIG_NAME: string, * DOCKER_BUILDKIT: number, * COMPOSE_PROJECT_NAME: string, * COMPOSE_FILE: string, * COMPOSE_PATH_SEPARATOR: string, + * DASH_CORE_NETWORK: string, * }} */packages/js-evo-sdk/src/sdk.ts (1)
160-174: Update JSDoc to document 'local' network option.The method signature now accepts 'local' as a valid network parameter, but the JSDoc comment doesn't document this option.
🔎 Suggested JSDoc update
/** * Create an EvoSDK instance configured with specific masternode addresses. * * @param addresses - Array of HTTPS URLs to masternodes (e.g., ['https://127.0.0.1:1443']) - * @param network - Network identifier: 'mainnet', 'testnet' (default: 'testnet') + * @param network - Network identifier: 'mainnet', 'testnet', 'local' (default: 'testnet') * @param options - Additional connection options * @returns A configured EvoSDK instance (not yet connected - call .connect() to establish connection) * * @example * ```typescript * const sdk = EvoSDK.withAddresses(['https://52.12.176.90:1443'], 'testnet'); * await sdk.connect(); + * // For local development: + * const localSdk = EvoSDK.withAddresses(['https://127.0.0.1:1443'], 'local'); + * await localSdk.connect(); * ``` */ static withAddresses(addresses: string[], network: 'mainnet' | 'testnet' | 'local' = 'testnet', options: ConnectionOptions = {}): EvoSDK {
🧹 Nitpick comments (13)
packages/dashmate/src/config/generateEnvsFactory.js (1)
6-16: Consider documenting pass-through behavior for unknown networks.The function correctly maps
localtoregtestand passes other values through unchanged. However, the JSDoc doesn't clarify what happens with unrecognized network names. Consider adding a note that unknown networks are returned as-is, or optionally add validation if only specific values are expected.📝 Optional: Document pass-through behavior
/** * Maps dashmate network name to Dash Core network name. + * Unknown network names are returned unchanged. * @param {string} network - dashmate network (local, devnet, testnet, mainnet) * @returns {string} Dash Core network name (regtest, devnet, testnet, mainnet) */ function getDashCoreNetwork(network) { if (network === NETWORK_LOCAL) { return 'regtest'; } return network; }packages/dapi-grpc/clients/platform/v0/objective-c/Platform.pbobjc.h (1)
8025-8171: New trunk/branch address state messages look correct; consider adding proto‑level docs (and possibly refining naming)The added
GetAddressesTrunkState*andGetAddressesBranchState*request/response types match the existing GPB patterns (versioned wrapper, innerV0message,ClearVersionOneOfCasehelpers, field enums) and are structurally sound.Two minor, non‑blocking suggestions:
- The semantics of
key,depth, andmerkProofare not documented, unlike many neighboring messages. If you’re still iterating on the.proto, consider adding brief comments there (e.g., expected key encoding, units/limits fordepth, what structuremerkProofrepresents), then regenerating this header. That will help SDK consumers use these RPCs correctly.- If it’s still easy to change the proto, you might want to rename
merkProof→merkleProof(and corresponding field name) for clarity; otherwise this can be left as‑is.Given this file is generated, any such tweaks should be done in
platform.protoand regenerated rather than hand‑editing the Objective‑C output.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
9927-10029: VerifyGetAddressesBranchStateResponseV0field naming and absence of metadata
GetAddressesBranchStateResponseV0currently exposes a singlemerkProofbytes field and noResponseMetadata, unlike most proof‑carrying responses in this file which also return metadata and often use the sharedProofmessage.Two things to verify against
platform.protoand the generated JS:
- The field is really named something equivalent to
merk_proofand notmerkle_proofor a structured proof type (e.g.Proof), to avoid a mismatch between the declaration and the wire schema.- The response is intentionally “proof‑only” (no metadata, no
ResultCaseoneof), and other language clients follow the same shape.If the proto actually uses a different name or includes metadata, this d.ts will need to be regenerated or adjusted accordingly.
As an optional cleanup, if the proto does in fact use
merkle_proof(or you plan to rename it), the TS surface could be made clearer like this (after updating proto + regenerating all bindings):Example diff if renaming to
merkleProofis correct- export class GetAddressesBranchStateResponseV0 extends jspb.Message { - getMerkProof(): Uint8Array | string; - getMerkProof_asU8(): Uint8Array; - getMerkProof_asB64(): string; - setMerkProof(value: Uint8Array | string): void; + export class GetAddressesBranchStateResponseV0 extends jspb.Message { + getMerkleProof(): Uint8Array | string; + getMerkleProof_asU8(): Uint8Array; + getMerkleProof_asB64(): string; + setMerkleProof(value: Uint8Array | string): void; @@ - export namespace GetAddressesBranchStateResponseV0 { - export type AsObject = { - merkProof: Uint8Array | string, - } + export namespace GetAddressesBranchStateResponseV0 { + export type AsObject = { + merkleProof: Uint8Array | string, + } }packages/dapi-grpc/clients/platform/v0/java/org/dash/platform/dapi/v0/PlatformGrpc.java (1)
1537-1597: New method descriptors for trunk/branch state look consistent
getGetAddressesTrunkStateMethodandgetGetAddressesBranchStateMethodfollow the existing pattern: correct request/response types, full method names, marshallers, and schema descriptors, with proper double‑checked locking on the static volatile fields. No changes needed.Given
@Generatedon this class, ensure these changes come from re‑running the proto compiler rather than hand‑editing, so regeneration stays reproducible.packages/dashmate/docs/services/quorum_list.md (1)
30-34: Clarify RPC endpoint port and additional API endpoints.Line 33 references
${CORE_RPC_PORT}which is a variable, but the actual port value is not documented in this file. Additionally, only the/quorumsendpoint is shown in the verification section (line 100), but other endpoints may be available.Consider adding:
- The actual Core RPC port value or reference to where it's configured
- Documentation of all available API endpoints beyond
/quorums(e.g.,/quorums/...,/masternode, etc.)packages/wasm-sdk/tests/functional/protocol.spec.mjs (1)
44-51: Consider using dynamic test data for consistency.Line 45 still uses a hardcoded
START_PROTXvalue while the test on lines 35-42 now uses the dynamicevonodeProTxHash. For consistency and maintainability, consider whether this test should also use the fixture data or add a comment explaining why a specific hardcoded value is required here.packages/wasm-sdk/tests/functional/epochs-blocks.spec.mjs (2)
44-55: Inconsistent use of hardcoded and dynamic epoch values.Line 49 uses a hardcoded epoch
8635, but line 51 uses the dynamicsampleEpochvariable. SinceevonodeProTxHashis required for this test (and triggers a skip when null), the hardcoded epoch at line 49 should likely also usesampleEpochfor consistency.🔎 Suggested refactor
- await client.getEvonodesProposedEpochBlocksByIds(8635, [EVONODE_ID]); + await client.getEvonodesProposedEpochBlocksByIds(sampleEpoch, [EVONODE_ID]);
57-75: Tests at lines 57-75 still use hardcodedEVONODE_IDand epoch values.These two tests continue to use hardcoded values (
'143dcd6a6b7684fde01e88a10e5d65de9a29244c5ecd586d14a342657025f113'and8635) instead of the dynamicevonodeProTxHashandsampleEpoch. This is inconsistent with the migration to local test data. Consider either:
- Adding conditional skips when
evonodeProTxHashorsampleEpochare unavailable, or- Updating these tests to use the dynamic values from the fixture.
packages/wasm-sdk/tests/functional/tokens.spec.mjs (1)
7-11: Redundant token contract aliases.Lines 10-11 create
TOKEN_CONTRACT_2andTOKEN_CONTRACT_3that simply alias the sameTOKEN_CONTRACTvalue. Since all three point to the same contract, consider removing the redundant aliases and usingTOKEN_CONTRACTdirectly throughout, or clarify if different contracts are intended in the future.🔎 Suggested refactor
const req = wasmFunctionalTestRequirements(); const TEST_IDENTITY = req.identityId; const TOKEN_CONTRACT = req.tokenContracts[0].contractId; - const TOKEN_CONTRACT_2 = TOKEN_CONTRACT; - const TOKEN_CONTRACT_3 = TOKEN_CONTRACT;Then update the test calls to use
TOKEN_CONTRACTconsistently.packages/wasm-sdk/tests/functional/token-pricing.spec.mjs (1)
21-23: Inconsistent test data initialization pattern.This test calls
wasmFunctionalTestRequirements()inline at line 22, whereas other functional tests (e.g.,tokens.spec.mjs,dpns.spec.mjs,identities.spec.mjs) initialize test data at the describe block level. Consider moving this to the describe level for consistency and to facilitate reuse if additional tests are added.🔎 Suggested refactor
describe('Token pricing', function describeTokenPricing() { this.timeout(60000); + const { tokenContracts } = wasmFunctionalTestRequirements(); let client; let builder; // ... it('calculates token id and fetches price by contract', async () => { - const CONTRACT_ID = wasmFunctionalTestRequirements().tokenContracts[0].contractId; + const CONTRACT_ID = tokenContracts[0].contractId; // ... }); });packages/wasm-sdk/tests/functional/addresses.spec.mjs (1)
7-7: Unused function call.Line 7 calls
wasmFunctionalTestRequirements()but doesn't use its return value. Since this test suite relies on hardcoded genesis addresses (as documented in lines 19-22) rather than fixture data, this call can be removed.🔎 Suggested fix
- wasmFunctionalTestRequirements(); - let client;packages/wasm-sdk/src/sdk.rs (2)
254-309: Consider refactoring to reduce code duplication.The three network branches (Dash, Testnet, Regtest) follow nearly identical patterns, differing only in the context cache source, constructor, and network type. This duplication could be reduced by extracting the common logic.
💡 Refactoring suggestion
Consider extracting a helper function that takes the network type, context cache, and context constructor as parameters:
fn build_sdk_with_context( address_list: dash_sdk::sdk::AddressList, network: Network, cached_context: Option<WasmTrustedContext>, create_context: impl FnOnce() -> Result<WasmTrustedContext, WasmSdkError>, ) -> Result<SdkBuilder, WasmSdkError> { let context = cached_context .map(Ok) .unwrap_or_else(create_context)?; Ok(SdkBuilder::new(address_list) .with_network(network) .with_context_provider(context)) }Then each branch becomes:
Network::Dash => { let guard = MAINNET_TRUSTED_CONTEXT.lock().unwrap(); build_sdk_with_context( address_list.clone(), network, guard.clone(), || WasmTrustedContext::new_mainnet() .map_err(|e| WasmSdkError::from(dash_sdk::Error::from(e))), )? }This preserves the exact same behavior while eliminating repetition.
330-342: LGTM: Local builder with default gateway address.The
new_local()method provides a convenient builder for local development using dashmate's default gateway address. The implementation correctly uses the untrusted context and Regtest network, consistent with the mainnet/testnet non-trusted variants.Optional: Consider replacing unwrap with proper error handling
Line 334 uses
.unwrap()on a hardcoded URL. While this is safe since the URL is guaranteed valid, for consistency with the error handling patterns used throughout this file, you could use:let local_addresses = vec![ "https://127.0.0.1:2443" .parse() .expect("Hardcoded local address must be valid") ];Or propagate any potential error through the return type (though this changes the method signature).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/js-evo-sdk/src/identities/facade.ts (1)
12-12: Inconsistent representation of missing values:undefinedvsnull.The
fetchmethod returnsPromise<wasm.Identity | undefined>for a missing identity, whilebyPublicKeyHashreturnsPromise<wasm.Identity | null>. This inconsistency requires consumers to handle absence differently across similar query methods.Consider standardizing on either
nullorundefinedthroughout the facade for missing entities.🔎 Suggested approach to standardize on null
-async fetch(identityId: wasm.IdentifierLike): Promise<wasm.Identity | undefined> { +async fetch(identityId: wasm.IdentifierLike): Promise<wasm.Identity | null> { const w = await this.sdk.getWasmSdkConnected(); return w.getIdentity(identityId); }Note: Verify that the underlying
w.getIdentity()returnsnullrather thanundefined, or add a conversion if needed.Also applies to: 87-90
🧹 Nitpick comments (1)
packages/wasm-sdk/src/queries/identity.rs (1)
797-808: Consider consistent naming for Map variables.The method
get_identities_balancesusesresults_map(line 797) whileget_identities_balances_with_proof_infousesbalances_map(line 1163). Since these methods have identical logic, consistent naming would improve maintainability.🔎 Suggested naming alignment
- let results_map = Map::new(); + let balances_map = Map::new(); for identifier in identifiers { let key = JsValue::from(IdentifierWasm::from(identifier)); let value = match balances_result.get(&identifier) { Some(Some(balance)) => BigInt::from(*balance).into(), _ => JsValue::NULL, }; - results_map.set(&key, &value); + balances_map.set(&key, &value); } - Ok(results_map) + Ok(balances_map)Also applies to: 1163-1177
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/dashmate/docs/services/quorum_list.mdpackages/js-evo-sdk/src/identities/facade.tspackages/wasm-sdk/package.jsonpackages/wasm-sdk/src/queries/identity.rspackages/wasm-sdk/tests/functional-bootstrap.cjspackages/wasm-sdk/tests/karma/karma.functional.conf.cjs
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/wasm-sdk/package.json
- packages/dashmate/docs/services/quorum_list.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use 2-space indent for JS/TS files
Use camelCase for variables and functions in JS/TS
Use PascalCase for class names in JS/TS
Use ESLint with Airbnb/TypeScript rules for JS/TS files
Files:
packages/js-evo-sdk/src/identities/facade.ts
packages/**/!(node_modules)/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames within JS packages
Files:
packages/js-evo-sdk/src/identities/facade.ts
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/wasm-sdk/src/queries/identity.rs
🧠 Learnings (21)
📓 Common learnings
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-04T09:08:48.470Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
Learnt from: shumkov
Repo: dashpay/platform PR: 2206
File: packages/rs-platform-version/src/version/protocol_version.rs:155-157
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the dashmate default configurations, the default protocol version for local networks has been removed, and an update mechanism for the protocol version in the consensus parameters has been implemented.
📚 Learning: 2024-11-06T07:27:01.722Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2314
File: packages/wallet-contract/test/bootstrap.js:14-16
Timestamp: 2024-11-06T07:27:01.722Z
Learning: In `packages/wallet-contract/test/bootstrap.js`, for Mocha tests in Node.js, async functions like `loadWasmDpp` can be assigned directly to `beforeAll` without wrapping them in another async function.
Applied to files:
packages/wasm-sdk/tests/karma/karma.functional.conf.cjspackages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2025-10-29T11:37:57.299Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2818
File: packages/dashmate/test/unit/templates/dynamicCompose.spec.js:1-4
Timestamp: 2025-10-29T11:37:57.299Z
Learning: In the dashpay/platform repository's dashmate package (packages/dashmate), test files do not need to import `expect` from chai because test/bootstrap.js sets it up globally with `global.expect = expect;`. The bootstrap file is automatically loaded by Mocha.
Applied to files:
packages/wasm-sdk/tests/karma/karma.functional.conf.cjspackages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2025-09-03T14:42:29.958Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/docs.html:1970-1971
Timestamp: 2025-09-03T14:42:29.958Z
Learning: In packages/wasm-sdk/, the docs.html file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in docs.html, as manual changes to docs.html would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/tests/karma/karma.functional.conf.cjs
📚 Learning: 2025-07-31T18:15:51.813Z
Learnt from: thephez
Repo: dashpay/platform PR: 2715
File: packages/wasm-sdk/test/ui-automation/run-ui-tests.sh:6-286
Timestamp: 2025-07-31T18:15:51.813Z
Learning: In packages/wasm-sdk/test/ui-automation/run-ui-tests.sh, the script doesn't require cleanup handling because it relies on Playwright's built-in webServer configuration to manage the HTTP server lifecycle automatically. The script only runs foreground processes through npm/Playwright commands.
Applied to files:
packages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2024-10-24T04:58:02.843Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2270
File: packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js:19-19
Timestamp: 2024-10-24T04:58:02.843Z
Learning: For operations involving Tenderdash where no client library exists yet, it's acceptable to use the Node.js built-in `crypto` module.
Applied to files:
packages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2024-10-18T15:37:36.387Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:77-78
Timestamp: 2024-10-18T15:37:36.387Z
Learning: In `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests to increase readability, rather than refactoring them into shared `beforeEach` blocks or helper functions.
Applied to files:
packages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2024-10-18T15:37:21.329Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2249
File: packages/dashmate/test/unit/status/scopes/platform.spec.js:466-467
Timestamp: 2024-10-18T15:37:21.329Z
Learning: In test files for the dashmate project, such as `packages/dashmate/test/unit/status/scopes/platform.spec.js`, prefer to keep mocks explicitly in tests rather than consolidating them, to increase readability.
Applied to files:
packages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2025-11-25T13:10:47.943Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T13:10:47.943Z
Learning: Applies to **/tests/**/*.{js,jsx,ts,tsx,rs} : Unit and integration tests should not perform network calls; mock dependencies
Applied to files:
packages/wasm-sdk/tests/functional-bootstrap.cjs
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
Applied to files:
packages/js-evo-sdk/src/identities/facade.tspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-08-05T13:55:39.147Z
Learnt from: thephez
Repo: dashpay/platform PR: 2718
File: packages/wasm-sdk/index.html:0-0
Timestamp: 2025-08-05T13:55:39.147Z
Learning: The get_identity_keys_with_proof_info function in the Rust WASM bindings does not support the "search" key request type and lacks the searchPurposeMap parameter. When proof mode is enabled with keyRequestType === 'search', the implementation falls back to the non-proof version (get_identity_keys) to maintain functionality.
Applied to files:
packages/js-evo-sdk/src/identities/facade.tspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-09-03T19:33:21.688Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/api-definitions.json:1285-1285
Timestamp: 2025-09-03T19:33:21.688Z
Learning: In packages/wasm-sdk/api-definitions.json, thephez prefers to keep the existing "ripemd160hash20bytes1234" placeholder for ECDSA_HASH160 data field in documentation examples rather than using a valid base64-encoded format, maintaining consistency with the previous documentation approach.
Applied to files:
packages/js-evo-sdk/src/identities/facade.tspackages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-02-10T11:26:36.709Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/wasm-sdk/src/verify.rs:26-68
Timestamp: 2025-02-10T11:26:36.709Z
Learning: In the wasm-sdk package, empty vectors and placeholder values are intentionally used in verification functions during the proof-of-concept stage to ensure proper compilation and type checking.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2024-10-03T11:51:06.980Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-03T11:51:06.980Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-09-03T14:41:16.196Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/AI_REFERENCE.md:766-766
Timestamp: 2025-09-03T14:41:16.196Z
Learning: In packages/wasm-sdk/, the AI_REFERENCE.md file is auto-generated from api-definitions.json. Any documentation fixes should be made in api-definitions.json rather than directly in AI_REFERENCE.md, as manual changes to AI_REFERENCE.md would be overwritten during regeneration.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WASM bindings to connect Rust and JavaScript code
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2024-11-03T10:38:00.762Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/info.rs:0-0
Timestamp: 2024-11-03T10:38:00.762Z
Learning: In this context, using `.unwrap()?` on the result of `root_hash` is acceptable and will not cause a panic.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/wasm-sdk/src/queries/identity.rs
🧬 Code graph analysis (1)
packages/js-evo-sdk/src/identities/facade.ts (2)
packages/js-evo-sdk/src/sdk.ts (1)
wasm(71-74).pnp.cjs (1)
bigint(22431-22431)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (wasm-dpp2) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (5)
packages/wasm-sdk/tests/functional-bootstrap.cjs (2)
1-2: LGTM: TLS override appropriate for functional tests with self-signed certificates.The approach is correct for local testing with self-signed certificates. The comment and file location clearly indicate test-only usage.
4-5: LGTM: Bootstrap loading is correct.Loading the shared bootstrap after setting environment variables ensures proper test initialization.
packages/wasm-sdk/tests/karma/karma.functional.conf.cjs (1)
7-13: LGTM: Bootstrap file reference updated consistently.The change from
bootstrap.cjstofunctional-bootstrap.cjsis applied correctly in both the files array and preprocessors configuration. The updated comment accurately describes the functional bootstrap's behavior.packages/js-evo-sdk/src/identities/facade.ts (1)
37-95: The wasm SDK methods and their return types are already correctly defined and aligned with the facade.ts signatures. All methods in packages/wasm-sdk/src/queries/identity.rs have explicitunchecked_return_typedeclarations that match the TypeScript facade methods:
getIdentityNonce,getIdentityBalance,getIdentityContractNoncereturnbigint | nullgetIdentitiesBalancesreturnsMap<Identifier, bigint | null>getIdentityBalanceAndRevisionreturnsIdentityBalanceAndRevision | nullgetIdentityByPublicKeyHashreturnsIdentity | null- All
*WithProofInfovariants returnProofMetadataResponseTyped<T>Functional tests confirm these types are working correctly. No verification needed.
packages/wasm-sdk/src/queries/identity.rs (1)
405-427: LGTM! Consistent refactoring to nullable and Map-based return types.The refactoring follows a clear, well-structured pattern across all identity query methods:
- Nullable types (
bigint | null,Identity | null,IdentityBalanceAndRevision | null) correctly useJsValue::NULLfor missing data- Map-based returns properly populate with nullable values
- BigInt conversions are correct for u64 values
- TypeScript type safety preserved via
unchecked_return_typeannotationsThis improves the API by providing more idiomatic JavaScript representations and explicit null handling.
Also applies to: 620-671, 684-743, 754-809, 820-872, 1099-1126, 1130-1178, 1182-1253
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
shumkov
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/wasm-sdk/tests/functional/groups.spec.mjs (1)
25-35: Comment doesn't match implementation: network errors will cause test failure.The comment states "These calls may fail in offline runs; permit network errors," but no error handling (try-catch) is implemented. If network errors occur, the test will fail rather than being permitted. Consider either:
- Adding try-catch blocks if network errors should truly be tolerated, or
- Removing the misleading comment if test failures on network errors are expected.
Additionally, the test lacks assertions to verify the returned data. While this may be intentional for smoke testing, consider adding basic checks (e.g., result structure validation) to increase test value.
🔎 Example: Add error handling if network errors should be tolerated
const { dpnsContractId: DPNS_CONTRACT, identityId: IDENTITY } = ( wasmFunctionalTestRequirements() ); - // These calls may fail in offline runs; permit network errors - await client.getIdentityGroups({ - identityId: IDENTITY, - memberDataContracts: [DPNS_CONTRACT], - }); - await client.getGroupMembers({ - dataContractId: DPNS_CONTRACT, - groupContractPosition: 0, - limit: 10, - }); + // These calls may fail in offline runs; permit network errors + try { + const groups = await client.getIdentityGroups({ + identityId: IDENTITY, + memberDataContracts: [DPNS_CONTRACT], + }); + // Optional: Add basic assertion + // assert(groups !== null, 'Expected groups result'); + } catch (error) { + // Tolerate network errors in offline runs + if (!error.message?.includes('network')) throw error; + } + + try { + const members = await client.getGroupMembers({ + dataContractId: DPNS_CONTRACT, + groupContractPosition: 0, + limit: 10, + }); + // Optional: Add basic assertion + // assert(members !== null, 'Expected members result'); + } catch (error) { + // Tolerate network errors in offline runs + if (!error.message?.includes('network')) throw error; + }packages/js-evo-sdk/src/sdk.ts (1)
164-164: Update JSDoc to include 'local' network option.The method signature at line 174 now accepts
'local'as a valid network option, but the JSDoc comment doesn't mention it.🔎 Proposed fix
- * @param network - Network identifier: 'mainnet', 'testnet' (default: 'testnet') + * @param network - Network identifier: 'mainnet', 'testnet', 'local' (default: 'testnet')
🧹 Nitpick comments (1)
packages/wasm-sdk/tests/functional/addresses.spec.mjs (1)
7-7: Remove or utilize the unused function call.The call to
wasmFunctionalTestRequirements()returns test data (identityId, dpnsContractId, etc.) that is not used anywhere in this test file. This test uses hardcoded genesis addresses instead.🔎 Proposed fix
- wasmFunctionalTestRequirements(); -
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/dashmate/configs/getConfigFileMigrationsFactory.jspackages/dashmate/docs/services/quorum_list.mdpackages/js-evo-sdk/src/sdk.tspackages/js-evo-sdk/tests/functional/epoch.spec.mjspackages/js-evo-sdk/tests/functional/protocol.spec.mjspackages/rs-dapi-client/Cargo.tomlpackages/wasm-sdk/tests/functional/addresses.spec.mjspackages/wasm-sdk/tests/functional/contracts.spec.mjspackages/wasm-sdk/tests/functional/fixtures/requiredTestData.mjspackages/wasm-sdk/tests/functional/groups.spec.mjs
💤 Files with no reviewable changes (2)
- packages/js-evo-sdk/tests/functional/protocol.spec.mjs
- packages/js-evo-sdk/tests/functional/epoch.spec.mjs
✅ Files skipped from review due to trivial changes (1)
- packages/dashmate/docs/services/quorum_list.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/wasm-sdk/tests/functional/contracts.spec.mjs
- packages/dashmate/configs/getConfigFileMigrationsFactory.js
- packages/wasm-sdk/tests/functional/fixtures/requiredTestData.mjs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use 2-space indent for JS/TS files
Use camelCase for variables and functions in JS/TS
Use PascalCase for class names in JS/TS
Use ESLint with Airbnb/TypeScript rules for JS/TS files
Files:
packages/js-evo-sdk/src/sdk.ts
packages/**/!(node_modules)/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames within JS packages
Files:
packages/js-evo-sdk/src/sdk.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: thephez
Repo: dashpay/platform PR: 2816
File: packages/js-dapi-client/lib/networkConfigs.js:17-40
Timestamp: 2025-10-23T13:04:39.160Z
Learning: For the testnet DAPI address whitelist in packages/js-dapi-client/lib/networkConfigs.js, nodes may be intentionally excluded even if they are ENABLED if they are running pre-2.1 versions. Version compatibility is an important filtering criterion beyond just enabled status.
📚 Learning: 2024-11-06T07:27:01.722Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2314
File: packages/wallet-contract/test/bootstrap.js:14-16
Timestamp: 2024-11-06T07:27:01.722Z
Learning: In `packages/wallet-contract/test/bootstrap.js`, for Mocha tests in Node.js, async functions like `loadWasmDpp` can be assigned directly to `beforeAll` without wrapping them in another async function.
Applied to files:
packages/wasm-sdk/tests/functional/addresses.spec.mjspackages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2025-09-03T19:33:21.688Z
Learnt from: thephez
Repo: dashpay/platform PR: 2754
File: packages/wasm-sdk/api-definitions.json:1285-1285
Timestamp: 2025-09-03T19:33:21.688Z
Learning: In packages/wasm-sdk/api-definitions.json, thephez prefers to keep the existing "ripemd160hash20bytes1234" placeholder for ECDSA_HASH160 data field in documentation examples rather than using a valid base64-encoded format, maintaining consistency with the previous documentation approach.
Applied to files:
packages/wasm-sdk/tests/functional/addresses.spec.mjs
📚 Learning: 2025-07-28T20:00:24.323Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/docs.html:2359-2383
Timestamp: 2025-07-28T20:00:24.323Z
Learning: In packages/wasm-sdk/docs.html, QuantumExplorer confirmed that placeholder private keys in documentation examples are acceptable as they are not real keys, though field name accuracy for the SDK API should still be maintained.
Applied to files:
packages/wasm-sdk/tests/functional/addresses.spec.mjspackages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2025-07-28T20:00:08.502Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
Applied to files:
packages/wasm-sdk/tests/functional/addresses.spec.mjspackages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2025-02-10T11:26:36.709Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/wasm-sdk/src/verify.rs:26-68
Timestamp: 2025-02-10T11:26:36.709Z
Learning: In the wasm-sdk package, empty vectors and placeholder values are intentionally used in verification functions during the proof-of-concept stage to ensure proper compilation and type checking.
Applied to files:
packages/wasm-sdk/tests/functional/addresses.spec.mjspackages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2024-10-29T10:42:00.521Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/Cargo.toml:22-22
Timestamp: 2024-10-29T10:42:00.521Z
Learning: In `packages/rs-dapi-client/Cargo.toml`, `backon` will not work without the `tokio-sleep` feature in our setup, so it's unnecessary to declare `tokio-sleep` as a separate feature in the `[features]` section.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2024-10-29T14:44:01.184Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-dapi-client/src/executor.rs:38-38
Timestamp: 2024-10-29T14:44:01.184Z
Learning: In `packages/rs-dapi-client/src/executor.rs`, for the structs `ExecutionError<E>` and `ExecutionResponse<R>`, only the serde derives (`Serialize`, `Deserialize`) should be conditional based on the "mocks" feature; the other derives (`Debug`, `Clone`, `Eq`, `PartialEq`) should always be present.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2024-10-22T10:53:12.111Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2259
File: packages/rs-dapi-client/src/dapi_client.rs:137-139
Timestamp: 2024-10-22T10:53:12.111Z
Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc<AtomicUsize>` is necessary for the retry counter.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-01-19T07:36:46.042Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` is currently only used in tests, and its WebAssembly implementation is deferred until there's a user request for it.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-05-28T16:22:26.334Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2644
File: packages/rs-drive/src/cache/system_contracts.rs:18-19
Timestamp: 2025-05-28T16:22:26.334Z
Learning: In packages/rs-drive/src/cache/system_contracts.rs, the `active_since_protocol_version` field in `ActiveSystemDataContract` struct is intentionally added for future use, not current implementation. QuantumExplorer confirmed this is "meant for later" when questioned about the `#[allow(unused)]` attribute.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2024-10-29T14:16:00.141Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/dapi-grpc/src/mock.rs:125-145
Timestamp: 2024-10-29T14:16:00.141Z
Learning: Using `serde_json` encoding produces deterministic output required to identify requests and responses by hash, therefore `serde_json` is used as the basic serialization algorithm for mocking.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2024-09-27T08:23:18.028Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2139
File: packages/rs-sdk/Cargo.toml:0-0
Timestamp: 2024-09-27T08:23:18.028Z
Learning: `bincode` is used in the `mocks` feature of the `rs-sdk`.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-01-23T09:23:32.740Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2405
File: packages/rs-sdk/src/sync.rs:88-95
Timestamp: 2025-01-23T09:23:32.740Z
Learning: The `block_on` function in `packages/rs-sdk/src/sync.rs` requires true blocking behavior, which is why a non-blocking WebAssembly implementation using `wasm_bindgen_futures::spawn_local` is not suitable.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WASM bindings to connect Rust and JavaScript code
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2024-10-18T15:39:51.172Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2254
File: packages/rs-sdk/src/sdk.rs:585-585
Timestamp: 2024-10-18T15:39:51.172Z
Learning: The 'platform' project uses Rust version 1.80, so code in 'packages/rs-sdk' can use features available in Rust 1.80, such as the `abs_diff()` method.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-11-25T13:10:23.481Z
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use WebAssembly as a bridge between Rust and JavaScript implementations
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-01-23T09:43:25.080Z
Learnt from: lklimek
Repo: dashpay/platform PR: 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.
Applied to files:
packages/rs-dapi-client/Cargo.toml
📚 Learning: 2025-07-31T18:15:51.813Z
Learnt from: thephez
Repo: dashpay/platform PR: 2715
File: packages/wasm-sdk/test/ui-automation/run-ui-tests.sh:6-286
Timestamp: 2025-07-31T18:15:51.813Z
Learning: In packages/wasm-sdk/test/ui-automation/run-ui-tests.sh, the script doesn't require cleanup handling because it relies on Playwright's built-in webServer configuration to manage the HTTP server lifecycle automatically. The script only runs foreground processes through npm/Playwright commands.
Applied to files:
packages/wasm-sdk/tests/functional/groups.spec.mjs
📚 Learning: 2024-10-30T11:19:59.163Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/tests/fetch/config.rs:233-233
Timestamp: 2024-10-30T11:19:59.163Z
Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.
Applied to files:
packages/wasm-sdk/tests/functional/groups.spec.mjs
🧬 Code graph analysis (3)
packages/wasm-sdk/tests/functional/addresses.spec.mjs (1)
packages/wasm-sdk/tests/functional/fixtures/requiredTestData.mjs (1)
wasmFunctionalTestRequirements(8-26)
packages/js-evo-sdk/src/sdk.ts (1)
packages/wasm-sdk/tests/unit/builder-with-addresses.spec.mjs (7)
builder(19-22)builder(33-40)builder(48-51)builder(59-62)builder(145-148)builder(159-162)builder(173-176)
packages/wasm-sdk/tests/functional/groups.spec.mjs (1)
packages/wasm-sdk/tests/functional/fixtures/requiredTestData.mjs (1)
wasmFunctionalTestRequirements(8-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Rust packages (rs-dapi-client) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (wasm-dpp2) / Formatting
- GitHub Check: Rust packages (wasm-dpp2) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp2) / Linting
- GitHub Check: Rust packages (rs-sdk-ffi) / Tests
- GitHub Check: Rust packages (wasm-dpp2) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Rust packages (wasm-sdk) / Unused dependencies
- GitHub Check: Rust packages (wasm-sdk) / Tests
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Rust crates security audit
- 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 JS packages / Build JS
🔇 Additional comments (7)
packages/rs-dapi-client/Cargo.toml (1)
34-34: LGTM: wasmbind feature correctly gated to wasm32 targets.The chrono dependency with
wasmbindfeature is appropriately placed under the wasm32-specific target configuration. This ensures that wasm-bindgen and js-sys integration is only pulled in for WebAssembly builds, while non-wasm targets use the base chrono dependency (line 61) without the additional overhead. This is the correct approach for conditional platform-specific features.packages/wasm-sdk/tests/functional/groups.spec.mjs (3)
2-2: LGTM: Dynamic fixture import improves maintainability.The import of
wasmFunctionalTestRequirementsenables dynamic test data sourcing, replacing hardcoded constants with centralized fixture data.
10-19: LGTM: Local network migration aligns with PR objectives.The setup correctly migrates from testnet to local network mode using
prefetchTrustedQuorumsLocal()andlocalTrusted()builder, consistent with the PR's goal of adding local network support for SDK functional tests.
37-40: LGTM: Consistent fixture usage.The second test correctly uses
wasmFunctionalTestRequirements()for dynamic test data, maintaining consistency with the first test.packages/js-evo-sdk/src/sdk.ts (3)
157-158: Previous review concern has been addressed.The earlier review comment flagged these methods as identical. The current implementation correctly differentiates them:
local()does not settrusted(defaults tofalseper line 55)localTrusted()explicitly setstrusted: trueThis is now consistent with the
testnet/mainnetpattern.
100-101: LGTM! Local network implementation follows established patterns.The local network handling correctly mirrors the mainnet/testnet implementation:
- Prefetches trusted quorums before building
- Uses the appropriate builder method based on the
trustedflag- Maintains consistency with existing network configuration patterns
Also applies to: 112-116
31-31: LGTM! Network type extension is correct.Adding
'local'to the network type union properly supports the new local network functionality introduced throughout this file.
| // Custom masternode addresses. When provided, network and trusted options are ignored. | ||
| // Example: ['https://127.0.0.1:1443', 'https://192.168.1.100:1443'] |
There was a problem hiding this comment.
Update comment to reflect that network is used.
The comment states "network and trusted options are ignored" when addresses are provided, but the code at lines 96-101 and 103 clearly uses the network option to determine which prefetch method to call and passes it to withAddresses(). Only the trusted option appears to be ignored in this path.
🔎 Proposed fix
- // Custom masternode addresses. When provided, network and trusted options are ignored.
+ // Custom masternode addresses. When provided, the network option is used for quorum prefetching,
+ // but the trusted option is ignored (trust mode is determined by the addresses configuration).
// Example: ['https://127.0.0.1:1443', 'https://192.168.1.100:1443']📝 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.
| // Custom masternode addresses. When provided, network and trusted options are ignored. | |
| // Example: ['https://127.0.0.1:1443', 'https://192.168.1.100:1443'] | |
| // Custom masternode addresses. When provided, the network option is used for quorum prefetching, | |
| // but the trusted option is ignored (trust mode is determined by the addresses configuration). | |
| // Example: ['https://127.0.0.1:1443', 'https://192.168.1.100:1443'] |
🤖 Prompt for AI Agents
In packages/js-evo-sdk/src/sdk.ts around lines 33-34, the comment incorrectly
says "network and trusted options are ignored" when custom masternode addresses
are provided; update the comment to state that only the trusted option is
ignored and that the network option is still used (it selects the prefetch
method and is passed to withAddresses()), so change the wording to reflect that
network is respected while trusted is not.
| it('getAddressInfo() returns address info for funded address', async () => { | ||
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | ||
| const res = await client.getAddressInfo(testAddress); | ||
| // PLATFORM_ADDRESS_1 has nonce=5, balance=1_000_000 | ||
| expect(res).to.be.ok(); | ||
| expect(res.nonce).to.equal(5n); | ||
| expect(res.balance).to.equal(1000000n); | ||
| }); |
There was a problem hiding this comment.
Fix Chai assertion syntax error.
Line 35 uses expect(res).to.be.ok() which is incorrect Chai syntax. The .ok is a property getter, not a method, so it should not be called with parentheses. This will cause a TypeError at runtime.
🔎 Proposed fix
it('getAddressInfo() returns address info for funded address', async () => {
const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1);
const res = await client.getAddressInfo(testAddress);
// PLATFORM_ADDRESS_1 has nonce=5, balance=1_000_000
- expect(res).to.be.ok();
+ expect(res).to.be.ok;
expect(res.nonce).to.equal(5n);
expect(res.balance).to.equal(1000000n);
});📝 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.
| it('getAddressInfo() returns address info for funded address', async () => { | |
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const res = await client.getAddressInfo(testAddress); | |
| // PLATFORM_ADDRESS_1 has nonce=5, balance=1_000_000 | |
| expect(res).to.be.ok(); | |
| expect(res.nonce).to.equal(5n); | |
| expect(res.balance).to.equal(1000000n); | |
| }); | |
| it('getAddressInfo() returns address info for funded address', async () => { | |
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const res = await client.getAddressInfo(testAddress); | |
| // PLATFORM_ADDRESS_1 has nonce=5, balance=1_000_000 | |
| expect(res).to.be.ok; | |
| expect(res.nonce).to.equal(5n); | |
| expect(res.balance).to.equal(1000000n); | |
| }); |
🤖 Prompt for AI Agents
In packages/wasm-sdk/tests/functional/addresses.spec.mjs around lines 31 to 38,
the Chai assertion uses the property `.ok` as a method
(`expect(res).to.be.ok()`), which throws a TypeError; change it to use the
property form without parentheses (e.g., `expect(res).to.be.ok`) or use an
equivalent assertion like `expect(res).to.exist` so the test checks presence
without calling the property as a function.
| it('getAddressInfoWithProofInfo() returns proof info for address query', async () => { | ||
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | ||
| const res = await client.getAddressInfoWithProofInfo(testAddress); | ||
| expect(res).to.be.ok(); | ||
| expect(res.proof).to.be.ok(); | ||
| expect(res.metadata).to.be.ok(); | ||
| // data should be present for funded address | ||
| expect(res.data).to.be.ok(); | ||
| expect(res.data.nonce).to.equal(5n); | ||
| expect(res.data.balance).to.equal(1000000n); | ||
| }); |
There was a problem hiding this comment.
Fix multiple Chai assertion syntax errors.
Lines 43, 44, 45, and 47 all use expect(...).to.be.ok() which is incorrect. The .ok property should not be called with parentheses.
🔎 Proposed fix
it('getAddressInfoWithProofInfo() returns proof info for address query', async () => {
const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1);
const res = await client.getAddressInfoWithProofInfo(testAddress);
- expect(res).to.be.ok();
- expect(res.proof).to.be.ok();
- expect(res.metadata).to.be.ok();
+ expect(res).to.be.ok;
+ expect(res.proof).to.be.ok;
+ expect(res.metadata).to.be.ok;
// data should be present for funded address
- expect(res.data).to.be.ok();
+ expect(res.data).to.be.ok;
expect(res.data.nonce).to.equal(5n);
expect(res.data.balance).to.equal(1000000n);
});📝 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.
| it('getAddressInfoWithProofInfo() returns proof info for address query', async () => { | |
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const res = await client.getAddressInfoWithProofInfo(testAddress); | |
| expect(res).to.be.ok(); | |
| expect(res.proof).to.be.ok(); | |
| expect(res.metadata).to.be.ok(); | |
| // data should be present for funded address | |
| expect(res.data).to.be.ok(); | |
| expect(res.data.nonce).to.equal(5n); | |
| expect(res.data.balance).to.equal(1000000n); | |
| }); | |
| it('getAddressInfoWithProofInfo() returns proof info for address query', async () => { | |
| const testAddress = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const res = await client.getAddressInfoWithProofInfo(testAddress); | |
| expect(res).to.be.ok; | |
| expect(res.proof).to.be.ok; | |
| expect(res.metadata).to.be.ok; | |
| // data should be present for funded address | |
| expect(res.data).to.be.ok; | |
| expect(res.data.nonce).to.equal(5n); | |
| expect(res.data.balance).to.equal(1000000n); | |
| }); |
🤖 Prompt for AI Agents
In packages/wasm-sdk/tests/functional/addresses.spec.mjs around lines 40 to 50,
several assertions use the wrong Chai syntax `expect(...).to.be.ok()`; change
each to use the .ok property (no parentheses) e.g. `expect(...).to.be.ok` (or
`expect(...).to.exist` if preferred) for lines 43, 44, 45, and 47 so the
assertions use the correct Chai property access instead of calling it as a
function.
| it('getAddressesInfos() returns map of address infos', async () => { | ||
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | ||
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | ||
| const testAddresses = [testAddress1, testAddress2]; | ||
|
|
||
| const res = await client.getAddressesInfos(testAddresses); | ||
| expect(res).to.be.instanceOf(Map); | ||
| expect(res.size).to.equal(testAddresses.length); | ||
|
|
||
| // Check values for both funded addresses | ||
| // Note: Map keys are stringified addresses, so we need to check values | ||
| const entries = Array.from(res.entries()); | ||
| expect(entries.length).to.equal(2); | ||
|
|
||
| // Find entries by checking their nonce values (5n and 7n - BigInt) | ||
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | ||
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | ||
| expect(info1).to.be.ok(); | ||
| expect(info1[1].balance).to.equal(1000000n); | ||
| expect(info2).to.be.ok(); | ||
| expect(info2[1].balance).to.equal(2000000n); | ||
| }); |
There was a problem hiding this comment.
Fix Chai assertion syntax errors.
Lines 69 and 71 use expect(...).to.be.ok() which is incorrect Chai syntax.
🔎 Proposed fix
// Find entries by checking their nonce values (5n and 7n - BigInt)
const info1 = entries.find(([, v]) => v && v.nonce === 5n);
const info2 = entries.find(([, v]) => v && v.nonce === 7n);
- expect(info1).to.be.ok();
+ expect(info1).to.be.ok;
expect(info1[1].balance).to.equal(1000000n);
- expect(info2).to.be.ok();
+ expect(info2).to.be.ok;
expect(info2[1].balance).to.equal(2000000n);
});📝 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.
| it('getAddressesInfos() returns map of address infos', async () => { | |
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | |
| const testAddresses = [testAddress1, testAddress2]; | |
| const res = await client.getAddressesInfos(testAddresses); | |
| expect(res).to.be.instanceOf(Map); | |
| expect(res.size).to.equal(testAddresses.length); | |
| // Check values for both funded addresses | |
| // Note: Map keys are stringified addresses, so we need to check values | |
| const entries = Array.from(res.entries()); | |
| expect(entries.length).to.equal(2); | |
| // Find entries by checking their nonce values (5n and 7n - BigInt) | |
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | |
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | |
| expect(info1).to.be.ok(); | |
| expect(info1[1].balance).to.equal(1000000n); | |
| expect(info2).to.be.ok(); | |
| expect(info2[1].balance).to.equal(2000000n); | |
| }); | |
| it('getAddressesInfos() returns map of address infos', async () => { | |
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | |
| const testAddresses = [testAddress1, testAddress2]; | |
| const res = await client.getAddressesInfos(testAddresses); | |
| expect(res).to.be.instanceOf(Map); | |
| expect(res.size).to.equal(testAddresses.length); | |
| // Check values for both funded addresses | |
| // Note: Map keys are stringified addresses, so we need to check values | |
| const entries = Array.from(res.entries()); | |
| expect(entries.length).to.equal(2); | |
| // Find entries by checking their nonce values (5n and 7n - BigInt) | |
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | |
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | |
| expect(info1).to.be.ok; | |
| expect(info1[1].balance).to.equal(1000000n); | |
| expect(info2).to.be.ok; | |
| expect(info2[1].balance).to.equal(2000000n); | |
| }); |
🤖 Prompt for AI Agents
In packages/wasm-sdk/tests/functional/addresses.spec.mjs around lines 52 to 73,
the Chai assertions on lines 69 and 71 incorrectly use `.to.be.ok()` (calling ok
as a function); change them to the correct property usage such as
`expect(info1).to.be.ok;` and `expect(info2).to.be.ok;` (or use
`expect(info1).to.exist;` / `expect(info2).to.exist;`) so the assertions are
valid.
| it('getAddressesInfosWithProofInfo() returns proof info for multiple addresses', async () => { | ||
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | ||
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | ||
| const testAddresses = [testAddress1, testAddress2]; | ||
|
|
||
| const res = await client.getAddressesInfosWithProofInfo(testAddresses); | ||
| expect(res).to.be.ok(); | ||
| expect(res.proof).to.be.ok(); | ||
| expect(res.metadata).to.be.ok(); | ||
| expect(res.data).to.be.instanceOf(Map); | ||
| expect(res.data.size).to.equal(2); | ||
|
|
||
| // Check data values (nonce is BigInt) | ||
| const entries = Array.from(res.data.entries()); | ||
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | ||
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | ||
| expect(info1).to.be.ok(); | ||
| expect(info1[1].balance).to.equal(1000000n); | ||
| expect(info2).to.be.ok(); | ||
| expect(info2[1].balance).to.equal(2000000n); | ||
| }); |
There was a problem hiding this comment.
Fix Chai assertion syntax errors.
Lines 91 and 93 use expect(...).to.be.ok() which is incorrect Chai syntax.
🔎 Proposed fix
// Check data values (nonce is BigInt)
const entries = Array.from(res.data.entries());
const info1 = entries.find(([, v]) => v && v.nonce === 5n);
const info2 = entries.find(([, v]) => v && v.nonce === 7n);
- expect(info1).to.be.ok();
+ expect(info1).to.be.ok;
expect(info1[1].balance).to.equal(1000000n);
- expect(info2).to.be.ok();
+ expect(info2).to.be.ok;
expect(info2[1].balance).to.equal(2000000n);
});📝 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.
| it('getAddressesInfosWithProofInfo() returns proof info for multiple addresses', async () => { | |
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | |
| const testAddresses = [testAddress1, testAddress2]; | |
| const res = await client.getAddressesInfosWithProofInfo(testAddresses); | |
| expect(res).to.be.ok(); | |
| expect(res.proof).to.be.ok(); | |
| expect(res.metadata).to.be.ok(); | |
| expect(res.data).to.be.instanceOf(Map); | |
| expect(res.data.size).to.equal(2); | |
| // Check data values (nonce is BigInt) | |
| const entries = Array.from(res.data.entries()); | |
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | |
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | |
| expect(info1).to.be.ok(); | |
| expect(info1[1].balance).to.equal(1000000n); | |
| expect(info2).to.be.ok(); | |
| expect(info2[1].balance).to.equal(2000000n); | |
| }); | |
| it('getAddressesInfosWithProofInfo() returns proof info for multiple addresses', async () => { | |
| const testAddress1 = sdk.PlatformAddress.fromP2pkhHash(testHash1); | |
| const testAddress2 = sdk.PlatformAddress.fromP2shHash(testHash2); | |
| const testAddresses = [testAddress1, testAddress2]; | |
| const res = await client.getAddressesInfosWithProofInfo(testAddresses); | |
| expect(res).to.be.ok(); | |
| expect(res.proof).to.be.ok(); | |
| expect(res.metadata).to.be.ok(); | |
| expect(res.data).to.be.instanceOf(Map); | |
| expect(res.data.size).to.equal(2); | |
| // Check data values (nonce is BigInt) | |
| const entries = Array.from(res.data.entries()); | |
| const info1 = entries.find(([, v]) => v && v.nonce === 5n); | |
| const info2 = entries.find(([, v]) => v && v.nonce === 7n); | |
| expect(info1).to.be.ok; | |
| expect(info1[1].balance).to.equal(1000000n); | |
| expect(info2).to.be.ok; | |
| expect(info2[1].balance).to.equal(2000000n); | |
| }); |
🤖 Prompt for AI Agents
In packages/wasm-sdk/tests/functional/addresses.spec.mjs around lines 75 to 95,
the Chai assertions use the invalid syntax `expect(...).to.be.ok()` on lines ~91
and ~93; change those to the correct Chai property usage `expect(...).to.be.ok`
(remove the parentheses) so the assertions run correctly.
Issue being fixed or feature implemented
What was done?
New Features
Improvements
How Has This Been Tested?
Breaking Changes
JS Evo SDK public API is changed
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.