[ENG-159] Script to auto launch markets #2938
Conversation
|
""" WalkthroughA new Python script was introduced to automate the synchronization and launch of blockchain markets on a testnet. The script manages authority assignment, market map comparison between mainnet and testnet, governance proposal handling, and market creation, all via command-line interactions. Several helper functions and a command-line interface were added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant Blockchain CLI
participant Mainnet Node
participant Testnet Node
User->>Script: Run script with arguments
Script->>Blockchain CLI: Query authorities
alt Alice not authority
Script->>Blockchain CLI: Submit proposal to add Alice
Script->>Blockchain CLI: Vote "yes" on proposal
Script->>Blockchain CLI: Wait for proposal to pass
end
Script->>Mainnet Node: Download mainnet market map
Script->>Testnet Node: Download testnet market map
Script->>Script: Compare market maps
loop For each missing market (up to limit)
Script->>Blockchain CLI: Add market to testnet
Script->>Blockchain CLI: Wait for confirmation
Script->>Blockchain CLI: Launch market on listing module
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 10
🧹 Nitpick comments (5)
protocol/scripts/markets/launch_markets.py (5)
10-10: Remove unused importsThe imports
sysandListare not used in the code.-import sys import tempfile import yaml import json -from typing import Dict, Any, List +from typing import Dict, AnyAlso applies to: 14-14
17-21: Consider making hardcoded values configurableHardcoded addresses and endpoints reduce flexibility and maintainability. Consider moving these to configuration parameters or environment variables.
Consider implementing a configuration approach:
# Option 1: Environment variables alice_address = os.getenv("ALICE_ADDRESS", "dydx199tqg4wdlnu4qjlxchpd7seg454937hjrknju4") bob_address = os.getenv("BOB_ADDRESS", "dydx10fx7sy6ywd5senxae9dwytf8jxek3t2gcen2vs") mainnet_node = os.getenv("MAINNET_NODE", "https://dydx-ops-rpc.kingnodes.com:443") mainnet_chain = os.getenv("MAINNET_CHAIN", "dydx-mainnet-1") # Option 2: Configuration file # Load from config.json or config.yaml
144-164: Consider logging errors instead of printingThe function handles errors well, but consider using Python's logging module for better error tracking.
import logging def load_yml(file_path) -> Dict[str, Any]: """ Loads any yml file and returns the data as a dictionary. Args: file_path: Path to the yml file Returns: Dictionary containing the parsed data """ try: with open(file_path, 'r', encoding='utf-8') as file: data = yaml.safe_load(file) return data except FileNotFoundError: logging.error(f"File '{file_path}' not found.") return {} except yaml.YAMLError as e: logging.error(f"Error parsing YAML file: {e}") return {}
259-259: Fix misleading commentThe comment mentions "bob" but the --from parameter is "alice".
- "--from=alice", # bob has a decent amount of native tokens in subaccount 0 for gas. + "--from=alice",
1-286: Consider adding structured logging and configuration managementThe script would benefit from:
- Using Python's
loggingmodule instead of print statements- Implementing a configuration class or using a config file for all constants
- Adding a
--dry-runoption to preview actions without executing them- Consider breaking down the main function into smaller, testable functions
Would you like me to help implement a more robust configuration and logging system for this script?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
protocol/scripts/markets/launch_markets.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
protocol/scripts/markets/launch_markets.py
10-10: sys imported but unused
Remove unused import: sys
(F401)
14-14: typing.List imported but unused
Remove unused import: typing.List
(F401)
🪛 Flake8 (7.2.0)
protocol/scripts/markets/launch_markets.py
[error] 10-10: 'sys' imported but unused
(F401)
[error] 14-14: 'typing.List' imported but unused
(F401)
[error] 22-22: expected 2 blank lines, found 1
(E302)
[error] 36-36: expected 2 blank lines, found 1
(E302)
[error] 54-54: expected 2 blank lines, found 1
(E302)
[error] 144-144: expected 2 blank lines, found 1
(E302)
[error] 165-165: expected 2 blank lines, found 1
(E302)
[error] 191-191: expected 2 blank lines, found 1
(E302)
[error] 206-206: expected 2 blank lines, found 1
(E302)
[error] 259-259: at least two spaces before inline comment
(E261)
[error] 284-284: expected 2 blank lines after class or function definition, found 1
(E305)
🪛 Pylint (3.3.7)
protocol/scripts/markets/launch_markets.py
[refactor] 138-142: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
⏰ Context from checks skipped due to timeout of 90000ms (28)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: benchmark
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: build
- GitHub Check: build-and-push-mainnet
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: container-tests
- GitHub Check: build-and-push-testnet
- GitHub Check: golangci-lint
- GitHub Check: test-coverage-upload
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: test-race
- GitHub Check: liveness-test
- GitHub Check: Summary
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
protocol/scripts/markets/launch_markets.py (6)
23-35: Improve subprocess handling and eliminate temporary file usage.This function still uses the inefficient temporary file approach and lacks error handling that was flagged in previous reviews.
77-95: Address hardcoded governance values.The authority proposal still contains hardcoded addresses and deposit amounts that should be moved to constants as flagged in previous reviews.
170-194: Add error handling for subprocess commands.The subprocess calls still lack error handling as flagged in previous reviews.
196-209: Remove debug output and add safe dictionary access.The function still contains debug print statements and unsafe nested dictionary access as flagged in previous reviews.
238-239: Use command line arguments instead of hardcoded values.The subprocess commands still use hardcoded node and chain-id values instead of the provided arguments, as flagged in previous reviews.
Also applies to: 266-267
283-285: Improve exception handling specificity.The broad exception handling catches all exceptions which can mask programming errors, as flagged in previous reviews.
🧹 Nitpick comments (2)
protocol/scripts/markets/launch_markets.py (2)
10-10: Remove unused imports.The
sysandtyping.Listimports are not used in the code and should be removed for cleaner code.-import sys -from typing import Dict, Any, List +from typing import Dict, AnyAlso applies to: 14-14
23-23: Fix PEP8 formatting issues.Multiple PEP8 formatting violations detected:
- Missing blank lines between function definitions
- Insufficient spacing before inline comments
- Missing blank lines after class/function definitions
Consider running a code formatter like
blackorautopep8to automatically fix these formatting issues.Also applies to: 37-37, 57-57, 149-149, 170-170, 196-196, 211-211, 265-265, 291-291
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
protocol/scripts/markets/launch_markets.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
protocol/scripts/markets/launch_markets.py
10-10: sys imported but unused
Remove unused import: sys
(F401)
14-14: typing.List imported but unused
Remove unused import: typing.List
(F401)
🪛 Flake8 (7.2.0)
protocol/scripts/markets/launch_markets.py
[error] 10-10: 'sys' imported but unused
(F401)
[error] 14-14: 'typing.List' imported but unused
(F401)
[error] 23-23: expected 2 blank lines, found 1
(E302)
[error] 37-37: expected 2 blank lines, found 1
(E302)
[error] 57-57: expected 2 blank lines, found 1
(E302)
[error] 149-149: expected 2 blank lines, found 1
(E302)
[error] 170-170: expected 2 blank lines, found 1
(E302)
[error] 196-196: expected 2 blank lines, found 1
(E302)
[error] 211-211: expected 2 blank lines, found 1
(E302)
[error] 265-265: at least two spaces before inline comment
(E261)
[error] 291-291: expected 2 blank lines after class or function definition, found 1
(E305)
🪛 Pylint (3.3.7)
protocol/scripts/markets/launch_markets.py
[refactor] 143-147: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
[refactor] 221-289: Too many nested blocks (6/5)
(R1702)
⏰ Context from checks skipped due to timeout of 90000ms (27)
- GitHub Check: benchmark
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: test-race
- GitHub Check: test-coverage-upload
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: liveness-test
- GitHub Check: container-tests
- GitHub Check: build
- GitHub Check: golangci-lint
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: build-and-push-mainnet
- GitHub Check: build-and-push-testnet
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (3)
protocol/scripts/markets/launch_markets.py (3)
37-55: Good error handling implementation.The function now properly handles subprocess errors with appropriate exception raising, addressing the previous review feedback.
149-168: Well-implemented error handling.The function properly handles file and YAML parsing errors with appropriate fallbacks and clear error messages.
291-292: Standard main execution guard.Proper implementation of the main execution guard.
Kefancao
changed the title
Changelist
ENG-159
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit