feat: devgas query #325
feat: devgas query #325
Conversation
WalkthroughThe recent updates introduce the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
## [3.1.0](v3.0.14...v3.1.0) (2024-02-28) ### Features * devgas query ([#325](#325)) ([769831a](769831a)) [skip ci]
|
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (4)
- src/sdk/query/devgas.ts (1 hunks)
- src/sdk/query/index.ts (1 hunks)
- src/sdk/query/query.ts (3 hunks)
- src/sdk/tx/txClient.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- src/sdk/query/index.ts
Additional comments: 5
src/sdk/query/devgas.ts (1)
- 14-22: The
DevgasExtensioninterface is well-defined, providing a clear contract for interacting with the Devgas extension. Each method corresponds to a specific Devgas-related query, ensuring a modular and focused approach to querying Devgas data.src/sdk/tx/txClient.ts (2)
- 30-30: The inclusion of
setupDevgasExtensionin the import statement is correct and necessary for utilizing the Devgas extension within theNibiruTxClientclass.- 53-53: The addition of
setupDevgasExtensionin theNibiruExtensionsinitialization within theNibiruTxClientconstructor is correctly implemented. This ensures that the Devgas extension is available for transactional operations, aligning with the PR's objective to enhance the SDK's capabilities. However, ensure that the order of extension setup calls does not impact the initialization or usage of other extensions, as the order can sometimes be significant in composite setups.Verification successful
The addition of
setupDevgasExtensionat the beginning of the extension setup calls in theNibiruTxClientconstructor has been verified. The order of extension setup calls, as provided, does not indicate any obvious issues that could impact the initialization or usage of other extensions. However, without explicit documentation or comments indicating the significance of this order, it's advisable to consult the SDK's documentation or the developers to ensure that the order does not have specific implications that are not immediately apparent from the code alone.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the order of extension setup calls does not impact other extensions echo "Manual verification recommended: Check the initialization order of extensions in NibiruTxClient."Length of output: 202
Script:
#!/bin/bash # Search for the NibiruTxClient constructor and examine the order of extension setup calls rg "class NibiruTxClient" --context 10Length of output: 1093
Script:
#!/bin/bash # Search specifically for the constructor's body within the NibiruTxClient class, focusing on the initialization of extensions rg "constructor" -A 20 src/sdk/tx/txClient.tsLength of output: 654
src/sdk/query/query.ts (2)
- 39-40: The inclusion of
DevgasExtensionin theNibiruExtensionstype is correctly implemented, ensuring that the Devgas querying capabilities are recognized as part of the SDK's extension system. This change is crucial for type safety and code clarity.- 89-89: The setup of
DevgasExtensionin theNibiruQuerierclass is correctly implemented, aligning with the SDK's pattern for initializing extensions. This ensures that the Devgas querying capabilities are seamlessly integrated into the SDK's querying system. As with thetxClient.tsfile, ensure that the order of extension setup calls does not inadvertently affect the initialization or functionality of other extensions.Verification successful
The setup of
DevgasExtensionin theNibiruQuerierclass is correctly implemented, aligning with the SDK's pattern for initializing extensions. The detailed context around thesetupDevgasExtensionoccurrences and its placement among other extension setup calls does not indicate any potential issues with the initialization or functionality of other extensions. This verifies the original review comment.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the order of extension setup calls does not impact other extensions echo "Manual verification recommended: Check the initialization order of extensions in NibiruQuerier."Length of output: 200
Script:
#!/bin/bash # Find the file containing the NibiruQuerier class fd "query.ts" --exec rg "class NibiruQuerier"Length of output: 97
Script:
#!/bin/bash # Inspect the initialization of extensions within the NibiruQuerier class in query.ts rg "setupDevgasExtension" query.tsLength of output: 83
Script:
#!/bin/bash # Securely locate query.ts and inspect the initialization of extensions within the NibiruQuerier class fd query.ts | xargs rg "setupDevgasExtension"Length of output: 97
Script:
#!/bin/bash # Extract more detailed context around setupDevgasExtension occurrences in query.ts fd query.ts | xargs rg "setupDevgasExtension" -C 5Length of output: 652
| export const setupDevgasExtension = (base: QueryClient): DevgasExtension => { | ||
| const rpcClient = createProtobufRpcClient(base) | ||
| const queryService = new QueryClientImpl(rpcClient) | ||
|
|
||
| return { | ||
| devgas: { | ||
| feeShare: async (args: QueryFeeShareRequest) => { | ||
| const req = QueryFeeShareRequest.fromPartial(args) | ||
| const resp = await queryService.FeeShare(req) | ||
| return resp | ||
| }, | ||
| feeSharesByWithdrawer: async ( | ||
| args: QueryFeeSharesByWithdrawerRequest | ||
| ) => { | ||
| const req = QueryFeeSharesByWithdrawerRequest.fromPartial(args) | ||
| const resp = await queryService.FeeSharesByWithdrawer(req) | ||
| return resp | ||
| }, | ||
| feeShares: async (args: QueryFeeSharesRequest) => { | ||
| const req = QueryFeeSharesRequest.fromPartial(args) | ||
| const resp = await queryService.FeeShares(req) | ||
| return resp | ||
| }, | ||
| params: async (args: QueryParamsRequest) => { | ||
| const req = QueryParamsRequest.fromPartial(args) | ||
| const resp = await queryService.Params(req) | ||
| return resp | ||
| }, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
The setupDevgasExtension function correctly initializes the Devgas extension, leveraging the createProtobufRpcClient to create an RPC client and the QueryClientImpl for the query service. This setup is consistent with the expected pattern for initializing extensions in the SDK. However, consider adding error handling for the asynchronous operations within the method implementations to ensure robustness and reliability in case of RPC failures or data issues.
+ try {
+ const resp = await queryService.FeeShare(req);
+ return resp;
+ } catch (error) {
+ // Handle or log the error appropriately
+ throw new Error(`Failed to query fee share: ${error.message}`);
+ }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.
| export const setupDevgasExtension = (base: QueryClient): DevgasExtension => { | |
| const rpcClient = createProtobufRpcClient(base) | |
| const queryService = new QueryClientImpl(rpcClient) | |
| return { | |
| devgas: { | |
| feeShare: async (args: QueryFeeShareRequest) => { | |
| const req = QueryFeeShareRequest.fromPartial(args) | |
| const resp = await queryService.FeeShare(req) | |
| return resp | |
| }, | |
| feeSharesByWithdrawer: async ( | |
| args: QueryFeeSharesByWithdrawerRequest | |
| ) => { | |
| const req = QueryFeeSharesByWithdrawerRequest.fromPartial(args) | |
| const resp = await queryService.FeeSharesByWithdrawer(req) | |
| return resp | |
| }, | |
| feeShares: async (args: QueryFeeSharesRequest) => { | |
| const req = QueryFeeSharesRequest.fromPartial(args) | |
| const resp = await queryService.FeeShares(req) | |
| return resp | |
| }, | |
| params: async (args: QueryParamsRequest) => { | |
| const req = QueryParamsRequest.fromPartial(args) | |
| const resp = await queryService.Params(req) | |
| return resp | |
| }, | |
| }, | |
| } | |
| } | |
| export const setupDevgasExtension = (base: QueryClient): DevgasExtension => { | |
| const rpcClient = createProtobufRpcClient(base) | |
| const queryService = new QueryClientImpl(rpcClient) | |
| return { | |
| devgas: { | |
| feeShare: async (args: QueryFeeShareRequest) => { | |
| const req = QueryFeeShareRequest.fromPartial(args) | |
| try { | |
| const resp = await queryService.FeeShare(req); | |
| return resp; | |
| } catch (error) { | |
| // Handle or log the error appropriately | |
| throw new Error(`Failed to query fee share: ${error.message}`); | |
| } | |
| }, | |
| feeSharesByWithdrawer: async ( | |
| args: QueryFeeSharesByWithdrawerRequest | |
| ) => { | |
| const req = QueryFeeSharesByWithdrawerRequest.fromPartial(args) | |
| const resp = await queryService.FeeSharesByWithdrawer(req) | |
| return resp | |
| }, | |
| feeShares: async (args: QueryFeeSharesRequest) => { | |
| const req = QueryFeeSharesRequest.fromPartial(args) | |
| const resp = await queryService.FeeShares(req) | |
| return resp | |
| }, | |
| params: async (args: QueryParamsRequest) => { | |
| const req = QueryParamsRequest.fromPartial(args) | |
| const resp = await queryService.Params(req) | |
| return resp | |
| }, | |
| }, | |
| } | |
| } |
There was a problem hiding this comment.
We should not throw errors
There was a problem hiding this comment.
@cgilbe27, understood. Instead of throwing errors, we could handle them in a way that aligns with your project's error handling strategy. One approach could be to log the error and return a specific error response that the calling function can check for. Here's an example modification for the feeShare method:
feeShare: async (args: QueryFeeShareRequest) => {
const req = QueryFeeShareRequest.fromPartial(args)
try {
const resp = await queryService.FeeShare(req);
return resp;
} catch (error) {
console.error(`Failed to query fee share: ${error.message}`);
// Return a specific error response or null based on your project's strategy
return null;
}
},This way, we avoid throwing errors and instead log them for debugging purposes, returning null (or another appropriate value) to indicate an error occurred. Would this approach be more in line with your project's practices?
New Learnings Added
User: cgilbe27
PR: NibiruChain/ts-sdk#325
File: src/sdk/query/devgas.ts:25-55
Timestamp: 2024-02-28T18:58:23.997Z
Learning: cgilbe27 prefers not to use thrown errors for error handling in the `src/sdk/query/devgas.ts` file.
Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure path-based instructions.
|
Summary by CodeRabbit