Skip to content

feat(protocol): add Proposal0020 to revamp the security council - #22010

Open
ggonzalez94 wants to merge 9 commits into
mainfrom
revamp-security-council
Open

feat(protocol): add Proposal0020 to revamp the security council#22010
ggonzalez94 wants to merge 9 commits into
mainfrom
revamp-security-council

Conversation

@ggonzalez94

@ggonzalez94 ggonzalez94 commented Aug 13, 2026

Copy link
Copy Markdown
Member

Restructures the Security Council from 9 members to 5 according to the Board meeting decision and lowers the standard proposal threshold to 3/5 and the emergency threshold to 4/5. This PR adds the Proposal0020 spec and its forge script, which prints the six actions to paste into the Taiko DAO UI; the proposal itself has not been submitted yet.

These actions execute directly from the Aragon DAO rather than through controller.taiko.eth, which holds no permissions on the governance contracts — so BuildProposal doesn't apply, hence the BuildDirectProposal base that future signer and threshold changes can reuse.

Full specification, the exact actions and their mandatory ordering, prerequisites and post-execution steps: Proposal0020.md

Gustavo Gonzalez and others added 5 commits August 11, 2026 10:54
…e proposals

Sibling of BuildProposal for proposals whose actions the Aragon DAO
executes directly (SignerList, multisig plugins, EncryptionRegistry) —
the DAO Controller holds no permissions on those contracts, so the
Execute path cannot carry them. Provides print (actions for the DAO UI
calldata form), l1dryrun (fork rehearsal with baseline/post-state
hooks), and verify (byte-compares an on-chain proposal against the
built actions) modes, plus a proposal:verify pnpm alias. Adds the
EncryptionRegistry to LibL1Addrs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restructures the Security Council from 9 members to 5 (removing
Chainbound, Halborn, Drew Van der Werff, Toni Wahrstaetter and Gattaca;
retaining Taiko Labs, L2BEAT, Aragon and Nethermind; adding Gustavo
Gonzalez as an independent member), lowers the standard proposal
threshold from 5/9 to 3/5 and the emergency threshold from 7/9 to 4/5,
and drops SignerList minSignerListLength from 8 to 4. The new seat is
currently Taiko Labs' appointed encryption agent; Taiko Labs must
rotate agents before execution (documented in the spec, simulated by
the dryrun, and asserted in checkPostState).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Generated via P=0020 pnpm proposal; the six actions to paste into the
DAO UI, validated on a mainnet fork (end state: 5 members, 3/5
standard, 4/5 emergency).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keeps the base to the same two modes as BuildProposal (print and
dryrun) rather than introducing new repo-wide tooling alongside a
council change. Confirming that a UI-created proposal matches the
repo is now a manual getProposal comparison against the generated
action.md, documented in the proposal spec.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing consumed it and no other proposal carries one — the DAO UI
collects the metadata fields through its own form and pins them. The
exact text to paste now lives in a 'DAO UI fields' section of
Proposal0020.md, keeping a single source of truth.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52a52277ca

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/protocol/script/layer1/proposals/Proposal0020.md Outdated
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🐋 DeepSeek Code Review

🟡 Warnings

  • BuildDirectProposal.dryrunL1 rebuilds the DAO actions after simulatePreExecution().
    buildCreateProposalCalldata(...) calls buildDaoActions() internally, then later Action[] memory actions = buildDaoActions() is executed against the DAO. For this PR the actions are pure/constant, so it’s fine, but the base is explicitly intended for future view/state-dependent proposals. If a pre-execution step changes state used by buildDaoActions, the dryrun could execute a different action set than the one submitted to the multisig. Store the actions once before building createProposal and reuse that exact array.

  • simulatePreExecution does not faithfully model the mandatory rotation. It calls appointAgent(appointer) — making the Taiko Labs seat appoint itself — instead of appointing a replacement EOA, and it does not assert that appointerOf(SC_GUSTAVO_GONZALEZ) == SC_TAIKO_LABS. That means the dryrun can pass even if the on-chain premise changed or the real rotation path behaves differently. This is dryrun-only, but it weakens the rehearsal value.

🔵 Suggestions

  • In checkBaseline, explicitly assert the expected current agent relation:
    appointerOf(SC_GUSTAVO_GONZALEZ) == SC_TAIKO_LABS. Then simulate rotation with a dedicated replacement-agent address instead of appointer.

  • For future reuse, have dryrunL1 use the already-encoded action list for both proposal creation and execution, and optionally verify the on-chain getProposal actions match the local list before simulating execution.

🟢 What Looks Good

  • Baseline/post-state assertions are strong: they pin exact current membership, settings, and invariant state with clear failure messages.
  • Action ordering is sound: settings before member removal, add-before-remove, and threshold updates after the signer list reaches its final size.
  • The documentation is thorough on prerequisites, contract-enforced ordering, and post-execution steps; the direct-DAO vs BuildProposal distinction is clear.

Automatically triggered on PR update • model: deepseek-v4-pro

Comment thread packages/protocol/script/layer1/proposals/Proposal0020.action.md Outdated

dantaik commented Aug 13, 2026

Copy link
Copy Markdown
Member

In-depth review of Proposal0020

I reviewed this PR by independently deriving the required on-chain changes from the deployed contract code, verifying every byte of the committed actions, checking live mainnet state, and running the fork dryrun end-to-end.

Verdict: the proposal is correct and executable as specified. The six actions are exactly the minimal set the deployed contracts require, the committed calldata is byte-exact, every baseline assumption matches mainnet state, and l1dryrun passes green against a fork. There are a handful of documentation/tooling fixes worth making before signers rely on these docs — most notably a broken verification command in Proposal0020.md that approvers are instructed to run (F1).

What I verified

1. Goal → minimal action set, derived independently from deployed code. I pulled the verified sources of all target contracts from Etherscan — SignerList impl 0x584f…e070, Multisig impl 0x8510…3c25, EmergencyMultisig impl 0x437e…514b, EncryptionRegistry 0x2eFD…42d1 — and confirmed they are identical to taikoxyz/dao-contracts. Working only from that code, the 9→5 / 3-of-5 / 4-of-5 restructuring forces exactly this action set:

  • removeSigners reverts if the list would drop below minSignerListLength (currently 8), so the floor must drop first, and updateSettings takes the full (encryptionRegistry, minSignerListLength) struct — hence Action 1.
  • Membership changes are addSigners / removeSigners — Actions 2–3.
  • Both updateMultisigSettings take full structs, so unchanged fields must be restated — Actions 4–5. All restated values (864000, 1209600, signerList, onlyListed) match current on-chain state.
  • removeUnused() is the only way to prune the removed members from the registry's accountList — the removeUnused call embedded in addSigners fires at Action 2, before the removals, so the explicit Action 6 does the real pruning.

This matches the six actions in Proposal0020.md exactly, in a valid order.

2. Script and calldata. I recomputed all six calldata blobs from the function signatures with an independent encoder — byte-identical to Proposal0020.action.md. Regenerating the action file via MODE=print also produces a file identical to the committed one, so there is no drift between script and committed artifact.

3. Mainnet state (checked live). addresslistLength() == 9 and the nine listed seats are exactly the spec's member table; settings() == (0x2eFD…42d1, 8); standard multisigSettings() == (true, 5, 864000, SignerList, 1209600); emergency == (true, 7, SignerList, 1209600); appointerOf(0xAC58…7163) == 0xb47f…029a (Taiko Labs Safe) — the mandatory-rotation prerequisite is real; the new member address has no code (EOA); 0x989E…50BE is the OptimisticTokenVotingPlugin proxy; latest committed proposal is 0019 and proposalCount() == 22, so the numbering is right. The DAO also verifiably holds all four required permissions (UPDATE_SIGNER_LIST_PERMISSION and UPDATE_SIGNER_LIST_SETTINGS_PERMISSION on the SignerList, UPDATE_MULTISIG_SETTINGS_PERMISSION on both multisigs) — checked via DAO.hasPermission and exercised by the dryrun.

4. Fork dryrun. MODE=l1dryrun SENDER=<L2BEAT seat> against a mainnet fork: baseline checks pass, createProposal succeeds (id 22), the simulated agent rotation succeeds, all six actions execute as the DAO, and every post-state assertion passes.

5. Semantic claims in the comments/spec — confirmed against deployed code:

  • createProposal accepts a member or an appointed agent (isListedOrAppointedByListed, Multisig L178–185), so SENDER=<a member or agent> is correct.
  • Approvals are credited to the seat owner resolved at the proposal's snapshot block (getListedEncryptionOwnerAtBlock, approve() L245–248), and only the resolved agent can approve (_canApprove: "owners who appointed, can't"). The NEW_MEMBER comment block is therefore exactly right: executing before rotation leaves the Taiko Labs seat unable to approve new proposals (recoverable at any time), and the new member's approvals credit its own seat on post-execution snapshots.
  • appointAgent(self) is explicitly "the same as unappointing" (EncryptionRegistry L42–43), so the dryrun's rotation simulation is semantically valid, and a real rotation wipes the stored public key (L80–84), as the post-execution notes say.
  • minSignerListLength ≥ emergency minApprovals is a NatSpec-documented convention, not contract-enforced; 4 ≥ 4 holds.
  • allowFailureMap is hardcoded to 0 at creation (Multisig L206) — any reverting action aborts the whole execution.
  • Precedents check out: Proposal0008 was an updateMultisigSettings change, and the SignerList emitted membership-change events on exactly 2025-06-02 and 2026-01-19.

Findings

F1 — Should fix: the documented pre-approval verification command fails

Proposal0020.md (step 5 of "Building and submitting via the DAO UI") tells creators and approvers to verify the stored on-chain actions with:

cast call 0xD7dA1C25… "getProposal(uint256)(bool,(uint16,uint64,uint64),bytes,(address,uint256,bytes)[],address)" …

but getProposal returns six values — there is a uint16 approvals between executed and the parameters tuple (Multisig L276–286). The documented signature fails to decode (ABI decoding failed: buffer overrun; reproduced against mainnet proposal 21). Since this is the command gating "confirm what landed on-chain matches this repo before approving", it should be:

cast call 0xD7dA1C25E915438720692bC55eb3a7170cA90321 \
  "getProposal(uint256)(bool,uint16,(uint16,uint64,uint64),bytes,(address,uint256,bytes)[],address)" \
  <PROPOSAL_ID> --rpc-url <ETHEREUM_RPC>

(verified working against mainnet proposal 21).

F2 — Should fix: stale verify-mode instruction survives in the generated action file

Commit 6d57357 dropped the verify mode, but BuildDirectProposal.logProposalAction still writes "After creation, run the verify mode against the new proposal id before approving" into every generated action file (BuildDirectProposal.sol L77–79), and the committed Proposal0020.action.md (L4–6) carries it. A signer following that instruction hits the invalid-mode branch — which, per F3, exits successfully. Suggest pointing the template at the F1 getProposal comparison instead, and regenerating.

F3 — Nice to have: run() exits 0 on an invalid MODE

BuildDirectProposal.run() L45–47 logs "Invalid mode" and returns; forge reports success. This mirrors the existing BuildProposal convention, so it's consistent — but for a new base a revert would be strictly safer (and would have turned F2 into a hard error instead of a silent no-op).

F4 — Small doc fix: the replacement agent cannot use setOwnPublicKey

Post-execution step 2 says Taiko Labs' replacement agent "registers its key the same way" as step 1's setOwnPublicKey — but setOwnPublicKey reverts MustBeListed for anyone not on the SignerList (EncryptionRegistry L94–97), and an agent is not listed. The agent path is setPublicKey(owner, key) called by the appointed agent (L110–119). The DAO UI presumably picks the right call, but the spec should name the actual mechanism.

F5 — Doc nuance, no action needed: what Action 6 actually leaves behind

removeUnused() only prunes accountList; it does not clear accounts[member].appointedAgent or appointerOf[agent]. All five removed members currently have appointed agents and registered keys on-chain, so after execution the appointerOf entries for their five agent EOAs remain set permanently: unlisted members can't call appointAgent (MustBeListed), and appointing one of those five addresses for another seat later reverts (AlreadyAppointed). Harmless for council operation (the stale entries can never resolve to a listed owner), but the Action 6 comment in Proposal0020.s.sol ("prunes the removed members' EncryptionRegistry entries") slightly oversells what happens — worth one accurate sentence in the spec.

F6 — Optional precision: the "mandatory ordering" claim is half contract-enforced

Only "Action 1 before Action 3" is genuinely contract-enforced (with floor 8, removeSigners would revert: 10−5=5 < 8). "Actions 4–5 must follow Action 3" is not, for these values: minApprovals 3 and 4 satisfy minApprovals ≤ addresslistLength() at every intermediate size (9, 10, or 5), so Actions 4–5 would succeed in any position. The chosen order is still the right defensive pattern — just worth knowing which dependency is the hard one.

F7 — Nice to have: assert the EOA assumption in the dryrun

The spec calls the new seat an EOA and encryption UX depends on it, but checkBaseline never asserts code.length == 0 for the new member. One extra check line pins it (I verified the address currently has no code).

Considerations (informational)

  • Emergency fault tolerance shrinks by design: 7/9 tolerates 2 unavailable seats; 4/5 tolerates 1 — and if execution happens before the Taiko Labs rotation, emergency temporarily needs all 4 functional seats. If the council ever shrinks to the new floor of 4 members, emergency requires 4/4 unanimity. Presumably all accepted in the Board decision, but worth the council seeing stated.
  • Nothing on-chain enforces the rotation prerequisite — it's purely operational. The spec's recommendation to rotate when the proposal is forwarded to the veto stage is the right call; the failure mode (Taiko Labs seat can't approve until it rotates) is recoverable and correctly documented.
  • Dryrun scope: the dryrun exercises createProposal plus the six actions pranked as the DAO — a faithful model of the target-side permission checks — but not the multisig-forward/optimistic-veto pipeline in between. Fine here since prior proposals have exercised that path, just worth knowing what the green run does and doesn't prove.
  • Immutable spec link: the DAO-UI Description pins blob/main/...; consider pinning the merge commit hash instead so the document TAIKO holders read during the 10-day veto window can never drift.
  • deployments/mainnet-contract-logs-L1.md currently has no governance/SignerList section, so post-execution step 3 ("record execution" there) will be its first entry of this kind — fine, just noting it.

On the earlier automated review

Of the DeepSeek warnings: #1 and #2 remain valid (F3/F2 above); #3 is incorrectcreateProposal explicitly accepts appointed agents via isListedOrAppointedByListed (Multisig L178–185), so SENDER=<a member or agent> works, and an unlisted SENDER fails the dryrun loudly at check(ok, "createProposal reverted"); #4 is a fair nuance, but appointAgent(self) is defined as un-appointment in the deployed registry, so the simulation is a semantically valid model of "the new member is released", which is the invariant checkPostState asserts.


Companion PR: #22013 (stacked on this branch) moves all security council seat addresses into a dedicated SC_-prefixed section of LibL1Addrs.sol and renames NEW_MEMBERSC_GUSTAVO_GONZALEZ, with the action file verified byte-identical and the dryrun re-run green.


Generated by Claude Code

#22013)

Co-authored-by: Claude <dong77@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
dantaik and others added 2 commits August 14, 2026 12:31
Co-authored-by: Claude <dong77@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants