fix(sdk)!: bigint for uint64 values #2443
Conversation
|
Warning Rate limit exceeded@pshenmic has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request updates data type definitions across multiple packages dealing with protocol buffers, gRPC clients, and WebAssembly bindings. Changes include converting numeric fields to string or BigInt representations in response metadata, updating serialization/deserialization methods, and modifying test assertions. New classes for handling data contract history and various status responses have been introduced. The modifications span JS and TypeScript clients, test suites, and Rust-based WASM modules, ensuring consistency and improved type safety. Changes
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (1)
packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/GetIdentityByPublicKeyHashResponse.spec.js (1)
98-101: Fix inconsistent BigInt usage in metadata assertions.While other test cases use BigInt for height and timeMs, this test case still uses regular number comparison. For consistency and to prevent potential precision loss, these assertions should also use BigInt.
Apply this diff to fix the inconsistency:
- expect(getIdentityResponse.getMetadata().getHeight()) - .to.equal(metadataFixture.height); - expect(getIdentityResponse.getMetadata().getCoreChainLockedHeight()) - .to.equal(metadataFixture.coreChainLockedHeight); + expect(getIdentityResponse.getMetadata().getHeight()) + .to.deep.equal(BigInt(metadataFixture.height)); + expect(getIdentityResponse.getMetadata().getCoreChainLockedHeight()) + .to.deep.equal(metadataFixture.coreChainLockedHeight); + expect(getIdentityResponse.getMetadata().getTimeMs()) + .to.deep.equal(BigInt(metadataFixture.timeMs)); + expect(getIdentityResponse.getMetadata().getProtocolVersion()) + .to.deep.equal(metadataFixture.protocolVersion);
🧹 Nitpick comments (11)
packages/js-dapi-client/lib/methods/platform/getIdentityContractNonce/GetIdentityContractNonceResponse.js (1)
8-16: Add type validation in the constructor.While the JSDoc indicates
biginttype, there's no runtime validation to ensure the input is actually a BigInt.Consider adding type validation:
constructor(identityContractNonce, metadata, proof = undefined) { super(metadata, proof); + if (typeof identityContractNonce !== 'bigint') { + throw new InvalidResponseError('identityContractNonce must be a BigInt'); + } this.identityContractNonce = identityContractNonce; }packages/js-dapi-client/lib/methods/platform/getStatus/TimeStatus.js (1)
14-18: Standardize null checks in constructor.The null checks in the constructor are inconsistent. The
epochcheck uses the OR operator whileblockandgenesisuse type checking.- this.local = local; - this.block = typeof block === 'bigint' ? block : null; - this.genesis = typeof genesis === 'bigint' ? genesis : null; - this.epoch = epoch || null; + this.local = local; + this.block = block ?? null; + this.genesis = genesis ?? null; + this.epoch = epoch ?? null;Note: This assumes that
undefinedandnullare equivalent for these fields. If you need to specifically handle non-bigint values, keep the current implementation forblockandgenesis.packages/js-dapi-client/lib/methods/platform/getStatus/getStatusFactory.js (1)
48-48: Consider adding type validation for the response.The response creation could benefit from type validation before returning.
- return GetStatusResponse.createFromProto(getStatusResponse); + const response = GetStatusResponse.createFromProto(getStatusResponse); + if (!(response instanceof GetStatusResponse)) { + throw new InvalidResponseError('Invalid response type'); + } + return response;packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts (1)
26-57: Consider adding type safety for the return value.The return type is currently
any. Consider defining a proper type for the contract history object.-): Promise<any> { +interface ContractHistory { + [timestamp: number]: DataContract; +} +): Promise<ContractHistory | null> {packages/js-dapi-client/lib/methods/platform/getStatus/StateSyncStatus.js (1)
60-72: Fix incorrect JSDoc comments.The JSDoc comments for
getSnapshotHeightandgetSnapshotChunkCountincorrectly state "Chunk process average time" instead of their actual return values.- * @returns {bigint} Chunk process average time + * @returns {bigint} Snapshot height */ getSnapshotHeight() { return this.snapshotHeight; } /** - * @returns {bigint} Chunk process average time + * @returns {bigint} Snapshot chunks count */ getSnapshotChunkCount() {packages/wasm-dpp/src/metadata.rs (1)
71-73: Simplify type casting.The type casting can be done directly in the struct initialization.
- pub fn core_chain_locked_height(&self) -> u32 { - self.0.core_chain_locked_height as u32 + pub fn core_chain_locked_height(&self) -> u32 { + u32::try_from(self.0.core_chain_locked_height).expect("Value exceeds u32::MAX")packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.ts (1)
50-55: Consider adding type safety for Metadata constructor parameters.While the change simplifies the code, it could be prone to parameter order mistakes. Consider using a builder pattern or maintaining the object literal approach with TypeScript interfaces for better type safety.
- metadata = new Metadata( - responseMetadata.getHeight(), - responseMetadata.getCoreChainLockedHeight(), - responseMetadata.getTimeMs(), - responseMetadata.getProtocolVersion(), - ); + metadata = new Metadata({ + height: responseMetadata.getHeight(), + coreChainLockedHeight: responseMetadata.getCoreChainLockedHeight(), + timeMs: responseMetadata.getTimeMs(), + protocolVersion: responseMetadata.getProtocolVersion(), + });packages/js-dapi-client/lib/methods/platform/getStatus/ChainStatus.js (1)
11-11: Consider using bigint for coreChainLockedHeight.For consistency with other height fields, consider using bigint for coreChainLockedHeight instead of number.
- * @param {number=} coreChainLockedHeight - core chain locked height + * @param {bigint=} coreChainLockedHeight - core chain locked heightAlso applies to: 32-32
packages/js-dapi-client/lib/methods/platform/getDataContractHistory/getDataContractHistoryFactory.js (1)
56-75: Improve error handling in retry mechanism.The retry mechanism catches InvalidResponseError but might miss type-specific errors related to bigint conversion.
Consider adding specific error handling for bigint conversion:
} catch (e) { - if (e instanceof InvalidResponseError) { + if (e instanceof InvalidResponseError || e instanceof TypeError) { lastError = e; } else { throw e;packages/js-dapi-client/lib/methods/platform/getStatus/GetStatusResponse.js (1)
68-161: Consider adding error handling for proto object validation.The
createFromProtomethod should validate the proto object and its nested properties before accessing them to prevent potential runtime errors.static createFromProto(proto) { + if (!proto || !proto.getV0()) { + throw new Error('Invalid proto object: missing v0 data'); + } const v0 = proto.getV0(); + + if (!v0.getVersion() || !v0.getNode() || !v0.getChain() + || !v0.getNetwork() || !v0.getStateSync() || !v0.getTime()) { + throw new Error('Invalid proto object: missing required status components'); + }packages/wasm-dpp/src/identity/identity.rs (1)
263-265: Consider updating get_public_key_max_id to use u64.For consistency with other method signatures in this file, consider updating this method to use u64 instead of f64.
- pub fn get_public_key_max_id(&self) -> f64 { - self.inner.get_public_key_max_id() as f64 + pub fn get_public_key_max_id(&self) -> u64 { + self.inner.get_public_key_max_id() as u64
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (73)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(89 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(38 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(89 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(18 hunks)packages/js-dapi-client/lib/SimplifiedMasternodeListProvider/createMasternodeListStreamFactory.js(1 hunks)packages/js-dapi-client/lib/methods/core/subscribeToMasternodeListFactory.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/DataContractHistoryEntry.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/getDataContractHistoryFactory.js(2 hunks)packages/js-dapi-client/lib/methods/platform/getEpochsInfo/EpochInfo.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getEpochsInfo/GetEpochsInfoResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityContractNonce/GetIdentityContractNonceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityNonce/GetIdentityNonceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/ChainStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/GetStatusResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/NetworkStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/NodeStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/StateSyncStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/TimeStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/VersionStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/getStatusFactory.js(2 hunks)packages/js-dapi-client/lib/methods/platform/response/AbstractResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/response/Metadata.js(2 hunks)packages/js-dapi-client/lib/test/fixtures/getMetadataFixture.js(1 hunks)packages/js-dapi-client/lib/test/fixtures/getStatusFixture.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContract/GetDataContractResponse.spec.js(4 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContract/getDataContractFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js(8 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getDocuments/GetDocumentsResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getDocuments/getDocumentsFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/GetEpochsInfoResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/getEpochsInfoFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/GetIdentitiesContractKeysResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/getIdentitiesContractKeysFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentity/GetIdentityResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentity/getIdentityFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/getIdentityBalanceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/GetIdentityByPublicKeyHashResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/getIdentityByPublicKeyHashFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/GetIdentityContractNonce.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/getIdentityContractNonceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/GetIdentityKeys.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/getIdentityKeysFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/GetIdentityNonce.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/getIdentityNonceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/GetProtocolVersionUpgradeStateResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/getProtocolVersionUpgradeStateFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/GetProtocolVersionUpgradeVoteStatusResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/getProtocolVersionUpgradeVoteStatusFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getStatus/getStatusFactory.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/waitForStateTransitionResult/waitForStateTransitionResultFactory.spec.js(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.spec.ts(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts(6 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/update.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/get.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/identities/get.ts(1 hunks)packages/js-dash-sdk/tsconfig.json(1 hunks)packages/wasm-dpp/src/data_contract/data_contract.rs(1 hunks)packages/wasm-dpp/src/document/document_facade.rs(1 hunks)packages/wasm-dpp/src/document/factory.rs(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs(2 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/mod.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/fee/balance_is_not_enough_error.rs(1 hunks)packages/wasm-dpp/src/identity/identity.rs(1 hunks)packages/wasm-dpp/src/identity/state_transition/identity_credit_transfer_transition/transition.rs(3 hunks)packages/wasm-dpp/src/metadata.rs(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/js-dapi-client/lib/SimplifiedMasternodeListProvider/createMasternodeListStreamFactory.js
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/get.ts
- packages/wasm-dpp/src/document/document_facade.rs
🔇 Additional comments (277)
packages/js-dapi-client/lib/methods/platform/getStatus/NetworkStatus.js (2)
13-32: LGTM! Well-documented getter methods.The getter methods are consistently named, well-documented with JSDoc, and correctly implemented.
34-35: LGTM! Correct module export.The class is properly exported.
packages/js-dapi-client/lib/methods/platform/getIdentityContractNonce/GetIdentityContractNonceResponse.js (2)
4-4: Verify if the 40-bit bitmask is sufficient.The bitmask
0xFFFFFFFFFFonly covers 40 bits, which might be insufficient for filtering u64 values that require 64 bits.Please confirm if this is intentional or if it should be updated to
0xFFFFFFFFFFFFFFFFfor full u64 coverage.
19-23: LGTM!The return type update is consistent with the BigInt changes.
packages/js-dapi-client/lib/methods/platform/getEpochsInfo/GetEpochsInfoResponse.js (1)
44-44: LGTM! Converting to BigInt prevents precision loss.The conversion of
firstBlockHeightandstartTimeto BigInt is appropriate for handling large numeric values without precision loss.Please ensure that:
- The
EpochInfoconstructor has been updated to handle BigInt parameters- Tests have been updated to work with BigInt values
- Consider if
feeMultiplier(line 47) should also be converted to BigIntAlso applies to: 46-46
✅ Verification successful
BigInt Conversions Verified
- The conversion of
epoch.getFirstBlockHeight()andepoch.getStartTime()to BigInt in the response is appropriate, and theEpochInfoconstructor (as seen in its JSDoc) now correctly accepts BigInt values for these fields.- No explicit test files for
EpochInfowere found with the search pattern; please verify that any tests covering epoch metadata are updated to work with BigInt.- Additionally, consider if the
feeMultiplier(at line 47) should also be converted to BigInt, depending on its precision requirements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if EpochInfo class and tests have been updated for BigInt # Find EpochInfo class definition echo "Checking EpochInfo class definition:" rg -A 10 "class EpochInfo" packages/js-dapi-client/ # Find related test files echo -e "\nChecking test files:" fd "EpochInfo.*spec" packages/js-dapi-client/Length of output: 1358
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (87)
4667-4670: Switch from numeric to string fields is correct to preserve 64-bit integer precision.
4710-4713: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
4722-4725: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
4763-4764: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
4784-4785: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
4809-4821: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
4863-4875: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
7911-7911: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
7951-7951: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
7993-7993: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8021-8032: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8363-8366: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8403-8405: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8445-8450: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8473-8484: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8815-8818: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8855-8857: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8897-8902: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
8925-8936: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
9409-9410: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
9448-9455: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
9485-9493: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
9503-9535: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
13998-13999: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
14040-14042: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
14080-14084: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
14133-14146: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
15755-15756: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
15797-15800: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
15836-15841: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
15890-15901: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
20525-20527: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
20578-20580: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
20638-20642: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
20772-20785: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
21182-21185: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
21221-21224: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
21258-21262: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
21276-21289: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
28907-28910: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
28953-28964: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
29009-29013: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
29023-29027: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
29066-29079: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
29102-29115: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31170-31172: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31209-31212: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31246-31250: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31264-31277: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31330-31332: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31369-31372: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31406-31410: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
31424-31437: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
32023-32025: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
32062-32065: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
32099-32103: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
32117-32129: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33725-33728: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33774-33785: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33833-33837: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33847-33851: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33952-33965: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
33988-34002: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
38738-38741: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
38778-38781: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
38820-38825: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
38848-38859: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
39496-39499: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
39536-39539: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
39578-39583: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
39606-39617: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42427-42429: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42468-42479: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42513-42518: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42542-42574: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42596-42610: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42930-42935: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
42985-43004: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43056-43063: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43080-43091: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43207-43211: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43309-43313: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43317-43340: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43601-43609: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43646-43677: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43707-43760: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.
43767-43907: Same approach as above—string-based uint64 reading/writing is consistent and avoids precision loss.packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (89)
4667-4670: Switch from numeric to string fields
These changes (height, timeMs) ensure 64-bit safety in JavaScript. Make sure surrounding code handles string values correctly.
4710-4710: Use of readUint64String
Switching toreadUint64String()preserves full 64-bit values. Ensure consumers treat the field as a string or BigInt as needed.
4722-4722: Use of readUint64String
Again, this approach avoids precision loss for timeMs.
4763-4764: Conditional writing of string field
parseInt(f, 10) !== 0is a standard check to skip writing a zero value.
4784-4785: Conditional writing of string field
Similar zero-check pattern ensures no redundant serialization of timeMs.
4809-4821: Getter/Setter to string
Changing the doc and return type to string is consistent with preserving 64-bit values.
4863-4871: Getter/Setter to string
These lines align the timeMs field with 64-bit string usage.
7911-7911: String-based nonce
Using string for identityNonce is correct to avoid numeric overflow.
7951-7951: readUint64String
Reading identityNonce as a string ensures full precision.
7993-7993: Uint64 string assignment
Writing the nonce as a string is consistent with the change.
8021-8021: Getter/Setter for identityNonce
Returning a string is correct given Protojstype=JS_STRING.
8363-8363: String-based identityContractNonce
Same approach for contract nonce: no precision loss.
8403-8403: readUint64String
Ensures identityContractNonce is parsed as a full 64-bit string.
8445-8445: Uint64 string assignment
Consistently writing contract nonce value as a string.
8473-8473: Getter/Setter for identityContractNonce
Correctly returning and setting string values.
8815-8815: String-based balance
Preserving full 64-bit balance prevents overflow.
8855-8855: readUint64String
Reading the balance as a string.
8897-8897: Uint64 string assignment
Serializing balance as a string.
8925-8925: Getter/Setter for balance
Returning a string for balance is consistent with the PR’s objective.
9409-9410: Balance and Revision fields
Both switched to string to handle large numeric ranges.
9448-9452: readUint64String for balance & revision
Avoids losing precision on retrieval.
9485-9493: Conditional write of string fields
Using parseInt checks andwriteUint64Stringto safely handle zero-value.
9503-9533: Getter/Setter for balance & revision
All references updated to string-based fields.
13998-13998: EvonodeProposedBlocks: count as string
Maintains 64-bit accuracy for block counts.
14040-14040: readUint64String for count
Ensures no integer overflow.
14080-14081: Conditional write
Skipping zero for count in serialization is standard PB pattern.
14133-14145: Getter/Setter for count
String-based approach is consistent with the rest of the PR.
15755-15755: GetIdentitiesBalancesResponse: string balance
Prevents overflow on large identity balances.
15797-15797: Uint64 string read
Reading the balance as a string.
15836-15838: Conditional write
Only writes a nonzero balance field.
15890-15901: Getter/Setter for balance
Consistent with the PB approach for 64-bit fields as strings.
20525-20525: startAtMs as string
Ensures time fields are not truncated.
20578-20578: readUint64String
Reading startAtMs as string to avoid numeric limits.
20638-20639: Conditional write of startAtMs
Checking parseInt for zero.
20772-20784: Getter/Setter for startAtMs
Properly storing time-based fields as strings.
21182-21182: date field as string
Keeps large timestamps intact.
21221-21221: readUint64String for date
Full 64-bit preservation for date.
21258-21259: Conditional write
Skipping zero date if parseInt returns 0.
21276-21288: Getter/Setter for date
Allows data contract history timestamps as strings.
28907-28909: Epoch info fields
Converting firstBlockHeight & startTime to strings.
28953-28961: readUint64String
Safely retrieves large epoch values for firstBlockHeight & startTime.
29009-29010: Conditional write
Omits default zero from serialization for firstBlockHeight.
29023-29024: Conditional write
Same skipping approach for startTime.
29066-29078: Getter/Setter for firstBlockHeight
Storing as string ensures no overflow.
29102-29114: Getter/Setter for startTime
Again, string-based approach for large numeric fields.
31170-31170: startTimeMs
Ensuring poll times remain accurate for large values.
31209-31209: readUint64String
Reading poll startTimeMs as string.
31246-31247: Conditional write
Skipping zero for poll startTimeMs.
31264-31276: Getter/Setter for poll startTimeMs
Maintaining 64-bit timestamps as strings.
31330-31330: endTimeMs as string
Preserves full precision for poll end times.
31369-31369: readUint64String
Reading endTimeMs without losing numeric precision.
31406-31407: Conditional write
Conditionally serializing endTimeMs if nonzero.
31424-31436: Getter/Setter for endTimeMs
Ensures accurate poll end time in string form.
32023-32023: timestamp as string
VotePollsByTimestamp uses strings for large time values.
32062-32062: readUint64String
Reading poll timestamp from binary as string.
32099-32100: Conditional write
Skipping zero poll timestamp.
32117-32129: Getter/Setter for poll timestamp
Full 64-bit coverage with string-based fields.
33725-33727: FinishedVoteInfo with block heights/times
Strings for blockHeight/day/timeMs to avoid overflow.
33774-33782: readUint64String
Reading finishedAtBlockHeight and finishedAtBlockTimeMs as string.
33833-33834: Conditional write for finishedAtBlockHeight
Standard parseInt check.
33847-33848: Conditional write for finishedAtBlockTimeMs
Same approach to skip zero.
33952-33964: Getter/Setter for finishedAtBlockHeight
String for block height plus updated doc.
33988-34000: Getter/Setter for finishedAtBlockTimeMs
String-based time ensures full precision.
38738-38738: Prefunded specialized balance
Storing as string to maintain large balances.
38778-38778: readUint64String
Extracting balance from binary as a string.
38820-38821: Conditional write
Only writes the balance if nonzero.
38848-38859: Getter/Setter for specialized balance
Maintaining string-based representation.
39496-39496: Total credits in platform
Switch to string for the credits field.
39536-39536: readUint64String
Reading credits without overflow.
39578-39579: Conditional write
Avoid serializing zero credits field.
39606-39617: Getter/Setter for credits
String-based approach consistent with large integer handling in JS.
42427-42429: Time fields (local, block, genesis)
Using strings to prevent potential overflow for large timestamps.
42468-42476: readUint64String
Retrieving local, block, genesis as strings.
42513-42528: Conditional writes for time fields
Skipping zero if parseInt results in 0.
42545-42574: Getter/Setter for time fields
All time fields use strings for 64-bit safety.
42599-42610: Getter/Setter for genesis
Retaining string-based usage for large timestamps.
42930-42934: Chain fields to strings
latestBlockHeight, earliestBlockHeight, maxPeerBlockHeight switched to string.
42985-42985: readUint64String for latestBlockHeight
No numeric truncation.
42997-43001: readUint64String for earliestBlockHeight & maxPeerBlockHeight
Same approach to maintain large numeric values.
43059-43060: Conditional write
Skipping zero for latestBlockHeight.
43080-43088: Conditional write
Same zero-skip for earliestBlockHeight, maxPeerBlockHeight.
43207-43219: Getter/Setter for chain’s block heights
Ensuring string usage for large block heights.
43309-43339: All chain getters/setters
EarliestBlockHeight & MaxPeerBlockHeight are stored as strings.
43601-43608: StateSync fields
Switching totalSyncedTime, remainingTime, etc. to strings.
43646-43674: readUint64String for multiple StateSync fields
ChunkProcessAvgTime, snapshotHeight, etc. handled as strings with parseInt checks.
43707-43715: Conditional write
Similarly skipping zero for totalSyncedTime, remainingTime.
43728-43757: Conditional writes for multiple fields
Ensures zero-value fields are not redundantly serialized.
43767-43797: Continuation of conditional writes
Chunk process time, snapshot blocks, all safe as strings.
43821-43904: Getter/Setter for StateSync
All relevant time/block fields use strings for 64-bit fidelity.packages/js-dapi-client/lib/methods/platform/getStatus/NodeStatus.js (1)
1-24: LGTM! Well-structured class implementation.The class follows best practices with:
- Clear JSDoc annotations
- Proper handling of optional parameters
- Consistent getter method naming
packages/js-dapi-client/lib/methods/platform/getDataContractHistory/DataContractHistoryEntry.js (1)
1-24: LGTM! Well-structured class implementation.The class follows best practices with:
- Clear JSDoc annotations
- Proper use of bigint for timestamps
- Consistent getter method naming
packages/js-dapi-client/lib/methods/platform/getEpochsInfo/EpochInfo.js (2)
5-5: LGTM! Type conversion aligns with PR objectives.The conversion of
firstBlockHeightandstartTimeparameters fromnumbertobiginthelps prevent precision loss for large values.Also applies to: 7-7
26-26: LGTM! Return type updates maintain type safety.The return type updates in JSDoc for
getFirstBlockHeightandgetStartTimecorrectly reflect the bigint type conversion.Also applies to: 40-40
packages/js-dapi-client/lib/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.js (2)
6-6: LGTM! Type conversion ensures precision for balance values.The conversion of balance from
numbertobigintin both parameter and return types helps prevent precision loss for large balance values.Also applies to: 17-17
28-28: LGTM! Safe conversion to BigInt.The explicit conversion using
BigInt()increateFromProtoensures safe handling of large values from the protocol buffer response.packages/js-dapi-client/lib/methods/platform/response/Metadata.js (3)
4-6: LGTM! Flexible parameter types with safe conversion.The parameter type update to accept both
bigintandstringprovides flexibility while maintaining type safety through explicitBigIntconversion.
10-12: LGTM! Safe conversion in constructor.The constructor correctly converts input values to
BigInt, ensuring consistent type handling throughout the class.
18-18: LGTM! Return type updates maintain type safety.The return type updates in JSDoc for
getHeightandgetTimeMscorrectly reflect the bigint type conversion.Also applies to: 34-34
packages/js-dapi-client/lib/methods/platform/getIdentityNonce/GetIdentityNonceResponse.js (4)
4-4: LGTM! Proper use of BigInt for large integer filter.The change from number to BigInt ensures accurate handling of large u64 values without precision loss.
8-8: LGTM! Correct type documentation.The JSDoc update properly reflects the parameter type change to bigint.
19-19: LGTM! Return type properly documented.The JSDoc update correctly reflects the return type change to bigint.
31-32: Verify proto value handling for large numbers.The conversion to BigInt and bitwise filtering looks correct, but let's verify the proto implementation handles u64 values properly.
✅ Verification successful
Proto handling is correct.
The identity nonce is declared as a uint64 with
[jstype = JS_STRING], ensuring that large numbers are transmitted as strings which are then safely converted to BigInt. The bitwise AND operation in the conversion is valid and properly handles the u64 value.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check proto definition for identity nonce field rg -A 5 "identity_nonce" packages/dapi-grpc/protos/platform/v0/platform.proto # Check if jstype is properly set for u64 fields rg "jstype.*JS_STRING.*uint64" packages/dapi-grpc/protos/platform/v0/platform.protoLength of output: 287
packages/js-dash-sdk/src/SDK/Client/Platform/methods/identities/get.ts (1)
34-40: LGTM! Improved Metadata instantiation.The change from object literal to direct constructor arguments improves type safety and makes the code more maintainable.
packages/js-dapi-client/lib/methods/platform/response/AbstractResponse.js (1)
53-58: LGTM! Consistent Metadata instantiation pattern.The change follows the same pattern as seen in other files, using structured object initialization for Metadata.
packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js (1)
7-7: LGTM! Improved type safety with DataContractHistoryEntry.The change from object to typed array improves type safety and code clarity.
Also applies to: 18-18
packages/wasm-dpp/src/errors/consensus/fee/balance_is_not_enough_error.rs (2)
32-34: LGTM! Improved type safety by using u64.The change from f64 to u64 for balance prevents precision loss and aligns with the PR objectives.
37-39: Verify the fee type cast.The
as u64cast might need verification to ensure no precision loss occurs during conversion.✅ Verification successful
I'm going to run some additional commands to inspect the implementation details in the file and to locate the declaration of the fee() method. This will help us verify the underlying fee type and whether the conversion performed by
as u64is safe (i.e. that no precision loss will occur).
I'm going to run additional shell scripts to search for the underlying definition of the
Creditstype and for the full implementation ofBalanceIsNotEnoughError, which should help us verify whether theas u64cast is safe with respect to precision loss.
Verification of the fee type cast is correct.
- The underlying
fee()method returns a value of typeCredits, which is defined asu64inpackages/rs-dpp/src/balances/credits.rs.- Since both the original type and the target type are
u64, converting withas u64will not result in any precision loss.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for fee() method implementation to verify its return type ast-grep --pattern 'impl BalanceIsNotEnoughError { $$$ fn fee(&self) -> $type { $$$ } $$$ }'Length of output: 106
Script:
#!/bin/bash # 1. Print the beginning of the file to inspect the struct and its inner field. sed -n '1,150p' packages/wasm-dpp/src/errors/consensus/fee/balance_is_not_enough_error.rs # 2. Search across the repository for any definition of `fn fee(&self)` to find its return type. rg "fn fee(&self)" -t rustLength of output: 1818
Script:
#!/bin/bash echo "Searching for the Credits type definition:" rg "pub type Credits" -t rust -C 3 echo "Searching for BalanceIsNotEnoughError implementation and fee() method:" rg "impl BalanceIsNotEnoughError" -t rust -C 20Length of output: 9766
packages/js-dapi-client/lib/methods/platform/getStatus/getStatusFactory.js (1)
9-9: LGTM! Well-documented changes with proper error handling.The updated return types and JSDoc comments accurately reflect the new functionality.
Also applies to: 13-13, 17-20
packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts (1)
15-17: LGTM! Proper type safety for large integers.The change from number to bigint for startAtMs prevents precision loss and aligns with the PR objectives.
Also applies to: 23-23
packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/update.ts (1)
34-35: Verify nonce type consistency across the codebase.The removal of explicit
BigInt()conversion suggests thatidentityContractNonceis already a BigInt. Let's verify this assumption.✅ Verification successful
Nonce Type Consistency Verified
- The
NonceManager’sbumpIdentityContractNoncemethod performs addition using the BigInt literal (+ 1n), which implies that the nonce is expected to be a BigInt.- The returned nonce from
getIdentityContractNonce(via the dapi-client’sGetIdentityContractNonceResponse) is converted to BigInt before being utilized.- Although some test mocks resolve the nonce as a number (e.g.
1), the production code consistently operates on BigInt values, making the removal of the explicitBigInt()conversion inupdate.tscorrect.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for bumpIdentityContractNonce return type and usage ast-grep --pattern 'bumpIdentityContractNonce($$$)' # Search for other nonce-related methods rg -A 5 'identityContractNonce'Length of output: 42560
packages/js-dapi-client/lib/methods/platform/getStatus/VersionStatus.js (1)
1-77: LGTM! Well-structured class with proper documentation.The class is well-designed with:
- Clear JSDoc documentation
- Proper getter methods
- Appropriate use of number type for protocol versions
packages/wasm-dpp/src/metadata.rs (1)
37-45: LGTM! Constructor changes improve type safety.The explicit type parameters instead of JsValue improve type safety and align with the PR objective to fix uint64 handling.
packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts (2)
4-7: LGTM! Type changes align with uint64 handling requirements.The conversion from
numbertobigintfor thevalueproperty inNonceStatetype and thenonceparameter insetIdentityNoncemethod is appropriate for handling uint64 values without precision loss.Also applies to: 26-38
40-72: LGTM! Proper handling of bigint nonce values.The method correctly handles bigint values throughout its execution, maintaining precision for uint64 nonce values.
packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/GetIdentityNonce.spec.js (1)
25-25: LGTM! Tests properly validate bigint handling.The test cases have been correctly updated to use BigInt for nonce values and metadata assertions, ensuring proper validation of uint64 handling.
Also applies to: 104-114
packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.spec.js (1)
26-26: LGTM! Tests properly validate bigint balance handling.The test cases have been correctly updated to use BigInt for balance values and metadata assertions, ensuring proper validation of uint64 handling.
Also applies to: 104-113
packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/GetIdentityKeys.spec.js (1)
109-116: LGTM! Tests properly validate bigint metadata fields.The test cases have been correctly updated to use BigInt for metadata assertions, ensuring proper validation of uint64 handling in height and timeMs fields.
packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/getIdentityNonceFactory.spec.js (2)
28-28: LGTM! Proper handling of nonce as BigInt.The conversion of nonce to BigInt aligns with the PR objective to handle u64 values correctly.
85-92: LGTM! Enhanced metadata assertions with proper type handling.The granular assertions for metadata properties with explicit BigInt conversions improve type safety and test precision.
Also applies to: 121-128
packages/js-dapi-client/test/unit/methods/platform/getDocuments/GetDocumentsResponse.spec.js (2)
39-40: LGTM! Complete metadata setup.Added missing metadata fields (timeMs and protocolVersion) for comprehensive testing.
87-94: LGTM! Enhanced metadata assertions with proper type handling.The granular assertions for metadata properties with explicit BigInt conversions improve type safety and test precision.
Also applies to: 113-120
packages/js-dapi-client/test/unit/methods/platform/getIdentity/getIdentityFactory.spec.js (1)
87-94: LGTM! Enhanced metadata assertions with proper type handling.The granular assertions for metadata properties with explicit BigInt conversions improve type safety and test precision.
Also applies to: 123-130
packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/getIdentityBalanceFactory.spec.js (3)
30-30: LGTM! Proper handling of balance as BigInt.The conversion of balance to BigInt aligns with the PR objective to handle u64 values correctly.
88-95: LGTM! Enhanced metadata assertions with proper type handling.The granular assertions for metadata properties with explicit BigInt conversions improve type safety and test precision.
Also applies to: 124-131
122-122: LGTM! Proper handling of default balance as BigInt.The assertion correctly expects a BigInt(0) for undefined balance.
packages/js-dapi-client/lib/methods/platform/getStatus/GetStatusResponse.js (1)
1-67: LGTM! Well-structured class with proper encapsulation.The class follows good OOP principles with clear separation of concerns, proper encapsulation through getter methods, and thorough JSDoc documentation.
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs (1)
20-20: LGTM! Improved type safety with Option.The change from
JsValuetoOption<Revision>leverages Rust's type system better and provides clearer semantics about the possibility of missing revisions.Also applies to: 63-64
packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/GetIdentityContractNonce.spec.js (1)
25-25: LGTM! Consistent use of BigInt for uint64 values.The changes properly handle uint64 values by using BigInt, aligning with the PR's objective to prevent precision loss. The assertions are updated consistently to use BigInt comparisons.
Also applies to: 105-114
packages/js-dapi-client/test/unit/methods/platform/getDataContract/getDataContractFactory.spec.js (1)
88-95: LGTM! Consistent BigInt assertions for metadata fields.The test assertions are properly updated to use BigInt for uint64 values in metadata fields, maintaining consistency with the PR's objective to handle large numbers correctly.
Also applies to: 129-136
packages/js-dapi-client/test/unit/methods/platform/getIdentity/GetIdentityResponse.spec.js (1)
82-89: LGTM! Proper handling of uint64 values.The test now correctly validates uint64 fields (height and timeMs) using BigInt, preventing potential precision loss.
packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/getIdentityContractNonceFactory.spec.js (1)
29-29: LGTM! Proper initialization of nonce as BigInt.The nonce value is now correctly initialized as BigInt(1), preventing potential precision loss.
packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/getIdentityByPublicKeyHashFactory.spec.js (1)
92-99: LGTM! Proper validation of uint64 metadata fields.The test now correctly validates uint64 fields (height and timeMs) using BigInt, preventing potential precision loss.
packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/getProtocolVersionUpgradeStateFactory.spec.js (1)
91-98: LGTM! Proper handling of uint64 values using BigInt.The changes correctly update the metadata assertions to use BigInt for
heightandtimeMsfields, ensuring proper handling of uint64 values without precision loss.Also applies to: 127-134
packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/getIdentityKeysFactory.spec.js (1)
99-106: LGTM! Consistent BigInt handling for uint64 values.The changes maintain consistency with other files by properly handling uint64 values using BigInt for
heightandtimeMsfields in metadata assertions.Also applies to: 138-145
packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts (1)
131-134: LGTM! Clear parameter documentation.The JSDoc comments clearly document the parameter types, making it easy for developers to understand the expected types.
packages/js-dapi-client/test/unit/methods/platform/waitForStateTransitionResult/waitForStateTransitionResultFactory.spec.js (1)
58-65: LGTM! Comprehensive BigInt handling across test cases.The changes consistently update all metadata assertions across different test cases to properly handle uint64 values using BigInt, maintaining test coverage while improving type safety.
Also applies to: 100-107, 149-156
packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/getEpochsInfoFactory.spec.js (2)
30-30: LGTM: Proper BigInt usage for large integersThe initialization of
epochInfoFixturecorrectly usesBigIntfor values that could exceed JavaScript's Number.MAX_SAFE_INTEGER.
95-102: LGTM: Consistent BigInt assertions for metadata fieldsThe test assertions properly validate that height and timeMs are handled as BigInt values while maintaining regular number comparisons for other fields.
Also applies to: 134-141
packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/GetProtocolVersionUpgradeStateResponse.spec.js (1)
114-121: LGTM: Proper metadata field type assertionsThe test correctly validates that height and timeMs are handled as BigInt values while maintaining regular number comparisons for coreChainLockedHeight and protocolVersion.
packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/GetEpochsInfoResponse.spec.js (2)
26-26: LGTM: Proper BigInt initializationThe initialization of
epochInfoFixturecorrectly usesBigIntfor values that could exceed JavaScript's Number.MAX_SAFE_INTEGER.
83-90: LGTM: Comprehensive BigInt assertionsThe test cases thoroughly validate BigInt handling for metadata fields across different scenarios, ensuring consistent type handling.
Also applies to: 98-105, 125-132
packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/GetProtocolVersionUpgradeVoteStatusResponse.spec.js (1)
117-124: LGTM: Proper metadata field type assertionsThe test correctly validates that height and timeMs are handled as BigInt values while maintaining regular number comparisons for coreChainLockedHeight and protocolVersion.
packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/getProtocolVersionUpgradeVoteStatusFactory.spec.js (1)
101-108: LGTM! Test assertions updated to handle BigInt values.The test assertions have been correctly updated to handle
u64values usingBigIntforheightandtimeMsfields, while maintaining regular number comparisons for other fields. This aligns with the PR objectives.packages/js-dapi-client/test/unit/methods/platform/getDataContract/GetDataContractResponse.spec.js (1)
66-67: LGTM! Test coverage expanded for metadata fields.The test has been properly updated to:
- Set new metadata fields (
timeMsandprotocolVersion)- Assert BigInt values for numeric fields
Also applies to: 79-86
packages/js-dapi-client/test/unit/methods/platform/getDocuments/getDocumentsFactory.spec.js (1)
113-120: LGTM! Consistent BigInt handling across test cases.The test assertions have been systematically updated across all test cases to handle
u64values using BigInt, maintaining consistency throughout the test suite.Also applies to: 154-161, 195-202
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js (3)
45-52: LGTM! Clear and structured history fixture.The data contract history fixture has been well-structured with explicit
dateandvalueproperties, improving test readability.
129-136: LGTM! Consistent BigInt handling for metadata.The metadata assertions have been properly updated to handle
u64values using BigInt, maintaining consistency with other tests.Also applies to: 173-180
165-165: Verify the change in empty result representation.The test now expects
nullinstead of an empty array when no history is available. Please verify if this change is intentional and aligns with the API contract.✅ Verification successful
Review Verification: Empty Result Representation
The tests now consistently expect that when no data contract history is available the response represents it as
nullrather than an empty array. In various test scenarios (e.g. in both GetDataContractHistoryResponse and getDataContractHistoryFactory tests), the expectation is explicitly set tonull. This indicates that the change was intentional and is aligned with the API contract.No issues were found with this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of getDataContractHistory result handling rg -A 5 'getDataContractHistory\(\)' --type jsLength of output: 10483
packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/getIdentitiesContractKeysFactory.spec.js (1)
147-155: LGTM! Metadata assertions correctly updated to use BigInt.The changes properly handle uint64 values by using BigInt for height and timeMs fields in metadata assertions, which aligns with the PR objectives to prevent precision loss.
Also applies to: 182-189
packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/GetIdentitiesContractKeysResponse.spec.js (1)
114-121: LGTM! Metadata assertions correctly updated to use BigInt.The changes properly handle uint64 values by using BigInt for height and timeMs fields in metadata assertions, which aligns with the PR objectives to prevent precision loss.
Also applies to: 164-171
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js (2)
28-35: LGTM! Data contract history structure updated to use array format.The data contract history fixture has been correctly updated to use an array of objects with date and value properties, which provides a clearer and more structured representation.
151-158: LGTM! Metadata assertions correctly updated to use BigInt.The changes properly handle uint64 values by using BigInt for height and timeMs fields in metadata assertions, which aligns with the PR objectives to prevent precision loss.
packages/wasm-dpp/src/identity/identity.rs (2)
108-115: LGTM! Balance methods correctly updated to use u64.The balance-related methods have been properly updated to use u64 instead of f64, which prevents precision loss when handling large integers.
133-140: LGTM! Revision methods correctly updated to use u64.The revision-related methods have been properly updated to use u64 instead of f64, which prevents precision loss when handling large integers.
packages/wasm-dpp/src/identity/state_transition/identity_credit_transfer_transition/transition.rs (1)
93-94: LGTM! The changes correctly handle uint64 values.The modifications properly address the uint64 handling issue by:
- Using u64 instead of f64 for amount values
- Converting amounts to BigInt when serializing to JavaScript
Also applies to: 98-99, 164-164, 239-239
packages/js-dapi-client/test/unit/methods/platform/getStatus/getStatusFactory.spec.js (1)
1-279: LGTM! Comprehensive test coverage for getStatus functionality.The test suite is well-structured with:
- Complete coverage of success paths
- Proper BigInt handling verification
- Thorough assertions for all fields
- Edge case handling for optional fields
packages/wasm-dpp/src/data_contract/data_contract.rs (1)
316-317: LGTM! Improved parameter naming.The parameter rename from 'e' to 'nonce' enhances code readability while maintaining the same functionality.
packages/wasm-dpp/src/document/state_transition/document_batch_transition/mod.rs (1)
131-132: LGTM! The change aligns with the PR objectives.The parameter type change from
u32tou64forset_identity_contract_nonceis correct and necessary to prevent precision loss when handling large nonce values.Run the following script to verify the impact of this change:
✅ Verification successful
Nonce Type Update Verified: No Remaining
u32Occurrences FoundThe additional searches using
ripgrepshowed no instances ofnonce: u32usage in either type declarations or function parameters, indicating that the change fromu32tou64has been fully implemented.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining u32 nonce types in the codebase that might need similar updates. # Search for nonce-related type declarations ast-grep --pattern $'type $_nonce$_: u32' # Search for nonce-related function parameters ast-grep --pattern $'fn $_($_ nonce: u32)'Length of output: 86
Script:
#!/bin/bash # Description: Perform additional searches for 'nonce' references that may be using u32. # This script uses ripgrep (rg) to scan the entire repository for occurrences where nonce parameters or types are declared as u32. echo "Searching for explicit 'nonce: u32' occurrences..." rg -n "nonce: u32" echo -e "\nSearching for functions with 'nonce: u32' parameters..." rg -n "fn .*nonce: u32" echo -e "\nSearching for any lines with both 'nonce' and 'u32' for additional context..." rg -n "nonce.*u32|u32.*nonce"Length of output: 511
packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js (2)
99-104: LGTM! Proper BigInt conversions for large numbers.The test correctly converts numeric values that could exceed JavaScript's Number.MAX_SAFE_INTEGER to BigInt, preventing precision loss:
- Chain heights and peer heights
- State sync times and block counts
- Timestamps
Also applies to: 114-121, 125-128
194-268: LGTM! Comprehensive test coverage.The test suite thoroughly verifies:
- All status components (version, node, chain, network, state sync, time)
- Proper type conversions for all fields
- Instance creation from proto
- Getter methods for all properties
Also applies to: 270-346
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (9)
58-59: LGTM: ResponseMetadata type changes align with uint64 handlingThe changes to convert
heightandtimeMsfields fromnumbertostringtype in ResponseMetadata are consistent with the PR objective to fix uint64 handling and prevent precision loss.Also applies to: 67-68, 88-91
537-538: LGTM: GetIdentityNonceResponse type changesConverting
identityNoncefield fromnumbertostringtype is appropriate for handling uint64 values without precision loss.Also applies to: 563-563
606-607: LGTM: GetIdentityContractNonceResponse type changesConverting
identityContractNoncefield fromnumbertostringtype is consistent with the uint64 handling improvements.Also applies to: 632-632
675-676: LGTM: Balance and revision type changesConverting
balanceandrevisionfields fromnumbertostringtype in balance-related responses is appropriate for handling uint64 values.Also applies to: 701-701, 776-780, 794-795
2405-2406: LGTM: Data contract history timestamp type changesConverting timestamp fields (
startAtMsanddate) fromnumbertostringtype is consistent with handling large integer values.Also applies to: 2426-2426, 2494-2495, 2514-2514
3970-3971: LGTM: Vote poll timestamp type changesConverting timestamp fields (
startTimeMs,endTimeMs, andtimestamp) fromnumbertostringtype is appropriate for handling uint64 values.Also applies to: 3988-3989, 3994-3995, 4012-4013, 4081-4082, 4103-4104
5607-5618: LGTM: Status response time and block height type changesConverting time and block height related fields from
numbertostringtype in status responses is consistent with handling large integer values.Also applies to: 5764-5787
5035-5036: LGTM: Balance and credits type changesConverting balance and credits fields from
numbertostringtype in specialized balance and platform credits responses is appropriate for handling uint64 values.Also applies to: 5061-5061, 5152-5153, 5178-5178
58-5809: Verify the completeness of uint64 type changesLet's verify that all uint64 fields have been converted to string type consistently.
✅ Verification successful
uint64 Type Conversion Verification
The search results indicate that for the Platform client the uint64 fields that need conversion (for example, the height and timeMs fields) are now declared as strings as expected. The other numeric fields that remain (such as epoch and protocolVersion) are left as numbers—this is intentional because only fields that risk exceeding JavaScript’s safe number range (uint64 values) require conversion to string. In short, the conversion has been applied consistently based on each field’s requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining number type fields that might need conversion rg -A 1 'get\w+\(\):' | rg 'number'Length of output: 13148
🧰 Tools
🪛 Biome (1.9.4)
[error] 161-162: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 866-867: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 5387-5388: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
packages/js-dash-sdk/tsconfig.json (1)
2-4: Update TypeScript Target Version
Changing"target": "es2020"(from the older ES5 setting) enables modern JavaScript features—including native BigInt support—which is essential for safely handling large integer values in the codebase.packages/dapi-grpc/protos/platform/v0/platform.proto (15)
71-78: Ensure Consistent BigInt Handling in ResponseMetadata
In theResponseMetadatamessage, fields likeheightandtime_msare now annotated with[jstype = JS_STRING]to prevent precision loss for large blockchain values. Please verify that any downstream code expecting numeric types is updated to handle these as string representations.
163-174: BigInt Conversion in GetIdentityNonceResponse
Theidentity_noncefield now uses[jstype = JS_STRING]to safely representuint64values in JavaScript. Confirm that any consumers or serialization logic correctly handle this string conversion.
176-187: Safe Representation for Identity Contract Nonce
Theidentity_contract_noncefield has been updated with[jstype = JS_STRING]to avoid precision issues. Ensure that the client code deserializes this value appropriately.
189-200: BigInt Handling for Identity Balance
Thebalancefield inGetIdentityBalanceResponseis now expressed with[jstype = JS_STRING], which prevents precision loss when working with large balance figures. Verify that downstream consumers correctly process the new string format.
205-208: Ensure Precise Handling of Balance and Revision
InsideGetIdentityBalanceAndRevisionResponse, bothbalanceandrevisionfields now use[jstype = JS_STRING]. This update is crucial for accurately representing large numbers, so please confirm consistency across all client-side operations.
377-380: Convert Identity Balance to String
In theGetIdentitiesBalancesResponsemessage, the optionalbalancefield is now marked with[jstype = JS_STRING]to preserve numeric precision. Ensure that any code consuming these balance values handles them as strings or converts them safely.
510-513: String Representation for Start Timestamp
Thestart_at_msfield inGetDataContractHistoryRequestnow specifies[jstype = JS_STRING]. This change is critical for preventing precision loss in large millisecond timestamps. Please double-check that any filtering or comparison operations account for the string format.
521-523: Maintain Precision in Contract History Dates
Thedatefield within theDataContractHistoryEntrymessage now uses[jstype = JS_STRING], ensuring that large timestamp values are correctly preserved. Confirm that any date processing routines are updated accordingly.
1025-1028: Correct Data Type for Specialized Balance
InGetPrefundedSpecializedBalanceResponse, thebalancefield has been updated with[jstype = JS_STRING]. This modification prevents precision issues when dealing with large numbers. Please ensure that any consumer of this field correctly handles the string type.
1047-1050: BigInt Support for Total Credits
Thecreditsfield inGetTotalCreditsInPlatformResponsenow uses[jstype = JS_STRING]to accurately capture large numeric totals. Verify that subsequent processing logic treats these values appropriately.
731-742: Epoch Information: Handling Large Numbers
Within theEpochInfomessage ofGetEpochsInfoResponse, fields such asfirst_block_heightandstart_timeare now annotated with[jstype = JS_STRING]. This change is important for avoiding precision loss in blockchain metrics.
800-807: Timestamp Precision in Vote Polls Request
Bothstart_time_ms(inStartAtTimeInfo) andend_time_ms(inEndAtTimeInfo) inGetVotePollsByEndDateRequestnow use[jstype = JS_STRING]. This ensures that time-based filtering remains accurate even with very large numbers.
823-825: Vote Polls Timestamp as String
Thetimestampfield inSerializedVotePollsByTimestampwithinGetVotePollsByEndDateResponseis now marked with[jstype = JS_STRING]to support high-precision time values. Please ensure vote processing logic adheres to this new representation.
876-888: Accurate Vote State Timestamps and Block Height
Within theFinishedVoteInfoofGetContestedResourceVoteStateResponse, bothfinished_at_block_heightandfinished_at_block_time_msnow use[jstype = JS_STRING]. This is essential to maintain precision in vote result evaluations.
1130-1176: BigInt Handling in GetStatusResponse
Multiple numeric fields in theGetStatusResponsemessage—including time values in theTimemessage, block heights in theChainmessage, and various metrics inStateSync—are now annotated with[jstype = JS_STRING]. This comprehensive update ensures that large values are safely represented as strings throughout status reporting. Please review any client-side conversions to maintain consistency.
| class NetworkStatus { | ||
| /** | ||
| * @param {string} chainId - Chain id | ||
| * @param {number} peersCount - Peers count | ||
| * @param {boolean} listening - Is listening to P2P network | ||
| */ | ||
| constructor(chainId, peersCount, listening) { | ||
| this.chainId = chainId; | ||
| this.peersCount = peersCount; | ||
| this.listening = listening; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and consider BigInt for peersCount.
Given that this PR aims to fix u64 handling, consider the following improvements:
- Add input validation in the constructor to ensure type safety.
- Consider using BigInt for peersCount if it could exceed Number.MAX_SAFE_INTEGER.
Here's a suggested implementation:
constructor(chainId, peersCount, listening) {
+ if (typeof chainId !== 'string') {
+ throw new TypeError('chainId must be a string');
+ }
+ if (typeof peersCount !== 'number' || !Number.isInteger(peersCount) || peersCount < 0) {
+ throw new TypeError('peersCount must be a non-negative integer');
+ }
+ if (typeof listening !== 'boolean') {
+ throw new TypeError('listening must be a boolean');
+ }
this.chainId = chainId;
this.peersCount = peersCount;
this.listening = listening;
}📝 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.
| class NetworkStatus { | |
| /** | |
| * @param {string} chainId - Chain id | |
| * @param {number} peersCount - Peers count | |
| * @param {boolean} listening - Is listening to P2P network | |
| */ | |
| constructor(chainId, peersCount, listening) { | |
| this.chainId = chainId; | |
| this.peersCount = peersCount; | |
| this.listening = listening; | |
| } | |
| class NetworkStatus { | |
| /** | |
| * @param {string} chainId - Chain id | |
| * @param {number} peersCount - Peers count | |
| * @param {boolean} listening - Is listening to P2P network | |
| */ | |
| constructor(chainId, peersCount, listening) { | |
| if (typeof chainId !== 'string') { | |
| throw new TypeError('chainId must be a string'); | |
| } | |
| if (typeof peersCount !== 'number' || !Number.isInteger(peersCount) || peersCount < 0) { | |
| throw new TypeError('peersCount must be a non-negative integer'); | |
| } | |
| if (typeof listening !== 'boolean') { | |
| throw new TypeError('listening must be a boolean'); | |
| } | |
| this.chainId = chainId; | |
| this.peersCount = peersCount; | |
| this.listening = listening; | |
| } | |
| } |
| const identityContractNonce = BigInt(proto.getV0() | ||
| .getIdentityContractNonce()) & IDENTITY_CONTRACT_NONCE_VALUE_FILTER; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for BigInt conversion.
The code assumes the proto value can be safely converted to BigInt, but this might throw if the value is invalid.
Consider adding try-catch:
- const identityContractNonce = BigInt(proto.getV0()
- .getIdentityContractNonce()) & IDENTITY_CONTRACT_NONCE_VALUE_FILTER;
+ let identityContractNonce;
+ try {
+ identityContractNonce = BigInt(proto.getV0().getIdentityContractNonce()) & IDENTITY_CONTRACT_NONCE_VALUE_FILTER;
+ } catch (e) {
+ throw new InvalidResponseError('Invalid contract nonce value: cannot convert to BigInt');
+ }📝 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.
| const identityContractNonce = BigInt(proto.getV0() | |
| .getIdentityContractNonce()) & IDENTITY_CONTRACT_NONCE_VALUE_FILTER; | |
| let identityContractNonce; | |
| try { | |
| identityContractNonce = BigInt(proto.getV0().getIdentityContractNonce()) & IDENTITY_CONTRACT_NONCE_VALUE_FILTER; | |
| } catch (e) { | |
| throw new InvalidResponseError('Invalid contract nonce value: cannot convert to BigInt'); | |
| } |
| * height: bigint, | ||
| * coreChainLockedHeight: number, | ||
| * height: number, | ||
| * signature: Buffer, | ||
| * timeMs: bigint, | ||
| * protocolVersion: number, | ||
| * timeMs: number, | ||
| * }} |
There was a problem hiding this comment.
Inconsistent type annotations.
The JSDoc return type for coreChainLockedHeight is annotated as number but the implementation returns a string.
Update the return type annotation to match the implementation:
* @returns {{
* height: bigint,
- * coreChainLockedHeight: number,
+ * coreChainLockedHeight: string,
* timeMs: bigint,
* protocolVersion: number,
* }}📝 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.
| * height: bigint, | |
| * coreChainLockedHeight: number, | |
| * height: number, | |
| * signature: Buffer, | |
| * timeMs: bigint, | |
| * protocolVersion: number, | |
| * timeMs: number, | |
| * }} | |
| * height: bigint, | |
| * coreChainLockedHeight: string, | |
| * timeMs: bigint, | |
| * protocolVersion: number, | |
| * }} |
| height: BigInt(10), | ||
| coreChainLockedHeight: '42', | ||
| timeMs: new Date().getTime().toString(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify string conversion of timeMs.
The conversion of timeMs to string using toString() and then presumably back to bigint elsewhere could lead to precision issues. Consider using BigInt directly.
- timeMs: new Date().getTime().toString(),
+ timeMs: BigInt(new Date().getTime()),📝 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.
| height: BigInt(10), | |
| coreChainLockedHeight: '42', | |
| timeMs: new Date().getTime().toString(), | |
| height: BigInt(10), | |
| coreChainLockedHeight: '42', | |
| timeMs: BigInt(new Date().getTime()), |
| * @returns {bigint|null} Drive ABCI version | ||
| */ |
There was a problem hiding this comment.
Fix incorrect JSDoc comments.
The JSDoc comments for getBlockTime and getGenesisTime methods have incorrect descriptions.
- * @returns {bigint|null} Drive ABCI version
+ * @returns {bigint|null} Block time
- * @returns {bigint|null} Tenderdash version
+ * @returns {bigint|null} Genesis timeAlso applies to: 35-36
| nonceManager.setIdentityNonce(identityId, 1n); | ||
| expect(await nonceManager.getIdentityNonce(identityId)).to.be.equal(1); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent type usage in test assertions.
The test mixes bigint (1n) with number (1) in assertions, which could mask type-related issues.
- nonceManager.setIdentityNonce(identityId, 1n);
- expect(await nonceManager.getIdentityNonce(identityId)).to.be.equal(1);
+ nonceManager.setIdentityNonce(identityId, 1n);
+ expect(await nonceManager.getIdentityNonce(identityId)).to.be.equal(1n);📝 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.
| nonceManager.setIdentityNonce(identityId, 1n); | |
| expect(await nonceManager.getIdentityNonce(identityId)).to.be.equal(1); | |
| nonceManager.setIdentityNonce(identityId, 1n); | |
| expect(await nonceManager.getIdentityNonce(identityId)).to.be.equal(1n); |
| public async bumpIdentityNonce(identityId: Identifier): Promise<bigint> { | ||
| const nextIdentityNonce = (await this.getIdentityNonce(identityId)); | ||
| this.setIdentityNonce(identityId, nextIdentityNonce); | ||
| return nextIdentityNonce; | ||
| } |
There was a problem hiding this comment.
Fix missing nonce increment in bumpIdentityNonce.
The method is supposed to increment the nonce but currently just returns the current value without modification.
Apply this diff to fix the increment operation:
- const nextIdentityNonce = (await this.getIdentityNonce(identityId));
+ const nextIdentityNonce = (await this.getIdentityNonce(identityId)) + 1n;📝 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.
| public async bumpIdentityNonce(identityId: Identifier): Promise<bigint> { | |
| const nextIdentityNonce = (await this.getIdentityNonce(identityId)); | |
| this.setIdentityNonce(identityId, nextIdentityNonce); | |
| return nextIdentityNonce; | |
| } | |
| public async bumpIdentityNonce(identityId: Identifier): Promise<bigint> { | |
| - const nextIdentityNonce = (await this.getIdentityNonce(identityId)); | |
| + const nextIdentityNonce = (await this.getIdentityNonce(identityId)) + 1n; | |
| this.setIdentityNonce(identityId, nextIdentityNonce); | |
| return nextIdentityNonce; | |
| } |
| public async bumpIdentityContractNonce( | ||
| identityId: Identifier, | ||
| contractId: Identifier, | ||
| ): Promise<number> { | ||
| const nextIdentityContractNonce = await this | ||
| .getIdentityContractNonce(identityId, contractId) + 1; | ||
| ): Promise<bigint> { | ||
| const identityContractNonce = await this.getIdentityContractNonce(identityId, contractId); | ||
| // @ts-ignore | ||
| const nextIdentityContractNonce = identityContractNonce + 1n; | ||
| this.setIdentityContractNonce(identityId, contractId, nextIdentityContractNonce); | ||
| return nextIdentityContractNonce; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-ignore by ensuring proper types.
The method correctly uses bigint literal for increment but relies on @ts-ignore. This suggests a type system issue that should be addressed.
Remove the @ts-ignore by ensuring identityContractNonce is properly typed as bigint:
- // @ts-ignore
- const nextIdentityContractNonce = identityContractNonce + 1n;
+ const nextIdentityContractNonce = identityContractNonce + 1n;📝 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.
| public async bumpIdentityContractNonce( | |
| identityId: Identifier, | |
| contractId: Identifier, | |
| ): Promise<number> { | |
| const nextIdentityContractNonce = await this | |
| .getIdentityContractNonce(identityId, contractId) + 1; | |
| ): Promise<bigint> { | |
| const identityContractNonce = await this.getIdentityContractNonce(identityId, contractId); | |
| // @ts-ignore | |
| const nextIdentityContractNonce = identityContractNonce + 1n; | |
| this.setIdentityContractNonce(identityId, contractId, nextIdentityContractNonce); | |
| return nextIdentityContractNonce; | |
| } | |
| public async bumpIdentityContractNonce( | |
| identityId: Identifier, | |
| contractId: Identifier, | |
| ): Promise<bigint> { | |
| const identityContractNonce = await this.getIdentityContractNonce(identityId, contractId); | |
| const nextIdentityContractNonce = identityContractNonce + 1n; | |
| this.setIdentityContractNonce(identityId, contractId, nextIdentityContractNonce); | |
| return nextIdentityContractNonce; | |
| } |
| * @param id {Identifier} | ||
| * @param startAMs {bigint} | ||
| * @param limit {number} | ||
| * @param offset {number} |
There was a problem hiding this comment.
💡 Codebase verification
Action Required: Update Usage in Contracts History Module
The search confirms that one non-local usage of fetchDataContractHistory in packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts still passes a value (startAtMs) that might not explicitly be a bigint. Please verify if this variable is properly converted to a bigint or update its assignment accordingly. Also, document this breaking change in your changelog/migration guide.
🔗 Analysis chain
Breaking change: startAMs parameter type changed to bigint.
The parameter type change from number to bigint is a breaking change that could affect existing code. Ensure this change is documented in the changelog and migration guide.
Please verify if there are any existing usages of this method that need to be updated:
Also applies to: 138-138
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for calls to fetchDataContractHistory
rg -A 2 'fetchDataContractHistory\(' Length of output: 721
| @@ -128,7 +128,7 @@ impl DocumentFactoryWASM { | |||
| .for_each(|entry| { | |||
| let key_value = js_sys::Array::from(&entry); | |||
| let contract_id = identifier_from_js_value(&key_value.get(0)).unwrap(); | |||
| let nonce = key_value.get(1).as_f64().unwrap() as u64; | |||
| let nonce = key_value.get(1).as_string().unwrap().parse::<u64>().unwrap(); | |||
There was a problem hiding this comment.
Add proper error handling for nonce parsing.
The current implementation uses multiple unwrap calls which could panic if:
- The value is missing
- The string conversion fails
- The u64 parsing fails
Consider this safer implementation:
-let nonce = key_value.get(1).as_string().unwrap().parse::<u64>().unwrap();
+let nonce = key_value
+ .get(1)
+ .ok_or_else(|| JsError::new("Missing nonce value"))?
+ .as_string()
+ .ok_or_else(|| JsError::new("Nonce value must be a string"))?
+ .parse::<u64>()
+ .map_err(|e| JsError::new(&format!("Invalid nonce value: {}", e)))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let nonce = key_value.get(1).as_string().unwrap().parse::<u64>().unwrap(); | |
| let nonce = key_value | |
| .get(1) | |
| .ok_or_else(|| JsError::new("Missing nonce value"))? | |
| .as_string() | |
| .ok_or_else(|| JsError::new("Nonce value must be a string"))? | |
| .parse::<u64>() | |
| .map_err(|e| JsError::new(&format!("Invalid nonce value: {}", e)))?; |
pshenmic
changed the title
pshenmic
changed the title
pshenmic
force-pushed
the
fix/js-bigint
branch
from
282d16c to
78d2ac9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
1-1218:🧩 Analysis chain
Verify the impact on client implementations.
The widespread changes to uint64 field types will require updates in client implementations to handle string values instead of numbers.
Run the following script to find affected client implementations:
🏁 Script executed:
#!/bin/bash # Description: Find client implementations that might need updates # Search for direct number comparisons with affected fields rg -A 2 "(?:height|time_ms|balance|nonce|count|backfilled_blocks|chunk_process_avg_time)" --type js --type ts # Search for BigInt conversions rg "BigInt\(" --type js --type tsLength of output: 99171
Client Implementations Must Be Updated for uint64 Handling
Our investigation confirms that client code—both in generated JavaScript and corresponding TypeScript definitions—still treats several uint64 fields (e.g. for “height”, “balance”, “count”, etc.) as numbers (or as number|Long) rather than as strings. In several client files (such as those under the dapi‑grpc clients for both core and platform) we see that the generated methods still use numeric conversions (e.g. applying “>>> 0”, calling “uint32(…)”, etc.) and the d.ts files continue to specify these fields as numbers. This is in contrast with the proto update ([jstype = JS_STRING]) that now mandates string representation for uint64 values.
As a result, client logic that directly compares or operates on these fields as numbers will be affected—potentially causing precision or conversion issues. The client implementations need to be reviewed and modified to properly handle these fields as strings (or use a suitable library for 64-bit integer manipulation such as Long or BigInt) to avoid runtime errors or loss of precision.
- Affected Areas:
- Generated protocols in packages/dapi‑grpc/clients/core/v0/nodejs/core_pbjs.js still perform numeric conversions on fields like “height”, “count”, and “balance.”
- TypeScript definitions (e.g. in packages/dapi‑grpc/clients/core/v0/web/core_pb.d.ts) continue to type these fields as numbers.
- Any client code (e.g. in the bench-suite and elsewhere) that performs direct number comparisons or arithmetic on these fields will need adjustment.
🧰 Tools
🪛 Buf (1.47.2)
5-5: Files with package "org.dash.platform.dapi.v0" must be within a directory "org/dash/platform/dapi/v0" relative to root but were in directory "packages/dapi-grpc/protos/platform/v0".
(PACKAGE_DIRECTORY_MATCH)
♻️ Duplicate comments (1)
packages/js-dapi-client/lib/methods/platform/getStatus/NetworkStatus.js (1)
7-11: 🛠️ Refactor suggestionAdd input validation and use BigInt for peersCount.
Given this PR's focus on fixing u64 handling, consider:
- Adding input validation in the constructor
- Using BigInt for peersCount to prevent potential overflow
🧹 Nitpick comments (8)
packages/wasm-dpp/src/document/mod.rs (2)
129-131: Simplify the revision assignment.The
as u64cast is unnecessary since the input parameter is already of typeu64.- self.0.set_revision(revision.map(|r| r as u64)); + self.0.set_revision(revision);
134-135: Simplify the revision retrieval.The
mapoperation is unnecessary since it's not transforming the value.- self.0.revision().map(|r| r) + self.0.revision()packages/js-dapi-client/lib/methods/platform/getTotalCreditsInPlatform/GetTotalCreditsInPlatformResponse.js (1)
29-29: Add error handling for BigInt conversion.While the conversion to BigInt is correct, consider adding try-catch to handle invalid number strings gracefully.
- const totalCreditsInPlatform = BigInt(proto.getV0().getCredits()); + let totalCreditsInPlatform; + try { + totalCreditsInPlatform = BigInt(proto.getV0().getCredits()); + } catch (e) { + throw new InvalidResponseError('Invalid credits value: must be a valid number string'); + }packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js (2)
28-35: Consider using BigInt for date values.Since this PR aims to handle uint64 values consistently using BigInt, consider converting the
datevalues to BigInt to maintain consistency across the codebase.dataContractHistoryFixture = [{ - date: 10000, + date: BigInt(10000), value: dataContractFixture.toBuffer(), }, { - date: 20000, + date: BigInt(20000), value: dataContractFixture.toBuffer(), }];
151-158: Enhance BigInt conversion test coverage.While the test correctly validates BigInt conversions for metadata fields, consider adding explicit type checks to ensure the values are actually BigInt instances.
expect(getDataContractHistoryResponse.getMetadata().getHeight()) - .to.deep.equal(BigInt(metadataFixture.height)); + .to.deep.equal(BigInt(metadataFixture.height)); + expect(getDataContractHistoryResponse.getMetadata().getHeight()) + .to.be.instanceof(BigInt); expect(getDataContractHistoryResponse.getMetadata().getTimeMs()) - .to.deep.equal(BigInt(metadataFixture.timeMs)); + .to.deep.equal(BigInt(metadataFixture.timeMs)); + expect(getDataContractHistoryResponse.getMetadata().getTimeMs()) + .to.be.instanceof(BigInt);packages/wasm-dpp/lib/test/fixtures/getIdentityCreditTransferTransitionFixture.js (1)
9-9: Add input validation for amount.While the conversion to BigInt aligns with the PR objectives, consider adding validation to ensure the amount is non-negative.
- stateTransition.setAmount(BigInt(1000)); + const amount = BigInt(1000); + if (amount < 0) { + throw new Error('Amount must be non-negative'); + } + stateTransition.setAmount(amount);packages/platform-test-suite/lib/test/fixtures/getIdentityFixture.js (1)
30-30: Add validation for balance.While the conversion to BigInt aligns with the PR objectives, consider adding validation to ensure the balance is non-negative.
- identity.setBalance(BigInt(10000)); + const balance = BigInt(10000); + if (balance < 0) { + throw new Error('Balance must be non-negative'); + } + identity.setBalance(balance);packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js (1)
99-104: Consider adding edge case tests for BigInt conversions.While the tests cover the basic functionality, consider adding tests for edge cases such as:
- Maximum uint64 values
- Zero values
- Values near uint64 boundaries
Also applies to: 227-229
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js(89 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts(38 hunks)packages/dapi-grpc/clients/platform/v0/web/platform_pb.js(89 hunks)packages/dapi-grpc/protos/platform/v0/platform.proto(18 hunks)packages/js-dapi-client/lib/SimplifiedMasternodeListProvider/createMasternodeListStreamFactory.js(1 hunks)packages/js-dapi-client/lib/methods/core/subscribeToMasternodeListFactory.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/DataContractHistoryEntry.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getDataContractHistory/getDataContractHistoryFactory.js(2 hunks)packages/js-dapi-client/lib/methods/platform/getEpochsInfo/EpochInfo.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getEpochsInfo/GetEpochsInfoResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityContractNonce/GetIdentityContractNonceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getIdentityNonce/GetIdentityNonceResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/ChainStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/GetStatusResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/NetworkStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/NodeStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/StateSyncStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/TimeStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/VersionStatus.js(1 hunks)packages/js-dapi-client/lib/methods/platform/getStatus/getStatusFactory.js(2 hunks)packages/js-dapi-client/lib/methods/platform/getTotalCreditsInPlatform/GetTotalCreditsInPlatformResponse.js(3 hunks)packages/js-dapi-client/lib/methods/platform/response/AbstractResponse.js(1 hunks)packages/js-dapi-client/lib/methods/platform/response/Metadata.js(2 hunks)packages/js-dapi-client/lib/test/fixtures/getMetadataFixture.js(1 hunks)packages/js-dapi-client/lib/test/fixtures/getStatusFixture.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContract/GetDataContractResponse.spec.js(4 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContract/getDataContractFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js(8 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getDocuments/GetDocumentsResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getDocuments/getDocumentsFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/GetEpochsInfoResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/getEpochsInfoFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/GetIdentitiesContractKeysResponse.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/getIdentitiesContractKeysFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentity/GetIdentityResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentity/getIdentityFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/getIdentityBalanceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/GetIdentityByPublicKeyHashResponse.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/getIdentityByPublicKeyHashFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/GetIdentityContractNonce.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/getIdentityContractNonceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/GetIdentityKeys.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/getIdentityKeysFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/GetIdentityNonce.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/getIdentityNonceFactory.spec.js(3 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/GetProtocolVersionUpgradeStateResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/getProtocolVersionUpgradeStateFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/GetProtocolVersionUpgradeVoteStatusResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/getProtocolVersionUpgradeVoteStatusFactory.spec.js(2 hunks)packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/getStatus/getStatusFactory.spec.js(1 hunks)packages/js-dapi-client/test/unit/methods/platform/waitForStateTransitionResult/waitForStateTransitionResultFactory.spec.js(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.spec.ts(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts(6 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/update.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/get.ts(1 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/identities/get.ts(1 hunks)packages/js-dash-sdk/tsconfig.json(1 hunks)packages/platform-test-suite/lib/test/fixtures/getIdentityFixture.js(1 hunks)packages/wasm-dpp/.eslintrc(1 hunks)packages/wasm-dpp/lib/test/fixtures/getIdentityCreditTransferTransitionFixture.js(1 hunks)packages/wasm-dpp/lib/test/fixtures/getIdentityFixture.js(1 hunks)packages/wasm-dpp/src/data_contract/data_contract.rs(1 hunks)packages/wasm-dpp/src/document/document_facade.rs(1 hunks)packages/wasm-dpp/src/document/extended_document.rs(1 hunks)packages/wasm-dpp/src/document/factory.rs(2 hunks)packages/wasm-dpp/src/document/mod.rs(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_create_transition.rs(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs(4 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/mod.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/basic/identity/identity_insufficient_balance_error.rs(1 hunks)packages/wasm-dpp/src/errors/consensus/fee/balance_is_not_enough_error.rs(1 hunks)packages/wasm-dpp/src/identity/identity.rs(1 hunks)packages/wasm-dpp/src/identity/state_transition/identity_credit_transfer_transition/transition.rs(3 hunks)
⛔ Files not processed due to max files limit (15)
- packages/wasm-dpp/src/identity/state_transition/identity_update_transition/identity_update_transition.rs
- packages/wasm-dpp/src/identity/state_transition/identity_update_transition/to_object.rs
- packages/wasm-dpp/src/metadata.rs
- packages/wasm-dpp/test/integration/document/Document.spec.js
- packages/wasm-dpp/test/integration/document/DocumentFacade.spec.js
- packages/wasm-dpp/test/integration/identity/IdentityFacade.spec.js
- packages/wasm-dpp/test/integration/stateTransition/StateTransitionFacade.spec.js
- packages/wasm-dpp/test/unit/Metadata.spec.js
- packages/wasm-dpp/test/unit/dataContract/DataContract.spec.js
- packages/wasm-dpp/test/unit/document/Document.spec.js
- packages/wasm-dpp/test/unit/document/DocumentFactory.spec.js
- packages/wasm-dpp/test/unit/identity/Identity.spec.js
- packages/wasm-dpp/test/unit/identity/IdentityFactory.spec.js
- packages/wasm-dpp/test/unit/identity/stateTransition/IdentityUpdateTransition/IdentityUpdateTransition.spec.js
- packages/wasm-dpp/test/unit/identity/stateTransition/identityCreditTransferTransition/identityCreditTransferTransition.spec.js
🚧 Files skipped from review as they are similar to previous changes (56)
- packages/js-dapi-client/lib/SimplifiedMasternodeListProvider/createMasternodeListStreamFactory.js
- packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/GetProtocolVersionUpgradeVoteStatusResponse.spec.js
- packages/js-dapi-client/lib/methods/core/subscribeToMasternodeListFactory.js
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/identities/get.ts
- packages/js-dapi-client/lib/methods/platform/getDataContractHistory/DataContractHistoryEntry.js
- packages/wasm-dpp/src/errors/consensus/fee/balance_is_not_enough_error.rs
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/get.ts
- packages/js-dapi-client/lib/methods/platform/getEpochsInfo/GetEpochsInfoResponse.js
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/update.ts
- packages/js-dapi-client/test/unit/methods/platform/getDocuments/getDocumentsFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getDataContract/getDataContractFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/GetIdentityByPublicKeyHashResponse.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getDocuments/GetDocumentsResponse.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentity/getIdentityFactory.spec.js
- packages/js-dash-sdk/tsconfig.json
- packages/wasm-dpp/src/document/factory.rs
- packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/GetIdentityContractNonce.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/getEpochsInfoFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityContractNonce/getIdentityContractNonceFactory.spec.js
- packages/js-dapi-client/lib/methods/platform/getStatus/NodeStatus.js
- packages/js-dapi-client/lib/methods/platform/getDataContractHistory/getDataContractHistoryFactory.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/getIdentityNonceFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/GetProtocolVersionUpgradeStateResponse.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeState/getProtocolVersionUpgradeStateFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/waitForStateTransitionResult/waitForStateTransitionResultFactory.spec.js
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/get.ts
- packages/wasm-dpp/src/document/state_transition/document_batch_transition/mod.rs
- packages/js-dapi-client/test/unit/methods/platform/getEpochsInfo/GetEpochsInfoResponse.spec.js
- packages/js-dapi-client/lib/methods/platform/getStatus/StateSyncStatus.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/getIdentitiesContractKeysFactory.spec.js
- packages/wasm-dpp/src/document/document_facade.rs
- packages/wasm-dpp/src/data_contract/data_contract.rs
- packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/getIdentityKeysFactory.spec.js
- packages/js-dapi-client/lib/methods/platform/response/AbstractResponse.js
- packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts
- packages/js-dapi-client/lib/test/fixtures/getStatusFixture.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityByPublicKeyHash/getIdentityByPublicKeyHashFactory.spec.js
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts
- packages/js-dapi-client/test/unit/methods/platform/getProtocolVersionUpgradeVoteStatus/getProtocolVersionUpgradeVoteStatusFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentity/GetIdentityResponse.spec.js
- packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.spec.ts
- packages/js-dapi-client/test/unit/methods/platform/getIdentityNonce/GetIdentityNonce.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentityKeys/GetIdentityKeys.spec.js
- packages/js-dapi-client/test/unit/methods/platform/getStatus/getStatusFactory.spec.js
- packages/js-dapi-client/lib/methods/platform/response/Metadata.js
- packages/js-dapi-client/lib/methods/platform/getIdentityContractNonce/GetIdentityContractNonceResponse.js
- packages/js-dapi-client/lib/methods/platform/getEpochsInfo/EpochInfo.js
- packages/js-dapi-client/lib/methods/platform/getStatus/TimeStatus.js
- packages/js-dapi-client/lib/methods/platform/getIdentityBalance/GetIdentityBalanceResponse.js
- packages/js-dapi-client/lib/methods/platform/getStatus/VersionStatus.js
- packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js
- packages/js-dapi-client/test/unit/methods/platform/getIdentitiesContractKeys/GetIdentitiesContractKeysResponse.spec.js
- packages/js-dapi-client/lib/test/fixtures/getMetadataFixture.js
- packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts
🔇 Additional comments (173)
packages/wasm-dpp/src/document/mod.rs (1)
129-135: LGTM! The changes align with the PR objectives.The update from
u32tou64for the revision field is a good change that prevents potential data loss and maintains consistency with other components being updated in this PR.packages/js-dapi-client/lib/methods/platform/getTotalCreditsInPlatform/GetTotalCreditsInPlatformResponse.js (2)
6-6: LGTM! Note the breaking change.The type change from
numbertobigintaligns with the PR's goal to prevent precision loss in uint64 values. This is a breaking change that will require updates in consuming code.
17-17: LGTM! Return type matches parameter type.The return type change maintains consistency with the constructor parameter type, ensuring type safety throughout the class.
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js (3)
44-50: LGTM!The test correctly validates the new array-based structure for data contract history.
52-67: LGTM!The change to return and expect
nullinstead of an empty object when no history is present improves the API's clarity.
168-224: LGTM!The error handling tests are comprehensive and correctly adapted to work with the new array-based structure while maintaining proper validation of invalid responses.
packages/dapi-grpc/clients/platform/v0/nodejs/platform_protoc.js (112)
4667-4670: Convert default numeric values to string
Changing default values from numeric0to string"0"forheightandtimeMshelps avoid precision loss for uint64 fields.
4710-4710: Switch to readUint64String for height
ReplacingreadUint64()withreadUint64String()is appropriate to maintain precision for large uint64 values.
4722-4722: Switch to readUint64String for timeMs
Same rationale as height: ensures we preserve large values accurately.
4763-4764: parseInt check for height
UsingparseInt(f, 10) !== 0to skip writing zero is a common Protobuf pattern for optional fields.
4784-4785: parseInt check for timeMs
Consistent with the approach used forheight.
4809-4821: Update getter/setter docs and defaults for height
The JSDoc now correctly indicates string returns and parameters, aligning with the new uint64 as string approach.
4863-4866: Update getter/setter for timeMs
Replicated pattern for handlingtimeMsas a string with accurate default.
7911-7911: Use string for identityNonce
Consistent shift to string for large numbers.
7951-7951: readUint64String for identityNonce
Ensures no precision loss when deserializing.
7993-7993: Serialize identityNonce as string
UseswriteUint64String, matching the read pattern.
8021-8021: Setter for identityNonce
Storing as string is correct; no concern on usage.
8363-8363: Use string for identityContractNonce
Follows the established string-based convention for uint64 fields.
8403-8403: readUint64String for identityContractNonce
Retains large numeric precision for contract nonce.
8445-8445: Serialize identityContractNonce as string
Mirrors the read approach for consistency.
8473-8473: Getter/setter doc updates for identityContractNonce
Accurate doc strings reflect the new string type.
8815-8815: Use string for GetIdentityBalanceResponse
Prevents overflow by handling balance as string.
8855-8855: readUint64String for balance
Ensures correct deserialization of potentially large balances.
8897-8897: Serialize balance as string
Aligned with the rest of the changes for uint64 fields.
8925-8925: Getter/setter for balance
Correct default string usage.
9409-9410: BalanceAndRevision as strings
Both fields are handled as strings to preserve precision.
9448-9452: readUint64String for balance and revision
Typical approach for large numeric fields.
9485-9493: parseInt check, then writeUint64String
Same pattern ensuring optional field is skipped if zero.
9503-9533: Get/set for balance/revision
Doc references updated to reflect string usage, consistent with approach.
13998-13998: Use string for count
Switching to string for large block count values.
14040-14040: readUint64String for count
Ensuring correct handling of block count.
14080-14080: parseInt check for count
Remains consistent with optional field logic.
14133-14133: Getter/setter for count
Accurate JSDoc for string-based field.
15755-15755: Use string for identity balance
Maintains precision for balances.
15797-15797: readUint64String for balance
Consistent approach to avoid precision issues.
15836-15836: Serialize identity balance
Matches the read approach.
15890-15890: Getter/setter for identity balance
Default string usage is consistent.
20525-20525: Use string for startAtMs
Ensures large timestamps stay accurate.
20578-20578: readUint64String for startAtMs
Retains precise timestamp.
20638-20639: parseInt check for startAtMs
Skipping zero for optional field.
20772-20772: Getter/setter for startAtMs
String-based setter with correct default.
21182-21182: Use string for date
Avoids overflow on large time values.
21221-21221: readUint64String for date
Critical for preserving large timestamps.
21258-21259: parseInt check for date
Allows skipping zero-values.
21276-21288: Get/set for date
Accurate string-based date field.
28907-28909: Strings for firstBlockHeight and startTime
Both fields updated to strings for large numeric values.
28953-28954: readUint64String for firstBlockHeight
Retains full precision for block heights.
28961-28962: readUint64String for startTime
Ensures large epoch times remain intact.
29009-29010: parseInt check for firstBlockHeight
Follows same optional writing logic.
29023-29024: parseInt check for startTime
Skipped unless non-zero.
29066-29069: Getter/setter for firstBlockHeight
Correctly documented as string-based now.
29102-29105: Getter/setter for startTime
Same string-based pattern for epoch usage.
31170-31170: Use string for startTimeMs
Maintains large timestamp accuracy.
31209-31209: readUint64String for startTimeMs
Correct approach for large numeric data.
31246-31247: parseInt check for startTimeMs
Optional field logic remains consistent.
31264-31276: Get/set for startTimeMs
String usage in getters/setters looks good.
31330-31330: Use string for endTimeMs
Mirrors the approach for startTimeMs.
31369-31369: readUint64String for endTimeMs
Preserves large numeric precision.
31406-31407: parseInt check for endTimeMs
Consistent optional field skipping.
31424-31436: Get/set for endTimeMs
UsessetProto3StringIntFieldwith default"0".
32023-32023: Use string for timestamp
Protects against overflow and preserves large timestamps.
32062-32062: readUint64String for timestamp
Standard approach for big integer fields.
32099-32100: parseInt check for timestamp
Similar pattern: skip zero fields.
32117-32129: Getter/setter for timestamp
Correct doc changes for string usage.
33725-33727: Use string for finishedAtBlockHeight and finishedAtBlockTimeMs
Helps avoid precision issues for block data.
33774-33775: readUint64String for finishedAtBlockHeight
Maintains large numeric integrity.
33782-33783: readUint64String for finishedAtBlockTimeMs
Keeps large timestamps accurate.
33833-33834: parseInt check for finishedAtBlockHeight
Skipping zero as expected.
33847-33848: parseInt check for finishedAtBlockTimeMs
Same optional field approach.
33952-33955: Getter/setter for finishedAtBlockHeight
Now consistently treated as string.
33988-33991: Getter/setter for finishedAtBlockTimeMs
Matches the pattern of string-based block times.
38738-38738: Use string for prefunded specialized balance
Essential to safely handle large balances.
38778-38779: readUint64String for prefunded balance
Preserves large numeric amounts.
38820-38821: serializeBinaryToWriter with parseInt check
Aligns with the optional field handling logic.
38848-38859: Getter/setter for prefunded balance
Correct default and doc for string usage.
39496-39496: Use string for total credits
Ensures we don’t lose precision on potentially large values.
39536-39537: readUint64String for credits
Safeguards big integer data.
39578-39580: serializeBinaryToWriter for credits
Skipping zero with parseInt for optional fields.
39606-39617: Getter/setter for credits
Accurate string doc references.
42427-42429: Use string for local, block, genesis times
Guarantees large timestamps remain unaffected by JS number limits.
42468-42476: readUint64String for local/block/genesis
Ensures time fields remain valid for large durations.
42513-42514: parseInt checks for local time
Skipping zero values is consistent with other fields.
42519-42521: parseInt checks for block time
Same logic for block time field.
42526-42528: parseInt checks for genesis time
Uniform approach for skipping unused fields.
42545-42558: Getter/setter for local
String-based doc updates look good.
42563-42574: Getter/setter for block
Mirrors local time logic.
42599-42610: Getter/setter for genesis
Uses the same string-based approach as above.
42930-42934: Use string for latestBlockHeight, earliestBlockHeight, maxPeerBlockHeight
Essential for correct representation of large block heights.
42985-42986: readUint64String for latestBlockHeight
Preserves large chain height.
42997-42998: readUint64String for earliestBlockHeight
Same pattern for earliest block.
43001-43002: readUint64String for maxPeerBlockHeight
Keeps peer block height precise.
43059-43060: parseInt checks for latestBlockHeight
Optional writing if non-zero.
43080-43081: parseInt checks for earliestBlockHeight
Same skip-zero logic.
43087-43088: parseInt checks for maxPeerBlockHeight
Uniform code style across fields.
43207-43211: Getter/setter for latestBlockHeight
String-based block height matches the design.
43309-43313: Getter/setter for earliestBlockHeight
Correct doc references for string usage.
43327-43330: Getter/setter for maxPeerBlockHeight
Aligned with the approach for large height fields.
43601-43609: Use string for StateSync fields
Applying strings to times and block counts ensures no overflow.
43646-43650: readUint64String for totalSyncedTime/remainingTime
Keeps sync timings precise.
43658-43659: readUint64String for chunkProcessAvgTime
Consistent with rest of sync fields.
43662-43663: readUint64String for snapshotHeight
Large block heights remain safe as strings.
43666-43667: readUint64String for snapshotChunksCount
Ensures chunk count remains unrestricted by number limits.
43670-43671: readUint64String for backfilledBlocks
Handles big block ranges.
43674-43675: readUint64String for backfillBlocksTotal
Same pattern for total count.
43707-43708: parseInt check for totalSyncedTime
Skipping zero field if not set.
43714-43715: parseInt check for remainingTime
Matches the same logic.
43728-43729: parseInt check for chunkProcessAvgTime
Consistent with optional field style.
43735-43736: parseInt check for snapshotHeight
Avoids writing zero defaults.
43742-43743: parseInt check for snapshotChunksCount
Aligns with the rest of the optional fields.
43749-43750: parseInt check for backfilledBlocks
Same skip-zero approach.
43756-43757: parseInt check for backfillBlocksTotal
Matches the overall pattern.
43767-43771: Getter/setter for totalSyncedTime
Using strings ensures no numeric overflow.
43785-43789: Getter/setter for remainingTime
Doc updates reflecting strings.
43821-43825: Getter/setter for chunkProcessAvgTime
Matches other state sync fields.
43841-43843: Getter/setter for snapshotHeight
Same conversion to string type.
43859-43861: Getter/setter for snapshotChunksCount
Uniform approach for chunk counts.
43877-43879: Getter/setter for backfilledBlocks
Consistent with other large numeric fields.
43895-43897: Getter/setter for backfillBlocksTotal
Parallel pattern for total block count.packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (16)
58-59: Approve the conversion of numeric fields to string type in ResponseMetadata.The changes to convert
heightandtimeMsfields fromnumbertostringtype in ResponseMetadata are appropriate given the PR objective to handle uint64 values using BigInt. This ensures no precision loss for large numbers.Also applies to: 67-68, 88-91
537-538: Approve the conversion of nonce field to string type.The changes to convert
identityNoncefield fromnumbertostringtype in GetIdentityNonceResponse are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 563-563
606-607: Approve the conversion of contract nonce field to string type.The changes to convert
identityContractNoncefield fromnumbertostringtype in GetIdentityContractNonceResponse are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 632-632
675-676: Approve the conversion of balance field to string type.The changes to convert
balancefield fromnumbertostringtype in GetIdentityBalanceResponse are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 701-701
776-780: Approve the conversion of balance and revision fields to string type.The changes to convert
balanceandrevisionfields fromnumbertostringtype in BalanceAndRevision are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 794-795
1447-1448: Approve the conversion of count field to string type.The changes to convert
countfield fromnumbertostringtype in EvonodeProposedBlocks are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 1463-1463
2405-2406: Approve the conversion of startAtMs field to string type.The changes to convert
startAtMsfield fromnumbertostringtype in GetDataContractHistoryRequest are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 2426-2426
2494-2495: Approve the conversion of date field to string type.The changes to convert
datefield fromnumbertostringtype in DataContractHistoryEntry are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 2514-2514
3970-3971: Approve the conversion of time fields to string type.The changes to convert
startTimeMsandendTimeMsfields fromnumbertostringtype in StartAtTimeInfo and EndAtTimeInfo are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 3988-3989, 3994-3995, 4012-4012
4081-4082: Approve the conversion of timestamp field to string type.The changes to convert
timestampfield fromnumbertostringtype in SerializedVotePollsByTimestamp are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 4103-4103
5607-5608: Approve the conversion of time fields to string type in Time class.The changes to convert
local,block, andgenesisfields fromnumbertostringtype in Time class are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 5612-5613, 5617-5618, 5637-5640
5765-5769: Approve the conversion of time and block fields to string type in StateSync class.The changes to convert various time and block-related fields from
numbertostringtype in StateSync class are appropriate and align with the PR objective to handle uint64 values using BigInt. The affected fields include:
- totalSyncedTime
- remainingTime
- chunkProcessAvgTime
- snapshotHeight
- snapshotChunksCount
- backfilledBlocks
- backfillBlocksTotal
Also applies to: 5774-5775, 5777-5778, 5780-5781, 5783-5784, 5786-5787, 5801-5809
5035-5036: Approve the conversion of balance field to string type in GetPrefundedSpecializedBalanceResponse.The changes to convert
balancefield fromnumbertostringtype in GetPrefundedSpecializedBalanceResponse are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 5061-5061
5152-5153: Approve the conversion of credits field to string type in GetTotalCreditsInPlatformResponse.The changes to convert
creditsfield fromnumbertostringtype in GetTotalCreditsInPlatformResponse are appropriate and align with the PR objective to handle uint64 values using BigInt.Also applies to: 5178-5178
5688-5689: Approve the conversion of block height fields to string type in Chain class.The changes to convert various block height fields from
numbertostringtype in Chain class are appropriate and align with the PR objective to handle uint64 values using BigInt. The affected fields include:
- latestBlockHeight
- earliestBlockHeight
- maxPeerBlockHeight
Also applies to: 5701-5702, 5704-5705, 5727-5732
4338-4339: Approve the conversion of block height and time fields to string type in FinishedVoteInfo class.The changes to convert block height and time fields from
numbertostringtype in FinishedVoteInfo class are appropriate and align with the PR objective to handle uint64 values using BigInt. The affected fields include:
- finishedAtBlockHeight
- finishedAtBlockTimeMs
Also applies to: 4344-4345, 4364-4367
packages/dapi-grpc/clients/platform/v0/web/platform_pb.js (6)
4664-4670: LGTM! Core metadata fields correctly updated to handle uint64 values.The changes properly update the
heightandtimeMsfields inResponseMetadatato use string representation for uint64 values, which aligns with the PR objective of fixing precision loss issues.Also applies to: 4707-4712, 4719-4724, 4760-4767, 4781-4788, 4806-4822, 4860-4876
7908-7914: LGTM! Identity-related fields correctly updated to handle uint64 values.The changes properly update the identity-related fields (nonce, balance, revision) to use string representation for uint64 values across multiple response classes:
- GetIdentityNonceResponse
- GetIdentityContractNonceResponse
- GetIdentityBalanceResponse
- GetIdentityBalanceAndRevisionResponse
Also applies to: 7948-7954, 7990-7998, 8018-8032, 8360-8366, 8400-8406, 8442-8450, 8470-8484, 8812-8818, 8852-8858, 8894-8902, 8922-8936, 9406-9411, 9445-9454, 9482-9496, 9500-9516, 9519-9534
31167-31172: LGTM! Vote poll timestamp fields correctly updated to handle uint64 values.The changes properly update the vote poll timestamp fields to use string representation for uint64 values across:
- StartAtTimeInfo (startTimeMs)
- EndAtTimeInfo (endTimeMs)
- SerializedVotePollsByTimestamp (timestamp)
Also applies to: 31206-31212, 31243-31250, 31261-31277, 31327-31332, 31366-31372, 31403-31410, 31421-31437, 32020-32025, 32059-32065, 32096-32103, 32114-32130
42424-42431: LGTM! Status response fields correctly updated to handle uint64 values.The changes properly update the status response fields to use string representation for uint64 values across:
- Time (local, block, genesis)
- Chain (latestBlockHeight, earliestBlockHeight, maxPeerBlockHeight)
Also applies to: 42465-42479, 42510-42531, 42542-42574, 42596-42610, 42927-42936, 42982-42987, 42994-43004, 43056-43063, 43077-43091, 43204-43220, 43306-43340
43598-43611: LGTM! State sync fields correctly updated to handle uint64 values.The changes properly update all state sync fields to use string representation for uint64 values:
- totalSyncedTime
- remainingTime
- chunkProcessAvgTime
- snapshotHeight
- snapshotChunksCount
- backfilledBlocks
- backfillBlocksTotal
Also applies to: 43643-43677, 43704-43718, 43725-43760, 43764-43780, 43783-43798, 43818-43906
4664-43906: Verify the impact of these changes on the client applications.Since this is a breaking change that affects how uint64 values are handled across multiple response types, client applications will need to be updated to handle string values instead of numbers.
Run the following script to find potential client usage that needs to be updated:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Find potential client code that needs to be updated to handle string values # Search for direct usage of the affected response types rg -l "GetIdentityNonceResponse|GetIdentityBalanceResponse|GetStatusResponse" # Search for numeric comparisons that might need updating ast-grep --pattern 'getHeight() $$$' ast-grep --pattern 'getTimeMs() $$$' ast-grep --pattern 'getIdentityNonce() $$$' ast-grep --pattern 'getBalance() $$$'Length of output: 3485
Client Update Required: uint64 Handling Change Requires Client Code Updates
The search results confirmed that the affected response types (i.e.
GetIdentityNonceResponse,GetIdentityBalanceResponse, andGetStatusResponse) are used in multiple client repositories (such as the JS DAPI client, gRPC clients, and Rust implementations). This means that the breaking change—converting uint64 values from numbers to strings—will indeed impact client applications.
- JS Clients: Files in
packages/js-dapi-client(both source and tests) reference these response types, so their code will need to correctly handle string values.- Other Clients: Similar usages were found in gRPC and Rust-based client implementations.
Given the widespread references, client apps will need to be updated to process these as strings rather than numbers.
packages/wasm-dpp/lib/test/fixtures/getIdentityFixture.js (1)
37-37: Add validation for balance.While the conversion to BigInt aligns with the PR objectives, consider adding validation to ensure the balance is non-negative.
- identity.setBalance(BigInt(10000)); + const balance = BigInt(10000); + if (balance < 0) { + throw new Error('Balance must be non-negative'); + } + identity.setBalance(balance);packages/wasm-dpp/src/errors/consensus/basic/identity/identity_insufficient_balance_error.rs (1)
26-27: LGTM! Type change aligns with PR objectives.The change from
u32tou64for the balance method aligns with the PR's goal of fixing uint64 handling, ensuring no precision loss for large balance values.packages/js-dapi-client/lib/methods/platform/getIdentityNonce/GetIdentityNonceResponse.js (2)
4-4: LGTM! Constant updated to use BigInt.The constant is correctly updated to use BigInt, maintaining consistency with the PR's objective.
31-32: Verify BigInt conversion and bitwise operation.The bitwise operation with BigInt is implemented correctly, but ensure that the proto always returns a valid string for conversion.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if proto response always returns a string for identity nonce ast-grep --pattern 'getIdentityNonce() { return $$$; }'Length of output: 57
I'll generate an additional shell script to search for all occurrences of
getIdentityNonce(in the repository to inspect its implementation and verify that it always returns a valid string for conversion.
🏁 Script executed:
#!/bin/bash # Search for all occurrences of getIdentityNonce to inspect its implementation and return type rg "getIdentityNonce\\s*\\(" -n .Length of output: 7457
BigInt Conversion & Bitwise Operation Verified
The investigation confirms that the proto's getter method for identity nonce is defined to return a string (as seen in the generated TypeScript definitions in the web client), which ensures that converting it to BigInt for the subsequent bitwise operation is safe.
- The proto declaration (e.g., in
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts) shows thatgetIdentityNonce()returns a string.- This guarantees that
BigInt(proto.getV0().getIdentityNonce())receives a valid numeric string, making the BigInt conversion and subsequent bitwise operation withIDENTITY_NONCE_VALUE_FILTERvalid.packages/js-dapi-client/lib/methods/platform/getStatus/getStatusFactory.js (1)
48-48: LGTM! Improved response handling.The change to use
GetStatusResponse.createFromProtoprovides better structure and type safety for the response data.packages/js-dapi-client/lib/methods/platform/getStatus/ChainStatus.js (2)
91-96: Fix JSDoc return type for getCoreChainLockedHeight.The return type in JSDoc should indicate that the method can return null.
- * @returns {number} core chain locked height + * @returns {number|null} core chain locked height
6-6: Verify block height comparisons with BigInt.Since block heights are using BigInt, ensure that any comparisons or arithmetic operations in the codebase handle these values correctly.
Also applies to: 56-61
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for potential number vs bigint comparisons with block heights rg -A 2 "BlockHeight.*[<>=]"Length of output: 94613
Subject: Block Height Comparisons Using BigInt in ChainStatus.js Verified
After inspecting various files and specifically reviewing comparisons for block heights across the codebase, no instances of improper number versus BigInt operations were found in the JavaScript client. The file in question (ChainStatus.js) correctly annotates block heights as BigInt, and all the comparisons or arithmetic operations using these values in the codebase are implemented using BigInt semantics.
- No mixed-type comparisons or arithmetic operations involving plain numbers were detected in the js-dapi-client area.
- The code in ChainStatus.js (and related modules) consistently uses BigInt (with proper conversion/usage) for block height calculations.
Thus, the review comment’s concern has been validated as being handled correctly.
packages/js-dash-sdk/src/SDK/Client/Platform/methods/documents/broadcast.ts (1)
50-50: LGTM! Safe handling of large integers.Converting
identityContractNonceto string ensures proper handling of large integers in state transitions, preventing potential precision loss.packages/js-dapi-client/test/unit/methods/platform/getIdentityBalance/getIdentityBalanceFactory.spec.js (3)
30-30: LGTM! Proper initialization of test data.Using BigInt for balance initialization aligns with the new type requirements.
88-95: LGTM! Comprehensive metadata testing.The test assertions correctly verify metadata fields using BigInt comparisons where appropriate.
124-131: LGTM! Consistent metadata verification.The test maintains consistency in verifying metadata fields with BigInt comparisons.
packages/js-dapi-client/lib/methods/platform/getStatus/GetStatusResponse.js (4)
8-24: LGTM! Well-structured class implementation.The class provides a clean interface for encapsulating various status components with proper JSDoc documentation.
101-106: LGTM! Safe handling of chain heights.Proper conversion of block heights to BigInt prevents precision loss for large values.
126-134: LGTM! Comprehensive state sync status handling.All time and count fields are properly converted to BigInt to handle large values safely.
146-149: LGTM! Safe time value handling.Time values are correctly converted to BigInt to prevent precision loss.
packages/js-dapi-client/test/unit/methods/platform/getDataContract/GetDataContractResponse.spec.js (3)
66-67: LGTM! Complete metadata setup.Test properly sets up all required metadata fields including time and protocol version.
79-86: LGTM! Proper metadata verification.Test assertions correctly verify metadata fields using BigInt comparisons where appropriate.
118-125: LGTM! Consistent metadata verification in proof tests.Test maintains consistency in verifying metadata fields with BigInt comparisons in proof-related tests.
packages/wasm-dpp/src/identity/identity.rs (1)
108-109: LGTM! Type safety improvements for balance and revision handling.The changes from
f64tou64for balance and revision methods improve type safety and prevent potential precision loss when handling large numbers. This aligns well with the PR's objective of fixing uint64 handling.Also applies to: 113-114, 118-119, 123-124, 128-129, 133-134, 138-139
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs (3)
20-20: LGTM! Added Revision type import.Added import for the Revision type to support proper typing.
99-100: LGTM! Improved revision handling.Changed to return
Option<Revision>which better represents the optional nature of revision values.
235-236: LGTM! Enhanced serialization for large numbers.Configured the serializer to handle large number types as BigInts, preventing precision loss.
packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/document_create_transition.rs (1)
82-84: LGTM! Consistent revision type usage.Changed initial_revision to return u64 instead of u32, maintaining consistency with other revision-related changes and preventing potential overflow issues.
packages/wasm-dpp/src/identity/state_transition/identity_credit_transfer_transition/transition.rs (2)
93-94: LGTM! Improved amount handling precision.Changed amount methods from f64 to u64 to prevent floating-point precision loss when handling large numbers.
Also applies to: 98-99
184-184: LGTM! Enhanced JavaScript numeric interop.Updated to use bigint string representation when converting to JavaScript objects, ensuring precise handling of large numbers across the WASM boundary.
Also applies to: 259-259
packages/wasm-dpp/src/document/extended_document.rs (2)
136-140: LGTM: Type conversion looks correct.The conversion from
u64toRevisioninset_revisionis safe as it's using theasoperator with the same size types.
143-145: LGTM: Direct mapping of types.The
get_revisionmethod now correctly returnsOption<u64>without any type conversion needed.packages/js-dapi-client/test/unit/methods/platform/getStatus/GetStatusResponse.spec.js (1)
113-122: LGTM: Comprehensive state sync status tests.Good coverage of all state sync numeric fields with BigInt conversions.
Also applies to: 245-262
packages/wasm-dpp/.eslintrc (1)
3-5: LGTM: ES2020 environment enabled for BigInt support.Enabling ES2020 environment is required for BigInt syntax support, which aligns with the PR's objective of using BigInt for uint64 values.
packages/dapi-grpc/protos/platform/v0/platform.proto (4)
72-73: LGTM: ResponseMetadata fields correctly marked for string conversion.The height and time_ms fields in ResponseMetadata are correctly marked with jstype = JS_STRING to prevent precision loss.
Also applies to: 75-75
1131-1136: LGTM: Time-related fields properly configured for string conversion.All time-related uint64 fields (local, block, genesis) are correctly marked with jstype = JS_STRING.
1151-1155: LGTM: Chain height fields properly configured for string conversion.All block height related uint64 fields are correctly marked with jstype = JS_STRING:
- latest_block_height
- earliest_block_height
- max_peer_block_height
1168-1176: LGTM: State sync fields properly configured for string conversion.All state sync related uint64 fields are correctly marked with jstype = JS_STRING:
- total_synced_time
- remaining_time
- chunk_process_avg_time
- snapshot_height
- snapshot_chunks_count
- backfilled_blocks
- backfill_blocks_total
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/platform-test-suite/test/functional/platform/getStatus.spec.js (1)
20-22: Consider adding more comprehensive test coverage.While the current assertions verify the basic type and presence of version strings, consider:
- Adding assertions to validate version string format (e.g., semantic versioning)
- Testing other status properties that might be affected by the uint64 to BigInt changes
- Using more descriptive assertion messages
Example enhancement:
- expect(status.getVersionStatus().getDapiVersion()).to.be.a('string').to.exist(); + const versionStatus = status.getVersionStatus(); + expect(versionStatus.getDapiVersion(), 'DAPI version should be a valid semver') + .to.be.a('string') + .to.exist() + .and.match(/^\d+\.\d+\.\d+/); + + // Test uint64 fields if any + const timeStatus = status.getTimeStatus(); + expect(timeStatus.getAdjustedTime(), 'Adjusted time should be a BigInt') + .to.be.a('bigint');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js(8 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts(6 hunks)packages/platform-test-suite/lib/test/fixtures/getDataContractFixture.js(1 hunks)packages/platform-test-suite/test/functional/platform/getStatus.spec.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js
- packages/js-dash-sdk/src/SDK/Client/Platform/NonceManager/NonceManager.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Linting
- 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 (3)
packages/platform-test-suite/test/functional/platform/getStatus.spec.js (1)
20-22: LGTM! Good use of encapsulation with getter methods.The change from direct property access to getter methods improves encapsulation and maintainability.
packages/platform-test-suite/lib/test/fixtures/getDataContractFixture.js (2)
13-13: LGTM! Good type flexibility.The updated JSDoc type annotation allows for a flexible transition to BigInt while maintaining backward compatibility.
154-154: Verify BigInt conversion edge cases.The BigInt conversion looks good and ensures consistent type handling. However, let's verify the behavior with edge cases.
Let's add test cases to verify the BigInt conversion handles all input types correctly:
describe('getDataContractFixture', () => { const testCases = [ { input: '9007199254740991', expected: 9007199254740991n }, // Max safe integer as string { input: 42, expected: 42n }, // Number { input: 9007199254740993n, expected: 9007199254740993n }, // BigInt beyond safe integer ]; testCases.forEach(({ input, expected }) => { it(`should handle ${typeof input} input: ${input}`, async () => { const contract = await getDataContractFixture(input); // Verify the nonce was converted correctly // Note: Implementation details of how to access the nonce may vary }); }); });Would you like me to create a new issue to track the addition of these test cases?
✅ Verification successful
Error: No shell scripts or web queries found
BigInt conversion verified; consider tracking additional test cases.
The
BigInt(identityNonce)conversion at line 154 inpackages/platform-test-suite/lib/test/fixtures/getDataContractFixture.jscorrectly ensures consistent type handling across string, number, or bigint inputs. The proposed test cases (covering string representations, numbers, and already-bigint values) will further validate edge case behavior. Please proceed with opening an issue to track adding the tests if you agree.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/platform-test-suite/test/functional/platform/Identity.spec.js (1)
552-555: Update remaining balance comparisons for BigInt compatibility.Several balance comparisons in the test suite need to be updated to handle BigInt values correctly:
-expect(recipientBalanceAfter).to.be.equal(recipientBalanceBefore + transferAmount); +expect(recipientBalanceAfter).to.be.a('bigint').and.to.equal(recipientBalanceBefore + BigInt(transferAmount)); -expect(identityBalanceAfter).to.be.lessThan(identityBalanceBefore + transferAmount); +expect(identityBalanceAfter).to.be.a('bigint').and.to.be.below(identityBalanceBefore + BigInt(transferAmount)); -identity.getBalance() + 1, +identity.getBalance() + 1n,Also applies to: 569-569
🧹 Nitpick comments (1)
packages/platform-test-suite/test/functional/platform/Identity.spec.js (1)
320-320: Consider using a more idiomatic BigInt assertion.The change correctly handles BigInt comparison, but there's a more idiomatic way to write this assertion using Chai's built-in BigInt support.
-expect(fetchedIdentity.getBalance() > 0).to.be.true(); +expect(fetchedIdentity.getBalance()).to.be.a('bigint').and.to.be.above(0n);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/platform-test-suite/test/functional/platform/Identity.spec.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (1)
packages/platform-test-suite/test/functional/platform/Identity.spec.js (1)
324-324: Add test cases for large uint64 values.Given that this PR aims to fix precision loss issues with uint64 values, consider adding test cases that verify behavior with values exceeding
Number.MAX_SAFE_INTEGER(2^53 - 1).Example test cases to consider:
it('should handle large uint64 values correctly', async () => { const largeAmount = BigInt('18446744073709551615'); // max uint64 const mediumAmount = BigInt('9223372036854775807'); // max int64 // Test credit transfer with large amounts await client.platform.identities.creditTransfer( identity, recipient.getId(), largeAmount, ); const recipientAfterTransfer = await client.platform.identities.get( recipient.getId(), ); expect(recipientAfterTransfer.getBalance()).to.be.a('bigint') .and.to.equal(largeAmount); });
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/platform-test-suite/test/functional/platform/Identity.spec.js (1)
1-735: Consider adding explicit BigInt conversion tests.While the existing tests cover the functionality, consider adding explicit test cases that verify the behavior with values that exceed JavaScript's Number.MAX_SAFE_INTEGER to ensure the BigInt migration properly handles large uint64 values.
it('should handle large uint64 values', async () => { const largeAmount = BigInt(Number.MAX_SAFE_INTEGER) + BigInt(1); // Test with large values // ... });🧰 Tools
🪛 Biome (1.9.4)
[error] 225-225: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 228-228: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 313-313: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 316-316: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 627-627: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/platform-test-suite/test/e2e/withdrawals.spec.js(3 hunks)packages/platform-test-suite/test/functional/platform/Document.spec.js(1 hunks)packages/platform-test-suite/test/functional/platform/Identity.spec.js(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Tests
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (9)
packages/platform-test-suite/test/functional/platform/Document.spec.js (1)
303-303: LGTM! The BigInt conversion aligns with PR objectives.The change from numeric to BigInt increment is correct and consistent with the PR's goal of properly handling uint64 values.
Note that this change is within a skipped test case. Please verify if this test can be unskipped now that the BigInt conversion is in place.
packages/platform-test-suite/test/e2e/withdrawals.spec.js (3)
113-113: LGTM! Correct assertion style for BigInt comparison.The change to use native JavaScript comparison operators instead of Chai's built-in assertions is the right approach for comparing BigInt values.
175-175: LGTM! Correct BigInt multiplication.The explicit conversion of the multiplier to BigInt using
BigInt(2)is necessary as BigInt values can only be operated with other BigInt values.
190-190: LGTM! Correct BigInt division.The explicit conversion of the divisor to BigInt using
BigInt(2)is necessary as BigInt values can only be operated with other BigInt values.packages/platform-test-suite/test/functional/platform/Identity.spec.js (5)
232-232: LGTM! Balance comparison updates align with BigInt migration.The changes correctly update balance comparisons to use direct boolean expressions, which is more explicit and compatible with BigInt values.
Also applies to: 320-320, 433-435
422-422: LGTM! Proper BigInt calculation for credit amounts.The calculation correctly uses BigInt for both operands to prevent precision loss when dealing with large numbers.
551-552: LGTM! Credit transfer assertions properly handle BigInt values.The balance comparisons and arithmetic operations correctly use BigInt, ensuring precise credit transfer validation.
Also applies to: 555-556
570-570: LGTM! Proper BigInt usage in insufficient balance test.The test correctly uses BigInt addition to verify the behavior when attempting to transfer more credits than available.
622-622: LGTM! Revision comparisons updated to use BigInt.The identity revision comparisons are correctly updated to use BigInt arithmetic, maintaining consistency with the uint64 migration.
Also applies to: 654-654
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/wasm-dpp/src/metadata.rs (2)
37-48: LGTM! Consider simplifying protocol_version casting.The constructor changes improve type safety by using explicit numeric types. The use of
u64forblock_heightandtime_mseffectively prevents precision loss, which aligns with the PR objectives.Consider simplifying the double cast of
protocol_version:- protocol_version: protocol_version as u64 as ProtocolVersion, + protocol_version: ProtocolVersion::from(protocol_version as u64),
71-88: LGTM! Remove redundant cast in time_ms getter.The getter methods now correctly return native types (u64/u32) instead of f64, which prevents precision loss and aligns with the PR objectives.
The cast in the
time_msgetter is redundant sinceself.0.time_msis alreadyu64:- self.0.time_ms as u64 + self.0.time_ms
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js(3 hunks)packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts(2 hunks)packages/platform-test-suite/test/e2e/withdrawals.spec.js(4 hunks)packages/platform-test-suite/test/functional/platform/Epoch.spec.js(1 hunks)packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs(3 hunks)packages/wasm-dpp/src/metadata.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/wasm-dpp/src/document/state_transition/document_batch_transition/document_transition/mod.rs
- packages/js-dash-sdk/src/SDK/Client/Platform/methods/contracts/history.ts
- packages/platform-test-suite/test/e2e/withdrawals.spec.js
- packages/js-dapi-client/lib/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.js
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- 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 (1)
packages/platform-test-suite/test/functional/platform/Epoch.spec.js (1)
27-27: LGTM! Verify other uint64 fields in the response.The change to use
BigInt(1)aligns with the PR's objective to handle uint64 values correctly. This prevents potential precision loss issues.Please verify that all other uint64 fields in the
getEpochsInforesponse are also being handled as BigInt. Run this script to check the response structure:✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for other uint64 fields in the getEpochsInfo response that might need BigInt assertions # Search for field definitions in proto files rg -A 5 "uint64.*first_block_height|uint64.*epoch" packages/dapi-grpc/protos/Length of output: 957
Double-check BigInt Conversions in getEpochsInfo Response
The shell script confirms that, besides the already-updated
first_block_height, the proto defines another uint64 field:start_time(also annotated withjstype = JS_STRING). Please ensure that any tests or code usingstart_timesimilarly convert or assert its value as a BigInt where precision is critical.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js (1)
44-50: Consider adding specific BigInt value assertions.While the deep equality checks work, consider adding specific test cases to verify that the returned dates are actually BigInt instances and maintain precision.
it('should return data contract history', () => { const dataContractHistory = getDataContractHistoryResponse.getDataContractHistory(); const proof = getDataContractHistoryResponse.getProof(); expect(dataContractHistory).to.deep.equal(dataContractHistoryFixture); + expect(dataContractHistory[0].date).to.be.instanceof(BigInt); + expect(dataContractHistory[0].date.toString()).to.equal('10000'); expect(proof).to.equal(undefined); });Also applies to: 52-67
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js(8 hunks)packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/getDataContractHistoryFactory.spec.js
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/js-dapi-client/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js (2)
28-35: LGTM! Proper BigInt usage in fixture setup.The fixture correctly uses BigInt for date values, aligning with the PR's objective to handle uint64 values properly.
77-78: LGTM! Proper handling of proto conversion and metadata.The implementation correctly:
- Sets proto date values as strings
- Converts metadata numeric fields to BigInt
- Includes proper assertions for all metadata fields
Also applies to: 82-83, 151-158
...ent/test/unit/methods/platform/getDataContractHistory/GetDataContractHistoryResponse.spec.js
Show resolved
Hide resolved
Issue being fixed or feature implemented
Implements #2436
Currently, protobuf JS library incorrectly maps
u64values from the protocol to a JS typenumber, which is not intended for such big values. That leads to the precision loss on a nonce param (which is a value, not number), making SDK unable to submit any transactions to the given data contract anymore.While working on a fix, we found out that WASM DPP actually have the same issue
This PR implements BigInt JS type in dapi-grpc, js dash sdk, and wasm-dpp.
What was done?
[jstype = JS_STRING]).toObject()->ResponseClass)How Has This Been Tested?
Breaking Changes
All uint64 types from the DAPI GRPC now represent as
BigIntinstead ofnumberin the SDK, such as credits, timestamps (ms), platform block heights, and all related queries are affected.Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Summary by CodeRabbit
bigint.Metadataobjects to improve clarity and type consistency.bigint, enhancing the handling of large numeric values.numbertobigint.