chore: total inj burnt #530
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.tsOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "./../../eslintrc.js" to extend from. Please check that the name of the config is correct. The config "./../../eslintrc.js" was referenced from the config file in "/packages/sdk-ts/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThis pull request introduces a new feature to fetch INJ (Injective) burnt tokens in the Injective SDK. The changes span multiple files in the Changes
Sequence DiagramsequenceDiagram
participant Client
participant IndexerGrpcAuctionApi
participant GrpcClient
participant Transformer
Client->>IndexerGrpcAuctionApi: fetchInjBurnt()
IndexerGrpcAuctionApi->>GrpcClient: Call InjBurntEndpoint
GrpcClient-->>IndexerGrpcAuctionApi: Return InjBurntEndpointResponse
IndexerGrpcAuctionApi->>Transformer: Transform response
Transformer-->>IndexerGrpcAuctionApi: Return TotalInjBurnt
IndexerGrpcAuctionApi-->>Client: Return total INJ burnt
Possibly Related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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 (
|
shane-moore
requested review from
ThomasRalee,
ivan-angjelkoski and
Frederick-88
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.ts (1)
51-68: Enhance test coverage with edge casesWhile the basic happy path is covered, consider adding test cases for:
- Error scenarios (e.g., network errors)
- Response validation (e.g., negative values)
- Zero value responses
test('fetchInjBurnt handles errors', async () => { // Mock client to throw error const error = new Error('Network error'); jest.spyOn(indexerGrpcAuctionApi.client, 'InjBurntEndpoint').mockRejectedValue(error); await expect(indexerGrpcAuctionApi.fetchInjBurnt()).rejects.toThrow(); }); test('fetchInjBurnt handles zero value', async () => { // Test with zero burnt tokens const response = { totalInjBurnt: '0' }; jest.spyOn(indexerGrpcAuctionApi.client, 'InjBurntEndpoint').mockResolvedValue(response); const result = await indexerGrpcAuctionApi.fetchInjBurnt(); expect(result).toBe(0); });packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts (1)
87-112: LGTM: Implementation follows established patternsThe implementation correctly follows the existing patterns for error handling and retry mechanism. A few minor suggestions:
- Consider using
InjectiveAuctionRpc.InjBurntEndpointRequest.create()for consistency with other methods- Add JSDoc comments to document the method's purpose and return type
+ /** + * Fetches the total amount of INJ tokens that have been burnt + * @returns {Promise<TotalInjBurnt>} The total amount of burnt INJ tokens + */ async fetchInjBurnt() { - const request: InjectiveAuctionRpc.InjBurntEndpointRequest = {} + const request = InjectiveAuctionRpc.InjBurntEndpointRequest.create()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
packages/sdk-ts/package.json(1 hunks)packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.ts(1 hunks)packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts(1 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts(2 hunks)packages/sdk-ts/src/client/indexer/types/auction.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/sdk-ts/src/client/indexer/types/auction.ts
🔇 Additional comments (3)
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (1)
5-8: LGTM: Import statements are properly organized
The new type imports are correctly added and organized with related types.
packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.ts (1)
87-112: Verify endpoint usage and error handling
Let's verify the implementation across the codebase:
✅ Verification successful
Let me gather more information about the implementation and its usage.
Let me check the test implementation to ensure proper test coverage.
Implementation verified and properly integrated
The fetchInjBurnt implementation is correctly integrated within the codebase with:
- Proper error handling and consistent with other endpoints in the module
- Complete type definitions (
TotalInjBurnt) - Transformer implementation for response handling
- Test coverage in place
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any other usages of InjBurntEndpoint
rg -l "InjBurntEndpoint"
# Check for similar implementations in other modules
ast-grep --pattern 'class $_ extends BaseGrpcConsumer {
$$$
async fetch$_() {
$$$
}
}'
Length of output: 269
Script:
#!/bin/bash
# Check the transformer implementation
cat packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts
# Check for any tests related to this endpoint
rg -l "fetchInjBurnt"
# Look for any interface definitions or types related to this endpoint
rg "InjBurntEndpoint" -A 3 -B 3
Length of output: 5380
Script:
#!/bin/bash
# Check the test implementation
cat packages/sdk-ts/src/client/indexer/grpc/IndexerGrpcAuctionApi.spec.ts
# Check for any type definitions
rg "TotalInjBurnt" -A 3 -B 3
Length of output: 4058
packages/sdk-ts/package.json (1)
130-130: Verify the dependency version update.
The update of @injectivelabs/indexer-proto-ts to version 1.13.4 aligns with the PR objectives for implementing INJ burnt token functionality.
Let's verify the version and check for any security concerns:
✅ Verification successful
Dependency update to @injectivelabs/[email protected] is safe and up-to-date
The verification confirms:
- The version
1.13.4is the latest available version on npm registry - No security vulnerabilities were found for this package
- The patch version bump is appropriate for the feature addition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the dependency version and check for security advisories.
# Check if this is the latest version
curl -s "https://registry.npmjs.org/@injectivelabs%2Findexer-proto-ts" | jq -r '."dist-tags".latest'
# Check for any security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@injectivelabs/indexer-proto-ts") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check for compatibility with other @injectivelabs dependencies
rg '"@injectivelabs/[^"]+": "[^"]+"' package.json
Length of output: 523
| static injBurntResponseToInjBurnt( | ||
| response: InjectiveAuctionRpc.InjBurntEndpointResponse, | ||
| ): TotalInjBurnt { | ||
| return Number(response.totalInjBurnt) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using BigNumber for safe number conversion
The current implementation uses Number() for converting the total burnt amount. Since this could be a large value representing token amounts, consider using BigNumber to avoid potential precision loss or overflow issues.
- static injBurntResponseToInjBurnt(
- response: InjectiveAuctionRpc.InjBurntEndpointResponse,
- ): TotalInjBurnt {
- return Number(response.totalInjBurnt)
- }
+ static injBurntResponseToInjBurnt(
+ response: InjectiveAuctionRpc.InjBurntEndpointResponse,
+ ): TotalInjBurnt {
+ return new BigNumber(response.totalInjBurnt).toNumber()
+ }📝 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.
| static injBurntResponseToInjBurnt( | |
| response: InjectiveAuctionRpc.InjBurntEndpointResponse, | |
| ): TotalInjBurnt { | |
| return Number(response.totalInjBurnt) | |
| } | |
| static injBurntResponseToInjBurnt( | |
| response: InjectiveAuctionRpc.InjBurntEndpointResponse, | |
| ): TotalInjBurnt { | |
| return new BigNumber(response.totalInjBurnt).toNumber() | |
| } |
Summary by CodeRabbit
New Features
fetchInjBurntin the API, enhancing data retrieval capabilities.injBurntResponseToInjBurntfor transforming response data.TotalInjBurntadded to improve type definitions.Bug Fixes
Tests
fetchInjBurntmethod to ensure its reliability.