Problem: ica query not tested #1635
Conversation
Solution: - test module safe query
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1635 +/- ##
===========================================
- Coverage 36.31% 17.87% -18.44%
===========================================
Files 123 72 -51
Lines 9715 5168 -4547
===========================================
- Hits 3528 924 -2604
+ Misses 5773 4121 -1652
+ Partials 414 123 -291 |
yihuang
requested review from
mmsqe and
calvinaco
and removed request for
a team
WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
Signed-off-by: yihuang <[email protected]>
![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
🧹 Outside diff range and nitpick comments (2)
integration_tests/test_ica.py (1)
171-181: LGTM: Implemented safe query function with minor improvement suggestion.The
call_module_safe_queryfunction effectively implements the safe querying mechanism for ICA balance. It correctly uses thegen_query_balance_packetfunction and includes proper error handling.Consider making the gas limit configurable or estimate it dynamically instead of using a fixed value of 200000. This would make the function more flexible for different network conditions and query complexities.
integration_tests/test_ica_precompile.py (1)
453-459: LGTM! Consider adding balance verification.The addition of the balance query functionality enhances the test coverage for ICA operations. The implementation correctly generates the query packet, submits it as a transaction, and verifies the transaction success.
To further improve the test, consider adding a verification step to check if the queried balance matches the expected balance calculated during the test. This would ensure that the balance query is not only executed successfully but also returns the correct value.
Example:
expected_balance = balance # Use the balance variable tracked in the test queried_balance = tcontract.functions.getQueriedBalance().call() # Assuming such a function exists assert queried_balance == expected_balance, f"Expected balance {expected_balance}, but got {queried_balance}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- integration_tests/configs/ibc.jsonnet (1 hunks)
- integration_tests/ibc_utils.py (3 hunks)
- integration_tests/test_ica.py (2 hunks)
- integration_tests/test_ica_precompile.py (2 hunks)
🧰 Additional context used
🔇 Additional comments (4)
integration_tests/test_ica.py (3)
12-12: LGTM: New import for balance query packet generation.The addition of
gen_query_balance_packetimport is consistent with the existing code style and aligns with the PR objective of implementing safe querying for Inter-Chain Accounts.
168-168: LGTM: Added balance query test.The addition of
call_module_safe_queryat the end of thetest_icafunction is a good placement for testing the balance query after all other operations. This change aligns well with the PR objective of implementing and testing safe querying for Inter-Chain Accounts.
Line range hint
1-181: Overall assessment: Well-implemented ICA safe querying test.The changes in this file successfully implement and integrate the safe querying mechanism for Inter-Chain Accounts. The new functionality is well-placed within the existing test structure, and the implementation is clean and consistent with the codebase style. The only suggestion for improvement is to consider making the gas limit in the
call_module_safe_queryfunction more flexible.To ensure that the
gen_query_balance_packetfunction is correctly implemented in theibc_utils.pyfile, please run the following verification script:integration_tests/configs/ibc.jsonnet (1)
114-114: Adding 'MsgModuleQuerySafe' to allowed messagesThe inclusion of
/ibc.applications.interchain_accounts.host.v1.MsgModuleQuerySafein theallow_messageslist enables the host chain to processMsgModuleQuerySafemessages. This is necessary for testing ICA queries as outlined in the PR objectives.
18c337a
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
New Features
cronos_777-1andchainmain-1, including new parameters for message handling and account definitions.Bug Fixes
Documentation