Enforce governance delays, etc.#9
Open
EndymionJkb wants to merge 13 commits into
Open
Conversation
# Conflicts: # test/utils/RoycoDayTestBase.sol
…ed numbers from comments.
# Conflicts: # script/Deploy.s.sol # src/libraries/Constants.sol # test/concrete/Factory/Test_DeployScriptConfig.t.sol # test/fork/kernels/Test_KernelSuiteBase.t.sol
# Conflicts: # src/factory/templates/Identical_ERC4626_ST_JT_SharePriceToChainlinkOracle_BalancerV3GyroECLP_LT_DeploymentTemplate.sol
# Conflicts: # src/accountant/RoycoDayAccountant.sol # src/factory/templates/Identical_ERC4626_ST_JT_SharePriceToChainlinkOracle_BalancerV3GyroECLP_LT_DeploymentTemplate.sol
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.
Enforce governance delays, cap the fixed term, and split consequential roles
Motivation
Without execution delays, an admin can bypass the timelock those delays are meant to provide. The clearest case is the upgrader: with zero delay, an admin can replace an implementation instantly, and an upgrade can change the system entirely.
The same logic applies to any role that can make a consequential change. This PR assigns delays based on what each role can do, so no admin key can move funds, change code, or override pricing faster than the long delay. Immediate actions are limited to protective actions whose worst outcome is halting or excluding.
What this PR does
setConversionRateinto its own long-delay role, while keeping oracle-source setters operational.DeployScript.getRoleConfigas the single source of truth.Delay model
Two constants in
Constants.solare used by both the deploy script and the accountant:SHORT_DELAY_SECONDSis two days. It applies to operational changes to live market configuration: kernel, accountant, protocol fee setter, oracle quoter, market ops, Balancer pool manager, unpauser, and blacklist removal.LONG_DELAY_SECONDSis fifteen days. It applies to root admin execution, upgrader execution, grant delay on consequential roles, target admin delay on deployed proxies, sanctions-source changes,setConversionRate, and template registration.The guiding rule is that emergency protection can be immediate, but reversal or consequential change must wait. The pauser has zero delay; the unpauser waits the short delay. Blacklist add is immediate; blacklist removal waits the short delay. The upgrader waits the long delay because an upgrade can replace implementations and change the system completely.
Fixed-term cap
MAX_FIXED_TERM_SECONDSis fourteen days. The deploy script asserts that the long delay is at least the fixed-term cap and at least the short delay. That preserves the core invariant: a committed user can exit before a governance change takes effect, and the admin path is never faster than a delay it can reconfigure.The invariant is also protected at runtime. AccessManager applies delay changes through
Time.Delay.withUpdate, so reducing a delay waits a setback equal to the reduction. Every path that can reduce a delay is itself bounded by the long delay: the governance admin has the long execution delay, and the factory's zero-delay admin can only be reached through a registered template, whose registration now also waits the long delay.Role splits
The blacklist functions and conversion-rate override previously fell back to the admin role. They now have dedicated roles.
blacklistAccountsis assigned toADMIN_BLACKLIST_ROLEwith zero delay, since adding an address only excludes it and should be available immediately in an emergency.unblacklistAccountsis assigned toADMIN_UNBLACKLIST_ROLEwith the short delay, since it removes protection for specific accounts but is not a system-wide security change.setSanctionsListis assigned toADMIN_SANCTIONS_ROLEwith the long delay, since changing the sanctions source changes how every account is screened.setConversionRateis assigned toADMIN_CONVERSION_RATE_ROLEwith the long delay. It is a direct price override and the most powerful function on the oracle quoter. The oracle-source setters,setChainlinkOracle,setBPTOracle, andsetMaxReinvestmentSlippage, remain operational at the short delay.Factory admin
The factory keeps the admin role at zero delay so it can wire roles during deployment. A registered template can reach that admin through
executeAsFactory.This PR bounds that path by moving template registration to the long delay. A malicious template therefore cannot be registered inside the reaction window, and the factory admin can only be driven by an already-registered trusted template during its deployment flow.
Removing the standing factory admin entirely is deferred as defense-in-depth against a template bug, not as part of the compromised-key path. The residual risk is documented at
executeAsFactory, and the full admin-key-to-consequential-change path map is indocs/balancer-review/admin-delay-hardening.md.Cleanup
RoleConfigUtils.getRoleConfigduplicatedDeployScript.getRoleConfigand had only one consumer,UpgradeFactoryModule.UpgradeFactoryModuleno longer matches the current design. Itspreparepath reads from the retired architecture, where the factory itself was the access manager, so it now reverts.Both files are removed, leaving
DeployScript.getRoleConfigas the single role-configuration source.Open question
No outside party can cancel the root admin's scheduled operations, because the admin role's guardian is the admin role itself. An independent veto would require a second admin-role holder. Balancer's
TimelockAuthorizer, for comparison, had separate executors and cancelers.That is a key-management decision for the deployer, not a code change, and is left open deliberately.
Example Illustration