Create delisting gov proposal script #2722
Conversation
WalkthroughA new Python script is introduced to automate the creation of a delisting proposal within a decentralized finance context. The script fetches market data via HTTP requests, applies helper functions to derive mappings for trading pairs, and then constructs a proposal with messages to update market status and disable tickers. Finally, the proposal is printed in JSON format. The file includes the main orchestration function and two helper functions for data extraction. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as main()
participant H1 as get_ticker_to_perpetual_id()
participant H2 as get_perpetual_id_to_clob_pair()
participant MD as Market Data API
U->>M: Execute script
M->>MD: HTTP GET market data
MD-->>M: Return market data
M->>H1: Retrieve ticker-perpetual mapping
H1-->>M: Return mapping
M->>H2: Retrieve perpetual-CLOB pair mapping
H2-->>M: Return mapping
M->>M: Loop through tickers for delisting
M->>M: Append proposal update messages
M->>U: Output complete proposal in JSON
Poem
✨ Finishing Touches
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
🧹 Nitpick comments (4)
protocol/scripts/governance/create_delisting_proposal.py (4)
1-2: Add error handling for requests import.Consider adding error handling for the requests import and specify the minimum required version.
import json +try: import requests +except ImportError: + raise ImportError( + "The 'requests' package is required. " + "Please install it via 'pip install requests>=2.31.0'" + )
5-8: Consider making URLs configurable.The hardcoded URLs limit the script's flexibility across different environments.
Consider:
- Moving URLs to a configuration file
- Adding environment variables support
- Adding command-line arguments for URL override
+import os +from typing import Final + -MAINNET_URL_BASE = "https://dydx-ops-rest.kingnodes.com" +MAINNET_URL_BASE: Final = os.getenv( + "DYDX_API_URL", + "https://dydx-ops-rest.kingnodes.com" +)
73-74: Add logging configuration.Configure logging to help with debugging and monitoring.
if __name__ == '__main__': + logging.basicConfig( + level=logging.INFO, + format='%(asctime)s - %(levelname)s - %(message)s' + ) main()
1-74: Add unit tests for governance operations.Given that this script handles critical governance operations for delisting markets, it's essential to have comprehensive test coverage.
Consider adding:
- Unit tests for helper functions
- Integration tests with mock API responses
- Test cases for error scenarios
- Validation of proposal structure
- End-to-end tests with a test environment
Would you like me to help create a test suite for this script?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
protocol/scripts/governance/create_delisting_proposal.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: test-race
- GitHub Check: test-coverage-upload
- GitHub Check: build
- GitHub Check: liveness-test
- GitHub Check: golangci-lint
- GitHub Check: benchmark
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: container-tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
| # Create clob delisting proposal message | ||
| clob_delisting_message = {} | ||
| clob_delisting_message["@type"] = "/dydxprotocol.clob.MsgUpdateClobPair" | ||
| clob_delisting_message["authority"] = "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move hardcoded authority address to configuration.
The authority address is hardcoded which makes the script less maintainable and could lead to errors if the address changes.
+import os
+from typing import Final
+
+AUTHORITY_ADDRESS: Final = os.getenv(
+ "DYDX_AUTHORITY_ADDRESS",
+ "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
+)
+
- clob_delisting_message["authority"] = "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
+ clob_delisting_message["authority"] = AUTHORITY_ADDRESS
- marketmap_disable_message["authority"] = "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
+ marketmap_disable_message["authority"] = AUTHORITY_ADDRESSAlso applies to: 51-51
| def main(): | ||
| marketmap_data = requests.get(MARKETMAP_URL).json() | ||
| perpetuals_data = requests.get(PERPETUALS_URL).json() | ||
| clob_data = requests.get(CLOB_URL).json() | ||
|
|
There was a problem hiding this comment.
Add error handling for HTTP requests.
The script doesn't handle potential HTTP request failures or invalid JSON responses.
Add proper error handling:
+import logging
+from typing import Any, Dict
+
+def fetch_data(url: str) -> Dict[str, Any]:
+ try:
+ response = requests.get(url, timeout=30)
+ response.raise_for_status()
+ return response.json()
+ except requests.exceptions.RequestException as e:
+ logging.error(f"Failed to fetch data from {url}: {e}")
+ raise
+ except ValueError as e:
+ logging.error(f"Invalid JSON response from {url}: {e}")
+ raise
+
def main():
- marketmap_data = requests.get(MARKETMAP_URL).json()
- perpetuals_data = requests.get(PERPETUALS_URL).json()
- clob_data = requests.get(CLOB_URL).json()
+ validate_tickers()
+ marketmap_data = fetch_data(MARKETMAP_URL)
+ perpetuals_data = fetch_data(PERPETUALS_URL)
+ clob_data = fetch_data(CLOB_URL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def main(): | |
| marketmap_data = requests.get(MARKETMAP_URL).json() | |
| perpetuals_data = requests.get(PERPETUALS_URL).json() | |
| clob_data = requests.get(CLOB_URL).json() | |
| import logging | |
| from typing import Any, Dict | |
| import requests # assuming requests is not already imported elsewhere | |
| def fetch_data(url: str) -> Dict[str, Any]: | |
| try: | |
| response = requests.get(url, timeout=30) | |
| response.raise_for_status() | |
| return response.json() | |
| except requests.exceptions.RequestException as e: | |
| logging.error(f"Failed to fetch data from {url}: {e}") | |
| raise | |
| except ValueError as e: | |
| logging.error(f"Invalid JSON response from {url}: {e}") | |
| raise | |
| def main(): | |
| validate_tickers() | |
| marketmap_data = fetch_data(MARKETMAP_URL) | |
| perpetuals_data = fetch_data(PERPETUALS_URL) | |
| clob_data = fetch_data(CLOB_URL) |
| def get_ticker_to_perpetual_id(perpetuals_data): | ||
| ticker_to_perpetual_id = {} | ||
| for data in perpetuals_data["perpetual"]: | ||
| ticker = data["params"]["ticker"] | ||
| perpetual_id = data["params"]["id"] | ||
| ticker_to_perpetual_id[ticker] = perpetual_id | ||
| return ticker_to_perpetual_id | ||
|
|
||
| def get_perpetual_id_to_clob_pair(clob_data): | ||
| perpetual_id_to_clob_pair = {} | ||
| for data in clob_data["clob_pair"]: | ||
| perpetual_id = data["perpetual_clob_metadata"]["perpetual_id"] | ||
| perpetual_id_to_clob_pair[perpetual_id] = data | ||
| return perpetual_id_to_clob_pair |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add type hints and input validation to helper functions.
The helper functions lack type hints and input validation which could lead to runtime errors.
-def get_ticker_to_perpetual_id(perpetuals_data):
+from typing import Dict
+
+def get_ticker_to_perpetual_id(perpetuals_data: Dict[str, Any]) -> Dict[str, str]:
+ if not isinstance(perpetuals_data, dict) or "perpetual" not in perpetuals_data:
+ raise ValueError("Invalid perpetuals data structure")
+
ticker_to_perpetual_id = {}
for data in perpetuals_data["perpetual"]:
+ if not isinstance(data, dict) or "params" not in data:
+ logging.warning(f"Skipping invalid perpetual data: {data}")
+ continue
ticker = data["params"]["ticker"]
perpetual_id = data["params"]["id"]
ticker_to_perpetual_id[ticker] = perpetual_id
return ticker_to_perpetual_id
-def get_perpetual_id_to_clob_pair(clob_data):
+def get_perpetual_id_to_clob_pair(clob_data: Dict[str, Any]) -> Dict[str, Dict[str, Any]]:
+ if not isinstance(clob_data, dict) or "clob_pair" not in clob_data:
+ raise ValueError("Invalid CLOB data structure")
+
perpetual_id_to_clob_pair = {}
for data in clob_data["clob_pair"]:
+ if not isinstance(data, dict) or "perpetual_clob_metadata" not in data:
+ logging.warning(f"Skipping invalid CLOB pair data: {data}")
+ continue
perpetual_id = data["perpetual_clob_metadata"]["perpetual_id"]
perpetual_id_to_clob_pair[perpetual_id] = data
return perpetual_id_to_clob_pair📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_ticker_to_perpetual_id(perpetuals_data): | |
| ticker_to_perpetual_id = {} | |
| for data in perpetuals_data["perpetual"]: | |
| ticker = data["params"]["ticker"] | |
| perpetual_id = data["params"]["id"] | |
| ticker_to_perpetual_id[ticker] = perpetual_id | |
| return ticker_to_perpetual_id | |
| def get_perpetual_id_to_clob_pair(clob_data): | |
| perpetual_id_to_clob_pair = {} | |
| for data in clob_data["clob_pair"]: | |
| perpetual_id = data["perpetual_clob_metadata"]["perpetual_id"] | |
| perpetual_id_to_clob_pair[perpetual_id] = data | |
| return perpetual_id_to_clob_pair | |
| from typing import Dict, Any | |
| import logging | |
| def get_ticker_to_perpetual_id(perpetuals_data: Dict[str, Any]) -> Dict[str, str]: | |
| if not isinstance(perpetuals_data, dict) or "perpetual" not in perpetuals_data: | |
| raise ValueError("Invalid perpetuals data structure") | |
| ticker_to_perpetual_id = {} | |
| for data in perpetuals_data["perpetual"]: | |
| if not isinstance(data, dict) or "params" not in data: | |
| logging.warning(f"Skipping invalid perpetual data: {data}") | |
| continue | |
| ticker = data["params"]["ticker"] | |
| perpetual_id = data["params"]["id"] | |
| ticker_to_perpetual_id[ticker] = perpetual_id | |
| return ticker_to_perpetual_id | |
| def get_perpetual_id_to_clob_pair(clob_data: Dict[str, Any]) -> Dict[str, Dict[str, Any]]: | |
| if not isinstance(clob_data, dict) or "clob_pair" not in clob_data: | |
| raise ValueError("Invalid CLOB data structure") | |
| perpetual_id_to_clob_pair = {} | |
| for data in clob_data["clob_pair"]: | |
| if not isinstance(data, dict) or "perpetual_clob_metadata" not in data: | |
| logging.warning(f"Skipping invalid CLOB pair data: {data}") | |
| continue | |
| perpetual_id = data["perpetual_clob_metadata"]["perpetual_id"] | |
| perpetual_id_to_clob_pair[perpetual_id] = data | |
| return perpetual_id_to_clob_pair |
|
|
||
| DELISTED_CLOB_STATUS = "STATUS_FINAL_SETTLEMENT" | ||
|
|
||
| TICKERS_TO_DELIST = [] # should be in BASE-QUOTE format (ex. PAIN-USD) |
There was a problem hiding this comment.
Validate TICKERS_TO_DELIST is not empty.
An empty TICKERS_TO_DELIST list could lead to creating an invalid proposal.
Add validation at the start of the main function:
TICKERS_TO_DELIST = [] # should be in BASE-QUOTE format (ex. PAIN-USD)
+
+def validate_tickers():
+ if not TICKERS_TO_DELIST:
+ raise ValueError(
+ "TICKERS_TO_DELIST cannot be empty. "
+ "Please specify tickers in BASE-QUOTE format (ex. PAIN-USD)"
+ )Committable suggestion skipped: line range outside the PR's diff.
Changelist
[Describe or list the changes made in this PR]
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