-
Notifications
You must be signed in to change notification settings - Fork 240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Problem: v0.50.x is outdated #1722
Conversation
WalkthroughThis pull request involves updates to the project's Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Mod as Module Dependencies
participant Proj as Project Codebase
Dev->>Mod: Update dependencies
Mod-->>Dev: Provide new library versions
Dev->>Proj: Replace dependencies in go.mod
Dev->>Proj: Update CHANGELOG.md
Proj-->>Dev: Reflect changes
Possibly related PRs
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1722 +/- ##
===========================================
+ Coverage 16.87% 35.56% +18.68%
===========================================
Files 72 126 +54
Lines 6163 11915 +5752
===========================================
+ Hits 1040 4237 +3197
- Misses 5000 7254 +2254
- Partials 123 424 +301 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
integration_tests/test_basic.py (3)
958-960
: Consider using constants for magic numbersThe staking and token amounts are hardcoded. Consider defining these as constants at the module level for better maintainability and clarity.
+# Constants for token amounts +VALIDATOR_STAKE_AMOUNT = 1000000000000000000 # 1 TCRO +VALIDATOR_TOKEN_AMOUNT = 50000000000000000 # 0.05 TCRO - staked = 1000000000000000000 - amt = 50000000000000000 - coin = f"{staked}stake,{amt}basetcro" + coin = f"{VALIDATOR_STAKE_AMOUNT}stake,{VALIDATOR_TOKEN_AMOUNT}basetcro"
983-989
: Consider parameterizing validator configurationThe validator creation parameters are hardcoded. Consider making them configurable for better test flexibility.
+ validator_config = { + "moniker": moniker, + "commission_rate": "1.0", + "commission_max_rate": "2.0", + "commission_max_change_rate": "0.1", + } res = cli1.create_validator( f"{staked}stake", - { - "moniker": moniker, - }, + validator_config, gas_prices=DEFAULT_GAS_PRICE, )
1000-1004
: Document commission rate valuesThe commission rate values are using a specific format (18 decimal places). Consider adding a comment explaining the format and why these specific values are used.
+ # Commission rates are in decimal format with 18 decimal places + # rate: 0.1 (10%) + # max_rate: 0.2 (20%) + # max_change_rate: 0.01 (1%) assert val["commission"]["commission_rates"] == { "rate": "100000000000000000", "max_rate": "200000000000000000", "max_change_rate": "10000000000000000", }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
integration_tests/test_basic.py
(2 hunks)
🔇 Additional comments (3)
integration_tests/test_basic.py (3)
949-961
: LGTM: Node setup and initialization looks good
The initial setup of the test environment, including node creation and port configuration, follows the established patterns.
966-975
: LGTM: JSON-RPC configuration is properly set up
The configuration of JSON-RPC endpoints follows security best practices by binding to localhost.
991-999
: LGTM: Comprehensive validation checks
The test includes thorough validation of the validator's status, including checks for jailing status, bonding status, tokens, and moniker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
integration_tests/test_basic.py (2)
962-964
: Consider extracting magic numbers into named constantsThe staking and transfer amounts are hardcoded. Consider defining these as named constants at the module level for better maintainability and clarity.
+# Constants for validator setup +VALIDATOR_STAKE_AMOUNT = 1000000000000000000 # 1 TCRO +VALIDATOR_INITIAL_TRANSFER = 50000000000000000 # 0.05 TCRO + def test_join_validator(cronos): # ... - staked = 1000000000000000000 - amt = 50000000000000000 + staked = VALIDATOR_STAKE_AMOUNT + amt = VALIDATOR_INITIAL_TRANSFER
987-994
: Consider parameterizing validator configurationThe validator creation uses hardcoded values for commission rates and other parameters. Consider making these configurable or extracting them into constants.
+# Constants for validator configuration +DEFAULT_COMMISSION_RATE = "100000000000000000" # 0.1 +DEFAULT_MAX_RATE = "200000000000000000" # 0.2 +DEFAULT_MAX_CHANGE_RATE = "10000000000000000" # 0.01 + def test_join_validator(cronos): # ... res = cli1.create_validator( f"{staked}stake", { "moniker": moniker, + "commission_rate": DEFAULT_COMMISSION_RATE, + "max_rate": DEFAULT_MAX_RATE, + "max_change_rate": DEFAULT_MAX_CHANGE_RATE, }, gas_prices=DEFAULT_GAS_PRICE, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sum
is excluded by!**/*.sum
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)go.mod
(1 hunks)integration_tests/test_basic.py
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- CHANGELOG.md
- go.mod
🔇 Additional comments (3)
integration_tests/test_basic.py (3)
953-959
: LGTM: Clear test setup with well-defined parameters
The initial setup of the test function is clear and follows good practices by:
- Using descriptive variable names
- Setting up a new node with unique ports
- Properly initializing the test environment
980-984
: Add error handling for node startup
The node startup process should include error handling in case the node fails to start or sync.
1010-1021
: LGTM: Good use of helper function for status checking
The implementation of status checking and unbonding test is well-structured:
- Uses a helper function for status verification
- Properly waits for status changes
- Verifies the unbonding process
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
for more info
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
CHANGELOG.md
to include new bug fix entries and reorganized existing entries for clarity.go.mod
to newer versions and alternative sources for improved functionality and security.