feat: migrate governance scripts from deployment-scripts branch#853
Open
evgeniko wants to merge 24 commits into
Open
feat: migrate governance scripts from deployment-scripts branch#853evgeniko wants to merge 24 commits into
evgeniko wants to merge 24 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end deployment documentation and new EVM + Solana TypeScript tooling, scripts, and configs to deploy, configure, sign, post, and verify governance VAAs and NTT components across EVM and Solana networks. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant EvmEnv as evm/ts-scripts/env.ts
participant CreateMsg as createGovernanceMessage.ts
participant Guardian as Guardian (secP256k1 key)
participant EvmChain as EVM Node / Contract Registry
Dev->>CreateMsg: run script
CreateMsg->>EvmEnv: init(), loadOperatingChains(), loadContracts()
loop per chain
CreateMsg->>EvmEnv: getContractAddress(chainId)
CreateMsg->>CreateMsg: build governance payload (solidityPack)
CreateMsg->>Guardian: sign payload (ECDSA)
CreateMsg->>CreateMsg: assemble VAA (hashes + sig)
CreateMsg->>EvmChain: output/store VAA for chain
end
Dev->>EvmChain: later run executeGovernanceVaa.ts to call performGovernance(vaa)
sequenceDiagram
participant Dev as Developer
participant SolEnv as solana/ts/scripts/env.ts
participant Ledger as Ledger Signer
participant Wormhole as Wormhole Program
participant NTTGov as Governance Program
participant Solana as Solana RPC
Dev->>SolEnv: run executeGovernanceVaa.ts
SolEnv->>SolEnv: loadGovernanceVaa(), getProgramAddresses()
SolEnv->>Ledger: getSigner()
SolEnv->>Wormhole: postVaaSolana(vaa, signCallback)
Ledger->>Wormhole: sign transaction (addLedgerSignature)
Wormhole->>SolEnv: confirmation of post
SolEnv->>Solana: parseVaa()
SolEnv->>NTTGov: createGovernanceVaaInstruction(parsedVaa)
SolEnv->>Ledger: ledgerSignAndSend([instruction])
Ledger->>Solana: send signed tx
Solana->>Dev: return confirmed tx signature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
evm/ts-scripts/config/mainnet/chains.json (1)
52-57:⚠️ Potential issue | 🔴 CriticalDo not commit localhost RPC URL in mainnet configuration.
The MegaETH RPC is set to
http://localhost:50444in a mainnet config file. This will cause script failures for anyone running governance scripts against MegaETH mainnet, asgetProvider()(see context snippet 1) uses this URL directly. This appears to be a local development artifact.🐛 Proposed fix to restore mainnet RPC
{ "description": "MegaETH", "evmNetworkId": 4326, "chainId": 64, - "rpc": "http://localhost:50444" + "rpc": "https://mainnet.megaeth.com/rpc" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/config/mainnet/chains.json` around lines 52 - 57, The mainnet config for "MegaETH" currently contains a localhost RPC URL which must be removed; update the "rpc" value for the MegaETH entry in chains.json (the object with "description": "MegaETH") to a production/public RPC endpoint or to an empty/placeholder value so getProvider() does not use a localhost URL for mainnet; ensure getProvider() (the provider resolution path) will instead read the correct env/config value (e.g., an env var like PROVIDER_URL) or a valid public RPC string rather than the local dev URL.evm/ts-scripts/config/mainnet/contracts.json (1)
1-192:⚠️ Potential issue | 🟡 MinorMissing
TrimmedAmountLibskey from JSON schema.The
ContractsJsontype inevm/ts-scripts/src/env.ts(lines 17-32) definesTrimmedAmountLibsas a required property, but this key is missing from the JSON file. While current scripts may not query this key, adding it would ensure type consistency and prevent potential runtime errors if accessed.💡 Proposed fix - add empty array before closing brace
{ "address": "0x574B7864119C9223A9870Ea614dC91A8EE09E512", "chainId": 64 } - ] + ], + "TrimmedAmountLibs": [] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/config/mainnet/contracts.json` around lines 1 - 192, The JSON file is missing the required TrimmedAmountLibs key declared on the ContractsJson type in evm/ts-scripts/src/env.ts; add a "TrimmedAmountLibs": [] entry (an empty array) near the other top-level arrays (e.g., alongside WormholeCoreContracts, WormholeRelayers, GeneralPurposeGovernances) before the final closing brace so the JSON conforms to the ContractsJson schema and satisfies any runtime/type expectations.
🧹 Nitpick comments (15)
solana/ts/scripts/executeGovernanceVaa_short.ts (1)
28-34: Hardcoded signer file path should use environment variable.The path
"YOUR_SIGNER_FILE"is a placeholder. For operational use, this should read from an environment variable to avoid hardcoding sensitive paths.♻️ Proposed fix
+const signerFilePath = process.env.SIGNER_FILE_PATH; +if (!signerFilePath) { + throw new Error("SIGNER_FILE_PATH environment variable is required"); +} + const payerSecretKey = Uint8Array.from( JSON.parse( - fs.readFileSync(`YOUR_SIGNER_FILE`, { + fs.readFileSync(signerFilePath, { encoding: "utf-8", }) ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/executeGovernanceVaa_short.ts` around lines 28 - 34, The code currently reads a hardcoded signer path ("YOUR_SIGNER_FILE") when building payerSecretKey; change it to read the path from an environment variable (e.g., process.env.SIGNER_FILE or process.env.PAYER_SIGNER) and fail fast if the variable is missing. Update the fs.readFileSync call used to construct payerSecretKey (and any related constant) to read the env var instead of the literal string, and add a clear error/throw if the env var is not set so the script does not attempt to parse a missing file.evm/ts-scripts/shell/verify-governance-contract.sh (1)
55-59: Quote the command substitution to prevent word splitting.The
cast abi-encodeoutput should be quoted to prevent potential word splitting issues if the output contains spaces or special characters.🔧 Proposed fix
forge verify-contract --chain "$evm_chain_id" \ --etherscan-api-key "$etherscan_api_key" \ "$governance_address" \ src/wormhole/Governance.sol:Governance --watch \ - --constructor-args $(cast abi-encode "constructor(address)" "$core_wormhole_address") + --constructor-args "$(cast abi-encode "constructor(address)" "$core_wormhole_address")" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/shell/verify-governance-contract.sh` around lines 55 - 59, The shell command is vulnerable to word-splitting because the command substitution for constructor args is unquoted; update the forge verify-contract invocation so the output of cast abi-encode is quoted (i.e., wrap $(cast abi-encode "constructor(address)" "$core_wormhole_address") in quotes) when passed to --constructor-args to ensure the encoded constructor argument is treated as a single token (references: the --constructor-args argument and the cast abi-encode invocation using core_wormhole_address).evm/ts-scripts/src/executeGovernanceVaa.ts (1)
55-76: Unusedloghelper function.The
logfunction is defined on line 58 but never used. The subsequent logging statements useconsole.logdirectly instead.♻️ Either use the log helper or remove it
Option 1: Use the log helper for consistent prefixed logging:
const log = (...args: any[]) => console.log(`[${chain.chainId}]`, ...args); const { vaa } = await getChainConfig<GovernanceConfig>("governance-vaas", chain.chainId); const governanceContract = await getGovernanceContract(chain); const vaaHex = Buffer.from(vaa, "base64").toString("hex"); - log("Executing governance with VAA: ", vaaHex); + log("Executing governance with VAA:", vaaHex); const tx = await governanceContract.performGovernance( `0x${vaaHex}`, {} // overrides ); - log(`Submitted governance transaction: ${tx.hash}`); + log("Submitted governance transaction:", tx.hash); await tx.wait(); - log("success."); + log("Success.");Option 2: Remove the unused helper:
async function executeGovernance( chain: ChainInfo, ) { - const log = (...args: any[]) => console.log(`[${chain.chainId}]`, ...args); const { vaa } = await getChainConfig<GovernanceConfig>("governance-vaas", chain.chainId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/src/executeGovernanceVaa.ts` around lines 55 - 76, The file defines a local helper log(...) inside executeGovernance but all logging uses console.log directly; either remove the unused log function or replace the console.log calls with the helper for consistent prefixed output: locate the executeGovernance function and either delete the const log = (...) declaration, or change the three console.log/... uses (the "Executing governance with VAA:", "Submitted governance transaction:", and "success.") to call log(...) so logs include the `[${chain.chainId}]` prefix before interacting with governanceContract.performGovernance and awaiting tx.wait().solana/ts/scripts/executeGovernanceVaas.ts (1)
69-75: Consider transaction size limits when batching multiple VAAs.All governance instructions are sent in a single transaction. Solana transactions have size limits (~1232 bytes) and account limits (64 unique accounts). If the VAA file contains many entries, this could cause transaction failures. Consider batching instructions into multiple transactions if needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/executeGovernanceVaas.ts` around lines 69 - 75, The current code sends all governance instructions in one transaction which can exceed Solana transaction size and account limits; update the logic that calls ledgerSignAndSend with the full instructions array to instead chunk the instructions into safe-sized batches (e.g., respect ~1200 byte transaction payload and <=64 unique accounts per tx) and send each batch sequentially: create a helper to split the instructions array by estimated serialized size and distinct account count, call ledgerSignAndSend for each batch, await connection.confirmTransaction for each returned tx.signature before proceeding to the next batch, and log each batch's signature (references: the instructions variable, ledgerSignAndSend, connection.confirmTransaction, tx.signature).deployments.md (3)
14-14: Minor: Use hyphen in compound adjective.Per static analysis: "High level" should be "High-level" when used as an adjective.
-### High level description of the process: +### High-level description of the process:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments.md` at line 14, Change the heading text "High level description of the process:" to use the compound adjective form "High-level description of the process:"; locate the heading string "High level description of the process" and replace it with "High-level description of the process" so the static analysis warning is resolved.
143-143: Typo: "ETTHERSCAN" should be "ETHERSCAN".- "etherscan": "YOUR-ETTHERSCAN-API_KEY" + "etherscan": "YOUR-ETHERSCAN-API-KEY"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments.md` at line 143, Fix the typo in the example API key value for the "etherscan" entry: replace the placeholder "YOUR-ETTHERSCAN-API_KEY" with the correctly spelled "YOUR-ETHERSCAN-API_KEY" so the key name "etherscan" maps to the proper ETHERSCAN placeholder.
44-48: Add language specifiers to fenced code blocks.Several code blocks are missing language identifiers, which affects syntax highlighting and linting. For example, lines 44, 95, 99, 104, and 148 should specify
shellorbash.♻️ Example fix for line 44
-``` +```shell export FOUNDRY_PROFILE=prodAlso applies to: 95-96, 99-100, 104-105, 148-149
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments.md` around lines 44 - 48, The fenced code blocks containing environment variable exports (e.g., lines with FOUNDRY_PROFILE, WALLET_KEY, LEDGER_BIP32_PATH, ENV) are missing language specifiers; update each affected fenced block to start with a language tag like ```shell or ```bash for proper syntax highlighting and linting (apply this to the blocks around the exports at the mentioned locations such as the snippet containing "export FOUNDRY_PROFILE=prod" and the other blocks at lines 95-96, 99-100, 104-105, 148-149).evm/ts-scripts/src/createGovernanceMessage.ts (1)
1-4: Consider using ES import forellipticfor consistency.Line 2 uses CommonJS
requirewhile the rest of the file uses ES imports. This is a minor inconsistency.♻️ Suggested change
import { ethers } from "ethers"; -const elliptic = require("elliptic"); +import elliptic from "elliptic"; import { inspect } from "util";You may need to add
@types/ellipticas a dev dependency for proper typing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/src/createGovernanceMessage.ts` around lines 1 - 4, The file uses CommonJS require for elliptic; replace "const elliptic = require('elliptic')" with an ES import (e.g., import elliptic from 'elliptic') to match the other imports in createGovernanceMessage.ts and update any usages (like new elliptic.ec(...)) accordingly; also add `@types/elliptic` as a devDependency if TypeScript type errors appear so functions/types from the elliptic package are properly typed.solana/ts/scripts/env_short.ts (1)
42-51: The!configcheck doesn't catch JSON parse errors.
JSON.parsethrows on invalid JSON, so theif (!config)check only catches the edge case where the file literally containsnull. Consider wrapping in try-catch for clearer error messages.♻️ Suggested improvement
export function loadScriptConfig(filename: string): any { - const configFile = fs.readFileSync( - `./ts/scripts/config/${env}/${filename}.json` - ); - const config = JSON.parse(configFile.toString()); - if (!config) { - throw Error("Failed to pull config file!"); + const path = `./ts/scripts/config/${env}/${filename}.json`; + try { + const configFile = fs.readFileSync(path); + const config = JSON.parse(configFile.toString()); + if (!config) { + throw new Error("Config file parsed to null/undefined"); + } + return config; + } catch (err) { + throw new Error(`Failed to load config from ${path}: ${err}`); } - return config; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/env_short.ts` around lines 42 - 51, The loadScriptConfig function should catch JSON parsing and file read errors instead of relying on the unreachable if (!config) check: wrap the fs.readFileSync and JSON.parse calls for loadScriptConfig in a try-catch, include the filename and env values in the thrown/logged error message, and rethrow or throw a new Error with the original error attached so callers get a clear message when reading or parsing `./ts/scripts/config/${env}/${filename}.json` fails.solana/ts/sdk/governance.ts (1)
73-74: ExtractedgovernanceProgramIdis unused.
verifyGovernanceHeaderreturns[governanceProgramId, ixData], butgovernanceProgramIdis never used. Consider either validating it againstthis.program.programIdor removing the extraction.♻️ Optional validation
const [governanceProgramId, ixData] = verifyGovernanceHeader(args.vaa.payload); + if (!governanceProgramId.equals(this.program.programId)) { + throw new Error("VAA governance program ID does not match configured program"); + } const ix = deserializeInstruction(ixData);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/sdk/governance.ts` around lines 73 - 74, verifyGovernanceHeader returns [governanceProgramId, ixData] but governanceProgramId is never used; either remove the unused destructuring or validate it—update the code around the verifyGovernanceHeader call to compare the returned governanceProgramId with this.program.programId and throw/log an error if they differ, otherwise drop the governanceProgramId variable and only use ixData for deserializeInstruction(ixData); reference verifyGovernanceHeader, governanceProgramId, this.program.programId, ixData, and deserializeInstruction when making the change.evm/ts-scripts/src/env.ts (2)
75-85: Functions markedasyncwithout async operations.
getChainConfigandgetAllContractsare markedasyncbut only call synchronous functions. While harmless, removing unnecessaryasync/awaitimproves clarity.♻️ Example for getAllContracts
-export async function getAllContracts(contractName: ContractTypes) { - return (await loadContracts())[contractName]; +export function getAllContracts(contractName: ContractTypes) { + return loadContracts()[contractName]; }Similar changes would be needed for callers expecting promises.
Also applies to: 220-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/src/env.ts` around lines 75 - 85, The functions getChainConfig and getAllContracts are declared async but perform only synchronous work (they call loadScriptConfig synchronously), so remove the unnecessary async to improve clarity: change their signatures to non-async (drop the async keyword) and return the value directly (no awaiting), and then update any callers that currently rely on them returning Promises to handle the now-synchronous return (or wrap them in Promise.resolve(...) at call sites if needed). Ensure you update function declarations for getChainConfig and getAllContracts and run TypeScript checks to fix any callsites expecting a Promise.
199-217: Unnecessary recursive call inloadContracts.Line 216 calls
return loadContracts()recursively after settingcontracts. Sincecontractsis already assigned, this can simplyreturn contracts.♻️ Suggested fix
// NOTE: We assume that the contracts.json file is correctly formed... contracts = JSON.parse(contractsFile.toString()) as ContractsJson; - return loadContracts(); + return contracts; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/src/env.ts` around lines 199 - 217, The function loadContracts currently assigns the parsed JSON to the module-level variable contracts and then calls return loadContracts() recursively; change this so that after parsing and assigning contracts (ContractsJson) the function returns contracts directly instead of recursing. Update the return at the end of loadContracts to return contracts to avoid the unnecessary infinite/extra call and ensure the function short-circuits correctly when contracts is already set.solana/ts/scripts/executeGovernanceVaa.ts (2)
22-24: Type assertionas anybypasses type safety.The
governanceProgramId as anycast suppresses type checking. Consider validating thatgovernanceProgramIdmatches one of the expectedGovProgramIdvalues at runtime, or adjusting the type to accept arbitrary program IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/executeGovernanceVaa.ts` around lines 22 - 24, The code currently uses a type-assertion ("governanceProgramId as any") when constructing NTTGovernance which bypasses type safety; replace this cast by validating or converting governanceProgramId before passing it to the NTTGovernance constructor: add a runtime check that governanceProgramId matches an allowed GovProgramId (or map/parse it to the expected enum/type), throw or return a clear error if invalid, or update the variable's type so it is already the expected GovProgramId; reference the symbols governanceProgramId and NTTGovernance (and the GovProgramId enum/type) to locate and fix the code path that supplies the programId argument.
55-55: Log message is misleading for a generic governance VAA execution script.The message "claiming ownership of NTT Program" is specific to one type of governance action, but this script executes any governance VAA. Consider a more generic message.
♻️ Suggested change
- console.log(`Account ${signerPk.toBase58()} is claiming ownership of NTT Program ${nttProgramId}.`); + console.log(`Account ${signerPk.toBase58()} is executing governance VAA on program ${governanceProgramId}.`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/executeGovernanceVaa.ts` at line 55, The log message is overly specific; replace the console.log that references signerPk and nttProgramId so it reports a generic governance VAA execution (e.g., "Signer <signerPk> executing governance VAA for program <nttProgramId>"). Use signerPk and nttProgramId to build the message and, if available, append VAA identifiers/summary from variables like vaa or governanceVaa to make the log generic and informative.solana/ts/scripts/env.ts (1)
5-11: Module-level environment checks may block scripts unnecessarily.The
LEDGER_DERIVATION_PATHandENVchecks run immediately on import. For scripts that might only need to read configs (likereadManagerConfiguration.ts), this forces setting Ledger environment variables even when not using hardware signing.Consider lazy validation or moving checks into
getSigner()for the Ledger path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/env.ts` around lines 5 - 11, The module-level throws for LEDGER_DERIVATION_PATH and ENV cause imports to fail; move these checks into lazy validation (e.g., inside the getSigner() function) or guard them so only code paths that require a Ledger or specific ENV fail; specifically remove the top-level throws for LEDGER_DERIVATION_PATH and ENV and add checks inside getSigner() to validate LEDGER_DERIVATION_PATH when hardware signing is requested, and validate ENV at the point of configuration loading (e.g., within readManagerConfiguration or its caller), ensuring error messages reference LEDGER_DERIVATION_PATH, ENV, and getSigner so failures remain discoverable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evm/ts-scripts/.env.sample`:
- Around line 1-3: Reorder the environment variables in .env.sample to satisfy
dotenv-linter and add a trailing newline: sort the keys alphabetically (replace
the current order of WALLET_KEY, LEDGER_BIP32_PATH, GUARDIAN_KEY with
GUARDIAN_KEY, LEDGER_BIP32_PATH, WALLET_KEY) and ensure the file ends with a
single newline character; no other content changes required.
In `@evm/ts-scripts/.gitignore`:
- Around line 1-4: Add a rule to the .gitignore in the evm/ts-scripts directory
to ignore the local .env file to prevent accidental commits of secrets; update
the existing entries (output, contract-bindings, .env.testnet, .env.mainnet) by
appending a new line with ".env" so that files like WALLET_KEY and GUARDIAN_KEY
stored in .env are not tracked.
In `@evm/ts-scripts/config/mainnet/contracts.json`:
- Around line 162-163: The JSON in the GeneralPurposeGovernances array is
invalid because there's a missing comma between adjacent objects (the snippet
shows "}" followed immediately by "{"); edit the GeneralPurposeGovernances array
to insert the missing comma between those two object entries so the array
contains properly comma-separated objects (look for the objects in the
GeneralPurposeGovernances array and add a comma after the preceding "}" to fix
the parse error).
In `@evm/ts-scripts/config/testnet/governance-vaas.json`:
- Around line 1-18: The governance-vaas.json is missing an entry for XRPL EVM
Testnet (chainId 57) which causes executeGovernanceVaa.ts (which calls
getChainConfig("governance-vaas", chain.chainId)) to fail; add a JSON object
with "chainId": 57 and the appropriate base64 VAA string to governance-vaas.json
matching the format used for other entries, ensuring the chainId matches the
XRPL EVM Testnet entry in chains.json and the GeneralPurposeGovernance contract
referenced in contracts.json so executeGovernanceVaa.ts can find the VAA for
chainId 57.
In `@evm/ts-scripts/package.json`:
- Around line 2-4: Add the "private": true flag to this package.json to prevent
accidental npm publication; update the root object (near the existing "name":
"ts-scripts" and "version": "1.0.0" entries) to include the "private": true
property so npm will refuse to publish this internal scripts package.
In `@evm/ts-scripts/shell/verify-governance-contract.sh`:
- Around line 47-53: The conditional currently checks governance_address twice;
update the second duplicated condition to check core_wormhole_address instead so
the if statement becomes: if [ "$governance_address" = "" ] || [
"$core_wormhole_address" = "" ] || [ "$etherscan_api_key" = "null" ] || [
"$evm_chain_id" = "null" ]; then ...; this ensures missing core_wormhole_address
causes the script to skip verification and prevents passing invalid arguments to
forge verify-contract.
- Around line 1-4: Add a shebang as the first line of
verify-governance-contract.sh to explicitly select the shell interpreter (e.g.
#!/usr/bin/env bash) so the ENV check (test -z "$ENV") and subsequent behavior
run under a consistent shell; place the shebang at the top of the file and
ensure the script file is executable (chmod +x) so it always runs with the
chosen interpreter.
In `@evm/ts-scripts/src/createGovernanceMessage.ts`:
- Around line 100-110: The solidityPack call in createGovernanceMessage.ts is
malformed: fix the types array to use "uint16" (not "uin16") and insert a uint16
callDataLength field before callData so the values array has seven entries;
compute callDataLength using ethers.utils.hexDataLength(callData) (or
equivalent) and pass it as the uint16 field alongside governanceModule, action
(1), chain.chainId, generalPurposeGovernanceAddress, tokenAddress,
callDataLength, and callData so the payload matches
parseGeneralPurposeGovernanceMessage's expected MODULE, action, chain,
governanceContract, governedContract, callDataLength, callData sequence.
- Around line 151-159: The VAA signature assembly is nesting packSig inside
another array which causes comma-separated toString() output; in the vm
construction in createGovernanceMessage.ts replace the nested [packSig] entry
with packSig (i.e., spread or directly include the packSig string/array elements
so they concatenate as hex without commas) so the final vm string correctly
concatenates all hex segments (reference variable names: vm and packSig and
function createGovernanceMessage).
In `@evm/ts-scripts/src/readTransceiverConfig.ts`:
- Line 50: Fix the typo in the console output string: change "Configuration
succeded for chain ${result.chainId}" to use "succeeded" instead; update the
console.log call in readTransceiverConfig.ts where the message is constructed
(the one referencing result.chainId) so it prints "Configuration succeeded for
chain ${result.chainId}".
In `@solana/ts/scripts/env_short.ts`:
- Around line 14-20: The current fallback RPC URL ("INSERT_RPC_URL") for rpcUrl
risks creating a Connection to an invalid endpoint and causing confusing runtime
errors; update the initialization of rpcUrl/connection (variables rpcUrl,
connectionCommitmentLevel, and connection) to validate
process.env.SOLANA_RPC_URL at startup and if missing or still the placeholder,
throw or surface a clear error (or log and exit) describing that SOLANA_RPC_URL
is required, so the Connection is never constructed with an invalid URL.
In `@solana/ts/scripts/env.ts`:
- Line 82: The module currently evaluates guardianKey at import time (export
const guardianKey = getEnv("GUARDIAN_KEY")) which forces all importers to
require GUARDIAN_KEY; change this to a lazy accessor by removing the
module-level evaluation and adding a function getGuardianKey() that calls
getEnv("GUARDIAN_KEY") when invoked, export getGuardianKey instead of
guardianKey, and update all consumers to call getGuardianKey() (replace
references to guardianKey) so only scripts that need the key require the env
var.
In `@solana/ts/scripts/executeGovernanceVaas.ts`:
- Around line 17-20: The thrown error incorrectly names the env var; update the
error message in the governanceVaasFileName check to reference the correct
environment variable GOVENANCE_VAAS_FILE_PATH
(process.env.GOVERNANCE_VAAS_FILE_PATH) so the message matches the actual check
performed by the governanceVaasFileName variable; locate the check around the
governanceVaasFileName declaration and change the Error text accordingly.
In `@solana/ts/scripts/readManagerConfiguration.ts`:
- Line 18: The console.log in readManagerConfiguration.ts contains a typo:
change the message string in the console.log call that currently reads "pared
account info" to "parsed account info" so logs are clear; locate the console.log
statement referencing accountInfo.value.data and update its message text
accordingly.
In `@solana/ts/sdk/governance.ts`:
- Around line 40-44: GOV_PROGRAM_IDS contains a duplicate entry; remove the
repeated "NGoD1yTeq5KaURrZo7MnCTFzTA4g62ygakJCnzMLCfm" so the array only
includes the NTT Manager ID and the single governance program ID; update the
export const GOV_PROGRAM_IDS declaration to have exactly two distinct entries
and run tests/lints to ensure no trailing commas or type changes.
---
Outside diff comments:
In `@evm/ts-scripts/config/mainnet/chains.json`:
- Around line 52-57: The mainnet config for "MegaETH" currently contains a
localhost RPC URL which must be removed; update the "rpc" value for the MegaETH
entry in chains.json (the object with "description": "MegaETH") to a
production/public RPC endpoint or to an empty/placeholder value so getProvider()
does not use a localhost URL for mainnet; ensure getProvider() (the provider
resolution path) will instead read the correct env/config value (e.g., an env
var like PROVIDER_URL) or a valid public RPC string rather than the local dev
URL.
In `@evm/ts-scripts/config/mainnet/contracts.json`:
- Around line 1-192: The JSON file is missing the required TrimmedAmountLibs key
declared on the ContractsJson type in evm/ts-scripts/src/env.ts; add a
"TrimmedAmountLibs": [] entry (an empty array) near the other top-level arrays
(e.g., alongside WormholeCoreContracts, WormholeRelayers,
GeneralPurposeGovernances) before the final closing brace so the JSON conforms
to the ContractsJson schema and satisfies any runtime/type expectations.
---
Nitpick comments:
In `@deployments.md`:
- Line 14: Change the heading text "High level description of the process:" to
use the compound adjective form "High-level description of the process:"; locate
the heading string "High level description of the process" and replace it with
"High-level description of the process" so the static analysis warning is
resolved.
- Line 143: Fix the typo in the example API key value for the "etherscan" entry:
replace the placeholder "YOUR-ETTHERSCAN-API_KEY" with the correctly spelled
"YOUR-ETHERSCAN-API_KEY" so the key name "etherscan" maps to the proper
ETHERSCAN placeholder.
- Around line 44-48: The fenced code blocks containing environment variable
exports (e.g., lines with FOUNDRY_PROFILE, WALLET_KEY, LEDGER_BIP32_PATH, ENV)
are missing language specifiers; update each affected fenced block to start with
a language tag like ```shell or ```bash for proper syntax highlighting and
linting (apply this to the blocks around the exports at the mentioned locations
such as the snippet containing "export FOUNDRY_PROFILE=prod" and the other
blocks at lines 95-96, 99-100, 104-105, 148-149).
In `@evm/ts-scripts/shell/verify-governance-contract.sh`:
- Around line 55-59: The shell command is vulnerable to word-splitting because
the command substitution for constructor args is unquoted; update the forge
verify-contract invocation so the output of cast abi-encode is quoted (i.e.,
wrap $(cast abi-encode "constructor(address)" "$core_wormhole_address") in
quotes) when passed to --constructor-args to ensure the encoded constructor
argument is treated as a single token (references: the --constructor-args
argument and the cast abi-encode invocation using core_wormhole_address).
In `@evm/ts-scripts/src/createGovernanceMessage.ts`:
- Around line 1-4: The file uses CommonJS require for elliptic; replace "const
elliptic = require('elliptic')" with an ES import (e.g., import elliptic from
'elliptic') to match the other imports in createGovernanceMessage.ts and update
any usages (like new elliptic.ec(...)) accordingly; also add `@types/elliptic` as
a devDependency if TypeScript type errors appear so functions/types from the
elliptic package are properly typed.
In `@evm/ts-scripts/src/env.ts`:
- Around line 75-85: The functions getChainConfig and getAllContracts are
declared async but perform only synchronous work (they call loadScriptConfig
synchronously), so remove the unnecessary async to improve clarity: change their
signatures to non-async (drop the async keyword) and return the value directly
(no awaiting), and then update any callers that currently rely on them returning
Promises to handle the now-synchronous return (or wrap them in
Promise.resolve(...) at call sites if needed). Ensure you update function
declarations for getChainConfig and getAllContracts and run TypeScript checks to
fix any callsites expecting a Promise.
- Around line 199-217: The function loadContracts currently assigns the parsed
JSON to the module-level variable contracts and then calls return
loadContracts() recursively; change this so that after parsing and assigning
contracts (ContractsJson) the function returns contracts directly instead of
recursing. Update the return at the end of loadContracts to return contracts to
avoid the unnecessary infinite/extra call and ensure the function short-circuits
correctly when contracts is already set.
In `@evm/ts-scripts/src/executeGovernanceVaa.ts`:
- Around line 55-76: The file defines a local helper log(...) inside
executeGovernance but all logging uses console.log directly; either remove the
unused log function or replace the console.log calls with the helper for
consistent prefixed output: locate the executeGovernance function and either
delete the const log = (...) declaration, or change the three console.log/...
uses (the "Executing governance with VAA:", "Submitted governance transaction:",
and "success.") to call log(...) so logs include the `[${chain.chainId}]` prefix
before interacting with governanceContract.performGovernance and awaiting
tx.wait().
In `@solana/ts/scripts/env_short.ts`:
- Around line 42-51: The loadScriptConfig function should catch JSON parsing and
file read errors instead of relying on the unreachable if (!config) check: wrap
the fs.readFileSync and JSON.parse calls for loadScriptConfig in a try-catch,
include the filename and env values in the thrown/logged error message, and
rethrow or throw a new Error with the original error attached so callers get a
clear message when reading or parsing
`./ts/scripts/config/${env}/${filename}.json` fails.
In `@solana/ts/scripts/env.ts`:
- Around line 5-11: The module-level throws for LEDGER_DERIVATION_PATH and ENV
cause imports to fail; move these checks into lazy validation (e.g., inside the
getSigner() function) or guard them so only code paths that require a Ledger or
specific ENV fail; specifically remove the top-level throws for
LEDGER_DERIVATION_PATH and ENV and add checks inside getSigner() to validate
LEDGER_DERIVATION_PATH when hardware signing is requested, and validate ENV at
the point of configuration loading (e.g., within readManagerConfiguration or its
caller), ensuring error messages reference LEDGER_DERIVATION_PATH, ENV, and
getSigner so failures remain discoverable.
In `@solana/ts/scripts/executeGovernanceVaa_short.ts`:
- Around line 28-34: The code currently reads a hardcoded signer path
("YOUR_SIGNER_FILE") when building payerSecretKey; change it to read the path
from an environment variable (e.g., process.env.SIGNER_FILE or
process.env.PAYER_SIGNER) and fail fast if the variable is missing. Update the
fs.readFileSync call used to construct payerSecretKey (and any related constant)
to read the env var instead of the literal string, and add a clear error/throw
if the env var is not set so the script does not attempt to parse a missing
file.
In `@solana/ts/scripts/executeGovernanceVaa.ts`:
- Around line 22-24: The code currently uses a type-assertion
("governanceProgramId as any") when constructing NTTGovernance which bypasses
type safety; replace this cast by validating or converting governanceProgramId
before passing it to the NTTGovernance constructor: add a runtime check that
governanceProgramId matches an allowed GovProgramId (or map/parse it to the
expected enum/type), throw or return a clear error if invalid, or update the
variable's type so it is already the expected GovProgramId; reference the
symbols governanceProgramId and NTTGovernance (and the GovProgramId enum/type)
to locate and fix the code path that supplies the programId argument.
- Line 55: The log message is overly specific; replace the console.log that
references signerPk and nttProgramId so it reports a generic governance VAA
execution (e.g., "Signer <signerPk> executing governance VAA for program
<nttProgramId>"). Use signerPk and nttProgramId to build the message and, if
available, append VAA identifiers/summary from variables like vaa or
governanceVaa to make the log generic and informative.
In `@solana/ts/scripts/executeGovernanceVaas.ts`:
- Around line 69-75: The current code sends all governance instructions in one
transaction which can exceed Solana transaction size and account limits; update
the logic that calls ledgerSignAndSend with the full instructions array to
instead chunk the instructions into safe-sized batches (e.g., respect ~1200 byte
transaction payload and <=64 unique accounts per tx) and send each batch
sequentially: create a helper to split the instructions array by estimated
serialized size and distinct account count, call ledgerSignAndSend for each
batch, await connection.confirmTransaction for each returned tx.signature before
proceeding to the next batch, and log each batch's signature (references: the
instructions variable, ledgerSignAndSend, connection.confirmTransaction,
tx.signature).
In `@solana/ts/sdk/governance.ts`:
- Around line 73-74: verifyGovernanceHeader returns [governanceProgramId,
ixData] but governanceProgramId is never used; either remove the unused
destructuring or validate it—update the code around the verifyGovernanceHeader
call to compare the returned governanceProgramId with this.program.programId and
throw/log an error if they differ, otherwise drop the governanceProgramId
variable and only use ixData for deserializeInstruction(ixData); reference
verifyGovernanceHeader, governanceProgramId, this.program.programId, ixData, and
deserializeInstruction when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 316f5bbc-4f79-4456-bee9-20e50d7ae0ad
📒 Files selected for processing (32)
deployments.mdevm/.gitignoreevm/ts-scripts/.env.sampleevm/ts-scripts/.gitignoreevm/ts-scripts/config/mainnet/chains.jsonevm/ts-scripts/config/mainnet/contracts.jsonevm/ts-scripts/config/testnet/contracts.jsonevm/ts-scripts/config/testnet/governance-vaas.jsonevm/ts-scripts/package.jsonevm/ts-scripts/shell/verify-governance-contract.shevm/ts-scripts/src/createGovernanceMessage.tsevm/ts-scripts/src/env.tsevm/ts-scripts/src/executeGovernanceVaa.tsevm/ts-scripts/src/readTransceiverConfig.tssolana/package.jsonsolana/ts/scripts/.env.samplesolana/ts/scripts/claimOwnership.tssolana/ts/scripts/config/devnet/governance-vaa.jsonsolana/ts/scripts/config/devnet/ntt.jsonsolana/ts/scripts/config/devnet/programs.jsonsolana/ts/scripts/config/mainnet/governance-vaas-june-05.jsonsolana/ts/scripts/config/mainnet/ntt.jsonsolana/ts/scripts/config/mainnet/programs.jsonsolana/ts/scripts/env.tssolana/ts/scripts/env_short.tssolana/ts/scripts/executeGovernanceVaa.tssolana/ts/scripts/executeGovernanceVaa_short.tssolana/ts/scripts/executeGovernanceVaas.tssolana/ts/scripts/helpers.tssolana/ts/scripts/readManagerConfiguration.tssolana/ts/scripts/transferOwnershipToGov.tssolana/ts/sdk/governance.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@evm/ts-scripts/src/env.ts`:
- Around line 180-182: The construction of the StaticJsonRpcProvider is
formatted in a way that trips Prettier; locate the provider instantiation (the
let provider = new ethers.providers.StaticJsonRpcProvider(...) line using
providerRpc) and reformat the argument list to satisfy Prettier (e.g., keep the
constructor call on one line or place the argument on the same line as the call)
so the provider declaration adheres to project Prettier rules and `prettier
--check` passes. Ensure you only change whitespace/formatting around the
ethers.providers.StaticJsonRpcProvider and providerRpc usage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5040ef67-abc3-4e22-a581-acf1d14fdbd0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
evm/ts-scripts/src/env.ts
- Move governance.ts from solana/ts/sdk/ to solana/ts/scripts/ so it doesn't break the SDK build (it depends on @certusone/wormhole-sdk and local IDL artifacts not available in CI) - Update import paths in all scripts that reference it - Fix IDL import paths (../../target -> ../../../target) - Add .env to evm/ts-scripts/.gitignore to prevent secret leakage - Make guardianKey lazy (getGuardianKey function) so scripts that don't need it aren't forced to set the env var - Add "private": true to evm/ts-scripts/package.json - Fix prettier violation in evm env.ts provider construction
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
evm/ts-scripts/src/env.ts (1)
59-64:⚠️ Potential issue | 🟡 MinorResolve remaining Prettier violations to unblock CI.
prettier --checkis still failing for this file; the long signatures/expressions around Line 63 and Line 208 are likely the current offenders.Suggested formatting patch
type ChainConfig = { chainId: ChainId; -} +}; -export async function getChainConfig<T extends ChainConfig>(filename: string, chainId: ChainId): Promise<T> { +export async function getChainConfig<T extends ChainConfig>( + filename: string, + chainId: ChainId +): Promise<T> { const scriptConfig: T[] = await loadScriptConfig(filename); @@ -export async function getContractAddress(contractName: ContractTypes, chainId: ChainId): Promise<string> { - const contract = (await getAllContracts(contractName))?.find((c) => c.chainId === chainId)?.address; +export async function getContractAddress( + contractName: ContractTypes, + chainId: ChainId +): Promise<string> { + const contract = (await getAllContracts(contractName))?.find( + (c) => c.chainId === chainId + )?.address;Also applies to: 208-209
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evm/ts-scripts/src/env.ts` around lines 59 - 64, Prettier is failing due to long lines in the type/signature area; reflow the declarations for ChainConfig and the getChainConfig function so they adhere to the project's line-length rules (wrap the object type, move generic/type params and parameter list onto multiple lines if needed), and ensure the call to loadScriptConfig in getChainConfig is formatted across lines to avoid an overly long expression; target the ChainConfig type, the getChainConfig<T extends ChainConfig>(filename: string, chainId: ChainId): Promise<T> signature, and the loadScriptConfig(...) usage for reformatting so the file passes prettier --check.solana/ts/scripts/governance.ts (1)
40-44:⚠️ Potential issue | 🟡 MinorThe duplicate governance program ID is still present.
Line 43 repeats the same literal from Line 42, so
GOV_PROGRAM_IDSstill exposes a phantom third value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/governance.ts` around lines 40 - 44, GOV_PROGRAM_IDS contains a duplicate literal (the second and third entries are identical); remove the repeated entry so the array only lists unique program IDs (update the export const GOV_PROGRAM_IDS array in governance.ts to eliminate the phantom third value) and keep the remaining values as const.
🧹 Nitpick comments (1)
solana/ts/scripts/env.ts (1)
17-24: Cache the in-flight signer initialization.If two code paths hit
getSigner()before the firstSolanaLedgerSigner.create()resolves, Line 20 can open two Ledger sessions/prompts. Memoizing the promise avoids that race.💡 Suggested fix
-let signer; +let signerPromise: Promise<SolanaLedgerSigner> | undefined; export async function getSigner(): Promise<SolanaLedgerSigner> { - if (!signer) { - signer = await SolanaLedgerSigner.create(derivationPath); - } - - return signer; + signerPromise ??= SolanaLedgerSigner.create(derivationPath); + return signerPromise; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/env.ts` around lines 17 - 24, The current getSigner() stores the resolved SolanaLedgerSigner in the module variable signer which allows two concurrent callers to race and call SolanaLedgerSigner.create(derivationPath) twice; change the memoization to cache the in-flight promise instead: make signer hold the Promise returned by SolanaLedgerSigner.create and assign signer = SolanaLedgerSigner.create(derivationPath) the first time getSigner() is called, then await that promise and return the resolved SolanaLedgerSigner so concurrent calls share the same creation promise; update types accordingly and keep the exported getSigner signature the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@solana/ts/scripts/governance.ts`:
- Around line 73-74: verifyGovernanceHeader(...) returns a parsed
governanceProgramId that is currently ignored; update the script to immediately
compare the returned governanceProgramId with args.programId and fail fast if
they differ — e.g., throw or log an error and exit when governanceProgramId !==
args.programId — so the script stops before building/signing the transaction;
locate the call to verifyGovernanceHeader and use the governanceProgramId
variable for this check.
---
Duplicate comments:
In `@evm/ts-scripts/src/env.ts`:
- Around line 59-64: Prettier is failing due to long lines in the type/signature
area; reflow the declarations for ChainConfig and the getChainConfig function so
they adhere to the project's line-length rules (wrap the object type, move
generic/type params and parameter list onto multiple lines if needed), and
ensure the call to loadScriptConfig in getChainConfig is formatted across lines
to avoid an overly long expression; target the ChainConfig type, the
getChainConfig<T extends ChainConfig>(filename: string, chainId: ChainId):
Promise<T> signature, and the loadScriptConfig(...) usage for reformatting so
the file passes prettier --check.
In `@solana/ts/scripts/governance.ts`:
- Around line 40-44: GOV_PROGRAM_IDS contains a duplicate literal (the second
and third entries are identical); remove the repeated entry so the array only
lists unique program IDs (update the export const GOV_PROGRAM_IDS array in
governance.ts to eliminate the phantom third value) and keep the remaining
values as const.
---
Nitpick comments:
In `@solana/ts/scripts/env.ts`:
- Around line 17-24: The current getSigner() stores the resolved
SolanaLedgerSigner in the module variable signer which allows two concurrent
callers to race and call SolanaLedgerSigner.create(derivationPath) twice; change
the memoization to cache the in-flight promise instead: make signer hold the
Promise returned by SolanaLedgerSigner.create and assign signer =
SolanaLedgerSigner.create(derivationPath) the first time getSigner() is called,
then await that promise and return the resolved SolanaLedgerSigner so concurrent
calls share the same creation promise; update types accordingly and keep the
exported getSigner signature the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16d87dbb-b277-47da-9a39-1bb915dd532f
📒 Files selected for processing (9)
evm/ts-scripts/.gitignoreevm/ts-scripts/package.jsonevm/ts-scripts/src/env.tssolana/ts/scripts/env.tssolana/ts/scripts/executeGovernanceVaa.tssolana/ts/scripts/executeGovernanceVaa_short.tssolana/ts/scripts/executeGovernanceVaas.tssolana/ts/scripts/governance.tssolana/ts/scripts/transferOwnershipToGov.ts
✅ Files skipped from review due to trivial changes (5)
- evm/ts-scripts/.gitignore
- evm/ts-scripts/package.json
- solana/ts/scripts/transferOwnershipToGov.ts
- solana/ts/scripts/executeGovernanceVaas.ts
- solana/ts/scripts/executeGovernanceVaa_short.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- solana/ts/scripts/executeGovernanceVaa.ts
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
solana/ts/scripts/executeGovernanceVaas.ts (1)
17-20:⚠️ Potential issue | 🟡 MinorFix the env-var name in the thrown error.
The guard checks
GOVERNANCE_VAAS_FILE_PATH, so the current message points users to the wrong setting.📝 Proposed fix
if (!governanceVaasFileName) { - throw new Error("GOVERNANCE_VAAS_FILE_NAME is required."); + throw new Error("GOVERNANCE_VAAS_FILE_PATH is required."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@solana/ts/scripts/executeGovernanceVaas.ts` around lines 17 - 20, The thrown error message uses the wrong env var name; update the guard that checks governanceVaasFileName (from process.env.GOVERNANCE_VAAS_FILE_PATH) so the Error text references GOVERNANCE_VAAS_FILE_PATH instead of GOVERNANCE_VAAS_FILE_NAME, e.g., change the message in the throw new Error(...) near the governanceVaasFileName declaration to mention the correct env var name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deployments.md`:
- Around line 54-59: Add explicit language identifiers (e.g., shell) to all
unlabeled fenced code blocks that contain shell commands; locate the blocks
containing the snippets like "export FOUNDRY_PROFILE=prod", "export
NTT_PROGRAM_ID=<your-program-key>", "cd solana" / "anchor build --arch sbf", the
"solana program -k usb://ledger?key=" deploy line, and the block with "export
SCANNER_TOKENS_FILE_PATH=" and prefix each triple-backtick fence with "shell"
(```shell) so markdownlint MD040 is satisfied.
- Line 45: Update the incorrect documentation path
"ts-scripts/output/<scripts-name>/<timestamp>.json" mentioned in the sentence
(and the other occurrences that currently point to wrong EVM config/output
locations) to the actual script output path used by the repo; locate every
instance of that path string in the file and replace it with the correct output
location used by the deploy scripts (ensure the phrasing matches the script
layout and output structure so operators are directed to the real JSON files
produced by the deploy scripts).
In `@solana/ts/scripts/executeGovernanceVaa.ts`:
- Around line 16-17: The log text in executeGovernanceVaa.ts incorrectly
mentions "claiming NTT ownership" and pulls in nttProgramId only for that
message; change the log(s) produced where getProgramAddresses() is destructured
(nttProgramId, wormholeProgramId, governanceProgramId) and the messages around
lines 52-54 to a generic message such as "Executing governance payload" (or
"Executing governance VAA") that does not reference NTT-specific semantics, and
remove nttProgramId from the destructure and any unused references so only
wormholeProgramId and governanceProgramId remain.
---
Duplicate comments:
In `@solana/ts/scripts/executeGovernanceVaas.ts`:
- Around line 17-20: The thrown error message uses the wrong env var name;
update the guard that checks governanceVaasFileName (from
process.env.GOVERNANCE_VAAS_FILE_PATH) so the Error text references
GOVERNANCE_VAAS_FILE_PATH instead of GOVERNANCE_VAAS_FILE_NAME, e.g., change the
message in the throw new Error(...) near the governanceVaasFileName declaration
to mention the correct env var name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2714e59-c036-4b30-9bc1-3013afa81565
📒 Files selected for processing (21)
deployments.mdevm/ts-scripts/config/testnet/governance-vaas.jsonevm/ts-scripts/src/env.tsevm/ts-scripts/src/executeGovernanceVaa.tsevm/ts-scripts/src/readTransceiverConfig.tssolana/ts/scripts/claimOwnership.tssolana/ts/scripts/config/devnet/governance-vaa.jsonsolana/ts/scripts/config/devnet/ntt.jsonsolana/ts/scripts/config/devnet/programs.jsonsolana/ts/scripts/config/mainnet/governance-vaas-june-05.jsonsolana/ts/scripts/config/mainnet/ntt.jsonsolana/ts/scripts/config/mainnet/programs.jsonsolana/ts/scripts/env.tssolana/ts/scripts/env_short.tssolana/ts/scripts/executeGovernanceVaa.tssolana/ts/scripts/executeGovernanceVaa_short.tssolana/ts/scripts/executeGovernanceVaas.tssolana/ts/scripts/governance.tssolana/ts/scripts/helpers.tssolana/ts/scripts/readManagerConfiguration.tssolana/ts/scripts/transferOwnershipToGov.ts
✅ Files skipped from review due to trivial changes (12)
- solana/ts/scripts/config/mainnet/programs.json
- evm/ts-scripts/config/testnet/governance-vaas.json
- solana/ts/scripts/config/devnet/programs.json
- solana/ts/scripts/config/devnet/governance-vaa.json
- solana/ts/scripts/config/devnet/ntt.json
- solana/ts/scripts/config/mainnet/ntt.json
- solana/ts/scripts/config/mainnet/governance-vaas-june-05.json
- solana/ts/scripts/claimOwnership.ts
- solana/ts/scripts/transferOwnershipToGov.ts
- solana/ts/scripts/executeGovernanceVaa_short.ts
- solana/ts/scripts/helpers.ts
- solana/ts/scripts/env.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- solana/ts/scripts/readManagerConfiguration.ts
- evm/ts-scripts/src/executeGovernanceVaa.ts
- evm/ts-scripts/src/readTransceiverConfig.ts
- solana/ts/scripts/env_short.ts
…ed for governance)
…nfigs, evm gitignore, NttConfig type)
evan-gray
reviewed
Mar 27, 2026
Contributor
There was a problem hiding this comment.
shouldn't all this go in a different package so as not to conflict with or pollute the solana ts route?
Contributor
Author
There was a problem hiding this comment.
good point, moving the Solana scripts into their own package
scnale
reviewed
Mar 27, 2026
Contributor
There was a problem hiding this comment.
Hmm, is this necessary? I think this was used with the old NTT deployment scripts, right? This is not consumed by the NTT CLI as far as I know. But it's been a long time since I looked at these scripts so I'm not sure. The same would apply to the testnet equivalent.
Contributor
Author
There was a problem hiding this comment.
Good call, createGovernanceMessage.ts only uses token and chainId from this config so I trimmed the managers.json down to just those two fields and removed the unused deployment fields
Moves governance scripts from solana/ts/scripts/ into solana/ts-scripts/ with its own package.json (private: true) to avoid polluting the published @wormhole-foundation/sdk-solana-ntt SDK with script-only dependencies like @xlabs-xyz/ledger-signer-solana and dotenv. Mirrors the existing evm/ts-scripts/ pattern.
- Regenerate bun.lock after removing script deps from solana/package.json - Trim managers.json configs to only token + chainId (the only fields used by createGovernanceMessage.ts) - Fix prettier formatting on moved env files
Remove the EVM createGovernanceMessage script which is not needed for governance VAA execution. Also removes managers.json configs and loadGuardianSetIndex from env.ts as they were only used by this script.
scnale
reviewed
Mar 30, 2026
| "@types/node": "^20.11.22", | ||
| "@xlabs-xyz/ledger-signer": "^0.0.3", | ||
| "dotenv": "^16.4.5", | ||
| "ethers": "^5.7.2", |
Contributor
There was a problem hiding this comment.
It might be worth bumping this to ^5.8.0 so that it contains the security fixes for elliptic.
We might want to add:
"overrides": {
"elliptic": "6.6.1"
}
To ensure no other dependency pulls in the old version too.
Contributor
Author
There was a problem hiding this comment.
great suggestion, thank you.
Done: 702cb47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates governance scripts from the
deployment-scriptsbranch tomain— VAA execution and governance message creation for both Solana and EVM.Excludes deployment/config scripts (NTT CLI handles those), quoter, and special relaying (deprecated). Nearly all files are as-is cherry-picks from
deployment-scriptswith minimal changes.What's included
Solana
governance.tsNTTGovernanceclass — VAA parsing, PDA derivation, instruction creationexecuteGovernanceVaa.tsexecuteGovernanceVaas.tsexecuteGovernanceVaa_short.tsenv.ts/env_short.ts/helpers.tsEVM
executeGovernanceVaa.tscreateGovernanceMessage.tsenv.tsWhat's excluded (and why)
deployManagers,deployTransceivers,initializeNtt, etc.) — NTT CLI handles deploymentconfigureManagers,updatePeerAddresses,registerPeers, limits) — NTT CLI handles configurationconfigureQuoter,registerManagers,quoteSpecialRelays) — quoter is deprecatedtransferManagerOwnership.ts— not neededupgradeTransceivers.ts— not neededreadTransceiverConfig.ts— not neededtransferOwnershipToGov.ts/claimOwnership.ts/readManagerConfiguration.ts— depend on oldNTTSDK class which was rewritten toSolanaNtton main; need adaptation in a follow-upFiles with code changes
All other files are straight cherry-picks from
deployment-scripts.solana/ts/scripts/governance.ts— moved fromsolana/ts/sdk/tosolana/ts/scripts/(SDK structure differs on main), inlinedderivePdafunction, fixed IDL import pathsevm/ts-scripts/src/createGovernanceMessage.ts— replaced cross-repoguardianKeyimport with local env var read; inlinedManagerConfigtype from non-migratedconfigureManagers.tssolana/ts/scripts/env.ts— removed quoter types/functions, madeguardianKeylazyevm/ts-scripts/src/env.ts— trimmedContractsJsontype to onlyGeneralPurposeGovernancesOther minor changes
solana/package.json— added@xlabs-xyz/ledger-signer-solanaanddotenvdev dependenciesevm/ts-scripts/.env.sample— addedGUARDIAN_KEY=evm/ts-scripts/.gitignore— added.envto prevent secret leakageevm/ts-scripts/package.json— added"private": trueevm/ts-scripts/config/— contracts.json contains onlyGeneralPurposeGovernancesaddressesexecuteGonvernanceVaas.ts→executeGovernanceVaas.ts