Skip to content

Commit

Permalink
update gotchas docs: AIP 6 & 7 info, other cleanup / tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
DZGoldman committed Sep 26, 2023
1 parent ec2deff commit 95c1636
Showing 1 changed file with 19 additions and 9 deletions.
28 changes: 19 additions & 9 deletions docs/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,29 @@ _The following is a list of quirks and/or potentially unexpected behaviors in th


- **Abstain Vote**
Voting “abstain” on a core-governor or governor proposal does not count as either a “for” or “against” vote, but does count towards reaching quorum (5% or 3% of votable tokens, respectively).
Voting “abstain” on a core-governor or treasury governor proposal does not count as either a “for” or “against” vote, but does count towards reaching quorum (5% or 3% of votable tokens, respectively). Voting abstain on a security council member removal proposal is disallowed.

- **Late Quorum Extension**
The core, treasury, and security-council-member-removal governors all have a minimum 14-day voting period, and use open zeppelin's "late quorum" module to add a late-quorum extension of 2 days. This ensures that there are always at least 2 days of voting after a proposal's quorum is reached; i.e., the maximum total voting period would be 16 days, when/if quorum is reached at the very end of the initial 14-day period.

- **Timelock vs Governor Execution**
An operated queued in the core-governor-timelock or the treasury-governor-timelock can be executed permissionlessly on either its associated governor (typical) or on the timelock itself (atypical). The execution will be the same in either case, but in the later case, the governor’s `ProposalExecuted` event will not be emitted.
- **L1-to-L2 Message Fees in scheduleBatch**
When executing a batch of operations on the L1ArbitrumTimelock, if more than one operation creates a retryable ticket, the full `msg.value` value will be forwarded to each one. For the execution to be successful, the `msg.value` should be set to `m `and the L1ArbitrumTimelock should be prefunded with at least `m * n` ETH, where `m` is the max out of the costs of each retryable ticket, and `n` is the number of retryable tickets created.
An operation queued in the core-governor-timelock or the treasury-governor-timelock can be executed permissionlessly on either its associated governor (typical) or on the timelock itself (atypical). The execution will be the same in either case, but in the later case, the governor’s `ProposalExecuted` event will not be emitted.

- **Two L1 Proxy Admins**
There are two L1 proxy admins - one for the governance contracts, once for the governed core Nitro contracts. Note that both proxy admins have the same owner (the DAO), and thus this has no material effect on the DAO's affordances.

- **Non-excluded L2 Timelock**
In the both treasury timelock and the DAO treasury can be transfered via treasury gov DAO vote; however, only ARB in the DAO treasury is excluded from the quorum numerator calculation. Thus, the DAO’s ARB should ideally all be stored in the DAO Treasury. (Currently, the sweepReceiver in the TokenDistributor is set to the timelock, not the DAO treasury.)
ARB in the both treasury timelock and the DAO treasury can be transferred via treasury gov DAO vote; however, only ARB in the DAO treasury is excluded from the quorum numerator calculation. Thus, the DAO’s ARB should ideally all be stored in the DAO Treasury.

- **L2ArbitrumGovernoer onlyGovernance behavior**
Typically, for a timelocked OZ governror, the `onlyGovernance` modifier ensures a call is made from the timelock; in L2ArbitrumGoverner, the _executor() method is overriden such that `onlyGovernance` enforces a call from the governor contract itself. This ensures calls guarded by `onlyGovernance` go through the full core proposal path, as calls from the governor could only be sent via `relay`. See the code comment on `relay` in [L2ArbitrumGoveror](../src/L2ArbitrumGovernor.sol) for more.

- The `UpgradeExecRouteBuilder` contract is immutable; instead of upgrading it, it can be redeployed, in which case any references to its address should be updated as well (currently only referenced in SecurityCouncilManager).
- The `UpgradeExecRouteBuilder`'s l1TimelockMinDelay variable should be equal to the minimum timelock delay on the core L1 timelock. If, for whatever reason, the value on the L1 timelock is ever increased, the UpgradeExecRouteBuilder should be redeployed with the new value accordingly.
- Changes to members of the security council should be initiated via the SecurityCouncilManager, not via calling addOwner/removeOwner on the multisigs directly. This ensures that the security council's two cohorts remain properly tracked.
- Voting abstain on a SecurityCouncilMemberRemovalGovernor proposal is disallowed.
- **UpgradeExecRouteBuilder Quirks**
- The `UpgradeExecRouteBuilder` contract is immutable; instead of upgrading it, it can be redeployed, in which case any references to its address should be updated as well (currently only referenced in SecurityCouncilManager). Additionally, UpgradeExecRouteBuilder`'s l1TimelockMinDelay variable should be equal to the minimum timelock delay on the core L1 timelock. If, for whatever reason, the value on the L1 timelock is ever increased, the UpgradeExecRouteBuilder should be redeployed with the new value accordingly.

- **Security Council Member Updates**
- Changes to members of the Security Council should be initiated via the SecurityCouncilManager, not via calling addOwner/removeOwner on the multisigs directly. This ensures that the security council's two cohorts remain properly tracked in the SecurityCouncilManager contract.

- **UpgradeExecutor Affordance**
Affordances are always given to the DAO via an UpgradeExecutor contract, which grants affordance to both the core governor proposal path and the Security Council. This includes abilities that are intended only for the Security Council; for example, proposal cancellation, practically speaking, could/would only ever be preformed by the Security Council (since the DAO wouldn't have time to vote on and execute a cancellation). Still, for this case, the affordance is given to the UpgradeExecutor; this is done for clarity, consistency, and to ensure that the UpgradeExecutor is the single source of truth for execution rights.

0 comments on commit 95c1636

Please sign in to comment.