Skip to content

feat(contract-manager): multi-contract support + safety in sync_governance_vaas#3663

Open
aditya520 wants to merge 1 commit into
mainfrom
feat/sync-governance-vaas-multi-contract
Open

feat(contract-manager): multi-contract support + safety in sync_governance_vaas#3663
aditya520 wants to merge 1 commit into
mainfrom
feat/sync-governance-vaas-multi-contract

Conversation

@aditya520
Copy link
Copy Markdown
Member

@aditya520 aditya520 commented May 5, 2026

Summary

Adds the ability to sync many contracts in one invocation and a few safety improvements that came out of running the q2 fee proposals across 56 pricefeed + 15 entropy chains.

New CLI options

Flag Purpose
--contract <id> repeatable; replaces the previous single-contract form (still works for one)
--contracts-file <path> newline-separated file of ids (supports # comments)
--all iterate every mainnet pricefeed/executor contract in DefaultStore
--gas-price-gwei <n> override gasPrice; useful on chains with fast-moving base fees (e.g. Arbitrum)
--dry-run decode + print without submitting

Bug fixes / behaviour changes

  • Fall back to DefaultStore.executor_contracts so executor contract ids resolve. Previously threw Contract <id> not found for entropy/executor proposals.
  • Coerce lastExecutedGovernanceSequence to Number; EvmExecutorContract returns a string, so lastExecuted + 1 was string-concatenating ("647" + 1 = "6471").
  • Use contract.chain.wormholeChainName instead of contract.getChain().wormholeChainName; EvmExecutorContract has no getChain().
  • Wrap executeGovernanceInstruction in try/catch so a single bad VAA (e.g. signed by an expired guardian set) doesn't kill the whole loop.
  • Log a friendly reached end of VAA queue instead of dumping a stack trace when fetchVaa fails at end-of-queue.
  • Skip non-EVM contracts gracefully when iterating with --all.

Test plan

  • pnpm test:lint clean
  • pnpm build (contract_manager) clean
  • pnpm tsc --noEmit clean for the modified file
  • Smoke test: --dry-run against a known-current contract walks the queue and exits cleanly with no transactions sent
  • Reviewer should run --dry-run --all against a funded key on a low-stakes branch to confirm --all iteration works as expected

🤖 Generated with Claude Code


Open in Devin Review

…nance_vaas

Adds the ability to sync many contracts in one invocation and a few safety
improvements to the script that came out of running the q2 fee proposals.

New CLI options:
  --contract <id>       repeatable; replaces the previous single-contract form
  --contracts-file <p>  newline-separated file of ids (supports # comments)
  --all                 iterate every mainnet pricefeed/executor contract
  --gas-price-gwei <n>  override gasPrice; useful on chains with fast-moving
                        base fees (e.g. Arbitrum)
  --dry-run             decode and print without submitting transactions

Bug fixes / behaviour:
- fall back to DefaultStore.executor_contracts so executor contract ids
  resolve (previously threw "Contract not found")
- coerce lastExecutedGovernanceSequence to Number; EvmExecutorContract
  returns a string so "lastExecuted + 1" was string-concatenating
- use contract.chain.wormholeChainName instead of contract.getChain().*;
  EvmExecutorContract has no getChain()
- wrap executeGovernanceInstruction in try/catch so a single bad VAA
  (e.g. signed by an expired guardian set) doesn't kill the whole loop
- log a friendly "reached end of VAA queue" instead of dumping a stack
  trace when fetchVaa fails at end-of-queue
- skip non-EVM contracts gracefully when iterating with --all

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented May 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-reference Ready Ready Preview, Comment May 5, 2026 7:24pm
component-library Ready Ready Preview, Comment May 5, 2026 7:24pm
developer-hub Ready Ready Preview, Comment May 5, 2026 7:24pm
4 Skipped Deployments
Project Deployment Actions Updated (UTC)
entropy-explorer Skipped Skipped May 5, 2026 7:24pm
insights Skipped Skipped May 5, 2026 7:24pm
proposals Skipped Skipped May 5, 2026 7:24pm
staking Skipped Skipped May 5, 2026 7:24pm

Request Review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03c554812f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

} != ${action.targetChainId}, skipping`,
);
}
lastExecuted++;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop advancing sequence after failed execution

When executeGovernanceInstruction throws, the script logs and continues, but lastExecuted is still incremented unconditionally. This means a transient failure on sequence N causes the loop to fetch N+1; if N+1 succeeds, the on-chain lastExecutedGovernanceSequence jumps past N (see verifyGovernanceVM in target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol, which accepts any vm.sequence > lastExecuted and then sets it), making N permanently unexecutable as an old message. This introduces a durable desync risk from a single temporary RPC/gas failure.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 No test coverage for new CLI features

This PR adds several new features (--all, --contracts-file, --dry-run, --gas-price-gwei, multi-contract support, error-and-continue behavior) but does not include any tests. Per REVIEW.md, new functionality should have tests. While this is a CLI script that is harder to unit test, the core logic (contract filtering, gas price override, VAA processing loop) could be extracted into testable functions. At minimum, the --dry-run mode could serve as a basis for integration testing.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant