Conversation
* Problem: wrong cmd runs with chain_binary * Revert "Problem: wrong cmd runs with chain_binary" This reverts commit 8f26321. * Problem: cmd flag don't support multiple chains * changelog * Update pystarport/cluster.py Signed-off-by: yihuang <huang@crypto.com> --------- Signed-off-by: yihuang <huang@crypto.com> Co-authored-by: HuangYi <huang@crypto.com>
* Problem: debug-addr rename to debug-listen-addr in relayer for more info, https://github.com/cosmos/relayer/pull/1504/files * keep previous
* Problem: event-query-tx-for not compatible with chain-main * Update CHANGELOG.md Signed-off-by: yihuang <huang@crypto.com> * fix lint --------- Signed-off-by: yihuang <huang@crypto.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request introduces a new Flake8 configuration file and updates the changelog with a new entry for external community pool queries. It modifies the cluster management and Cosmos CLI modules by updating method signatures to accept flexible keyword arguments, adding a coin type parameter, and introducing new methods for querying IBC denominations. Conditional logic is added to detect the availability of a new event query subcommand, affecting transaction processing. The cluster initialization flow is enhanced for multi-chain commands and debugging options. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ClusterCLI as Cluster CLI
participant Account as create_account
User->>ClusterCLI: Invoke create_node(coin_type, ...)
ClusterCLI->>Account: Call create_account(name, coin_type, **kwargs)
Account-->>ClusterCLI: Return account details
ClusterCLI-->>User: Return node creation result
sequenceDiagram
participant User
participant CosmosCLI as Cosmos CLI
participant ChainCmd as ChainCommand
User->>CosmosCLI: Invoke transaction-related command
CosmosCLI->>ChainCmd: Call prob_event_query_tx_for()
ChainCmd-->>CosmosCLI: Return event query availability (True/False)
alt Event Query Available
CosmosCLI->>User: Execute event query transaction flow
else Event Query Not Available
CosmosCLI->>User: Proceed with standard transaction processing
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (2)
CHANGELOG.md (1)
28-30: New changelog entries align with code changesThe changelog entries accurately reflect the changes implemented in the code base, providing good documentation of the new features and compatibility improvements.
Minor grammatical issue in the third entry - consider changing "binary that don't" to "binaries that don't" or "binary that doesn't" for grammatical correctness.
- - [#145](https://github.com/crypto-com/pystarport/pull/145) Backward compatible with binary that don't have event-query-tx-for. + - [#145](https://github.com/crypto-com/pystarport/pull/145) Backward compatible with binaries that don't have event-query-tx-for.🧰 Tools
🪛 LanguageTool
[grammar] ~30-~30: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...5) Backward compatible with binary that don't have event-query-tx-for. Feb 7, 2023...(SINGULAR_NOUN_AGREEMENT_WHO_DONT)
pystarport/cluster.py (1)
1253-1259: Added support for multiple chain commandsThis enhancement allows different commands to be specified for different chains, making the configuration more flexible.
Consider using a ternary operator for conciseness:
- if cmd is not None: - cmds = cmd.split(",") - else: - cmds = [None] * len(chains) + cmds = cmd.split(",") if cmd is not None else [None] * len(chains)Additionally, consider adding validation to ensure that when
cmdis provided, the number of comma-separated commands matches the number of chains:if cmd is not None: cmds = cmd.split(",") if len(cmds) != len(chains): raise ValueError(f"Number of commands ({len(cmds)}) does not match number of chains ({len(chains)})") else: cmds = [None] * len(chains)🧰 Tools
🪛 Ruff (0.8.2)
1254-1257: Use ternary operator
cmds = cmd.split(",") if cmd is not None else [None] * len(chains)instead ofif-else-blockReplace
if-else-block withcmds = cmd.split(",") if cmd is not None else [None] * len(chains)(SIM108)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.flake8(1 hunks)CHANGELOG.md(1 hunks)pystarport/cluster.py(9 hunks)pystarport/cosmoscli.py(31 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
pystarport/cluster.py (2)
pystarport/cosmoscli.py (5)
create_account(133-157)create_account_ledger(159-187)make_multisig(584-594)ibc_denom(1476-1487)account(335-340)pystarport/cli.py (1)
cli(174-181)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~30-~30: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...5) Backward compatible with binary that don't have event-query-tx-for. Feb 7, 2023...
(SINGULAR_NOUN_AGREEMENT_WHO_DONT)
🪛 Ruff (0.8.2)
pystarport/cluster.py
1254-1257: Use ternary operator cmds = cmd.split(",") if cmd is not None else [None] * len(chains) instead of if-else-block
Replace if-else-block with cmds = cmd.split(",") if cmd is not None else [None] * len(chains)
(SIM108)
🔇 Additional comments (20)
.flake8 (1)
1-4: LGTM: Well-configured Flake8 settingsThe Flake8 configuration follows good Python practices:
- Line length of 88 characters aligns with Black formatter
- Ignoring E203 (whitespace before ':') is standard when using Black
- Appropriate exclusions for generated files and non-source directories
pystarport/cosmoscli.py (10)
71-79: Good feature detection implementationThe addition of this method to check for the availability of the "event-query-tx-for" subcommand is a good practice for ensuring backward compatibility.
113-113: LGTM: Initializing the feature detection flagProperly initializes the feature flag during object construction, which will be used throughout the class.
133-157: Method signature enhancement for flexibilityAdding
**kwargsto the method signature allows passing additional parameters likecoin_typewithout breaking existing code.
159-187: Flexible parameter passing for ledger account creationSimilar to the previous change, this makes the method more adaptable to future parameter additions.
294-296: Added handling for nested commission response formatThis change improves the robustness of the code by handling both direct and nested commission response formats.
405-412: Improved backward compatibility for event queriesThe conditional check now verifies if the event-query-tx-for feature is available before attempting to use it.
446-453: Feature availability check for ledger transfersSimilar to the previous change, this ensures backward compatibility when transferring from a ledger device.
507-509: Consistent feature checking across all transaction methodsThe pattern of checking for feature availability has been consistently applied across all transaction methods, ensuring complete backward compatibility.
Also applies to: 529-531, 560-562, 580-582, 724-726, 773-775, 823-825, 863-865, 957-959, 980-982, 1057-1059, 1095-1097, 1154-1156, 1191-1193, 1223-1225, 1253-1255, 1326-1328, 1386-1388, 1441-1443, 1459-1461
584-594: Enhanced multisig account creation with flexible parametersAdded
**kwargsto allow passing additional parameters likecoin_typeto multisig account creation.
1476-1488: Added IBC denomination query capabilityThis new method provides functionality to query IBC denominations by path, enhancing the IBC-related capabilities of the client.
pystarport/cluster.py (9)
189-189: Added coin_type parameter for more flexible chain configurationThis new parameter allows specifying the coin type when creating a node, supporting different blockchain ecosystems.
261-262: Properly passing coin_type to account creationThe coin_type parameter is correctly passed to the validator account creation.
315-318: Enhanced account creation with flexible parametersUpdated method signature to accept
**kwargs, allowing for more flexible parameter passing including coin_type.
319-322: Flexible parameters for ledger account creationSimilar to the regular account creation, this enhances flexibility for ledger accounts.
514-516: Updated multisig creation with flexible parametersConsistent with other account creation methods, this now accepts additional parameters.
838-840: Added IBC denomination query capabilityThis method provides a convenient way to access IBC denomination information for a chain node.
907-917: Improved account creation with coin type supportAccount creation now properly extracts and passes the coin-type parameter from the configuration.
1023-1026: Enhanced validator account creation with coin type supportValidator accounts are now created with the specified coin type from configuration.
1301-1303: Added debug server configurationThe configuration enables a debug server on port 5183 and specifies the listen address for debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
CHANGELOG.md (2)
30-30: Fix grammar in changelog entry.There's a subject-verb agreement issue in the PR crypto-com#145 description.
-Backward compatible with binary that don't have event-query-tx-for. +Backward compatible with binary that doesn't have event-query-tx-for.🧰 Tools
🪛 LanguageTool
[grammar] ~30-~30: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...5) Backward compatible with binary that don't have event-query-tx-for. - [crypto-com#147](https...(SINGULAR_NOUN_AGREEMENT_WHO_DONT)
31-31: Fix typo in changelog entry.There's a minor typo in the PR crypto-com#147 description.
-suppport query exposed by the external community pool. +support query exposed by the external community pool.pystarport/cluster.py (1)
1253-1258: Simplify conditional logic.This code can be simplified using a ternary operator.
-# for multiple chains, there can be multiple cmds splited by `,` -if cmd is not None: - cmds = cmd.split(",") -else: - cmds = [None] * len(chains) +# for multiple chains, there can be multiple cmds split by `,` +cmds = cmd.split(",") if cmd is not None else [None] * len(chains)🧰 Tools
🪛 Ruff (0.8.2)
1254-1257: Use ternary operator
cmds = cmd.split(",") if cmd is not None else [None] * len(chains)instead ofif-else-blockReplace
if-else-block withcmds = cmd.split(",") if cmd is not None else [None] * len(chains)(SIM108)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)pystarport/cluster.py(10 hunks)pystarport/cosmoscli.py(31 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
pystarport/cluster.py (2)
pystarport/cosmoscli.py (6)
create_account(133-157)create_account_ledger(159-187)distribution_community(298-319)make_multisig(595-605)ibc_denom(1487-1498)account(346-351)pystarport/cli.py (1)
cli(174-181)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~30-~30: There seems to be a noun/verb agreement error. Did you mean “doesn't” or “didn't”?
Context: ...5) Backward compatible with binary that don't have event-query-tx-for. - [crypto-com#147](https...
(SINGULAR_NOUN_AGREEMENT_WHO_DONT)
🪛 Ruff (0.8.2)
pystarport/cluster.py
1254-1257: Use ternary operator cmds = cmd.split(",") if cmd is not None else [None] * len(chains) instead of if-else-block
Replace if-else-block with cmds = cmd.split(",") if cmd is not None else [None] * len(chains)
(SIM108)
🔇 Additional comments (12)
pystarport/cosmoscli.py (6)
71-79: Properly handle event query feature detection.This new method is well implemented and follows the existing pattern for feature detection. Good approach to ensure backward compatibility with binaries that don't support this feature.
113-113: Good feature detection implementation.Setting this flag in the constructor allows conditional execution throughout the codebase, ensuring backward compatibility.
133-156: Great flexibility improvement with kwargs.Adding
**kwargsto these methods improves flexibility and extendability, allowing additional parameters to be passed through to the underlying CLI commands.Also applies to: 159-173, 595-605
298-319: Robust implementation for community pool queries.This enhancement properly handles queries to both the "distribution" and "protocolpool" modules with appropriate error handling. The approach to gracefully fall back if a specific error is encountered is well done.
416-422: Consistent conditional event query checks.These conditional checks properly guard against calling the event query feature when it's not available, ensuring backward compatibility with older binaries.
Also applies to: 457-463, 518-519, 540-541, 571-572, 591-592, 734-736, 784-785, 834-835, 874-875, 968-969, 991-992, 1068-1069, 1106-1107, 1165-1166, 1202-1203, 1234-1235, 1264-1265, 1337-1338, 1397-1398, 1452-1454, 1470-1471
1487-1498: New IBC denomination query method.The
ibc_denommethod complements the existingibc_denom_tracemethod and expands the functionality for IBC-related queries.pystarport/cluster.py (6)
189-189: Support for coin type in account creation.Adding
coin_typeparameter to node creation and validator account setup allows for greater flexibility when working with different chains and wallet types.Also applies to: 261-261
315-317: Consistent API enhancement with kwargs support.These methods now properly forward additional keyword arguments to the underlying CLI commands, providing a consistent interface with the cosmoscli module.
Also applies to: 319-321, 372-373, 514-515
838-840: Exposed IBC denomination functionality.Adding the
ibc_denommethod exposes the new functionality from theCosmosCLIclass to the cluster level.
907-917: Account creation with coin type support.The code now properly extracts and uses the coin type parameter when creating accounts, which enables better support for different blockchain networks.
Also applies to: 1023-1026
1252-1263: Good support for multiple chain commands.The ability to specify different commands for different chains is a nice feature, allowing more flexible configurations when dealing with multi-chain setups.
🧰 Tools
🪛 Ruff (0.8.2)
1254-1257: Use ternary operator
cmds = cmd.split(",") if cmd is not None else [None] * len(chains)instead ofif-else-blockReplace
if-else-block withcmds = cmd.split(",") if cmd is not None else [None] * len(chains)(SIM108)
1301-1302: Added debug server configuration.New debug server settings enhance the troubleshooting capabilities for the relayer configuration.
Bumps [setuptools](https://github.com/pypa/setuptools) from 70.0.0 to 78.1.1. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst) - [Commits](pypa/setuptools@v70.0.0...v78.1.1) --- updated-dependencies: - dependency-name: setuptools dependency-version: 78.1.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Chores
Documentation