feat: account parser #374
feat: account parser #374
Conversation
* 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]>
### [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]
WalkthroughThis pull request introduces several changes aimed at improving issue management and account handling within the project. A new GitHub Actions workflow is created to automate the addition of newly opened issues to a project board and to apply a specific label. Additionally, new functions for converting Ethereum accounts to Cosmos accounts are added, along with corresponding unit tests. The versioning in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 4
🧹 Outside diff range and nitpick comments (11)
.github/issue-labeler-config.yml (3)
1-3: LGTM with minor suggestions for improvementThe implementation of the issue labeler configuration is correct and will successfully add the "S-triage" label to all new issues. However, I have a few minor suggestions to improve the code:
- Fix the typo in the comment (see separate comment).
- Simplify the regular expression (see separate comment).
- Consider adding more context about the purpose of the "S-triage" label and how it fits into your issue management workflow. This could help other team members understand the labeling strategy.
1-1: Fix typo in the commentThere's a small typo in the comment.
Please apply this change:
- # Adds the "S-triage" label ot any issue that gets opened. + # Adds the "S-triage" label to any issue that gets opened.
3-3: Simplify the regular expressionThe current regular expression has an unnecessary trailing forward slash.
Consider simplifying it as follows:
- - "/.*/" + - ".*"This change removes the forward slashes, which are typically used as delimiters in many regex engines but are not needed in this YAML configuration. The simplified version
".*"will still match any string, effectively labeling all issues.src/sdk/utils/t.json (1)
1-41: Overall, the configuration looks well-structured and follows Cosmos standards.The JSON configuration for the "cataclysm-1" chain is comprehensive and follows the standard practices for Cosmos-based chains. All necessary sections are present and consistently configured.
To ensure the reliability and correctness of this configuration:
- Thoroughly test this configuration in a staging environment before deploying to production.
- Verify that all parameters align with the intended design and economics of the "cataclysm-1" chain.
- Consider creating a separate documentation file explaining the rationale behind key configuration choices, which will be helpful for future maintenance and audits.
As the project evolves, consider implementing a process for managing and updating these configurations across different environments (e.g., testnet, mainnet). This could involve creating templates or using a configuration management tool to ensure consistency and ease of updates.
.github/workflows/gh-issues.yml (1)
16-23: LGTM with a suggestion: 'add-to-project' job is well-configured.The job is correctly set up to add issues to the specified project board. The use of a secret for the GitHub token is a good security practice.
Consider using a repository variable for the project URL instead of hardcoding it. This would improve maintainability and make it easier to update if the project board changes in the future. For example:
with: project-url: ${{ vars.PROJECT_BOARD_URL }} github-token: ${{ secrets.NIBIRU_PM }}Then, set the
PROJECT_BOARD_URLvariable in your repository settings.src/sdk/tx/txClient.ts (2)
21-21: Consider grouping related imports together.The new import statement for
accountFromNibiruis related to the Nibiru-specific functionality. Consider moving this import statement to be grouped with other Nibiru-related imports for better code organization.
73-73: Consider adding documentation for the newaccountParseroption.To improve code maintainability and ease of use, consider adding a comment or documentation explaining the purpose and expected behavior of the
accountParseroption. This will help other developers understand when and how to use this new feature.src/sdk/tx/account.test.ts (4)
7-30: LGTM: Well-structured test case foraccountFromEthAccount.This test case effectively covers the happy path scenario for
accountFromEthAccount. It creates a mockEthAccountobject with specific values and verifies that the returned account matches these values.Consider adding more edge cases to increase test coverage, such as:
- Testing with very large
Longvalues foraccountNumberandsequence.- Testing with an empty public key.
- Testing with a different
typeUrlfor the public key.
32-41: LGTM: Good edge case coverage foraccountFromEthAccount.This test case effectively handles the scenario where
baseAccountis undefined, which is an important edge case to consider.Consider adding the following assertions to make the test more robust:
- Verify that
account.pubkeyis explicitlynull(not just falsy).- Check that
account.addressis an empty string (not just falsy).- Ensure that
account.accountNumberandaccount.sequenceare explicitly of typeLong.
45-71: LGTM: Comprehensive test case foraccountFromNibiruwith EthAccount typeUrl.This test case effectively covers the scenario of parsing an EthAccount from the Any type. It creates a mock Any object with EthAccount typeUrl and verifies that the returned account matches the input values.
Consider adding the following assertions to make the test more robust:
- Verify that the returned
accountobject is not null or undefined.- Check that
account.pubkeyis not null and is of the expected type.- Ensure that
account.accountNumberandaccount.sequenceare explicitly of typeLong.
1-83: Overall, well-structured and comprehensive test suite.The test file provides good coverage for the
accountFromEthAccountandaccountFromNibirufunctions, including both happy path scenarios and some edge cases. The use of Jest as the testing framework is appropriate, and the test structure is clear and easy to follow.To further improve the test suite:
- Consider adding more edge cases and boundary conditions to increase test coverage.
- Enhance assertions in some test cases as suggested in previous comments.
- Use mocking for external dependencies to isolate the units under test.
- Add error handling test cases to ensure robustness.
These improvements will make the test suite more comprehensive and resilient to future changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- .github/issue-labeler-config.yml (1 hunks)
- .github/workflows/gh-issues.yml (1 hunks)
- CHANGELOG.md (1 hunks)
- package.json (1 hunks)
- src/sdk/tx/account.test.ts (1 hunks)
- src/sdk/tx/account.ts (1 hunks)
- src/sdk/tx/txClient.ts (2 hunks)
- src/sdk/utils/t.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
6-6: null
Bare URL used(MD034, no-bare-urls)
1-1: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (12)
src/sdk/utils/t.json (5)
6-10: Stake currency configuration looks good.The stake currency configuration follows the standard format for Cosmos-based chains. The use of "unibi" as the minimal denomination (representing micro-NIBI) is consistent with common practices.
11-13: BIP44 configuration is correct.The coin type 118 is the standard for Cosmos-based chains in the BIP44 specification, so this configuration is correct.
14-21: Bech32 configuration is consistent and follows standards.The bech32 address prefixes are consistently based on "nibi", which aligns with the chain name. The configuration follows the standard format for Cosmos-based chains, defining prefixes for account, validator, and consensus addresses and their public keys.
22-28: Verify if single currency configuration is intended.The currencies section defines only NIBI, which is consistent with the stake currency configuration. This is common for many chains, but some networks support multiple currencies.
Please confirm:
- Is it intentional to have only NIBI as the supported currency?
- Are there plans to add more currencies in the future?
If additional currencies are planned, consider adding a TODO comment or creating a separate issue to track this task.
1-5: Verify chain identification and endpoints.The chain identification and network endpoints look good. However, please confirm:
- Is it intentional that both
chainIdandchainNameare set to "cataclysm-1"?- Are the RPC and REST endpoints correct and operational?
To verify the endpoints, you can run the following curl commands:
Both commands should return "cataclysm-1" if the endpoints are correct and operational.
✅ Verification successful
Chain identification and endpoints verified.
All configurations are correct for "cataclysm-1".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify RPC endpoint curl -s https://rpc.nibiru.fi/status | jq '.result.node_info.network' # Verify REST endpoint curl -s https://lcd.nibiru.fi/cosmos/base/tendermint/v1beta1/node_info | jq '.default_node_info.network'Length of output: 202
.github/workflows/gh-issues.yml (3)
1-8: LGTM: Workflow name and trigger events are well-defined.The workflow name "Auto-add GH issues to project" clearly describes its purpose. The trigger events are correctly set to run on issue creation and labeling, which aligns with the workflow's objectives.
10-12: LGTM: Permissions are correctly set.The permissions are appropriately configured with the principle of least privilege. Write access to issues and read access to contents are the minimum required for the workflow's tasks.
25-38: LGTM with questions: 'label-triage' job is well-configured, but clarification needed.The job is set up correctly to label new issues using a configuration file. The condition to run only on newly opened issues without labels is appropriate.
The 'not-before' date is set to May 1, 2024. Is this intentional? This will prevent the action from running until that date, which is several months in the future.
The job name (line 26) seems to be incorrect. It's currently set to "Add GH ticket to project", which is the same as the previous job and doesn't reflect its actual function of labeling issues.
Consider updating the job name to better reflect its purpose:
- name: "Add GH ticket to project" + name: "Label new issues"To check if there are any other instances of potentially incorrect job names or dates, let's run the following script:
src/sdk/tx/txClient.ts (3)
73-73: LGTM: Enhanced flexibility with account parser option.The addition of the
accountParseroption withaccountFromNibiruenhances the flexibility of theNibiruTxClient. This change allows for custom account parsing specific to Nibiru accounts.
Line range hint
1-99: Summary of changes and suggestionsThe changes to
src/sdk/tx/txClient.tsenhance theNibiruTxClientby adding a custom account parser option. While the implementation looks correct, consider the following suggestions:
- Group the new import statement with other related imports.
- Add documentation for the new
accountParseroption to improve code maintainability.- Verify the impact of this change on other parts of the codebase that use
NibiruTxClientorconnectWithSigner.These improvements will enhance code organization, maintainability, and ensure the change doesn't introduce unexpected behavior in other parts of the codebase.
73-73: Verify the impact of the newaccountParseroption.The addition of the
accountParseroption might affect other parts of the codebase that interact withNibiruTxClient. Let's verify its usage and potential impact.✅ Verification successful
The impact of the new
accountParseroption has been verified and is contained within the expected areas of the codebase. No unintended effects were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of NibiruTxClient and potential impacts of the new accountParser option. # Search for NibiruTxClient usage echo "Searching for NibiruTxClient usage:" rg --type typescript "NibiruTxClient" -A 5 # Search for connectWithSigner method calls echo "\nSearching for connectWithSigner method calls:" rg --type typescript "connectWithSigner" -A 5 # Search for accountFromNibiru usage echo "\nSearching for accountFromNibiru usage:" rg --type typescript "accountFromNibiru"Length of output: 511
Script:
#!/bin/bash # Description: Check for usage of NibiruTxClient and potential impacts of the new accountParser option. # Search for NibiruTxClient usage echo "Searching for NibiruTxClient usage:" rg --type ts "NibiruTxClient" -A 5 # Search for connectWithSigner method calls echo "\nSearching for connectWithSigner method calls:" rg --type ts "connectWithSigner" -A 5 # Search for accountFromNibiru usage echo "\nSearching for accountFromNibiru usage:" rg --type ts "accountFromNibiru"Length of output: 6471
src/sdk/tx/account.test.ts (1)
1-5: LGTM: Import statements are well-organized and relevant.The import statements are logically ordered and include all necessary dependencies for the tests.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/sdk/tx/account.test.ts (1 hunks)
- src/sdk/tx/account.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/sdk/tx/account.test.ts
🔇 Additional comments (2)
src/sdk/tx/account.ts (2)
12-25: LGTM! UndefinedbaseAccounthandling implemented.The function now properly checks for undefined
baseAccountand throws an error if it's undefined. This addresses the concern raised in the previous review. The rest of the implementation looks correct and efficient.
1-11: Well-structured code with good documentationThe file is well-organized with appropriate imports and clear JSDoc comments for both functions. This improves code readability and maintainability.
Also applies to: 26-32
|
Unsupported type: '/eth.types.v1.EthAccount'with brand new keplr wallet #371This is the first step in closing #371. Since we support Eth accounts, we need to add a custom Account Parser into both the
SigningCosmWasmClientandSigningStargateClient. Luckily, there is already an option for injecting custom a Account Parser intoSigningStargateClient(PR for reference).We would also need to make a change to
SigningCosmWasmClientOptionsincosmjs/cosmwasm-stargate, to also accept custom parsers cosmos/cosmjs#1610. Until then, in a separate PR, we would expose asignWithKeplrEVMto mitigate developers facing #371.Summary by CodeRabbit
Release Notes for Version 4.5.2
New Features
Chores