chore: main to develop #381
Conversation
…380) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * refactor(faucet)!: set default tokens (#369) * chore: develop -> main (#368) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * chore(release): 4.5.1 ### [4.5.1](v4.5.0...v4.5.1) (2024-08-09) ### Miscellaneous Chores * develop -> main ([#368](#368)) ([c6d6570](c6d6570)), closes [#362](#362) [#366](#366) [#367](#367) [skip ci] * fix(faucet): remove unused tokens from default faucet request * fix: bump test --------- Co-authored-by: Cameron Gilbert <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> * chore: main to develop (#375) * chore: develop -> main (#370) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * refactor(faucet)!: set default tokens (#369) * chore: develop -> main (#368) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * chore(release): 4.5.1 ### [4.5.1](v4.5.0...v4.5.1) (2024-08-09) ### Miscellaneous Chores * develop -> main ([#368](#368)) ([c6d6570](c6d6570)), closes [#362](#362) [#366](#366) [#367](#367) [skip ci] * fix(faucet): remove unused tokens from default faucet request * fix: bump test --------- Co-authored-by: Cameron Gilbert <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> --------- Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> * chore(github): Add project automation for https://tinyurl.com/25uty9w5 * chore(release): 4.5.2 ### [4.5.2](v4.5.1...v4.5.2) (2024-09-24) ### Miscellaneous Chores * develop -> main ([#370](#370)) ([ec2a25b](ec2a25b)), closes [#362](#362) [#366](#366) [#367](#367) [#369](#369) [#368](#368) [#362](#362) [#366](#366) [#367](#367) [#362](#362) [#366](#366) [#367](#367) * **github:** Add project automation for https://tinyurl.com/25uty9w5 ([c2c27e5](c2c27e5)) [skip ci] --------- Co-authored-by: Cameron Gilbert <[email protected]> Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> Co-authored-by: Unique Divine <[email protected]> * feat: account parser (#374) * chore: develop -> main (#370) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * refactor(faucet)!: set default tokens (#369) * chore: develop -> main (#368) * revert: cosmos submodule only (#362) * revert: cosmos submodule only * fix: rem * fix: rem * fix: update * feat: add msg client * fix: paths * fix: try chaosnet ibc * fix: path again * fix: try hm * fix: fixes to pass * feat: eth protos (#366) * fix: eth protos * fix: client * fix: fixes * fix: try older nibiru * fix: index * fix: mainnet * fix: import * revert: build change * chore: tests (#367) * fix: all query tests * chore: final tests * fix: buf * fix: fix * fix: pull latest * fix: build * fix: build * chore(release): 4.5.1 ### [4.5.1](v4.5.0...v4.5.1) (2024-08-09) ### Miscellaneous Chores * develop -> main ([#368](#368)) ([c6d6570](c6d6570)), closes [#362](#362) [#366](#366) [#367](#367) [skip ci] * fix(faucet): remove unused tokens from default faucet request * fix: bump test --------- Co-authored-by: Cameron Gilbert <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> --------- Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> * chore(github): Add project automation for https://tinyurl.com/25uty9w5 * chore(release): 4.5.2 ### [4.5.2](v4.5.1...v4.5.2) (2024-09-24) ### Miscellaneous Chores * develop -> main ([#370](#370)) ([ec2a25b](ec2a25b)), closes [#362](#362) [#366](#366) [#367](#367) [#369](#369) [#368](#368) [#362](#362) [#366](#366) [#367](#367) [#362](#362) [#366](#366) [#367](#367) * **github:** Add project automation for https://tinyurl.com/25uty9w5 ([c2c27e5](c2c27e5)) [skip ci] * feat: nibiru account parser * refactor: throw if baseaccount is undefined * test: fixing tests * chore: removing unnecessary ? * refactor: matching cosmjs implementation * chore: removing t.json * chore: pr comments --------- Co-authored-by: Cameron Gilbert <[email protected]> Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> Co-authored-by: Unique Divine <[email protected]> * chore: remove stats and update default feature flags * feat: cosmwasmclient extension & signingcosmwasmclient implementation (#379) * feat: nibicosmwasmclient * feat: nibi signing cosm wasm client * refactor: adding nibi account parser to nibi signingcosmwasmclient * test: remove unused test file * test: take signingcosmwasmclient from coverage * chore: fix coverage --------- Co-authored-by: Kevin Yang <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> Co-authored-by: Calico Nino <[email protected]> Co-authored-by: Unique Divine <[email protected]>
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on the removal of statistical functionalities from the heart monitor module and the introduction of new client classes for interacting with CosmWasm smart contracts. Key changes include the addition of a pre-commit hook command, updates to Jest configuration for coverage exclusions, and enhancements to account handling in the Cosmos ecosystem. The overall structure remains intact, but significant functionalities related to statistics have been eliminated. Changes
Assessment against linked issues
Possibly related PRs
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: 7
🧹 Outside diff range and nitpick comments (6)
.husky/pre-commit (1)
Line range hint
1-5: LGTM! Consider optimizing the pre-commit hook.The addition of
yarn generate-barrelsto the pre-commit hook is a good practice to ensure barrel files are always up-to-date. However, consider the following suggestions:
Verify the order of operations: Ensure that running
generate-barrelsafterlint-stageddoesn't create any conflicts or undo any linting fixes.For efficiency, you might want to combine these commands using the
&&operator:yarn lint-staged && yarn generate-barrelsThis ensures that
generate-barrelsonly runs iflint-stagedpasses, potentially saving time on failed commits.src/sdk/tx/account.test.ts (3)
16-49: LGTM: Test suite for accountFromEthAccount is comprehensive.The test suite for
accountFromEthAccountcovers both error handling and successful execution scenarios. The tests are well-structured and use appropriate assertions.Consider adding the following test cases to improve coverage:
- Test with a baseAccount that has a null pubKey.
- Test with extreme values for accountNumber and sequence.
- Test with a baseAccount that has all fields set to their minimum allowed values.
51-105: LGTM: Test suite for accountFromNibiru is well-structured.The test suite for
accountFromNibirucovers both EthAccount and non-EthAccount scenarios. The tests are comprehensive and use appropriate mocking techniques.Consider the following improvements:
- Add a test case for handling invalid input (e.g., malformed Any object).
- Test edge cases such as empty address or extremely large accountNumber/sequence values.
- In the second test case, consider asserting on more properties of the returned account object, not just the address.
To improve test isolation, consider moving the mock setup for
cosmjs.accountFromAnyoutside the test case:let mockAccountFromAny: jest.SpyInstance; beforeEach(() => { mockAccountFromAny = jest.spyOn(cosmjs, "accountFromAny").mockReturnValue({ address: "nibi1otheraddress", pubkey: null, accountNumber: 789, sequence: 3, }); }); afterEach(() => { mockAccountFromAny.mockRestore(); });This setup allows for easier modification of the mock behavior in individual test cases if needed.
1-105: Overall, the test file is well-structured and provides good coverage.The test suites for both
accountFromEthAccountandaccountFromNibiruare comprehensive and well-implemented. They cover the main scenarios and use appropriate mocking and assertion techniques.To further improve the test file:
- Consider adding more edge cases and error scenarios to increase robustness.
- Implement parameterized tests for scenarios that can be tested with multiple inputs.
- Add tests for any untested error conditions in the original functions.
- Consider using a test coverage tool to identify any uncovered code paths.
These improvements will help ensure the reliability and maintainability of the account-related functions in your SDK.
src/sdk/core/cosmwasmclient.ts (1)
54-55: Clarify the error message when an account does not existThe current error message suggests sending tokens to the account, which might not always be the solution. An account can exist without tokens, especially in certain blockchain networks.
Consider revising the error message for accuracy:
`Account '${address}' does not exist on chain. Send some tokens there before trying to query sequence.` +`Account '${address}' does not exist on the chain. Ensure the account has been initialized before querying the sequence.`src/sdk/core/signingcosmwasmclient.ts (1)
199-243: Efficient compression level in 'upload' methodIn the
uploadmethod (lines 199-243), the WASM bytecode is compressed usingpako.gzipwith a compression level of 9. While level 9 offers maximum compression, it may have performance implications. Consider using a lower compression level (e.g., 6 or 7) for a balance between compression efficiency and performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
- .husky/pre-commit (1 hunks)
- jest.config.ts (1 hunks)
- package.json (2 hunks)
- src/gql/heart-monitor/heart-monitor.test.ts (0 hunks)
- src/gql/heart-monitor/heart-monitor.ts (0 hunks)
- src/gql/query/index.ts (0 hunks)
- src/gql/query/stats.ts (0 hunks)
- src/gql/utils/defaultObjects.ts (1 hunks)
- src/sdk/core/cosmwasmclient.ts (1 hunks)
- src/sdk/core/signingcosmwasmclient.ts (1 hunks)
- src/sdk/tx/account.test.ts (1 hunks)
- src/sdk/tx/account.ts (1 hunks)
- src/sdk/tx/index.ts (1 hunks)
- src/sdk/tx/txClient.ts (3 hunks)
💤 Files with no reviewable changes (4)
- src/gql/heart-monitor/heart-monitor.test.ts
- src/gql/heart-monitor/heart-monitor.ts
- src/gql/query/index.ts
- src/gql/query/stats.ts
✅ Files skipped from review due to trivial changes (1)
- src/sdk/tx/index.ts
🧰 Additional context used
🔇 Additional comments (15)
jest.config.ts (1)
25-26: Exclusion of Cosmjs implementations from coverage is appropriate.The addition of these two files to the coverage exclusion list is reasonable:
src/sdk/core/cosmwasmclient.tssrc/sdk/core/signingcosmwasmclient.tsThese files are noted as implementations from Cosmjs, which is an external library. It's generally a good practice to exclude external implementations from your project's test coverage metrics, as you're not responsible for testing third-party code.
However, to ensure this change aligns with the project's goals:
- Verify that these files are indeed direct implementations from Cosmjs without significant modifications.
- Ensure that the functionality these files provide is still adequately tested through integration or end-to-end tests.
- Consider adding a comment in the respective files indicating why they are excluded from coverage, to prevent future confusion.
To confirm the nature of these files, let's examine their contents:
This will help us confirm that these files are indeed Cosmjs implementations and that excluding them from coverage is appropriate.
✅ Verification successful
Exclusion of Cosmjs Implementations from Coverage Confirmed.
Based on the contents of the excluded files:
src/sdk/core/cosmwasmclient.tssrc/sdk/core/signingcosmwasmclient.tsBoth files primarily import and extend classes from Cosmjs libraries without significant custom modifications. Excluding these files from test coverage is appropriate, as they rely on well-tested external implementations.
Recommendations:
Add Explanatory Comments: Include comments in each excluded file explaining the reason for their exclusion from coverage. This will aid future developers in understanding the rationale and prevent unnecessary confusion.
// Excluded from test coverage as this is a direct implementation from Cosmjs.Ensure Adequate Testing: Verify that the functionalities provided by these implementations are covered through integration or end-to-end tests to maintain overall test reliability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the contents of the excluded files # Check if the files exist and display their contents for file in src/sdk/core/cosmwasmclient.ts src/sdk/core/signingcosmwasmclient.ts; do if [ -f "$file" ]; then echo "Contents of $file:" cat "$file" | head -n 20 # Display first 20 lines echo "..." else echo "File $file does not exist." fi doneLength of output: 1669
src/sdk/tx/account.test.ts (2)
1-7: LGTM: Import statements are appropriate and concise.The import statements are well-organized and include all necessary dependencies for the tests. No unused imports are present.
9-14: LGTM: Mock setup for decodeOptionalPubkey is correct.The mock setup for
decodeOptionalPubkeyis implemented correctly using Jest. The use ofjest.Mockfor typing is a good practice for maintaining type safety in tests.package.json (2)
Line range hint
1-124: Changes align with PR objectives and version update.The modifications to
package.jsonare minimal and focused, adding only the necessary dependencies. The version number (4.5.2) is consistent with the release update mentioned in the PR objectives. These changes contribute to the overall goal of improving the stability and functionality of the codebase.
49-50: LGTM. Please provide more context on the usage of pako.The addition of
pakoand its type definitions is syntactically correct and follows best practices. However, it would be helpful to understand the specific use case for this compression library in the project.To ensure
pakois being utilized in the project, let's search for its usage:Can you provide more information on where and how
pakowill be used in the project?Also applies to: 71-71
✅ Verification successful
Verified usage of
pakoin the project.The
pakolibrary is imported insrc/sdk/core/signingcosmwasmclient.ts, confirming its usage within the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for pako usage in TypeScript files rg --type typescript 'import.*pako' || echo "No direct imports of pako found." rg --type typescript 'require.*pako' || echo "No require statements for pako found."Length of output: 306
Script:
#!/bin/bash # Search for pako usage in TypeScript (.ts and .tsx) files rg --glob "*.ts" 'import.*pako' || echo "No direct imports of pako found in .ts files." rg --glob "*.ts" 'require.*pako' || echo "No require statements for pako found in .ts files." rg --glob "*.tsx" 'import.*pako' || echo "No direct imports of pako found in .tsx files." rg --glob "*.tsx" 'require.*pako' || echo "No require statements for pako found in .tsx files."Length of output: 826
src/gql/utils/defaultObjects.ts (1)
345-346: LGTM. Verify impact and update documentation.The changes to
defaultFeatureFlagslook good. Disabling theperpandspotflags by default aligns with the PR objectives of transitioning between branches and implementing various fixes.To ensure this change doesn't introduce unintended side effects:
- Verify that all code depending on these flags handles the disabled state correctly.
- Update any relevant documentation or configuration guides to reflect this change.
Run the following script to find potential areas affected by this change:
Review the results to ensure that the disabled state is handled appropriately in all cases.
✅ Verification successful
Verified: No usages of
perpandspotfeature flags found.The changes to
defaultFeatureFlagsare safe and do not affect other parts of the codebase. Disabling theperpandspotflags by default does not introduce any unintended side effects.
- No references to
featureFlags.perporfeatureFlags.spotwere found in the codebase.- Ensure that any related documentation is updated to reflect these changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of perp and spot feature flags echo "Searching for perp feature flag usage:" rg --type typescript 'featureFlags.perp' -C 3 echo "Searching for spot feature flag usage:" rg --type typescript 'featureFlags.spot' -C 3Length of output: 335
Script:
#!/bin/bash # Search for usages of perp and spot feature flags in .ts files echo "Searching for perp feature flag usage:" fd --extension ts --type file | xargs rg 'featureFlags\.perp' -C 3 echo "Searching for spot feature flag usage:" fd --extension ts --type file | xargs rg 'featureFlags\.spot' -C 3Length of output: 311
src/sdk/tx/account.ts (1)
1-6: LGTM on import statementsThe import statements are well-structured and correctly include all necessary dependencies. They align with the project's conventions.
src/sdk/core/cosmwasmclient.ts (2)
51-61: 🛠️ Refactor suggestionLeverage existing methods from the superclass
The
getSequencemethod may already be provided by theCosmWasmClientsuperclass. Reimplementing it can lead to code duplication.Check if
CosmWasmClienthas agetSequencemethod and use it directly:-public async getSequence(address: string): Promise<SequenceResponse> { - const account = await this.getAccount(address) - if (!account) { - throw new Error( - `Account '${address}' does not exist on the chain. Ensure the account has been initialized before querying the sequence.` - ) - } - return { - accountNumber: account.accountNumber, - sequence: account.sequence, - } -} +// Remove this method if it's already implemented in the superclassIf additional functionality is needed, consider calling
super.getSequence(address)and extending the result.Likely invalid or redundant comment.
32-32:⚠️ Potential issueAvoid accessing private properties of the superclass
Accessing
cosmWasmClient["cometClient"]directly may lead to issues ifcometClientis a private or protected property ofCosmWasmClient. This approach can break encapsulation and may cause errors if the internal implementation changes in future versions.Consider modifying the
CosmWasmClientclass or using public APIs to obtain the necessary client. Here's a possible correction assumingtmClientis a public property:-return new NibiCosmWasmClient(cosmWasmClient["cometClient"], options) +return new NibiCosmWasmClient(cosmWasmClient.tmClient, options)If
tmClientis not accessible, you might need to adjust your approach to avoid accessing private properties.⛔ Skipped due to learnings
Learnt from: CalicoNino PR: NibiruChain/ts-sdk#379 File: src/sdk/core/cosmwasmclient.ts:32-32 Timestamp: 2024-10-16T19:03:21.863Z Learning: In our codebase, it's acceptable to access private properties of base classes using bracket notation when necessary.src/sdk/core/signingcosmwasmclient.ts (1)
659-697: Ensure consistent signer type checks in 'sign' methodIn the
signmethod, the check forisOfflineDirectSigner(this.signer)is performed, and based on that, it calls eithersignDirectorsignAmino. Ensure that the signer supports both signing methods or handle cases where only one method is supported to prevent runtime errors.src/sdk/tx/txClient.ts (5)
55-55: Update calls to 'connectWithSigner' with new options typeThe
connectWithSignermethod now acceptsNibiSigningCosmWasmClientOptionsforwasmOptions. Ensure that:
- All calls to
NibiruTxClient.connectWithSignerare updated to pass the correctwasmOptions.- The options provided are compatible with
NibiSigningCosmWasmClientOptions.Run the following script to locate and review method calls:
#!/bin/bash # Description: Find all calls to 'NibiruTxClient.connectWithSigner' and check the options parameter. # Test: Search for 'NibiruTxClient\.connectWithSigner' calls. # Expect: 'wasmOptions' parameter matches the new type. rg 'NibiruTxClient\.connectWithSigner' --type ts -A 5
29-29: Ensure compatibility with 'NibiSigningCosmWasmClient'The
wasmClientproperty and constructor parameter have been updated to useNibiSigningCosmWasmClient. Verify that:
- All methods and properties used from
wasmClientare compatible withNibiSigningCosmWasmClient.- Any extensions or plugins relying on
SigningCosmWasmClientare updated accordingly.Run the following script to find usages of
wasmClientmethods:#!/bin/bash # Description: Find all usages of 'wasmClient' methods to ensure compatibility. # Test: Search for 'this\.wasmClient\.\w+' pattern. # Expect: Methods used are compatible with 'NibiSigningCosmWasmClient'. rg 'this\.wasmClient\.\w+' --type tsAlso applies to: 35-35
63-63: Validate 'accountParser' implementation with 'accountFromNibiru'The
accountParseroption is set toaccountFromNibiru. Ensure that:
accountFromNibirucorrectly implements the expected interface.- The parser properly handles account data in the context of Nibiru.
Run the following script to inspect the implementation:
#!/bin/bash # Description: Verify that 'accountFromNibiru' conforms to expected standards. # Test: Check the definition of 'accountFromNibiru'. # Expect: Function matches the required signature and behavior. fd 'account.ts' | xargs rg 'export function accountFromNibiru' -A 5Also applies to: 74-74
58-63: Confirm proper usage of 'NibiSigningCosmWasmClient.connectWithSigner'The instantiation of
NibiSigningCosmWasmClientreplaces the previous client. Verify that:
- The parameters passed to
connectWithSignerare correct and complete.- The
gasPriceandaccountParseroptions are appropriate for the new client.Run the following script to check for other invocations and ensure consistency:
✅ Verification successful
Usage of
NibiSigningCosmWasmClient.connectWithSigneris properly configuredParameters passed to
connectWithSignerare correct and complete. BothgasPriceandaccountParseroptions are appropriately set and consistent with the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all invocations of 'NibiSigningCosmWasmClient.connectWithSigner'. # Test: Search for 'NibiSigningCosmWasmClient\.connectWithSigner' calls. # Expect: Parameters are correctly set. rg 'NibiSigningCosmWasmClient\.connectWithSigner' --type ts -A 5Length of output: 140
Script:
#!/bin/bash # Description: Find the definition of 'GasPrice.fromString'. rg 'GasPrice\.fromString' --type ts -A 5Length of output: 689
Script:
#!/bin/bash # Description: Find the definition and usages of 'accountFromNibiru'. rg 'accountFromNibiru' --type ts -A 5Length of output: 2822
15-21: Verify new imports and dependenciesThe imports of
setupWasmExtension,accountFromNibiru,NibiSigningCosmWasmClient, andNibiSigningCosmWasmClientOptionsintroduce new dependencies and modules.
- Ensure that these modules are correctly installed and the import paths are accurate.
- Verify that the versions of the imported packages are compatible with the existing codebase.
Run the following script to verify the existence and export of these modules:
| * @param {EthAccount} ethAccount - The EthAccount object containing the account's base information. | ||
| * @returns {Account} The Cosmos account object. | ||
| */ | ||
| export const accountFromEthAccount = ({ |
There was a problem hiding this comment.
Fix parameter type mismatch in JSDoc comment
The JSDoc comment specifies that the parameter ethAccount is of type EthAccount, but the function actually destructures a BaseAccount. To maintain accuracy and avoid confusion, update the JSDoc comment to reflect the correct parameter type.
Apply this diff to correct the JSDoc comment:
/**
- * @param {EthAccount} ethAccount - The EthAccount object containing the account's base information.
+ * @param {BaseAccount} baseAccount - The BaseAccount object containing the account's base information.
* @returns {Account} The Cosmos account object.
*/
export const accountFromEthAccount = ({📝 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.
| * @param {EthAccount} ethAccount - The EthAccount object containing the account's base information. | |
| * @returns {Account} The Cosmos account object. | |
| */ | |
| export const accountFromEthAccount = ({ | |
| /** | |
| * @param {BaseAccount} baseAccount - The BaseAccount object containing the account's base information. | |
| * @returns {Account} The Cosmos account object. | |
| */ | |
| export const accountFromEthAccount = ({ |
| const baseAccount = EthAccount.decode(value).baseAccount | ||
| assert(baseAccount) | ||
| return accountFromEthAccount(baseAccount) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for missing baseAccount
Currently, the code uses assert(baseAccount) to ensure baseAccount exists. If baseAccount is undefined, this will throw a generic assertion error. Consider throwing a more descriptive error to provide clearer context during debugging.
Apply this diff to improve error handling:
if (typeUrl === "/eth.types.v1.EthAccount") {
const baseAccount = EthAccount.decode(value).baseAccount
- assert(baseAccount)
+ if (!baseAccount) {
+ throw new Error("Failed to decode baseAccount from EthAccount.")
+ }
return accountFromEthAccount(baseAccount)
}📝 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 baseAccount = EthAccount.decode(value).baseAccount | |
| assert(baseAccount) | |
| return accountFromEthAccount(baseAccount) | |
| const baseAccount = EthAccount.decode(value).baseAccount | |
| if (!baseAccount) { | |
| throw new Error("Failed to decode baseAccount from EthAccount.") | |
| } | |
| return accountFromEthAccount(baseAccount) |
| ) | ||
| return account ? this.accountParser(account) : null | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| } catch (error: any) { |
There was a problem hiding this comment.
Avoid suppressing ESLint rules by properly typing errors
Disabling ESLint rules can mask potential issues. Instead of using any for the error type and disabling the rule, use unknown and perform type checking within the catch block.
Here's how you can modify the code:
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-} catch (error: any) {
+} catch (error: unknown) {Then, inside the catch block, you can narrow down the error type:
if (error instanceof Error) {
// Now you can safely access error properties
}|
|
||
| public async getAccount(searchAddress: string): Promise<Account | null> { | ||
| try { | ||
| const account = await this.forceGetQueryClient().auth.account( |
There was a problem hiding this comment.
Use public methods instead of protected ones
The method forceGetQueryClient() is a protected method in CosmWasmClient. Calling it from a subclass can lead to tight coupling and potential issues if the superclass changes its implementation.
Use the public getQueryClient() method instead:
-const account = await this.forceGetQueryClient().auth.account(
+const account = await this.getQueryClient().auth.account(
searchAddress
)📝 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 account = await this.forceGetQueryClient().auth.account( | |
| const account = await this.getQueryClient().auth.account( |
| } catch (error: any) { | ||
| if (/rpc error: code = NotFound/i.test(error.toString())) { | ||
| return null | ||
| } | ||
| throw error |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling by avoiding string matching
Relying on regular expressions to match error messages can be fragile and may fail if the error message format changes. It's better to handle errors based on error codes or specific error types.
If the error object contains a code property, you can handle it like this:
-} catch (error: any) {
- if (/rpc error: code = NotFound/i.test(error.toString())) {
+} catch (error) {
+ if (error.code === "NotFound" || error.code === 5) { // 5 is the gRPC code for NOT_FOUND
return null
}
throw error
}Ensure that the error object conforms to a known error interface to reliably access the code property.
Committable suggestion was skipped due to low confidence.
| public async signAndBroadcast( | ||
| signerAddress: string, | ||
| messages: readonly EncodeObject[], | ||
| fee: FeeOption, | ||
| memo = "", | ||
| timeoutHeight?: bigint | ||
| ): Promise<DeliverTxResponse> { | ||
| let usedFee: StdFee | ||
| if (fee == "auto" || typeof fee === "number") { | ||
| assertDefined( | ||
| this.gasPrice, | ||
| "Gas price must be set in the client options when auto gas is used." | ||
| ) | ||
| const gasEstimation = await this.simulate(signerAddress, messages, memo) | ||
| const multiplier = | ||
| typeof fee === "number" ? fee : this.defaultGasMultiplier | ||
| usedFee = calculateFee( | ||
| Math.round(gasEstimation * multiplier), | ||
| this.gasPrice | ||
| ) | ||
| } else { | ||
| usedFee = fee | ||
| } | ||
| const txRaw = await this.sign( | ||
| signerAddress, | ||
| messages, | ||
| usedFee, | ||
| memo, | ||
| undefined, | ||
| timeoutHeight | ||
| ) | ||
| const txBytes = TxRaw.encode(txRaw).finish() | ||
| return this.broadcastTx( | ||
| txBytes, | ||
| this.broadcastTimeoutMs, | ||
| this.broadcastPollIntervalMs | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated code in 'signAndBroadcast' methods
The signAndBroadcast (lines 570-607) and signAndBroadcastSync (lines 625-657) methods contain duplicated code for fee calculation and transaction signing. To follow the DRY (Don't Repeat Yourself) principle, consider extracting the shared logic into a private helper method.
Apply this diff to refactor the common code:
+private async prepareSignedTx(
+ signerAddress: string,
+ messages: readonly EncodeObject[],
+ fee: FeeOption,
+ memo: string,
+ timeoutHeight?: bigint
+): Promise<TxRaw> {
+ let usedFee: StdFee
+ if (fee == "auto" || typeof fee === "number") {
+ assertDefined(
+ this.gasPrice,
+ "Gas price must be set in the client options when auto gas is used."
+ )
+ const gasEstimation = await this.simulate(signerAddress, messages, memo)
+ const multiplier = typeof fee === "number" ? fee : this.defaultGasMultiplier
+ usedFee = calculateFee(
+ Math.round(gasEstimation * multiplier),
+ this.gasPrice
+ )
+ } else {
+ usedFee = fee
+ }
+ return await this.sign(
+ signerAddress,
+ messages,
+ usedFee,
+ memo,
+ undefined,
+ timeoutHeight
+ )
+}
public async signAndBroadcast(
signerAddress: string,
messages: readonly EncodeObject[],
fee: FeeOption,
memo = "",
timeoutHeight?: bigint
): Promise<DeliverTxResponse> {
- let usedFee: StdFee
- if (fee == "auto" || typeof fee === "number") {
- assertDefined(
- this.gasPrice,
- "Gas price must be set in the client options when auto gas is used."
- )
- const gasEstimation = await this.simulate(signerAddress, messages, memo)
- const multiplier =
- typeof fee === "number" ? fee : this.defaultGasMultiplier
- usedFee = calculateFee(
- Math.round(gasEstimation * multiplier),
- this.gasPrice
- )
- } else {
- usedFee = fee
- }
- const txRaw = await this.sign(
- signerAddress,
- messages,
- usedFee,
- memo,
- undefined,
- timeoutHeight
- )
+ const txRaw = await this.prepareSignedTx(
+ signerAddress,
+ messages,
+ fee,
+ memo,
+ timeoutHeight
+ )
const txBytes = TxRaw.encode(txRaw).finish()
return this.broadcastTx(
txBytes,
this.broadcastTimeoutMs,
this.broadcastPollIntervalMs
)
}
public async signAndBroadcastSync(
signerAddress: string,
messages: readonly EncodeObject[],
fee: FeeOption,
memo = "",
timeoutHeight?: bigint
): Promise<string> {
- let usedFee: StdFee
- if (fee == "auto" || typeof fee === "number") {
- assertDefined(
- this.gasPrice,
- "Gas price must be set in the client options when auto gas is used."
- )
- const gasEstimation = await this.simulate(signerAddress, messages, memo)
- const multiplier =
- typeof fee === "number" ? fee : this.defaultGasMultiplier
- usedFee = calculateFee(
- Math.round(gasEstimation * multiplier),
- this.gasPrice
- )
- } else {
- usedFee = fee
- }
- const txRaw = await this.sign(
- signerAddress,
- messages,
- usedFee,
- memo,
- undefined,
- timeoutHeight
- )
+ const txRaw = await this.prepareSignedTx(
+ signerAddress,
+ messages,
+ fee,
+ memo,
+ timeoutHeight
+ )
const txBytes = TxRaw.encode(txRaw).finish()
return this.broadcastTxSync(txBytes)
}Also applies to: 625-657
| isDeliverTxFailure, | ||
| logs, | ||
| } from "@cosmjs/stargate" | ||
| import { findAttribute } from "@cosmjs/cosmwasm-stargate/build/signingcosmwasmclient" |
There was a problem hiding this comment.
Incorrect import path for 'findAttribute'
The import statement on line 56 is importing findAttribute from an internal build path, which is not recommended and could lead to issues if the internal structure changes. It's better to import from the public API provided by the package.
Apply this diff to correct the import path:
-import { findAttribute } from "@cosmjs/cosmwasm-stargate/build/signingcosmwasmclient"
+import { findAttribute } from "@cosmjs/cosmwasm-stargate"📝 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.
| import { findAttribute } from "@cosmjs/cosmwasm-stargate/build/signingcosmwasmclient" | |
| import { findAttribute } from "@cosmjs/cosmwasm-stargate" |
…(#380)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
refactor(faucet)!: set default tokens (refactor(faucet)!: set default tokens #369)
chore: develop -> main (chore: develop -> main #368)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
chore(release): 4.5.1
4.5.1 (2024-08-09)
Miscellaneous Chores
[skip ci]
fix(faucet): remove unused tokens from default faucet request
fix: bump test
chore: main to develop (chore: main to develop #375)
chore: develop -> main (chore: develop -> main #370)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
refactor(faucet)!: set default tokens (refactor(faucet)!: set default tokens #369)
chore: develop -> main (chore: develop -> main #368)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
chore(release): 4.5.1
4.5.1 (2024-08-09)
Miscellaneous Chores
[skip ci]
fix(faucet): remove unused tokens from default faucet request
fix: bump test
chore(github): Add project automation for https://tinyurl.com/25uty9w5
chore(release): 4.5.2
4.5.2 (2024-09-24)
Miscellaneous Chores
[skip ci]
feat: account parser (feat: account parser #374)
chore: develop -> main (chore: develop -> main #370)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
refactor(faucet)!: set default tokens (refactor(faucet)!: set default tokens #369)
chore: develop -> main (chore: develop -> main #368)
revert: cosmos submodule only (revert: cosmos submodule only #362)
revert: cosmos submodule only
fix: rem
fix: rem
fix: update
feat: add msg client
fix: paths
fix: try chaosnet ibc
fix: path again
fix: try hm
fix: fixes to pass
feat: eth protos (feat: eth protos #366)
fix: eth protos
fix: client
fix: fixes
fix: try older nibiru
fix: index
fix: mainnet
fix: import
revert: build change
chore: tests (chore: tests #367)
fix: all query tests
chore: final tests
fix: buf
fix: fix
fix: pull latest
fix: build
fix: build
chore(release): 4.5.1
4.5.1 (2024-08-09)
Miscellaneous Chores
[skip ci]
fix(faucet): remove unused tokens from default faucet request
fix: bump test
chore(github): Add project automation for https://tinyurl.com/25uty9w5
chore(release): 4.5.2
4.5.2 (2024-09-24)
Miscellaneous Chores
[skip ci]
feat: nibiru account parser
refactor: throw if baseaccount is undefined
test: fixing tests
chore: removing unnecessary ?
refactor: matching cosmjs implementation
chore: removing t.json
chore: pr comments
chore: remove stats and update default feature flags
feat: cosmwasmclient extension & signingcosmwasmclient implementation (feat: cosmwasmclient extension & signingcosmwasmclient implementation #379)
feat: nibicosmwasmclient
feat: nibi signing cosm wasm client
refactor: adding nibi account parser to nibi signingcosmwasmclient
test: remove unused test file
test: take signingcosmwasmclient from coverage
chore: fix coverage
Summary by CodeRabbit
Release Notes
New Features
NibiCosmWasmClientfor enhanced interaction with CosmWasm smart contracts.NibiSigningCosmWasmClientfor signing transactions and managing contract interactions.Bug Fixes
Chores
Documentation