Skip to content

Commit f0d6d97

Browse files
sveitsersveitser
authored
feat(deployer): add verify-proposal and auto-generate upgrade proposals (#4618)
* feat(deployer): add verify-proposal and auto-generate upgrade proposals - Add `deploy verify-proposal --contract <kind> --input schedule.json --input execute.json`: independently verify a timelock (schedule+execute) upgrade proposal against on-chain state and shipped bytecode, without trusting Etherscan or the JSON description - Bytecode check: compare the on-chain impl to the contract binding's DEPLOYED_BYTECODE, normalizing trailing solc CBOR metadata and the UUPS __self immutable; classify FullMatch / CodeMatchMetaDiffers / Mismatch - Governance checks: proxy owner()/currentAdmin() == mapped timelock, delay >= getMinDelay, value/predecessor zero, init selector, version prerequisite; assert schedule and execute share inner payload, salt, value, predecessor - Compute Safe EIP-712 domain/message/tx hashes so signers can match their Ledger - deploy writes proposals into contracts/deployments/proposals/<network>/<date>-<slug>/ (network from chain id; --network/--proposal-slug/--proposals-root overrides): schedule.json, execute.json, a contract kind file, and a templated README - Auto-fill the README Safe hashes: resolve the signing Safe from the deployment-info toml and read its nonce on-chain; skip with a warning when the signer set is ambiguous or lookup fails - Add a CI job to verify committed proposals under contracts/deployments/proposals/** * fix(deployer): verify-proposal takes a proposal directory; fix lint - Replace the two --input files with a positional proposal directory; read schedule.json, execute.json, and the contract kind from <dir>/contract (--contract now optional, cross-checked against the file). Fixes the broken num_args=2 usage that rejected two separate --input flags - Simplify the CI job to verify-proposal "$dir"; update README template (PROPOSAL_DIR token), docs, and the helper script comment to the dir form - Remove unused test imports (OpsTimelock, ext::AnvilApi) that failed clippy --all-targets -D warnings; apply nightly rustfmt import grouping * feat(deployer): validate proposals via proposal.toml, drop per-proposal README - Proposal PRs add only data: schedule.json, execute.json, and proposal.toml (contract kind, per-phase safe/nonce, all six Safe hashes, and metadata). The author is untrusted; correctness is established by verify-proposal, not by text - verify-proposal recomputes proxy/impl/salt/delay/predecessor/hashes from the JSONs + on-chain + shipped bytecode and asserts they match proposal.toml; the per-phase safe is cross-checked against deployment-info; nonce drift warns - Drop --safe/--nonce from verify (read from the toml); deploy writes the toml (auto-resolves signers, --safe override for ambiguous networks) - Remove per-proposal README generation and the contract file; the only README is the static, trusted contracts/deployments/proposals/README.md describing the flow * docs(deployer): document the 5-step proposal flow in the proposals README * docs(deployer): tighten proposal flow to short steps * docs(deployer): signers confirm all three hashes on the Ledger * feat(deployer): default verify-proposal RPC to the network public node - verify-proposal reads chain_id from proposal.toml and defaults --rpc-url to the network's public node (1 -> mainnet, 11155111 -> sepolia publicnode); --rpc-url overrides - Run verify read-only via its own provider, dispatched before the wallet provider is built, so it needs no mnemonic and no localhost node * fix(ci): make verify-proposals workflow actually run - build with rustup/mold/protoc like build.yml instead of the nix dev shell - run ./target/debug/deploy; --profile test outputs to target/debug, so the old ./target/test/deploy path failed every run - fetch-depth: 0 so git diff against the base ref resolves - verify a proposal dir when any file in it changes; the .json-only grep skipped proposal.toml tampering and broke under pipefail on no match - drop the per-network RPC case; the binary resolves the public node itself - add Swatinem/rust-cache and raise the timeout to 30 minutes * fix(deployer): close verification bypasses in verify-proposal - require exactly one transaction per Safe batch; extra txs escaped checks - fail non-empty init data for kinds without an expected init selector - embed deployment-info TOMLs via include_str!; the CWD-relative path let checks silently skip outside the repo root - always emit timelock-addr-match and proxy-addr-match from deployment-info; previously skipped without contract-address env vars, letting a look-alike proxy owned by an attacker timelock pass - hard-fail Safe proposer/executor checks on unknown networks and cover the SafeExit timelock instead of skipping it - pin network name to chain id (mainnet/decaf/hoodi); the default RPC was chosen from toml.chain_id and compared against itself - honor ESPRESSO_L1_PROVIDER on the verify-proposal subcommand - strip CBOR metadata only for a bounded tail ending in the canonical solc entry; the old scan never matched real bindings and allowed oversized attacker tails - mask immutables at per-kind artifact offsets instead of a substring scan; offsets kept in sync with bindings by tests over all five contract kinds * feat(ci): post proposal verification results as a PR comment - capture each proposal dir's verify output and PASS/FAIL status into a markdown file, posted via marocchino/sticky-pull-request-comment like test.yml - four-backtick fences since proposal descriptions are untrusted - stale comment is deleted when nothing was verified - job gets pull-requests: write; contents stays read-only * fix(ci): comment on the PR when verification fails before running - build or setup failures previously posted nothing and deleted the previous sticky comment - write a fallback FAIL comment with the run link when the job fails without producing pr-comment.md * fix(deployer): offset-free bytecode check, version-aware init check - compare bytecode by explaining every difference: accept only 20-byte windows holding the impl address over reference zeros (UUPS __self slots), require the exact per-contract window count, and report the offset plus both values on mismatch - delete the per-contract immutable offset tables; pin each binding's zero runs to the expected slot count in tests instead - accept empty init data only when the proxy already reached the reinitializer's target major version; fail closed if the version query fails * docs(deployer): explain why LightClient verification is unsupported
1 parent 26603af commit f0d6d97

15 files changed

Lines changed: 3667 additions & 11 deletions

File tree

‎.github/workflows/verify-proposals.yml‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
name: Verify upgrade proposals
2+
3+
on:
4+
pull_request:
5+
paths:
6+
- "contracts/deployments/proposals/**"
7+
8+
concurrency:
9+
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
10+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
11+
12+
env:
13+
CARGO_INCREMENTAL: "0"
14+
15+
jobs:
16+
verify-proposals:
17+
timeout-minutes: 30
18+
runs-on: ubuntu-latest
19+
permissions:
20+
contents: read
21+
pull-requests: write
22+
steps:
23+
- uses: actions/checkout@v6
24+
with:
25+
submodules: recursive
26+
fetch-depth: 0
27+
28+
- uses: rui314/setup-mold@v1
29+
30+
- name: Install Protoc
31+
run: |
32+
sudo apt-get update
33+
sudo apt-get install -y protobuf-compiler
34+
35+
- name: Enable Rust Caching
36+
uses: Swatinem/rust-cache@v2
37+
with:
38+
prefix-key: v3-rust
39+
shared-key: verify-proposals
40+
41+
- name: Build deploy binary
42+
run: cargo build --locked --bin deploy --profile test
43+
44+
- name: Find and verify changed proposals
45+
run: |
46+
set -euo pipefail
47+
# Keep only dirs with at least 5 path components, i.e.
48+
# contracts/deployments/proposals/<network>/<slug>, so README or
49+
# network-level files do not trigger verification.
50+
mapfile -t proposal_dirs < <(
51+
git diff --name-only "origin/${{ github.base_ref }}...HEAD" -- 'contracts/deployments/proposals/**' \
52+
| xargs -r -n1 dirname \
53+
| awk -F/ 'NF >= 5' \
54+
| sort -u
55+
)
56+
if [[ "${#proposal_dirs[@]}" -eq 0 ]]; then
57+
echo "No proposal directories changed."
58+
exit 0
59+
fi
60+
comment=pr-comment.md
61+
run_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
62+
printf '## Upgrade proposal verification\n\n' > "${comment}"
63+
exit_code=0
64+
for dir in "${proposal_dirs[@]}"; do
65+
if [[ ! -d "${dir}" ]]; then
66+
echo "Skipping deleted directory ${dir}"
67+
printf '### %s\n\nDirectory deleted; skipped.\n\n' "${dir}" >> "${comment}"
68+
continue
69+
fi
70+
echo "Verifying ${dir} ..."
71+
rc=0
72+
output=$(./target/debug/deploy verify-proposal "${dir}" 2>&1) || rc=$?
73+
printf '%s\n' "${output}"
74+
if [[ "${rc}" -eq 0 ]]; then
75+
status="PASS ✅"
76+
else
77+
status="FAIL ❌"
78+
exit_code="${rc}"
79+
fi
80+
# Four-backtick fence: proposal descriptions are untrusted and may
81+
# contain three-backtick sequences.
82+
printf '### %s: %s\n\n````\n%s\n````\n\n' "${dir}" "${status}" "${output}" >> "${comment}"
83+
done
84+
printf '[Workflow run](%s)\n' "${run_url}" >> "${comment}"
85+
exit "${exit_code}"
86+
87+
- name: Note incomplete verification
88+
if: ${{ always() && job.status != 'success' && hashFiles('pr-comment.md') == '' }}
89+
run: |
90+
{
91+
printf '## Upgrade proposal verification\n\n'
92+
printf 'FAIL ❌: workflow failed before any proposal was verified.\n\n'
93+
printf '[Workflow run](%s)\n' \
94+
"${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}"
95+
} > pr-comment.md
96+
97+
- name: Post/update PR comment
98+
if: ${{ always() }}
99+
continue-on-error: true
100+
uses: marocchino/sticky-pull-request-comment@v2
101+
with:
102+
header: verify-proposals
103+
path: pr-comment.md
104+
# No comment file (nothing verified or build failed): remove any stale comment.
105+
delete: ${{ hashFiles('pr-comment.md') == '' }}

‎Cargo.lock‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎contracts/deployments/proposals/README.md‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Upgrade proposals
2+
3+
## Why
4+
5+
- Contract upgrades go through a timelock owned by a multisig; each signer must know exactly what they are approving.
6+
- The impl address and Safe transaction are opaque; a signer cannot eyeball whether they are correct.
7+
- The proposal author (and the PR text) is untrusted, so verification must be reproducible, not based on trust.
8+
- Every signer can independently confirm the proposal matches the shipped contract before signing.
9+
10+
## Trust model
11+
12+
The proposal PR author is untrusted. Correctness is established entirely by `deploy verify-proposal <dir>`, which runs
13+
in CI on every proposal PR and must be run locally by each signer before signing. Do not trust any free text in a PR
14+
description or commit message.
15+
16+
## Directory convention
17+
18+
```
19+
contracts/deployments/proposals/
20+
<network>/
21+
<YYYYMMDD>-<slug>/
22+
schedule.json # Safe-tx-builder batch for the timelock schedule call
23+
execute.json # Safe-tx-builder batch for the timelock execute call
24+
proposal.toml # generated by `deploy`; machine-checked by `deploy verify-proposal`
25+
```
26+
27+
A proposal PR adds exactly these three files and nothing else. No README is generated per proposal.
28+
29+
## proposal.toml
30+
31+
`proposal.toml` is generated by `deploy` and records the claimed values for `proxy`, `impl`, `timelock`, `salt`,
32+
`delay`, `predecessor`, `chain_id`, `network`, and both Safe phase hashes. `deploy verify-proposal` recomputes every
33+
field from the JSONs, the on-chain state, and the shipped bytecode, then asserts the TOML matches. A malicious or
34+
incorrect TOML fails verification.
35+
36+
## CI
37+
38+
The workflow `.github/workflows/verify-proposals.yml` runs `deploy verify-proposal <dir>` for every proposal directory
39+
touched in a PR, using a public RPC for the target network.
40+
41+
## Flow
42+
43+
1. Deployer runs `deploy --upgrade-<contract> --use-timelock-owner ...`; opens a PR adding the proposal dir.
44+
2. Signers verify: `deploy verify-proposal contracts/deployments/proposals/<network>/<proposal-dir>` (also run by CI).
45+
All rows PASS. Note the printed hashes.
46+
- No mnemonic required; RPC defaults to the public node for the chain in `proposal.toml`.
47+
- Override with `deploy verify-proposal --rpc-url <url> <dir>` or set `ESPRESSO_L1_PROVIDER`.
48+
- Note: `--rpc-url` must follow the `verify-proposal` subcommand; the top-level `deploy --rpc-url` does not apply
49+
here. CI no longer passes an explicit RPC.
50+
3. Signers approve and merge the PR.
51+
4. One signer imports `schedule.json` into the Safe and submits.
52+
5. Other signers reconfirm the step-2 `domain`/`message`/`safe_tx` hashes on their Ledger, then sign.
53+
6. After the timelock delay, repeat steps 4-5 with `execute.json` (nonce + 1).
54+
55+
Nonce drift: `proposal.toml` hashes use the Safe nonce at generation time; verify prints a WARN (not FAIL) if the
56+
on-chain nonce moved. Reconfirm the nonce in the Safe app before signing.
57+
58+
## Networks
59+
60+
- `decaf/`: Decaf testnet (Sepolia, chainId=11155111)
61+
- `hoodi/`: Hoodi testnet (chainId=560048)
62+
- `mainnet/`: Espresso mainnet (Ethereum mainnet, chainId=1)

‎contracts/rust/deployer/Cargo.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edition = { workspace = true }
88
[dependencies]
99
alloy = { workspace = true }
1010
anyhow = { workspace = true }
11+
chrono = { workspace = true }
1112
clap = { workspace = true }
1213
derive_builder = { workspace = true }
1314
derive_more = { workspace = true }
@@ -21,6 +22,7 @@ serde_json = { workspace = true }
2122
surf-disco = { workspace = true }
2223
tide-disco = { workspace = true }
2324
tokio = { workspace = true }
25+
toml = { workspace = true }
2426
tracing = { workspace = true }
2527
url = { workspace = true }
2628
vbs = { workspace = true }

‎contracts/rust/deployer/src/builder.rs‎

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! builder pattern for
22
3-
use std::{fs, path::PathBuf};
3+
use std::path::PathBuf;
44

55
use alloy::{
66
hex::FromHex,
@@ -30,6 +30,7 @@ use crate::{
3030
TimelockOperationType, derive_timelock_address_from_contract_type,
3131
perform_timelock_operation, upgrade_stake_table_v3_timelock_proposal,
3232
},
33+
write::{WriteProposalParams, resolve_network, write_stake_table_v3_proposal_dir},
3334
},
3435
};
3536

@@ -152,6 +153,15 @@ pub struct DeployerArgs<P: Provider + WalletProvider> {
152153
output_dir: Option<PathBuf>,
153154
#[builder(default)]
154155
chain_id: u64,
156+
/// Override network name (otherwise derived from chain_id).
157+
#[builder(default)]
158+
network: Option<String>,
159+
/// Override the proposal directory slug (otherwise the contract kind in kebab-case).
160+
#[builder(default)]
161+
proposal_slug: Option<String>,
162+
/// Root for `contracts/deployments/proposals/` tree.
163+
#[builder(default)]
164+
proposals_root: Option<PathBuf>,
155165
}
156166

157167
impl<P: Provider + WalletProvider> DeployerArgs<P> {
@@ -535,23 +545,49 @@ impl<P: Provider + WalletProvider> DeployerArgs<P> {
535545
)
536546
.await?;
537547

538-
let output_dir = self
539-
.output_dir
540-
.as_deref()
541-
.context("--calldata-out-dir required for StakeTableV3 timelock upgrade")?;
542-
fs::create_dir_all(output_dir).with_context(|| {
543-
format!("failed to create output dir {}", output_dir.display())
544-
})?;
548+
let network = resolve_network(self.chain_id, self.network.clone())?;
549+
let slug = self
550+
.proposal_slug
551+
.clone()
552+
.unwrap_or_else(|| "stake-table-v3".to_owned());
553+
// --proposals-root > --calldata-out-dir > default
554+
let proposals_root = self
555+
.proposals_root
556+
.clone()
557+
.or_else(|| self.output_dir.clone())
558+
.unwrap_or_else(|| PathBuf::from("contracts/deployments/proposals"));
559+
let proposal_dir = write_stake_table_v3_proposal_dir(
560+
WriteProposalParams {
561+
proposals_root,
562+
network,
563+
slug,
564+
chain_id: self.chain_id,
565+
proxy: proposal.proxy_addr,
566+
new_impl: proposal.v3_impl_addr,
567+
timelock: proposal.timelock_addr,
568+
salt,
569+
delay,
570+
schedule_calldata: proposal.schedule.data.clone(),
571+
execute_calldata: proposal.execute.data.clone(),
572+
safe_override: None,
573+
},
574+
provider,
575+
)
576+
.await?;
545577
output_safe_tx_builder(
546578
&proposal.schedule,
547-
Some(&output_dir.join("schedule.json")),
579+
Some(&proposal_dir.join("schedule.json")),
548580
self.chain_id,
549581
)?;
550582
output_safe_tx_builder(
551583
&proposal.execute,
552-
Some(&output_dir.join("execute.json")),
584+
Some(&proposal_dir.join("execute.json")),
553585
self.chain_id,
554586
)?;
587+
tracing::info!(
588+
path = %proposal_dir.display(),
589+
"wrote StakeTableV3 timelock proposal"
590+
);
555591
} else if use_multisig {
556592
let calldata = upgrade_stake_table_v3_multisig_owner(
557593
provider,

0 commit comments

Comments
 (0)