Add release qual script for performing a chain upgrade #3044
Conversation
WalkthroughAdds a new release qualification utility under protocol/scripts/release_qual/: a README documenting usage and safeguards, and a Python script that submits a software upgrade proposal on testnet/staging and auto-votes with predefined test validators, enforcing strict chain-id restrictions and interactive confirmation. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as submit_upgrade_proposal.py
participant C as dydxprotocold CLI
participant N as Node (RPC)
participant G as Gov Module
U->>S: Run with args/env (upgrade_name, blocks_to_wait, node, chain-id)
S->>S: load_config() (validate allow/block lists)
S-->>U: Prompt for confirmation
U-->>S: Confirm
S->>C: status --node <node>
C->>N: Query status
N-->>C: Current block height
C-->>S: Height
S->>S: Compute upgrade_height
S->>S: Write proposal.json (MsgSoftwareUpgrade)
S->>C: tx gov submit-proposal (from alice) --node <node>
C->>N: Broadcast tx
N->>G: Create proposal
N-->>C: txhash
C-->>S: txhash
S->>C: query gov proposals --node <node>
C->>N: List proposals
N-->>C: proposal_id
C-->>S: proposal_id
loop For each test validator
S->>C: tx gov vote <proposal_id> yes --from <validator> --node <node>
C->>N: Broadcast vote
N->>G: Record vote
end
S->>C: query gov proposal <proposal_id> --node <node>
C->>N: Get proposal status
N-->>C: Status (e.g., PASSED)
C-->>S: Status
S-->>U: Print outcome
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
jusbar23
changed the title
![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
🧹 Nitpick comments (6)
protocol/scripts/release_qual/README.md (1)
17-32: Usage: add executable bit guidance and alternative invocationTo avoid friction, add a note to mark the script executable or run it via python. This helps on systems where the file isn’t yet chmod’d.
Apply this diff to add a brief note:
**Usage:** ```bash # Basic usage - upgrade in 300 blocks (default) -./submit_upgrade_proposal.py v5.0.0 +chmod +x submit_upgrade_proposal.py +./submit_upgrade_proposal.py v5.0.0 + +# Or run via Python +python3 submit_upgrade_proposal.py v5.0.0</blockquote></details> <details> <summary>protocol/scripts/release_qual/submit_upgrade_proposal.py (5)</summary><blockquote> `69-82`: **Harden command execution: add timeouts and better error context** Long-running RPCs can hang. Add a timeout and improve error messages to include the command executed. Apply this diff: ```diff -def run_cmd(cmd, node=None): - """Run command and return stdout.""" +def run_cmd(cmd, node=None, timeout=60): + """Run command and return stdout.""" # Add node flag if provided if node and "--node" not in cmd: cmd.extend(["--node", node]) try: result = subprocess.run( - cmd, capture_output=True, text=True, check=True + cmd, capture_output=True, text=True, check=True, timeout=timeout ) return result.stdout - except subprocess.CalledProcessError as e: - print(f"Error: {e.stderr}") + except subprocess.TimeoutExpired: + print(f"Error: command timed out: {' '.join(cmd)}") + return None + except subprocess.CalledProcessError as e: + print(f"Error running: {' '.join(cmd)}") + print(f"stderr: {e.stderr}") return None
126-141: Height parsing: include ValueError in exception handling
int(status['sync_info']['latest_block_height'])can raiseValueError. It’s currently unhandled.Apply this diff:
- except (json.JSONDecodeError, KeyError) as e: + except (json.JSONDecodeError, KeyError, ValueError) as e: # Fallback if we can't get current height print(f"Could not parse block height, using default. Error: {e}") upgrade_height = 1000000 print(f"Using default upgrade height: {upgrade_height}")
199-216: Vote broadcasts: consider block mode for deterministic inclusionTo reduce flakiness and get immediate confirmation, broadcast votes in “block” mode as well, mirroring the proposal submission. This is especially helpful when loops send multiple txs quickly.
Apply this diff:
cmd = [ "dydxprotocold", "tx", "gov", "vote", str(proposal_id), "yes", "--from", voter, "--chain-id", chain_id, "--yes", + "--broadcast-mode", "block", "--gas", "auto", "--fees", "5000000000000000adv4tnt", "--keyring-backend", "test" ]
217-219: Ensure proposal.json is always cleaned up; prefer a temp fileIf the process exits early (e.g., KeyboardInterrupt),
proposal.jsonmay be left behind. Use a temp file and ensure cleanup in a finally block.Apply this diff to make cleanup resilient:
- # Clean up - os.remove("proposal.json") + # Clean up + try: + os.remove("proposal.json") + except FileNotFoundError: + passAdditionally, consider switching to a NamedTemporaryFile so you don’t clash with existing files:
# At top-level imports: import tempfile # Replace the hardcoded filename usage: with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as tf: json.dump(proposal, tf, indent=2) proposal_path = tf.name # Use proposal_path instead of "proposal.json" # And in cleanup: try: os.remove(proposal_path) except FileNotFoundError: pass
221-227: Avoid assuming 1s block time; poll voting_end_time or heightSleeping N seconds as a proxy for N blocks is brittle. Poll the proposal’s
voting_end_timeor current block height to decide when to check final status.Example approach:
- Parse
voting_end_timefromquery gov proposal <id> --output jsonand sleep until that time plus a small buffer.- Alternatively, poll status every few seconds until it transitions out of VOTING_PERIOD or a max timeout elapses.
I can wire either approach if you prefer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
protocol/scripts/release_qual/README.md(1 hunks)protocol/scripts/release_qual/submit_upgrade_proposal.py(1 hunks)
🧰 Additional context used
🪛 LanguageTool
protocol/scripts/release_qual/README.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...stnet/staging environments. Features: - Submits upgrade proposals with a specifi...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ... specified upgrade name and block height - Automatically votes "yes" with all test ...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...lly votes "yes" with all test validators - Restricted to testnet/staging environmen...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...nments only (mainnet explicitly blocked) - Configurable via command-line arguments ...
(QB_NEW_EN)
[grammar] ~34-~34: There might be a mistake here.
Context: ...py v5.0.0 ``` Configuration Priority: 1. Command-line arguments (highest priority...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...ommand-line arguments (highest priority) 2. Environment variables (DYDX_NODE, `DYD...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...variables (DYDX_NODE, DYDX_CHAIN_ID) 3. Default values (v4staging node and testn...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...nd testnet chain-id) Safety Features: - Only works on allowed testnet/staging ch...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ...ainst allowed list Test Validators: The script automatically votes with thes...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...ically votes with these test validators: - alice, bob, carl, dave, emily, fiona, gr...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...xprotocold` CLI installed and configured - Test validator keys in the keyring (usin...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: benchmark
- GitHub Check: container-tests
- GitHub Check: build
- GitHub Check: liveness-test
- GitHub Check: test-coverage-upload
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: test-race
- GitHub Check: check-sample-pregenesis-up-to-date
- GitHub Check: golangci-lint
- 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-ecs-service-socks / (socks) Build and Push
- 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-ender / (ender) 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-bazooka-lambda / (bazooka) Build and Push Lambda
- 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-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-roundtable / (roundtable) 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: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: build-and-push-testnet
- GitHub Check: build-and-push-mainnet
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (2)
protocol/scripts/release_qual/README.md (1)
39-44: Docs/code mismatch — not an issue: staging uses "dydxprotocol-testnet"Verified the repo: the script's allow-list and defaults already target the staging node/chain-id used in this repo, so the README's "testnet/staging" wording is accurate and the original comment can be ignored.
- Evidence:
- protocol/scripts/release_qual/submit_upgrade_proposal.py
- ALLOWED_CHAIN_IDS = ["dydxprotocol-testnet"]
- default_node = "https://validator.v4staging.dydx.exchange:443"
- default_chain_id = "dydxprotocol-testnet"
- Explicit mainnet blocking: BLOCKED_CHAIN_IDS and check for "mainnet" in chain_id.lower()
- protocol/scripts/release_qual/README.md
- Usage and examples reference --chain-id dydxprotocol-testnet and export DYDX_CHAIN_ID=dydxprotocol-testnet; describes "testnet/staging" and safety features
- Other files also set CHAIN_ID="dydxprotocol-testnet" (protocol/testing/testnet-staging/staging.sh, protocol/testing/testnet-dev/dev.sh, protocol/testing/snapshotting/snapshot.sh)
Recommendation (optional): if you expect distinct staging chain-id names in the future, either expand ALLOWED_CHAIN_IDS or switch to a safe pattern match (allow "testnet"/"staging" substrings while blocking "mainnet").
Likely an incorrect or invalid review comment.
protocol/scripts/release_qual/submit_upgrade_proposal.py (1)
33-35: Default node/chain-id pairing looks inconsistentDefault node points to v4staging, but default chain-id is testnet. If that staging endpoint doesn’t actually serve the testnet chain-id, the script will fail early on chain-id validation or during queries.
- Confirm that
https://validator.v4staging.dydx.exchange:443is indeed a node fordydxprotocol-testnet.- If not, consider changing either the default chain-id or default node so they align.
| # Allowed chain IDs for testnet/staging only (blocking mainnet) | ||
| ALLOWED_CHAIN_IDS = [ | ||
| "dydxprotocol-testnet", # Standard testnet/staging chain | ||
| ] | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Allow safe patterns for testnet/staging instead of a single explicit chain-id
The README says “testnet/staging only,” but the allow-list includes only dydxprotocol-testnet. This will block staging. Replace the strict allow-list with a safe pattern check while retaining the mainnet block-list.
Apply this diff to allow chain-ids containing “testnet” or “staging” (case-insensitive):
-# Allowed chain IDs for testnet/staging only (blocking mainnet)
-ALLOWED_CHAIN_IDS = [
- "dydxprotocol-testnet", # Standard testnet/staging chain
-]
+# Allowed chain-id patterns for testnet/staging only (blocking mainnet)
+ALLOWED_CHAIN_ID_PATTERNS = ("testnet", "staging")
@@
- # Validate chain_id is allowed
- if chain_id not in ALLOWED_CHAIN_IDS:
- print(f"Error: Chain ID '{chain_id}' is not in the allowed list.")
- print(f"Allowed chains: {', '.join(ALLOWED_CHAIN_IDS)}")
- print("This script is restricted to testnet/staging environments only.")
- sys.exit(1)
+ # Validate chain_id matches allowed patterns (still block mainnet above)
+ if not any(p in chain_id.lower() for p in ALLOWED_CHAIN_ID_PATTERNS):
+ print(f"Error: Chain ID '{chain_id}' is not allowed.")
+ print(f"Allowed chain-id patterns: {', '.join(ALLOWED_CHAIN_ID_PATTERNS)}")
+ print("This script is restricted to testnet/staging environments only.")
+ sys.exit(1)Also applies to: 57-63
🤖 Prompt for AI Agents
In protocol/scripts/release_qual/submit_upgrade_proposal.py around lines 16-20
(and also update lines 57-63), the current allow-list hardcodes
"dydxprotocol-testnet" which blocks other testnet/staging chain-ids; replace the
strict list with a case-insensitive pattern check that permits any chain-id
containing "testnet" or "staging" and keep the explicit mainnet block (e.g.,
check that chain_id.lower() contains "testnet" or "staging" and otherwise
reject, while still preventing mainnet values).
| # Create proposal.json | ||
| proposal = { | ||
| "messages": [{ | ||
| "@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade", | ||
| "authority": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky", | ||
| "plan": { | ||
| "name": upgrade_name, | ||
| "height": str(upgrade_height), | ||
| "info": f"Upgrade to {upgrade_name}" | ||
| } | ||
| }], | ||
| "metadata": "", | ||
| "deposit": "10000000000000000000000adv4tnt", | ||
| "title": f"Software Upgrade to {upgrade_name}", | ||
| "summary": f"Upgrade the chain to {upgrade_name}" | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t hardcode the upgrade authority; fetch it from the chain
Using a hardcoded authority risks unauthorized proposals if the address is wrong. Cosmos SDK exposes the upgrade module authority via a query. Use it.
Apply this diff to retrieve authority dynamically and fail fast if it can’t be retrieved:
- # Create proposal.json
- proposal = {
+ # Resolve upgrade module authority dynamically
+ authority_resp = run_cmd(
+ ["dydxprotocold", "query", "upgrade", "authority", "--output", "json"],
+ node=node
+ )
+ if not authority_resp:
+ print("Error: Could not fetch upgrade module authority from chain.")
+ sys.exit(1)
+ try:
+ authority = json.loads(authority_resp)["authority"]
+ except (json.JSONDecodeError, KeyError) as e:
+ print(f"Error parsing authority from response: {e}")
+ sys.exit(1)
+
+ # Create proposal.json
+ proposal = {
"messages": [{
"@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade",
- "authority": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky",
+ "authority": authority,
"plan": {
"name": upgrade_name,
"height": str(upgrade_height),
"info": f"Upgrade to {upgrade_name}"
}
}],
"metadata": "",
"deposit": "10000000000000000000000adv4tnt",
"title": f"Software Upgrade to {upgrade_name}",
"summary": f"Upgrade the chain to {upgrade_name}"
}Please confirm dydxprotocold query upgrade authority is supported in your chain version. If not, I can fall back to computing the gov module address or a configurable env var.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Showing file if present ==="
if [ -f protocol/scripts/release_qual/submit_upgrade_proposal.py ]; then
sed -n '1,240p' protocol/scripts/release_qual/submit_upgrade_proposal.py || true
else
echo "File not found: protocol/scripts/release_qual/submit_upgrade_proposal.py"
fi
echo
echo "=== Search for run_cmd usage and definition ==="
rg --line-number "run_cmd\(" || true
rg --line-number "def run_cmd" || true
echo
echo "=== Search for dydxprotocold occurrences ==="
rg --line-number "dydxprotocold" || true
echo
echo "=== Search for 'query upgrade authority' usage ==="
rg --line-number "query.*upgrade.*authority" || true
echo
echo "=== Search for literal \"authority\" occurrences in repo ==="
rg --line-number "\"authority\"" || trueLength of output: 44625
Don’t hardcode the upgrade authority — fetch it from the chain
Found a hardcoded authority in protocol/scripts/release_qual/submit_upgrade_proposal.py (the proposal JSON uses "dydx10d07..."). Hardcoding risks proposing with an incorrect/unauthorized address. Use the upgrade module's authority query and fail-fast if it can't be retrieved.
Files to update:
- protocol/scripts/release_qual/submit_upgrade_proposal.py
- protocol/scripts/markets/launch_markets.py
- protocol/scripts/revshare/add_order_router_rev_share.py
- protocol/scripts/governance/create_delisting_proposal.py
Apply this change to resolve the issue (uses existing run_cmd):
- # Create proposal.json
- proposal = {
+ # Resolve upgrade module authority dynamically
+ authority_resp = run_cmd(
+ ["dydxprotocold", "query", "upgrade", "authority", "--output", "json"],
+ node=node
+ )
+ if not authority_resp:
+ print("Error: Could not fetch upgrade module authority from chain.")
+ sys.exit(1)
+ try:
+ authority = json.loads(authority_resp)["authority"]
+ except (json.JSONDecodeError, KeyError) as e:
+ print(f"Error parsing authority from response: {e}")
+ sys.exit(1)
+
+ # Create proposal.json
+ proposal = {
"messages": [{
"@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade",
- "authority": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky",
+ "authority": authority,
"plan": {
"name": upgrade_name,
"height": str(upgrade_height),
"info": f"Upgrade to {upgrade_name}"
}
}],
"metadata": "",
"deposit": "10000000000000000000000adv4tnt",
"title": f"Software Upgrade to {upgrade_name}",
"summary": f"Upgrade the chain to {upgrade_name}"
}Please confirm that dydxprotocold query upgrade authority --output json is supported by the deployed CLI/binary in your environment; if it isn’t, fallback options are: compute the gov/module address programmatically or read the authority from a configurable env var.
📝 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.
| # Create proposal.json | |
| proposal = { | |
| "messages": [{ | |
| "@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade", | |
| "authority": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky", | |
| "plan": { | |
| "name": upgrade_name, | |
| "height": str(upgrade_height), | |
| "info": f"Upgrade to {upgrade_name}" | |
| } | |
| }], | |
| "metadata": "", | |
| "deposit": "10000000000000000000000adv4tnt", | |
| "title": f"Software Upgrade to {upgrade_name}", | |
| "summary": f"Upgrade the chain to {upgrade_name}" | |
| } | |
| # Resolve upgrade module authority dynamically | |
| authority_resp = run_cmd( | |
| ["dydxprotocold", "query", "upgrade", "authority", "--output", "json"], | |
| node=node | |
| ) | |
| if not authority_resp: | |
| print("Error: Could not fetch upgrade module authority from chain.") | |
| sys.exit(1) | |
| try: | |
| authority = json.loads(authority_resp)["authority"] | |
| except (json.JSONDecodeError, KeyError) as e: | |
| print(f"Error parsing authority from response: {e}") | |
| sys.exit(1) | |
| # Create proposal.json | |
| proposal = { | |
| "messages": [{ | |
| "@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade", | |
| "authority": authority, | |
| "plan": { | |
| "name": upgrade_name, | |
| "height": str(upgrade_height), | |
| "info": f"Upgrade to {upgrade_name}" | |
| } | |
| }], | |
| "metadata": "", | |
| "deposit": "10000000000000000000000adv4tnt", | |
| "title": f"Software Upgrade to {upgrade_name}", | |
| "summary": f"Upgrade the chain to {upgrade_name}" | |
| } |
🤖 Prompt for AI Agents
In protocol/scripts/release_qual/submit_upgrade_proposal.py around lines
142-158, the proposal JSON hardcodes the upgrade authority address; instead run
the chain query (e.g. use the existing run_cmd helper to execute `dydxprotocold
query upgrade authority --output json`), parse the returned authority, fail-fast
with an explicit error if the query or parsing fails, and replace the hardcoded
string with the queried authority; apply the same change pattern to
protocol/scripts/markets/launch_markets.py,
protocol/scripts/revshare/add_order_router_rev_share.py, and
protocol/scripts/governance/create_delisting_proposal.py, and if the CLI query
is not available fall back to either a configured env var or computing the
module/gov address programmatically, ensuring all scripts error out early when
no valid authority is obtained.
| cmd = [ | ||
| "dydxprotocold", "tx", "gov", "submit-proposal", "proposal.json", | ||
| "--from", "alice", | ||
| "--chain-id", chain_id, | ||
| "--yes", | ||
| "--broadcast-mode", "sync", | ||
| "--gas", "auto", | ||
| "--fees", "5000000000000000adv4tnt", | ||
| "--keyring-backend", "test" | ||
| ] | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make proposal ID retrieval deterministic (parse tx JSON; block-broadcast)
Relying on “last proposal” can race if multiple proposals exist. Broadcast the tx in “block” mode, request JSON output, and parse the emitted proposal_id. Retain a fallback to the proposals query if parsing fails.
Apply this diff:
# Submit proposal
cmd = [
"dydxprotocold", "tx", "gov", "submit-proposal", "proposal.json",
"--from", "alice",
"--chain-id", chain_id,
- "--yes",
- "--broadcast-mode", "sync",
+ "--yes",
+ "--broadcast-mode", "block",
"--gas", "auto",
"--fees", "5000000000000000adv4tnt",
- "--keyring-backend", "test"
+ "--keyring-backend", "test",
+ "--output", "json",
]
result = run_cmd(cmd, node=node)
if not result:
os.remove("proposal.json")
sys.exit(1)
- # Extract txhash
- for line in result.split('\n'):
- if 'txhash:' in line:
- print(f"Submitted: {line.split('txhash:')[1].strip()}")
-
- time.sleep(5)
-
- # Get proposal ID
- result = run_cmd(["dydxprotocold", "query", "gov", "proposals", "--output", "json"], node=node)
- if not result:
- os.remove("proposal.json")
- sys.exit(1)
-
- proposals = json.loads(result)
- proposal_id = proposals['proposals'][-1]['id']
- print(f"Proposal ID: {proposal_id}")
+ # Extract txhash and proposal_id from tx result
+ proposal_id = None
+ try:
+ tx = json.loads(result)
+ print(f"Submitted: {tx.get('txhash', '')}")
+ # Try to find proposal_id in events
+ for log in tx.get("logs", []):
+ for ev in log.get("events", []):
+ if ev.get("type") in ("submit_proposal", "proposal_submit"):
+ for attr in ev.get("attributes", []):
+ if attr.get("key") in ("proposal_id", "proposalId"):
+ proposal_id = attr.get("value")
+ break
+ if proposal_id:
+ break
+ if proposal_id:
+ break
+ except (json.JSONDecodeError, KeyError) as e:
+ print(f"Warning: could not parse tx json for proposal_id: {e}")
+
+ # Fallback: query proposals if not found
+ if not proposal_id:
+ time.sleep(5)
+ result = run_cmd(
+ ["dydxprotocold", "query", "gov", "proposals", "--output", "json"],
+ node=node
+ )
+ if not result:
+ os.remove("proposal.json")
+ sys.exit(1)
+ proposals = json.loads(result).get("proposals", [])
+ if not proposals:
+ print("Error: no proposals returned from chain")
+ os.remove("proposal.json")
+ sys.exit(1)
+ proposal_id = proposals[-1]["id"]
+
+ print(f"Proposal ID: {proposal_id}")Also applies to: 181-197
🤖 Prompt for AI Agents
In protocol/scripts/release_qual/submit_upgrade_proposal.py around lines 165-175
(and similarly adjust lines 181-197), the script currently relies on querying
the "last proposal" which can race; modify the tx submit command to use
"--broadcast-mode", "block" and add "--output", "json" so the node returns
transaction result JSON, then parse that JSON to extract the emitted
"proposal_id" deterministically from the tx response (events/logs) and use it;
keep the existing proposals query as a fallback if parsing fails, and ensure
errors are handled and logged if neither method yields a proposal_id.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
protocol/scripts/release_qual/submit_upgrade_proposal.py (3)
16-19: Allow safe testnet/staging patterns instead of a single chain-id; keep explicit mainnet blockCurrent allow-list only includes dydxprotocol-testnet, which blocks staging. Switch to a case-insensitive pattern check for "testnet" or "staging" while retaining the explicit mainnet block you already have.
Apply this diff:
-# Allowed chain IDs for testnet/staging only (blocking mainnet) -ALLOWED_CHAIN_IDS = [ - "dydxprotocol-testnet", # Standard testnet/staging chain -] +# Allowed chain-id patterns for testnet/staging only (blocking mainnet) +ALLOWED_CHAIN_ID_PATTERNS = ("testnet", "staging") @@ - # Validate chain_id is allowed - if chain_id not in ALLOWED_CHAIN_IDS: - print(f"Error: Chain ID '{chain_id}' is not in the allowed list.") - print(f"Allowed chains: {', '.join(ALLOWED_CHAIN_IDS)}") - print("This script is restricted to testnet/staging environments only.") - sys.exit(1) + # Validate chain_id matches allowed patterns (still blocked by mainnet check above) + if not any(p in chain_id.lower() for p in ALLOWED_CHAIN_ID_PATTERNS): + print(f"Error: Chain ID '{chain_id}' is not allowed.") + print(f"Allowed chain-id patterns: {', '.join(ALLOWED_CHAIN_ID_PATTERNS)}") + print("This script is restricted to testnet/staging environments only.") + sys.exit(1)Also applies to: 57-63
142-158: Don’t hardcode the upgrade authority — query it from the chain and fail fast if unavailableHardcoding the authority risks submitting an invalid/unauthorized proposal on different environments. Resolve the authority via the upgrade module query before building the proposal JSON.
Apply this diff:
- # Create proposal.json - proposal = { + # Resolve upgrade module authority dynamically + authority_resp = run_cmd( + ["dydxprotocold", "query", "upgrade", "authority", "--output", "json"], + node=node + ) + if not authority_resp: + print("Error: Could not fetch upgrade module authority from chain.") + sys.exit(1) + try: + authority = json.loads(authority_resp)["authority"] + except (json.JSONDecodeError, KeyError) as e: + print(f"Error parsing authority from response: {e}") + sys.exit(1) + + # Create proposal.json + proposal = { "messages": [{ "@type": "/cosmos.upgrade.v1beta1.MsgSoftwareUpgrade", - "authority": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky", + "authority": authority, "plan": { "name": upgrade_name, "height": str(upgrade_height), "info": f"Upgrade to {upgrade_name}" } }], "metadata": "", "deposit": "20000000adv4tnt", "title": f"Software Upgrade to {upgrade_name}", "summary": f"Upgrade the chain to {upgrade_name}" }If your chain/binary doesn’t support this query, let’s fall back to a configurable env var (e.g., DYDX_UPGRADE_AUTHORITY) as a temporary workaround.
165-175: Make proposal ID retrieval deterministic: block broadcast + parse JSON instead of “last proposal”Querying “last proposal” can race when multiple proposals are in-flight. Broadcast in block mode with JSON output and parse the emitted proposal_id from tx logs, with a safe fallback.
Apply this diff:
cmd = [ "dydxprotocold", "tx", "gov", "submit-proposal", "proposal.json", "--from", "alice", "--chain-id", chain_id, "--yes", - "--broadcast-mode", "sync", + "--broadcast-mode", "block", "--gas", "auto", "--fees", "5000000000000000adv4tnt", - "--keyring-backend", "test" + "--keyring-backend", "test", + "--output", "json", ] @@ - # Extract txhash - for line in result.split('\n'): - if 'txhash:' in line: - print(f"Submitted: {line.split('txhash:')[1].strip()}") - - time.sleep(5) - - # Get proposal ID - result = run_cmd(["dydxprotocold", "query", "gov", "proposals", "--output", "json"], node=node) - if not result: - os.remove("proposal.json") - sys.exit(1) - - proposals = json.loads(result) - proposal_id = proposals['proposals'][-1]['id'] - print(f"Proposal ID: {proposal_id}") + # Extract txhash and proposal_id from tx result + proposal_id = None + try: + tx = json.loads(result) + print(f"Submitted: {tx.get('txhash', '')}") + # Optionally surface error code + if tx.get("code", 0) != 0: + print(f"Error: proposal submit failed with code={tx.get('code')} log={tx.get('raw_log')}") + os.remove("proposal.json") + sys.exit(1) + # Try to find proposal_id in events + for log in tx.get("logs", []): + for ev in log.get("events", []): + if ev.get("type") in ("submit_proposal", "proposal_submit"): + for attr in ev.get("attributes", []): + if attr.get("key") in ("proposal_id", "proposalId"): + proposal_id = attr.get("value") + break + if proposal_id: + break + if proposal_id: + break + except (json.JSONDecodeError, KeyError) as e: + print(f"Warning: could not parse tx json for proposal_id: {e}") + + # Fallback: query proposals if not found + if not proposal_id: + time.sleep(5) + result = run_cmd( + ["dydxprotocold", "query", "gov", "proposals", "--output", "json"], + node=node + ) + if not result: + os.remove("proposal.json") + sys.exit(1) + proposals = json.loads(result).get("proposals", []) + if not proposals: + print("Error: no proposals returned from chain") + os.remove("proposal.json") + sys.exit(1) + proposal_id = proposals[-1]["id"] + + print(f"Proposal ID: {proposal_id}")Also applies to: 181-197
🧹 Nitpick comments (4)
protocol/scripts/release_qual/submit_upgrade_proposal.py (4)
69-82: Avoid mutating caller’s cmd list in run_cmd; make --node detection robustMutating the caller-provided list can cause subtle bugs if reused. Copy the list and detect existing --node even when passed as “--node=…”.
Apply this diff:
def run_cmd(cmd, node=None): """Run command and return stdout.""" - # Add node flag if provided - if node and "--node" not in cmd: - cmd.extend(["--node", node]) + # Work on a copy and add node flag if provided (idempotent) + cmd = list(cmd) + if node and not any(arg.startswith("--node") for arg in cmd): + cmd.extend(["--node", node]) try: result = subprocess.run( cmd, capture_output=True, text=True, check=True ) return result.stdout except subprocess.CalledProcessError as e: print(f"Error: {e.stderr}") return None
126-141: Be more robust when deriving current height: add a fallback to “query block” before defaultingStatus parsing can vary; try querying the latest block height if status JSON parsing fails or status isn’t available, before falling back to a magic number.
Apply this diff:
- # Get current block height - result = run_cmd(["dydxprotocold", "status"], node=node) - if result: - try: - status = json.loads(result) - current_height = int(status['sync_info']['latest_block_height']) - upgrade_height = current_height + wait_blocks - print(f"Current height: {current_height}, upgrade at: {upgrade_height}") - except (json.JSONDecodeError, KeyError) as e: - # Fallback if we can't get current height - print(f"Could not parse block height, using default. Error: {e}") - upgrade_height = 1000000 - print(f"Using default upgrade height: {upgrade_height}") - else: - upgrade_height = 1000000 + # Get current block height (with fallback) + result = run_cmd(["dydxprotocold", "status"], node=node) + parsed_height = None + if result: + try: + status = json.loads(result) + parsed_height = int(status["sync_info"]["latest_block_height"]) + except (json.JSONDecodeError, KeyError, ValueError) as e: + print(f"Warning: could not parse height from status: {e}") + if parsed_height is None: + block_res = run_cmd(["dydxprotocold", "query", "block", "--output", "json"], node=node) + if block_res: + try: + block = json.loads(block_res) + parsed_height = int(block["block"]["header"]["height"]) + except (json.JSONDecodeError, KeyError, ValueError) as e: + print(f"Warning: could not parse height from query block: {e}") + if parsed_height is None: + print("Could not determine current height reliably; using default upgrade height.") + upgrade_height = 1000000 + print(f"Using default upgrade height: {upgrade_height}") + else: + current_height = parsed_height + upgrade_height = current_height + wait_blocks + print(f"Current height: {current_height}, upgrade at: {upgrade_height}")
220-240: Replace fixed sleep with status polling up to a timeout; exit early on terminal statesRather than sleeping a fixed number of seconds, poll the proposal status at a short interval up to a max duration. Exit as soon as PASSED/REJECTED/CANCELLED is observed.
Apply this diff:
- # Wait for voting period (same as blocks to upgrade) - print(f"\nWaiting {wait_blocks} seconds (~{wait_blocks} blocks) for voting period...") - for i in range(0, wait_blocks, 30): - remaining = wait_blocks - i - if remaining > 0: - print(f" {remaining}s remaining...") - time.sleep(min(30, remaining)) - - # Check status - cmd = [ - "dydxprotocold", "query", "gov", "proposal", - str(proposal_id), "--output", "json" - ] - result = run_cmd(cmd, node=node) - if result: - data = json.loads(result) - status = data['proposal']['status'] - print(f"\nFinal status: {status}") - if "PASSED" in status: - print(f"✅ Upgrade to {upgrade_name} approved for height {upgrade_height}") + # Poll proposal status until terminal state or timeout + print(f"\nPolling proposal status for up to {wait_blocks}s (~{wait_blocks} blocks)...") + start = time.time() + terminal = ("PASSED", "REJECTED", "FAILED", "CANCELLED") + last_status = None + while time.time() - start < wait_blocks: + cmd = ["dydxprotocold", "query", "gov", "proposal", str(proposal_id), "--output", "json"] + result = run_cmd(cmd, node=node) + if result: + try: + data = json.loads(result) + status = data["proposal"]["status"] + if status != last_status: + print(f" Status: {status}") + last_status = status + if any(t in status for t in terminal): + print(f"\nFinal status: {status}") + if "PASSED" in status: + print(f"✅ Upgrade to {upgrade_name} approved for height {upgrade_height}") + break + except (json.JSONDecodeError, KeyError): + pass + time.sleep(10) + else: + # Timeout + print("\nTimed out waiting for final status. Please check the proposal manually.")
94-99: Nit: Align --node help example with the default value to reduce confusionDefault uses HTTPS on 443; the help example shows HTTP on 26657. Aligning these avoids mixed signals.
Apply this diff:
parser.add_argument( '--node', - help='Node RPC endpoint (e.g., http://validator.v4staging.dydx.exchange:26657)' + help='Node RPC endpoint (e.g., https://validator.v4staging.dydx.exchange:443)' )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
protocol/scripts/release_qual/submit_upgrade_proposal.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test-sim-multi-seed-short
- GitHub Check: test-sim-import-export
- GitHub Check: container-tests
- GitHub Check: test-coverage-upload
- GitHub Check: liveness-test
- GitHub Check: test-race
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: benchmark
- GitHub Check: Summary
🔇 Additional comments (2)
protocol/scripts/release_qual/submit_upgrade_proposal.py (2)
154-154: Confirm deposit denom/amount against chain params (min_deposit) and fee policyThe deposit and fee amounts/denom are hardcoded. Ensure they satisfy min_deposit and any staging fee requirements to avoid proposal rejection or non-determinism across environments.
Please verify on your target chain:
- dydxprotocold query gov params --output json
- Confirm that deposit >= params.deposit_params.min_deposit and denom matches.
- Confirm fee denom is accepted and gas/fee settings are sane for your test nodes.
If you want, I can refactor the script to read these from environment variables or query chain params and compute a compliant deposit automatically.
13-15: LGTM on validator list for staging/test environmentsThe validator aliases align with the documented test keyring and look correct for auto-voting in staging.
| # Vote | ||
| print(f"Voting with {len(VALIDATORS)} validators...") | ||
| for voter in VALIDATORS: | ||
| cmd = [ | ||
| "dydxprotocold", "tx", "gov", "vote", str(proposal_id), "yes", | ||
| "--from", voter, | ||
| "--chain-id", chain_id, | ||
| "--yes", | ||
| "--gas", "auto", | ||
| "--fees", "5000000000000000adv4tnt", | ||
| "--keyring-backend", "test" | ||
| ] | ||
| result = run_cmd(cmd, node=node) | ||
| if result and 'txhash:' in result: | ||
| print(f" {voter}: ✓") | ||
| else: | ||
| print(f" {voter}: ✗") | ||
| time.sleep(1) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Vote reliably: use block broadcast + JSON and check tx code instead of scanning for ‘txhash:’
String scanning is brittle. Broadcast in block mode, request JSON, and verify code==0. This yields deterministic success/failure per vote.
Apply this diff:
for voter in VALIDATORS:
cmd = [
"dydxprotocold", "tx", "gov", "vote", str(proposal_id), "yes",
"--from", voter,
"--chain-id", chain_id,
"--yes",
+ "--broadcast-mode", "block",
"--gas", "auto",
"--fees", "5000000000000000adv4tnt",
- "--keyring-backend", "test"
+ "--keyring-backend", "test",
+ "--output", "json",
]
result = run_cmd(cmd, node=node)
- if result and 'txhash:' in result:
- print(f" {voter}: ✓")
- else:
- print(f" {voter}: ✗")
+ ok = False
+ if result:
+ try:
+ vote_tx = json.loads(result)
+ ok = vote_tx.get("code", 0) == 0
+ except json.JSONDecodeError:
+ ok = 'txhash' in result # last-resort fallback
+ print(f" {voter}: {'✓' if ok else '✗'}")
time.sleep(1)📝 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.
| # Vote | |
| print(f"Voting with {len(VALIDATORS)} validators...") | |
| for voter in VALIDATORS: | |
| cmd = [ | |
| "dydxprotocold", "tx", "gov", "vote", str(proposal_id), "yes", | |
| "--from", voter, | |
| "--chain-id", chain_id, | |
| "--yes", | |
| "--gas", "auto", | |
| "--fees", "5000000000000000adv4tnt", | |
| "--keyring-backend", "test" | |
| ] | |
| result = run_cmd(cmd, node=node) | |
| if result and 'txhash:' in result: | |
| print(f" {voter}: ✓") | |
| else: | |
| print(f" {voter}: ✗") | |
| time.sleep(1) | |
| # Vote | |
| print(f"Voting with {len(VALIDATORS)} validators...") | |
| for voter in VALIDATORS: | |
| cmd = [ | |
| "dydxprotocold", "tx", "gov", "vote", str(proposal_id), "yes", | |
| "--from", voter, | |
| "--chain-id", chain_id, | |
| "--yes", | |
| "--broadcast-mode", "block", | |
| "--gas", "auto", | |
| "--fees", "5000000000000000adv4tnt", | |
| "--keyring-backend", "test", | |
| "--output", "json", | |
| ] | |
| result = run_cmd(cmd, node=node) | |
| ok = False | |
| if result: | |
| try: | |
| vote_tx = json.loads(result) | |
| ok = vote_tx.get("code", 0) == 0 | |
| except json.JSONDecodeError: | |
| ok = 'txhash' in result # last-resort fallback | |
| print(f" {voter}: {'✓' if ok else '✗'}") | |
| time.sleep(1) |
🤖 Prompt for AI Agents
In protocol/scripts/release_qual/submit_upgrade_proposal.py around lines 198 to
216, the vote loop currently scans the command output for the string "txhash:"
which is brittle; change the vote invocation to broadcast in block mode and
return JSON (add "--broadcast-mode", "block", "--output", "json"), then parse
the JSON response and check the tx response code (tx_response.code == 0) to
determine success or failure, printing a success mark when code == 0 and a
failure mark otherwise; ensure you handle and log JSON parse errors and non-zero
codes as failures and keep the 1s sleep between votes.
|
this doesn't have a protocol change, only a release qual script |
|
@Mergifyio backport release/protocol/v9.x |
✅ Backports have been created
|
(cherry picked from commit fb16503)
Changelist
Test Plan
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
New Features
Documentation