Skip to content

chore(deployments): reconcile 10.json to live chain + add timelock deploy infra#50

Merged
rplusq merged 2 commits into
mainfrom
chore/deployments-reconcile-timelock-infra
Jul 14, 2026
Merged

chore(deployments): reconcile 10.json to live chain + add timelock deploy infra#50
rplusq merged 2 commits into
mainfrom
chore/deployments-reconcile-timelock-infra

Conversation

@rplusq

@rplusq rplusq commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator
  • deployments/10.json was stale: LTS entries pointed at old immutable contracts; StakeWeight/Distributor/Pauser impls predated P3/fix(stakeweight): correct permanent lock epoch accounting #45. Reconciled all proxy entries to live on-chain values (verified via EIP-1967 slots + getPostClaimHandlers).
  • Add TimelockDeploy.s.sol + arbitrum/base RPC aliases so ProxyAdmin ownership and the token admin role can be moved behind a per-chain timelock.

🤖 Generated with Claude Code

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @rplusq's task in 2m 11s —— View job


PR Review

  • Gather context
  • Read changed files
  • Run data classification subagent
  • Synthesize findings and post review

Found 3 issue(s)

Issue 1: Arbitrum/Base not in BaseScript production-chain guard — test mnemonic fallback risk

ID: timelockdeploy-arb-base-mnemonic-guard-a3f1
File: evm/script/deploy/TimelockDeploy.s.sol:22 (root cause: evm/script/Base.s.sol:69-74)
Severity: HIGH
Category: security

Context:

  • Pattern: BaseScript constructor only gates the test-mnemonic fallback for mainnet and optimism. Arbitrum (42161) and Base (8453) are not in isMainnetOrOptimism, so a broadcaster is silently derived from the public test test test…junk mnemonic if ETH_FROM is unset.
  • Risk: The deployed Timelock would be administered by the well-known test key (0xf39F…2266). Anyone with that key could immediately call grantRole/revokeRole on it, subverting the CONT-10 remediation entirely.
  • Impact: Compromised ProxyAdmin ownership and DEFAULT_ADMIN_ROLE for WCT on Arbitrum/Base.
  • Trigger: Operator runs the script without setting ETH_FROM — easy to miss on a new chain.

The script already documents this via an IMPORTANT comment (line 18-20), but documentation doesn't prevent mistakes for a security-critical contract. The fix belongs in BaseScript:

bool isProduction =
    block.chainid == getChain("mainnet").chainId ||
    block.chainid == getChain("optimism").chainId ||
    block.chainid == 42161 || // Arbitrum One
    block.chainid == 8453;    // Base
if (isProduction && specifiedSender == address(0)) {
    revert("You must specify a sender for a production deployment");
}

Fix this →


Issue 2: No deployment artifact written for Arbitrum/Base timelock

ID: timelockdeploy-no-artifact-write-c2e8
File: evm/script/deploy/TimelockDeploy.s.sol:23-35
Severity: MEDIUM
Category: code_quality

Context:

  • Pattern: EthereumDeploy calls _writeEthereumDeployments + DeploymentJsonWriter.writeEthereumDeploymentsToJson to persist the deployed address. TimelockDeploy only emits console2.log.
  • Risk: The deployed Timelock address on Arbitrum/Base is only visible in broadcast output — not recorded in a machine-readable file.
  • Impact: The CONT-10 hand-off scripts (Safe bundles) that reference the timelock address must be manually copied from broadcast logs; a transcription error would point the ProxyAdmin/admin role at the wrong address.
  • Trigger: Any time the deployment output needs to be referenced programmatically or verified later.

Minimally: gate the JSON write behind BROADCAST/WRITE_JSON env vars like other scripts and use DeploymentJsonWriter (or create the Arbitrum/Base JSON files). At minimum, write the address to a deployment JSON for the corresponding chain ID.


Issue 3: Missing Etherscan config for Arbitrum and Base

ID: foundry-missing-etherscan-arb-base-d5b9
File: evm/foundry.toml:45-50
Severity: LOW
Category: code_quality

Context:

  • Pattern: [etherscan] section has entries for sepolia, mainnet, optimism, optimism_sepolia — but not arbitrum or base. RPC aliases were added for both chains.
  • Risk: forge script … --verify will silently fail or error on Arbitrum/Base with no Etherscan API config.
  • Impact: Timelock contract won't be verified on-chain, reducing transparency for a security-critical component.
  • Trigger: Running TimelockDeploy with --verify.
[etherscan]
  arbitrum = { key = "${API_KEY_ARBISCAN}" }
  base     = { key = "${API_KEY_BASESCAN}" }

Data classification: No issues — addresses are public on-chain data, env vars are used for all secrets.

/// CHAIN_ID=42161 ETH_FROM=<ledger-addr> ADMIN_ADDRESS=<multisig> TIMELOCK_CANCELLER_ADDRESS=<canceller> \
/// forge script script/deploy/TimelockDeploy.s.sol --rpc-url arbitrum --broadcast --ledger --sender $ETH_FROM
/// (repeat with CHAIN_ID=8453 --rpc-url base for Base)
contract TimelockDeploy is BaseScript {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Auto Review Issue: Arbitrum/Base not in BaseScript production-chain guard — test mnemonic fallback risk

Severity: HIGH
Category: security
Tool: Claude Auto Review

Context:

  • Pattern: BaseScript constructor only gates the test-mnemonic fallback for mainnet and optimism. Arbitrum (42161) and Base (8453) are not in isMainnetOrOptimism, so a broadcaster is silently derived from the public test test test…junk mnemonic if ETH_FROM is unset.
  • Risk: The deployed Timelock would be administered by the well-known test key (0xf39F…2266). Anyone with that key could immediately call grantRole/revokeRole on it, subverting the CONT-10 remediation entirely.
  • Impact: Compromised ProxyAdmin ownership and DEFAULT_ADMIN_ROLE for WCT on Arbitrum/Base.
  • Trigger: Operator runs the script without setting ETH_FROM — easy to miss on a new chain.

The script already documents this via an IMPORTANT comment (line 18-20), but documentation doesn't prevent mistakes for a security-critical contract. The fix belongs in BaseScript:

bool isProduction =
    block.chainid == getChain("mainnet").chainId ||
    block.chainid == getChain("optimism").chainId ||
    block.chainid == 42161 || // Arbitrum One
    block.chainid == 8453;    // Base
if (isProduction && specifiedSender == address(0)) {
    revert("You must specify a sender for a production deployment");
}

Fix this →

/// forge script script/deploy/TimelockDeploy.s.sol --rpc-url arbitrum --broadcast --ledger --sender $ETH_FROM
/// (repeat with CHAIN_ID=8453 --rpc-url base for Base)
contract TimelockDeploy is BaseScript {
function run() public broadcast returns (Timelock timelock) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Auto Review Issue: No deployment artifact written for Arbitrum/Base timelock

Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review

Context:

  • Pattern: EthereumDeploy calls _writeEthereumDeployments + DeploymentJsonWriter.writeEthereumDeploymentsToJson to persist the deployed address. TimelockDeploy only emits console2.log.
  • Risk: The deployed Timelock address on Arbitrum/Base is only visible in broadcast output — not recorded in a machine-readable file.
  • Impact: The CONT-10 hand-off scripts (Safe bundles) that reference the timelock address must be manually copied from broadcast logs; a transcription error would point the ProxyAdmin/admin role at the wrong address.
  • Trigger: Any time the deployment output needs to be referenced programmatically or verified later.

Minimally: gate the JSON write behind BROADCAST/WRITE_JSON env vars like other scripts and use DeploymentJsonWriter (or create the Arbitrum/Base JSON files). At minimum, write the address to a deployment JSON for the corresponding chain ID.

rplusq and others added 2 commits July 14, 2026 21:50
…melock deploy infra

- 10.json was stale: LTS entries pointed at old immutable contracts; StakeWeight/
  Distributor/Pauser impls predated P3/#45. Reconciled all proxy entries to live
  on-chain values (verified via EIP-1967 slots + getPostClaimHandlers).
- Add TimelockDeploy.s.sol + arbitrum/base RPC aliases so ProxyAdmin ownership and
  the token admin role can be moved behind a per-chain timelock.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Etherscan V2 uses one API key across all chains (routed by chainid); the
per-explorer V1 keys are deprecated. Add arbitrum/base [etherscan] entries
and collapse optimism + the Makefile onto ${API_KEY_ETHERSCAN}, dropping
API_KEY_OPTIMISTIC_ETHERSCAN from the env sample.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@rplusq rplusq force-pushed the chore/deployments-reconcile-timelock-infra branch from 7625647 to 2e58226 Compare July 14, 2026 20:51
@rplusq rplusq merged commit 12c2d7c into main Jul 14, 2026
7 of 8 checks passed
@rplusq rplusq deleted the chore/deployments-reconcile-timelock-infra branch July 14, 2026 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant