poc: app operations + trust model#9
Draft
solimander wants to merge 31 commits into
Draft
Conversation
11 tasks
- Add script/releases/v1.4.0-governance/ with upgrade.json, EOA deploy script (TimelockControllerImpl + SafeTimelockFactory proxy + new AppController impl), and multisig upgrade script - Extend Env.sol with SafeTimelockFactory (proxy + impl), TimelockControllerImpl, and Safe infrastructure (safeSingleton, safeProxyFactory, defaultFallbackHandler) accessors - Add SafeTimelockFactory.t.sol with MockSafeProxyFactory and 13 test cases covering timelock/safe deployment, deterministic addresses, and revert paths - Add migrateAdmins tests to AppController.t.sol (happy path, auth, non-existent app, no-admins, idempotency) - Add SafeTimelockFactory and TimelockControllerImpl to compile-bindings.sh
sepolia-dev Zeus environment is at 1.3.0, not 1.1.1.
EOA-owned apps retain direct upgradeApp() → AppUpgraded path. Transferring ownership to a SafeTimelockFactory-deployed Safe or Timelock auto-enables governance mode (governed=true), which blocks direct upgrades and requires the two-step flow: scheduleUpgrade(app, release, delay) → AppUpgradeScheduled (no controller action) executeUpgrade(app, release) → AppUpgraded (controller acts) Release data is committed by hash at schedule time and verified at execution, avoiding storage of dynamic arrays (OZ TimelockController pattern). SafeTimelockFactory wired as an immutable on AppController; Deploy.s.sol auto-deploys a stub factory when none is provided.
…enforces delay natively
- cancelUpgrade(app): deletes _pendingUpgrades[app], emits AppUpgradeCancelled - scheduleUpgrade now emits AppUpgradeCancelled when overwriting an existing pending upgrade - Add AppUpgradeCancelled event to IAppController - 8 new tests covering cancel and overwrite-emits-cancel behaviour
…p, and grantTeamRole(ADMIN) When an app is timelocked, transferOwnership and terminateApp now require msg.sender == owner (i.e. the Timelock itself), preventing any admin from bypassing the queue delay. Similarly, grantTeamRole(ADMIN) now requires the Timelock to be the caller when the team is a Timelock address. Also fixes a pre-existing bug in transferOwnership: activeAppCount was not moved from the old owner to the new owner, causing arithmetic underflow on terminate/suspend after an ownership transfer.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…constructor arg Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ops through Timelock All sensitive operations (upgradeApp, terminateApp, transferOwnership, grantTeamRole ADMIN) now require msg.sender == owner when timelocked, enforcing that they go through Timelock.schedule → execute. The AppController-level scheduleUpgrade/executeUpgrade/cancelUpgrade functions and _pendingUpgrades storage are removed — the Timelock's own queue provides equivalent guarantees for all operations uniformly.
Full redeploy including SafeTimelockFactory with deployer-indexed registry. All 15 contracts verified on Etherscan. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deploy.s.sol was hardcoding address(0) for safeSingleton, safeProxyFactory, and safeFallbackHandler, causing all Safe deployments via the factory to revert. Safe addresses are now read from config.json and validated before use. Also adds safeTimelockFactory to the output.json serialization so the address is captured without manual inspection of broadcast logs.
… sepolia-dev - TimelockControllerImpl overrides schedule/scheduleBatch/execute/executeBatch/cancel to maintain _pendingIds/_pendingOps storage - getPendingOperations() returns all queued ops with target, calldata, and executableAt without requiring event log scanning or any RPC block range - getPendingOperationIds() returns just the IDs - All existing Timelock clones upgrade automatically via SafeTimelockFactory impl update - Redeploy sepolia-dev at v1.5.0 with new AppController and SafeTimelockFactory
…address collisions
TimelockControllerImpl.schedule/scheduleBatch now calls canCall(address(this), data) on the target before queuing. If the target implements ICallValidator and returns false, the schedule reverts immediately — preventing operations that would always fail at execute-time from entering the queue. AppController implements ICallValidator.canCall: - App-scoped ops (upgrade, transfer, terminate, start, stop, updateMetadata): requires caller has ADMIN on the app's owner team - Team-scoped ops (createAppForTeam, grantTeamRole, revokeTeamRole): requires caller is team or has ADMIN on team - createApp: always allowed (runtime quota checks apply) - Everything else: returns false (blocked from scheduling)
master already ships a v1.4.0 (isolated-billing), so this release must apply on top of v1.4.0. Move the directory to v1.5.0-governance and update upgrade.json: from 1.4.0, to 1.5.0.
createAppForTeam only wrote _appConfigs[app].owner but never set _appConfigs[app].timelocked. If the team (and therefore the app's initial owner) is a factory-deployed Timelock, the app was created with timelocked=false — bypassing the runtime guards on upgradeApp, transferOwnership, terminateApp, and grantTeamRole(ADMIN). Any subsequent co-admin the Timelock granted could then perform sensitive ops directly, with no queue delay. Set timelocked = safeTimelockFactory.isTimelock(team) at creation, mirroring the check transferOwnership already performs. Also fix upgrade.json metadata that was committed with the old 1.3.0 → 1.4.0 values; this release must apply on top of v1.4.0 (which is already taken by isolated-billing on master). Added regression tests: - timelockedOwnerSetsTimelockedFlag: Timelock-created app has the flag - timelockedBlocksNonOwnerAdminUpgrade: co-admin upgrade reverts, Timelock upgrade succeeds (exploit path closed) - nonTimelockOwnerDoesNotSetTimelocked: EOA-owned app keeps flag false - safeOwnerDoesNotSetTimelocked: Safe-owned app keeps flag false
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
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.
Before & After — master vs soli/gov (this PR)
master (current — system v1.3.0)
graph TB classDef eigenlayer fill:#dbeafe,stroke:#3b82f6 classDef protocol fill:#f3f4f6,stroke:#6b7280 classDef governance fill:#fef9c3,stroke:#ca8a04 subgraph Gov["Protocol Governance"] computeOps["computeOpsMultisig"]:::governance computeWL["computeWhitelisterMultisig"]:::governance end subgraph Proxies["Contracts (all proxies via ProxyAdmin)"] PA["ProxyAdmin"]:::protocol AC["AppController\n(impl last deployed in v1.1.1)\n─────────────────\nstartApp: checkCanCall(app)\nstopApp: checkCanCall(app)\nupgradeApp: checkCanCall(app)\nterminateApp: checkCanCall(app)"]:::protocol CAVS["ComputeAVSRegistrar"]:::protocol CO["ComputeOperator"]:::protocol IA["ImageAllowlist\n(added v1.2.0)"]:::protocol USDC["USDCDeposit\n(added v1.3.0)"]:::protocol AppBeacon["App Beacon → App impl"]:::protocol end subgraph EL["EigenLayer"] PC["PermissionController\n(gates ALL app operations)"]:::eigenlayer AM["AllocationManager"]:::eigenlayer DM["DelegationManager"]:::eigenlayer RM["ReleaseManager"]:::eigenlayer end computeOps -->|owns| PA PA -->|upgrades| AC & CAVS & CO & IA & USDC AC -->|"checkCanCall(app) → is admin?"| PC AC --> CAVS & CO & RM & AppBeacon CAVS --> AM CO --> AM & DM computeWL -->|operator allowlist| CAVSsoli/gov (this PR — system v1.4.0) — green = new, yellow = changed
graph TB classDef eigenlayer fill:#dbeafe,stroke:#3b82f6 classDef protocol fill:#f3f4f6,stroke:#6b7280 classDef governance fill:#fef9c3,stroke:#ca8a04 classDef new fill:#bbf7d0,stroke:#16a34a,stroke-width:2px classDef changed fill:#fef08a,stroke:#ca8a04,stroke-width:2px subgraph Gov["Protocol Governance"] computeOps["computeOpsMultisig"]:::governance computeWL["computeWhitelisterMultisig"]:::governance end subgraph Proxies["Contracts"] PA["ProxyAdmin"]:::protocol AC["AppController v1.4.0\n─────────────────\nstartApp: ADMIN role\nstopApp: ADMIN or PAUSER role\nupgradeApp: ADMIN role (owner-only if timelocked)\nterminateApp: ADMIN role (owner-only if timelocked)\ntransferOwnership: ADMIN role (owner-only if timelocked)\ngrantTeamRole ADMIN: owner-only if timelocked\n+ grantTeamRole / revokeTeamRole\n+ migrateAdmins"]:::changed CAVS["ComputeAVSRegistrar"]:::protocol CO["ComputeOperator"]:::protocol IA["ImageAllowlist"]:::protocol USDC["USDCDeposit"]:::protocol AppBeacon["App Beacon → App impl"]:::protocol STF["SafeTimelockFactory\n(proxy + impl)"]:::new TCI["TimelockControllerImpl\n(clone target)"]:::new end subgraph TeamGov["Team Governance (deployed by teams via SafeTimelockFactory)"] TeamSafe["Gnosis Safe\n(team members)"]:::new TeamTC["TimelockController\n(clone, minDelay enforced)"]:::new end subgraph Roles["App Roles — stored in AppController (per team)"] ADMIN["ADMIN\nstart/stop/upgrade/terminate\ntransferOwnership/grantTeamRole"]:::new PAUSER["PAUSER\nstop only"]:::new DEV["DEVELOPER\nmetadata/builds"]:::new end subgraph EL["EigenLayer"] PC["PermissionController\n(protocol admin only)"]:::eigenlayer AM["AllocationManager"]:::eigenlayer DM["DelegationManager"]:::eigenlayer RM["ReleaseManager"]:::eigenlayer end computeOps -->|owns| PA PA -->|upgrades| AC & CAVS & CO & IA & USDC & STF TCI -.->|clone target| TeamTC STF -->|deploys| TeamSafe & TeamTC AC -->|"app ops → hasTeamRole?"| ADMIN & PAUSER & DEV AC -->|"protocol ops → checkCanCall"| PC AC --> CAVS & CO & RM & AppBeacon TeamSafe -->|PROPOSER + EXECUTOR| TeamTC TeamTC -->|"Timelock.schedule → execute (after minDelay)"| AC CAVS --> AM CO --> AM & DM computeWL -->|operator allowlist| CAVSTimelocked app — sensitive ops flow
When an app's owner is a
SafeTimelockFactory-deployed Timelock (AppConfig.timelocked = true), the four sensitive operations require the Timelock itself asmsg.sender. There are no dedicatedscheduleUpgrade/executeUpgradefunctions in AppController — delay is enforced entirely by the Timelock's own queue:The same pattern applies to
terminateApp,transferOwnership, andgrantTeamRole(ADMIN).Non-sensitive ops (
startApp,stopApp,updateAppMetadataURI,grantTeamRole(PAUSER/DEVELOPER)) are open to any ADMIN role member regardless of timelocked status.Summary
checkCanCall(app)→ PermissionControllerhasTeamRole→ ADMIN / PAUSER / DEVELOPERTimelock.getTimestamp(operationId)