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 11 minutes and 22 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 (74)
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
Possibly related PRs
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
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