feat(contract_manager): migrate tooling for MigrateGovernanceAndWormhole - #3939
Conversation
Add SetWormholeAddressAndDataSources payload helper, migrate script, and proposal verification for upgrading legacy proxies onto pro-compatible wormhole + data sources without changing the consumer address.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const matchesProduction = | ||
| JSON.stringify(action.dataSources) === | ||
| JSON.stringify(proProductionSources); | ||
| const matchesStaging = | ||
| JSON.stringify(action.dataSources) === | ||
| JSON.stringify(proStagingSources); |
There was a problem hiding this comment.
🟡 Migration proposal check always warns that data sources are wrong
The proposal's data sources are compared to the expected pro data sources by turning both into text and matching character-for-character (JSON.stringify(action.dataSources) === JSON.stringify(proProductionSources) at contract_manager/scripts/check_proposal.ts:158-163), so a correct proposal is still flagged as not matching.
Impact: Operators verifying a mainnet migration proposal will always see "data sources do not match" even when the proposal is correct, defeating the check.
Key-order mismatch between decoded payload and expected config
The decoded action's data sources are produced by the DataSourceLayout struct which lists fields in the order emitterChain then emitterAddress (see governance/xc_admin/packages/xc_admin_common/src/governance_payload/SetDataSources.ts:13-17), so each object serializes as {"emitterChain":26,"emitterAddress":"..."}.
The expected sources from getDefaultDeploymentConfig declare the fields in the opposite order — emitterAddress first, then emitterChain (contract_manager/src/core/base.ts:154-169, 246-253) — so they serialize as {"emitterAddress":"...","emitterChain":26}.
JSON.stringify preserves insertion order, so the two strings never match even when the data is identical, making both matchesProduction and matchesStaging always false and always taking the WARNING branch at check_proposal.ts:168-175.
| const matchesProduction = | |
| JSON.stringify(action.dataSources) === | |
| JSON.stringify(proProductionSources); | |
| const matchesStaging = | |
| JSON.stringify(action.dataSources) === | |
| JSON.stringify(proStagingSources); | |
| const normalizeSources = ( | |
| sources: { emitterChain: number; emitterAddress: string }[], | |
| ) => | |
| JSON.stringify( | |
| [...sources] | |
| .map((s) => ({ | |
| emitterChain: s.emitterChain, | |
| emitterAddress: s.emitterAddress | |
| .replace(/^0x/i, "") | |
| .toLowerCase(), | |
| })) | |
| .sort((a, b) => | |
| `${a.emitterChain}:${a.emitterAddress}`.localeCompare( | |
| `${b.emitterChain}:${b.emitterAddress}`, | |
| ), | |
| ), | |
| ); | |
| const matchesProduction = | |
| normalizeSources(action.dataSources) === | |
| normalizeSources(proProductionSources); | |
| const matchesStaging = | |
| normalizeSources(action.dataSources) === | |
| normalizeSources(proStagingSources); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| payloads.push( | ||
| chain.generateGovernanceSetWormholeAddressAndDataSourcesPayload( | ||
| proWormhole.address.replace("0x", ""), | ||
| proDataSources, | ||
| 0n, | ||
| 0n, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
🔍 Migrate script depends on codec not present in this branch
migrate_evm_pricefeed_to_pro.ts and check_proposal.ts both import SetWormholeAddressAndDataSources from @pythnetwork/xc-admin-common, but that codec does not exist in this branch (confirmed no matches under governance/). The PR description states it is stacked on the xc_admin codec PR. Until that merges, this code will not build/typecheck. Worth confirming the retarget/merge order noted in the description before merging.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (action.newFeeValue !== 0n || action.newFeeExpo !== 0n) { | ||
| console.log( | ||
| ` WARNING: expected fee 0/0 for pro migrate, got ${action.newFeeValue}/${action.newFeeExpo}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔍 Fee comparison assumes bigint decode type
check_proposal.ts:148 compares action.newFeeValue !== 0n / action.newFeeExpo !== 0n (bigint literals). If the not-yet-present codec decodes these fee fields as JS number rather than bigint, these strict comparisons would always be true (number !== bigint), producing a spurious fee warning. Confirm the codec decodes fee value/expo as bigint to match the u64be usage assumed here and the 0n/0n args passed in the migrate script (migrate_evm_pricefeed_to_pro.ts:178-179).
Was this helpful? React with 👍 or 👎 to provide feedback.
| const isMainnet = selectedChains[0]?.isMainnet() ?? false; | ||
| const vault = | ||
| DefaultStore.vaults[isMainnet ? MAINNET_VAULT_ID : DEVNET_VAULT_ID]; |
There was a problem hiding this comment.
🔍 Testnet migration routes proposal through devnet vault
Unlike upgrade_evm_pricefeed_contracts.ts which always uses the mainnet-beta vault, migrate_evm_pricefeed_to_pro.ts:106-108 selects DEVNET_VAULT_ID when the selected chains are testnet. Worth confirming that testnet EVM governance is actually proposed through the devnet Squads vault (devnet_6baWtW1zTUVMSJHJQVxDUXWzqrQeYBr6mu31j3bTKwY3) and that this key exists in DefaultStore.vaults; if the vault key is wrong, vault is undefined and the non-dry-run path silently does nothing (optional chaining) rather than erroring.
Was this helpful? React with 👍 or 👎 to provide feedback.
Keep the migrate tooling PR scoped to code changes only.
Assumes fee was already set to 0 via SetFee before migrate.
…ceAndWormhole Expand action 10 payloads with explicit governance emitter and index CLI args.
keyvankhademi
changed the title
Summary
migrate_evm_pricefeed_to_pro.tsto deploy/reuse pro wormhole + new Pyth impl, then propose[UpgradeContract, MigrateGovernanceAndWormhole]for legacy proxies.EvmChain.generateGovernanceMigrateGovernanceAndWormholePayload(...)wraps the xc_admin codec (wormhole, data sources, new governance emitter, index).--governance-emitter-chain,--governance-emitter-address,--governance-data-source-index(does not silently reuse legacy governance).check_proposal.tsverifies migrate targets including governance fields.0via existingSetFeebefore migrate.Depends on sibling PRs:
MigrateGovernanceAndWormhole)Test plan
check_proposalagainst a sample proposal with action 10