chore: add verifyBytecode script for governance proposal check

[melanciani]

I suggest to add verifyBytecode.js script: compares the runtime bytecode deployed at an on-chain address against a locally compiled Hardhat artifact. It loads the artifact's immutableReferences (via the .dbg.json → build-info) and masks those byte ranges on both sides before comparing, so immutables like OZ UUPSUpgradeable's address(this) don't cause a false mismatch. Takes

<address> <artifact-path> [--rpc <url>]; exits 0 (match), 1 (no match), 2 (usage/error).

[melanciani]

@claude review

[claude]

Claude finished @melanciani's request. — View run

[claude]

[AUTOMATED]

Code Review — verifyBytecode script

Overall, this is a well-written and useful addition to the governance tooling. The immutable-masking approach is correct and the script has good structure with proper exit codes, clear output, and exported functions for reuse. A few issues worth addressing below, mostly around edge-case robustness for a security-sensitive tool.

Summary

Category	Finding
🔴 Bug	--rpc as last arg silently falls back to default RPC
🟡 Robustness	Broad try/catch on immutableReferences masks distinct failure modes
🟡 Style	Inconsistent quoting in .env.example

[claude]

[AUTOMATED] Bug — --rpc as last argument silently uses default RPC

If a user runs node verifyBytecode.js 0xABC artifact.json --rpc (forgetting the URL), process.argv[rpcIdx + 1] is undefined. This silently falls back to DEFAULT_RPC_URL in verifyBytecode() via options.rpcUrl || DEFAULT_RPC_URL, so the user believes they specified a custom RPC but verification runs against the public default instead.

For a governance security tool, verifying against the wrong network/RPC is a meaningful failure mode.

Suggested fix: validate that --rpc has a following argument:

if (rpcIdx !== -1 && (rpcIdx + 1 >= process.argv.length || process.argv[rpcIdx + 1].startsWith('--'))) {
  console.error('Error: --rpc requires a URL argument')
  process.exit(2)
}

Confidence: 92/100

[claude]

[AUTOMATED] Nit: Broad try/catch could distinguish "missing .dbg.json" from "corrupted build-info"

The catch block on line 38 handles every failure the same way — a warning + fallback to no masking. This is safe (it can only produce false negatives, never false positives), but for a security-critical tool it would be helpful to distinguish:

1. .dbg.json not found → expected for some artifacts, warn and continue
2. Build-info exists but has unexpected structure → suggests a real problem the user should investigate

Currently both cases print a generic warning. Consider at minimum checking whether the .dbg.json exists before entering the try block, so that a missing file is a quiet/expected path, while structural errors in the build-info are more prominently surfaced.

Confidence: 82/100

[claude]

[AUTOMATED] Nit: Inconsistent value quoting

The new RPC_SEPOLIA entry uses double quotes around the URL:

RPC_SEPOLIA="https://ethereum-sepolia-rpc.publicnode.com/"

But existing entries don't use quotes:

RPC_ETHEREUM=https://eth.llamarpc.com
RPC_GATEWAY_MAINNET=https://rpc.mainnet.zama.org

For consistency, consider removing the quotes to match the existing style.

Confidence: 90/100

[claude]

The review has been posted successfully. Here's a summary of what was reviewed and found:

PR #179 Review Summary

PR: "chore: add verifyBytecode script for governance proposal check" by @melanciani

What the PR Does

Adds a verifyBytecode.js script to the governance-proposal-builder tooling that compares on-chain runtime bytecode against locally compiled Hardhat artifacts. It intelligently masks immutable references (e.g., OZ UUPSUpgradeable's address(this)) before comparing, so legitimate deployments don't produce false mismatches.

Findings Posted (3 inline comments)

Severity	File	Finding
🔴 Bug	verifyBytecode.js:100	--rpc as last argument silently falls back to default RPC — If a user passes --rpc without a URL, undefined silently falls through to the default public RPC. For a governance security tool, this could mean verification runs against the wrong network without the user knowing.
🟡 Nit	verifyBytecode.js:38	Broad try/catch masks distinct failure modes — Missing .dbg.json (expected) vs corrupted build-info (unexpected) both produce the same generic warning. Safe (only causes false negatives), but could be more informative.
🟡 Nit	.env.example:10	Inconsistent value quoting — RPC_SEPOLIA uses double quotes while all other entries are unquoted.

What Passed Review

• ✅ Core immutable-masking logic is correct
• ✅ Exit codes are well-designed (0/1/2) and CI-friendly
• ✅ ethers v6 getCode() empty-address handling is correct
• ✅ Functions are properly exported for reuse
• ✅ README documentation is thorough
• ✅ CommonJS style matches existing scripts

[jatZama]

I am noticing this PR only now, it is a very good initiative, and surely we need something like this, but I would advise to warn users more explicitely that wrong immutables could lead to catastrophic failure in production, and this script does not take into account checking of immutables values, but maybe there is a way to improve this. Also the same issue is present if passing wrong constant values, which are used a lot more often than immutables inside host and gateway contracts, btw this almost lead to a catastrophic failure when a hot fix proposal was done few months ago for FHEVMExecutor, this is why this proposal was dropped by being ignored for 5 days and replaced by correct upgrade with correct constants values. https://app.aragon.org/dao/ethereum-sepolia/0x08e8a84c3c8c7cba165B1adcf67Ae4639eF84f52/proposals/SETUP_1-24
Currently what I do for each DAO proposal review is grepping manually in deployed bytecode all imported constants when those are not exposed via getters, see for example what I did today here https://zama-ai.slack.com/archives/C0B7C90G6KG/p1783504901533989?thread_ts=1783429451.615379&cid=C0B7C90G6KG : this is obviously very painful and error prone, so I agree we should improve this, at least I requested to expose getters for all private constants.

TLDR: I think we should warn users explicitely about immutables not being checked in the verifyBytecode script, and on the other hand, to handle imported constant values, we should ask operator to recompile their contracts with the correct values inside FHEVMAddresses.sol and GatewayAddresses.sol file. Ideally any immutable or constant variable should be fetchable via a view function in said contracts, for easier reviewing by operators, but sometimes this is not possible (for eg many gateway contracts do not expose private constants).