Add biguint and bigdecimal bcs types #76
Conversation
WalkthroughThe changes involve updates to the Jest configuration for TypeScript support, an upgrade of the Changes
Sequence Diagram(s)sequenceDiagram
participant A as User
participant B as Jest
participant C as TypeScript
participant D as Bcs Module
A->>B: Run Tests
B->>C: Transform TypeScript Files
C-->>B: Processed Files
B->>D: Serialize/Parse Big Numbers
D-->>B: Test Results
B-->>A: Return Test Results
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 (
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Filespackage-lock.jsonpackage.json
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (7)
- jest.config.mjs (1 hunks)
- package.json (1 hunks)
- src/client/lcd/api/IbcTransferAPI.spec.ts (1 hunks)
- src/core/auth/BaseAccount.spec.ts (1 hunks)
- src/core/upgrade/Plan.spec.ts (1 hunks)
- src/util/bcs.spec.ts (1 hunks)
- src/util/bcs.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- src/client/lcd/api/IbcTransferAPI.spec.ts
- src/core/auth/BaseAccount.spec.ts
Additional comments not posted (6)
jest.config.mjs (2)
4-6: Correct implementation of TypeScript transformation in Jest configuration.The
transformproperty has been correctly added to handle.tsand.tsxfiles usingts-jest. This is a standard approach for integrating TypeScript with Jest, ensuring that TypeScript files are properly transformed during testing.
9-9: Proper use of semicolon in export statement.The addition of a semicolon at the end of the export statement is a minor but good practice for maintaining consistency in JavaScript syntax, especially in module files.
src/core/upgrade/Plan.spec.ts (2)
13-13: Verify the change in data type for theheightfield.The test case has been updated to expect the
heightfield as a number instead of a string. This change should be verified across the codebase to ensure consistency and correct handling of theheightfield in all related areas.Run the following script to verify the usage of the
heightfield:Verification successful
The
heightfield is consistently used as a number across the codebase.The change in the
heightfield from a string to a number in the test case is consistent with its usage in other parts of the codebase, such as inTxAPI.ts,UpgradeAPI.ts, andPlan.ts. This confirms the correctness of the test case update.
src/core/upgrade/Plan.spec.ts:heightis expected as a number.src/core/upgrade/Plan.ts:heightis parsed as a number.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `height` field across the codebase. # Test: Search for the `height` field usage. Expect: Consistent usage as a number. rg --type typescript -A 5 $'height'Length of output: 69
Script:
#!/bin/bash # Description: Verify the usage of the `height` field across the codebase. # Test: Search for the `height` field usage. Expect: Consistent usage as a number. rg --type ts -A 5 $'height'Length of output: 179493
13-13: Verify the removal of deprecated fields.The removal of the
timeandupgraded_client_statefields from the input object should be verified to ensure they are no longer required or used elsewhere in the codebase.Run the following script to verify the removal of these fields:
src/util/bcs.spec.ts (1)
1-1: Ensure proper import paths.Verify that the import path for the
bcsmodule is correct and that the module exports the necessary functions and types used in this file.package.json (1)
49-49: Approve thets-jestversion update.The update from
^29.1.1to^29.2.5ints-jestis noted. This change is likely aimed at leveraging improvements or bug fixes in the newer version. It's important to ensure that this update integrates well with the existing project setup and does not introduce any compatibility issues with other dependencies.Please verify the compatibility of the new
ts-jestversion with the entire project setup, especially with TypeScript configurations and other related dependencies. This can be done by running the test suite and checking for any new warnings or errors that might arise due to this version change.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (3)
src/client/lcd/api/SlashingAPI.spec.ts (1)
1-5: Action Required: API Endpoint Not Fully OperationalThe API endpoint
https://lcd.devnet.initia.xyz/is reachable, but it returned a501 Not Implementedstatus code. This indicates that the server does not support the functionality required to fulfill the request. Please verify the server configuration or contact the server administrator to ensure that the endpoint is fully operational and supports the necessary HTTP methods.
- Ensure that the server is correctly configured to handle requests.
- Verify that all necessary services are running on the server.
Analysis chain
Approved: Import and API endpoint changes.
The import of
Durationis correctly added for use in the test. The update of the APIRequester's base URL tohttps://lcd.devnet.initia.xyz/reflects a shift to a development environment. Ensure that this environment is fully operational and that all endpoints function as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the readiness of the new API endpoint. # Test: Check if the new API endpoint is reachable. curl -I https://lcd.devnet.initia.xyz/Length of output: 640
src/client/lcd/api/MstakingAPI.spec.ts (1)
5-5: API Endpoint Not Accessible: Server Returned 501 Status CodeThe API endpoint
https://lcd.devnet.initia.xyz/is not accessible as expected, returning a 501 status code. This indicates that the server does not support the functionality required to fulfill the request. Please verify the server configuration or endpoint setup to ensure it is correctly configured and operational in the development environment.Analysis chain
API endpoint updated to reflect environment shift.
The API endpoint for
APIRequesterhas been changed from'https://stone-rest.initia.tech/'to'https://lcd.devnet.initia.xyz/'. This update likely reflects a shift to a different testing or development environment. It's important to verify that this new endpoint is correctly configured and accessible in the development environment.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the accessibility of the new API endpoint. # Test: Check if the new API endpoint is reachable. Expect: HTTP 200 status code. curl -o /dev/null -s -w "%{http_code}\n" https://lcd.devnet.initia.xyz/ | grep 200Length of output: 84
Script:
#!/bin/bash # Description: Verify the accessibility of the new API endpoint and capture the HTTP status code. # Test: Check the HTTP status code returned by the new API endpoint. curl -o /dev/null -s -w "%{http_code}\n" https://lcd.devnet.initia.xyz/Length of output: 77
src/client/lcd/LCDClient.ts (1)
74-74: Endpoint Not Ready; Reconsider URL ChangeThe new endpoint
https://lcd.devnet.initia.xyzis not fully operational, as indicated by the "Not Implemented" response to a GET request. It is advisable to revert the URL change in the example code or ensure that the server is correctly configured and ready for use before proceeding with this change.
- Ensure the server is fully configured and operational.
- Consider reverting the URL change until the endpoint is ready.
Analysis chain
URL Change Approved; Verify Endpoint Configuration
The change in the URL within the example code is approved as it correctly reflects the shift to a development environment. However, it's crucial to verify that the new endpoint (
https://lcd.devnet.initia.xyz) is fully configured and operational to ensure seamless integration and functionality.Run the following script to verify the endpoint configuration:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new endpoint configuration. # Test: Check if the new URL is reachable and responds correctly. curl -I https://lcd.devnet.initia.xyzLength of output: 639
Script:
#!/bin/bash # Description: Verify the new endpoint configuration with a GET request. # Test: Check if the new URL responds correctly to a GET request. curl -X GET https://lcd.devnet.initia.xyzLength of output: 494
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- src/client/lcd/APIRequester.spec.ts (3 hunks)
- src/client/lcd/LCDClient.ts (1 hunks)
- src/client/lcd/api/AuthAPI.spec.ts (1 hunks)
- src/client/lcd/api/BankAPI.spec.ts (1 hunks)
- src/client/lcd/api/DistributionAPI.spec.ts (1 hunks)
- src/client/lcd/api/FeeGrantAPI.spec.ts (1 hunks)
- src/client/lcd/api/GovAPI.spec.ts (1 hunks)
- src/client/lcd/api/IbcAPI.spec.ts (1 hunks)
- src/client/lcd/api/IbcAPI.ts (1 hunks)
- src/client/lcd/api/IbcTransferAPI.spec.ts (1 hunks)
- src/client/lcd/api/MstakingAPI.spec.ts (1 hunks)
- src/client/lcd/api/RewardAPI.spec.ts (2 hunks)
- src/client/lcd/api/SlashingAPI.spec.ts (1 hunks)
- src/client/lcd/api/TendermintAPI.spec.ts (1 hunks)
- src/client/lcd/api/TxAPI.spec.ts (1 hunks)
- src/client/lcd/api/UpgradeAPI.spec.ts (2 hunks)
Files skipped from review due to trivial changes (4)
- src/client/lcd/api/AuthAPI.spec.ts
- src/client/lcd/api/BankAPI.spec.ts
- src/client/lcd/api/DistributionAPI.spec.ts
- src/client/lcd/api/IbcAPI.spec.ts
Files skipped from review as they are similar to previous changes (1)
- src/client/lcd/api/IbcTransferAPI.spec.ts
Additional comments not posted (16)
src/client/lcd/api/SlashingAPI.spec.ts (1)
12-15: Approved: Updated type expectations in tests.The changes in expected types for
min_signed_per_window,downtime_jail_duration,slash_fraction_double_sign, andslash_fraction_downtimeenhance type safety and accuracy. Verify that these changes accurately reflect the data types returned by the API.src/client/lcd/api/UpgradeAPI.spec.ts (2)
5-5: Updated API endpoint.The API endpoint has been updated from
'https://stone-rest.initia.tech/'to'https://lcd.devnet.initia.xyz/'. This change is likely to reflect a shift to a development environment, which is a common practice. Ensure that all references to the old endpoint are updated across the entire test suite to maintain consistency.Verification successful
Old API endpoint references removed.
The search for the old API endpoint
'https://stone-rest.initia.tech/'returned no results, indicating that all references have been successfully updated to the new endpoint. This ensures consistency across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all references to the old API endpoint are updated. # Test: Search for the old API endpoint. Expect: No occurrences. rg --type ts 'https://stone-rest.initia.tech/'Length of output: 46
25-25: Updated expected length of module versions.The expected length of the module versions returned by the
upgradeinstance has been updated from 22 to 34. This change suggests an update in the underlying data or functionality, potentially due to the introduction of new modules or changes in the API. Verify that this change is consistent with the updates made in the PR and accurately reflects the new state of the API.src/client/lcd/api/RewardAPI.spec.ts (2)
5-5: Approved: Base URL change for APIRequester.The update to the base URL from
'https://stone-rest.initia.tech/'to'https://lcd.devnet.initia.xyz/'is appropriate for development environments, ensuring that tests are isolated from production.
22-22: Approved: Change in expected type fordilution_period.Changing the expected type from
expect.any(String)toexpect.any(Duration)enhances type safety and clarity in the test. It's important to ensure that theDurationclass is implemented correctly and integrates well with the rest of the application.Verify the implementation of the
Durationclass:Verification successful
Verified: The
Durationclass is implemented correctly.The
Durationclass insrc/core/Duration.tsis well-implemented, providing comprehensive functionality for handling duration data. The change in the test expectation to useexpect.any(Duration)is appropriate and enhances type safety. No further issues were found with this change.
- Location of
Durationclass:src/core/Duration.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of the `Duration` class. # Test: Search for the `Duration` class definition and usage. Expect: Proper implementation and usage across the application. ast-grep --lang typescript --pattern $'class Duration { $$$ }'Length of output: 2389
src/client/lcd/APIRequester.spec.ts (3)
16-20: Verify the usage ofaxios.AxiosHeaders().The test uses
axios.AxiosHeaders()to set headers, which is not a standard export from axios. Please verify its existence or consider using a more typical approach, such as directly using an object for headers.
31-35: Verify URL construction and header usage.Ensure that the URL is constructed correctly when using a base URL with a path segment. Also, verify the usage of
axios.AxiosHeaders()as mentioned in the previous comment.
46-50: Verify URL construction with search parameters and header usage.Ensure that the URL with search parameters is constructed correctly and that the query string is preserved. Also, verify the usage of
axios.AxiosHeaders()as mentioned in previous comments.src/client/lcd/api/TendermintAPI.spec.ts (1)
6-6: Verify the new API endpoint URL.The change in the API endpoint URL from
'https://stone-rest.initia.tech/'to'https://lcd.devnet.initia.xyz/'is significant as it indicates a shift to a development environment. This change can affect the behavior of the API being tested. It is crucial to verify:
- The correctness of the new URL.
- That the development environment is properly configured to handle the requests made by these tests.
Run the following script to verify the correctness of the new URL:
src/client/lcd/api/FeeGrantAPI.spec.ts (1)
4-4: Address the commented-out test suite and URL change.The change in the URL from a production or staging environment to a development environment is noted. However, the entire test suite is commented out, which raises concerns about the current state of testing for this API. It's crucial to ensure that the tests are active and accurately reflect the intended environment to maintain the reliability of the test suite.
Please verify the following:
- The reason for commenting out the entire test suite.
- Plans to reactivate or update these tests to reflect the new environment.
src/client/lcd/api/MstakingAPI.spec.ts (2)
3-3: Updated imports to include new dependencies.The import statement has been expanded to include
DurationandValConsPublicKeyfrom the../../../coremodule. This change is consistent with the new test assertions that utilize these types, ensuring that the necessary classes are available for the tests.
11-11: Refined test assertions to use more specific types.The test assertions for
unbonding_timeandmin_commission_ratehave been updated:
unbonding_timenow expects aDurationtype instead of a genericNumber, which enhances the specificity and correctness of the test.min_comission_ratehas been added as a new expectation, and it expects aString. This addition helps ensure that the API's response adheres to the updated contract.These changes are crucial for maintaining the integrity of the tests and ensuring they align with the updated API specifications.
Also applies to: 17-17
src/client/lcd/api/GovAPI.spec.ts (1)
3-3: Imports and API endpoint setup are correctly updated.The imports for
CoinsandDurationare correctly added to support the new test cases. The update to the API endpoint URL is consistent with the shift to a development environment, aligning with the PR objectives.Also applies to: 5-5
src/client/lcd/api/TxAPI.spec.ts (1)
11-11: Verify the URL change forLCDClientinitialization.The URL has been changed from a production to a development environment. This is a significant change as it affects how the application interacts with the blockchain and could impact testing scenarios.
Please confirm that this change was intentional and verify that all related configurations and tests are aligned with this new environment.
src/client/lcd/api/IbcAPI.ts (2)
138-140: Endpoint Change inparametersMethodThe API endpoint in the
parametersmethod has been updated from/ibc/client/v1/paramsto/ibc/core/client/v1/params. This change is crucial as it may affect the data retrieved and the overall functionality of the API. Ensure that this change is reflected in all associated documentation and that any dependent systems are updated accordingly.Please verify that this endpoint change has been communicated to all stakeholders and that integration tests cover this new endpoint to ensure it behaves as expected.
138-140: Consistency of Endpoint ChangeThe updated endpoint in the
parametersmethod aligns with the other IBC-related API endpoints in this file, which also use the/ibc/core/...prefix. This consistency suggests that the change is part of a broader architectural update to standardize the API endpoint structure across the system.
Summary by CodeRabbit
New Features
bcsmodule, improving serialization and deserialization capabilities.bcsmodule to ensure accurate serialization and parsing of large numeric values.Bug Fixes
Improvements