From 55865bcc37d4d277d29eb784a9bcfb8660a7b8af Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 17:14:29 +0000 Subject: [PATCH 1/3] Give the timelock hand-off its own script, so a second pass can run it Arc and Robinhood both launched with every proxy, both beacons and the group factory owned by one deployer EOA with no code, while their 2-of-5 Safes and TimelockControllers sat deployed, correctly wired to each other, and owning nothing. Arc's ProxyAdmin has since been handed over; the rest has not, and Robinhood is untouched. The cause is structural rather than a typo. Those transfers live inside the `:deploy` scripts that create the contracts, and on a two-pass chain launch that is the wrong place for them: pass 1 runs with protocolTimelock unset, the transfer is skipped, the script records its migration id anyway, and pass 2 - the pass that has the timelock - never runs it again. The instruction those scripts print, "run deploy again after setting protocolTimelock", is not something anyone can actually follow. Clearing the stale records would unstick it, but it also re-runs deployBeacon against whatever contracts ref the run pins, shipping a new implementation when all that was wanted was to move an owner. So the transfer gets its own script and its own id, which no existing record blocks. It deploys nothing. Every action is guarded on the current owner: already the timelock is reported and left alone, the deployer is transferred, anything else is reported and never forced. A chain that is already correct is a no-op, and a chain handed to something other than the deployer is never quietly overwritten. A contract a chain does not have is skipped rather than taking the run down. The two scripts that caused this keep their own behaviour - they did deploy what they were asked to, and their ids are honestly recorded - but they no longer print an instruction that cannot be carried out. They point here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Gc9MzY447rkqkXopXT6vmP --- .../deploy/collateral/escrow_beacon.ts | 8 +- .../lender_commitment_group_v2_beacon.ts | 7 +- .../46_transfer_timelock_ownership.ts | 138 ++++++++++++++++++ 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 packages/contracts/deploy/upgrades/46_transfer_timelock_ownership.ts diff --git a/packages/contracts/deploy/collateral/escrow_beacon.ts b/packages/contracts/deploy/collateral/escrow_beacon.ts index 8a84cc6ec..58a6dfe20 100644 --- a/packages/contracts/deploy/collateral/escrow_beacon.ts +++ b/packages/contracts/deploy/collateral/escrow_beacon.ts @@ -12,7 +12,13 @@ const deployFn: DeployFunction = async (hre) => { //is this necessary ? const { protocolTimelock } = await hre.getNamedAccounts() if (protocolTimelock === '0x0000000000000000000000000000000000000000') { - hre.log('⚠️ protocolTimelock is zero address — skipping escrow beacon ownership transfer. Run deploy again after setting protocolTimelock.') + // Do not tell anyone to "run deploy again" here: this script records its + // id whether or not the transfer happened, and a recorded id never runs + // again, so a second pass cannot pick it up. That is how Arc and Robinhood + // both left this beacon on the deployer EOA. The transfer now has its own + // script, which is not blocked by this record: + // deploy/upgrades/46_transfer_timelock_ownership.ts + hre.log('⚠️ protocolTimelock is zero address — skipping escrow beacon ownership transfer. Run the `protocol:transfer-timelock-ownership` tag once the timelock is set.') } else { hre.log('Transferring ownership of CollateralEscrowBeacon to Protocol Timelock...') await collateralEscrowBeacon.transferOwnership(protocolTimelock) diff --git a/packages/contracts/deploy/lender_commitment_forwarder/extensions/lender_groups_v2/lender_commitment_group_v2_beacon.ts b/packages/contracts/deploy/lender_commitment_forwarder/extensions/lender_groups_v2/lender_commitment_group_v2_beacon.ts index 50d4190c9..75f18945d 100644 --- a/packages/contracts/deploy/lender_commitment_forwarder/extensions/lender_groups_v2/lender_commitment_group_v2_beacon.ts +++ b/packages/contracts/deploy/lender_commitment_forwarder/extensions/lender_groups_v2/lender_commitment_group_v2_beacon.ts @@ -61,7 +61,12 @@ const deployFn: DeployFunction = async (hre) => { const { protocolTimelock , protocolOwnerSafe } = await hre.getNamedAccounts() if (protocolTimelock === '0x0000000000000000000000000000000000000000') { - hre.log('⚠️ protocolTimelock is zero address — skipping beacon ownership transfer. Run deploy again after setting protocolTimelock.') + // Same trap as the escrow beacon: this script's id is recorded whether or + // not the transfer ran, so "run deploy again" is not something anyone can + // actually do. The transfer lives in + // deploy/upgrades/46_transfer_timelock_ownership.ts, which has its own id + // and is not blocked by this one. + hre.log('⚠️ protocolTimelock is zero address — skipping beacon ownership transfer. Run the `protocol:transfer-timelock-ownership` tag once the timelock is set.') } else { hre.log('Transferring ownership of CommitmentGroupBeacon to Gnosis Safe...') await commitmentGroupBeacon.transferOwnership(protocolTimelock) diff --git a/packages/contracts/deploy/upgrades/46_transfer_timelock_ownership.ts b/packages/contracts/deploy/upgrades/46_transfer_timelock_ownership.ts new file mode 100644 index 000000000..ebc834186 --- /dev/null +++ b/packages/contracts/deploy/upgrades/46_transfer_timelock_ownership.ts @@ -0,0 +1,138 @@ +import { DeployFunction } from 'hardhat-deploy/dist/types' +import { logTxLink } from 'helpers/logTxLink' + +const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' + +/** + * Hands the beacons, the group factory and the ProxyAdmin to the protocol + * timelock, on any chain where the deployer is still holding them. + * + * This exists because the transfers it performs live inside the `:deploy` + * scripts that create those contracts, and on a two-pass chain launch that is + * the wrong place for them. Pass 1 runs with `protocolTimelock` unset, the + * transfer is skipped, and the script records its migration id anyway - so + * pass 2, which is the pass that has the timelock, never runs it again. The + * instruction those scripts print, "run deploy again after setting + * protocolTimelock", cannot be followed. + * + * That is not hypothetical. It is how Arc and Robinhood both ended up with + * every proxy, both beacons and the group factory owned by a single deployer + * EOA with no code, while their 2-of-5 Safes and TimelockControllers sat + * deployed, correctly wired to each other, and owning nothing. + * + * Clearing those stale records would work, but it also re-runs `deployBeacon` + * against whatever contracts ref the run pins - shipping a new implementation + * when all that was wanted was to move an owner. So the transfer gets its own + * script with its own id instead. It deploys nothing, it is idempotent, and it + * is not blocked by any record already written. + * + * Every action is guarded on the current owner: + * - already the timelock -> reported and left alone + * - the deployer -> transferred + * - anyone else -> reported and left alone, never forced + * + * so a chain that is already correct is a no-op, and a chain that was handed + * to something other than the deployer is never quietly overwritten. + */ +const deployFn: DeployFunction = async (hre) => { + hre.log('=================================================================') + hre.log('') + hre.log(`${hre.network.name}: handing ownership to the protocol timelock`) + hre.log('') + + const { deployer, protocolTimelock } = await hre.getNamedAccounts() + + const lower = (a: string) => (a ?? '').toLowerCase() + const isTimelock = (a: string) => lower(a) === lower(protocolTimelock) + const isDeployer = (a: string) => lower(a) === lower(deployer) + + // The Ownable contracts. Absent from a chain's deployments is not a failure: + // not every chain has every one of these, and asking for one that was never + // deployed should skip rather than take the run down. + const ownableNames = [ + 'CollateralEscrowBeacon', + 'LenderCommitmentGroupBeaconV2', + 'LenderCommitmentGroupFactory_V2', + ] + + for (const name of ownableNames) { + let contract + try { + contract = await hre.contracts.get(name) + } catch { + hre.log(` – ${name} is not deployed on this chain. Skipping.`) + continue + } + + const currentOwner: string = await contract.owner() + + if (isTimelock(currentOwner)) { + hre.log(` ✅ ${name} is already owned by the timelock`) + } else if (isDeployer(currentOwner)) { + hre.log(` → transferring ${name} to the timelock...`) + const tx = await contract.transferOwnership(protocolTimelock) + await tx.wait(1) + hre.log(` ✅ ${name} transferred to ${protocolTimelock}`) + await logTxLink(hre, tx.hash) + } else { + hre.log( + ` ⚠️ ${name} is owned by ${currentOwner}, which is neither the ` + + `deployer nor the timelock. Left alone.` + ) + } + } + + // The ProxyAdmin, which is the one that matters most: it is the admin of + // every transparent proxy on the chain, TellerV2 included. + hre.log('') + const defaultProxyAdmin = await hre.upgrades.admin.getInstance() + const proxyAdminOwner: string = await defaultProxyAdmin.owner() + + if (isTimelock(proxyAdminOwner)) { + hre.log(' ✅ Default Proxy Admin is already owned by the timelock') + } else if (isDeployer(proxyAdminOwner)) { + hre.log(' → transferring Default Proxy Admin to the timelock...') + const signer = await hre.getNamedSigner('deployer') + await hre.upgrades.admin.transferProxyAdminOwnership( + protocolTimelock, + signer + ) + hre.log(` ✅ Default Proxy Admin transferred to ${protocolTimelock}`) + } else { + hre.log( + ` ⚠️ Default Proxy Admin is owned by ${proxyAdminOwner}, which is ` + + `neither the deployer nor the timelock. Left alone.` + ) + } + + hre.log('') + hre.log('done.') + hre.log('=================================================================') + + return true +} + +// tags and deployment +deployFn.id = 'protocol:transfer-timelock-ownership' +deployFn.tags = ['protocol', 'protocol:transfer-timelock-ownership'] +deployFn.dependencies = ['teller-v2:deploy'] + +deployFn.skip = async (hre) => { + if (!hre.network.live) return true + + // Nothing to hand over to. Deliberately `false` is not returned here the way + // it is in the scripts this replaces: skipping on an unset timelock is the + // one case where recording the id would be wrong, and hardhat-deploy only + // records what actually ran. + const { protocolTimelock } = await hre.getNamedAccounts() + if (!protocolTimelock || protocolTimelock === ZERO_ADDRESS) { + hre.log( + `protocolTimelock is unset on ${hre.network.name} - nothing to transfer ` + + `ownership to. Set _TIMELOCK_ADDRESS and run again.` + ) + return true + } + + return false +} +export default deployFn From 0727553140272e4322671bee8072819e66ef6e76 Mon Sep 17 00:00:00 2001 From: teller-deploy Date: Mon, 21 Sep 2026 17:23:19 +0000 Subject: [PATCH 2/3] Run tags [protocol:transfer-timelock-ownership] on arc Written by scripts/deploy-chain.sh. Timelock: not yet deployed --- packages/contracts/deployments/arc/.migrations.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts/deployments/arc/.migrations.json b/packages/contracts/deployments/arc/.migrations.json index c6ec252fb..fcd85ed97 100644 --- a/packages/contracts/deployments/arc/.migrations.json +++ b/packages/contracts/deployments/arc/.migrations.json @@ -27,5 +27,6 @@ "lender-commitment-forwarder:extensions:borrow-swap:rebind-quoter": 1789811385, "lender-commitment-forwarder:extensions:loan-referral-forwarder-v2:deploy": 1789989318, "lender-commitment-forwarder:extensions:flash-swap-rollover:weth9-upgrade-arc": 1790007076, - "default-proxy-admin:transfer": 1790010192 + "default-proxy-admin:transfer": 1790010192, + "protocol:transfer-timelock-ownership": 1790011094 } \ No newline at end of file From a9bbca06fcac3d79e9ee66234267a98d1e8d3755 Mon Sep 17 00:00:00 2001 From: teller-deploy Date: Mon, 21 Sep 2026 17:49:46 +0000 Subject: [PATCH 3/3] Run tags [protocol:transfer-timelock-ownership] on robinhood Written by scripts/deploy-chain.sh. Timelock: not yet deployed --- packages/contracts/deployments/robinhood/.migrations.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts/deployments/robinhood/.migrations.json b/packages/contracts/deployments/robinhood/.migrations.json index b6894be61..01f5a21e5 100644 --- a/packages/contracts/deployments/robinhood/.migrations.json +++ b/packages/contracts/deployments/robinhood/.migrations.json @@ -23,5 +23,6 @@ "validate-deployments": 1789464120, "default-proxy-admin:transfer": 1789464120, "lender-commitment-forwarder:deploy": 1789993859, - "lender-commitment-forwarder:extensions:loan-referral-forwarder-v2:deploy": 1789993860 + "lender-commitment-forwarder:extensions:loan-referral-forwarder-v2:deploy": 1789993860, + "protocol:transfer-timelock-ownership": 1790012191 } \ No newline at end of file