[VPD-1612] Revoke CriticalTimelock privileges and cleanup#740
Open
GitGuru7 wants to merge 6 commits into
Open
Conversation
GitGuru7
marked this pull request as ready for review
July 15, 2026 12:55
…missions Add simulations/vip-665/data/scannedCritical.ts: the complete CriticalTimelock permission set per chain (8 mainnets), reconstructed off-chain by replaying every AccessControlManager RoleGranted/RoleRevoked event from ACM deployment to a recent block (2026-07-16), with 100% role-hash preimage resolution and zero replay anomalies. Derived without reference to the VIP data files. Each simulation now asserts, per chain: - the VIP revoke set equals the scanned set (0 missing / 0 extra, bidirectional) - Critical holds every scanned permission at the fork block - Critical holds none of them after execution This closes the circularity of only checking the VIP against its own data files. BNB wildcards hash via legacyWildcardRole (32-byte zero prefix); remote wildcards use the zero-address target directly. Sim results: bscmainnet 59/59, ethereum/arbitrumone/basemainnet/opmainnet/ unichainmainnet/opbnbmainnet 16/16 each (zksync data included, sim needs zksynctestnode). Co-Authored-By: Claude Fable 5 <[email protected]>
Debugger022
self-requested a review
July 16, 2026 11:35
Debugger022
previously approved these changes
Jul 16, 2026
Co-Authored-By: Claude Fable 5 <[email protected]>
- README: anvil-zksync requirement v0.3.0 -> >=0.6.11 (zksync Era mainnet blocks use protocol version 29, older releases refuse to fork); note that the local node is stateful and needs a fresh fork per simulation run. - hardhat.config.zksync.ts: fall back to the well-known public hardhat test key for the zksynctestnode network (local fork node only; the framework impersonates and funds the signer). An empty accounts array crashed the vip-framework at getSigners()[0] with an unrelated-looking TypeError that made VIP execution silently no-op. Real networks still require DEPLOYER_PRIVATE_KEY. Verified: simulations/vip-645/zksyncmainnet.ts passes 17/17 against a fresh anvil-zksync 0.6.11 fork at block 71188600 with no DEPLOYER_PRIVATE_KEY set. Co-Authored-By: Claude Fable 5 <[email protected]>
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
3 participants
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.
VIP-645: [Multi-Chain] Remove all CriticalTimelock Privileges
Summary
This proposal revokes every permission the CriticalTimelock holds across all eight Venus mainnets — including its emergency pause powers — leaving it with zero privileges and no permission moved to a Guardian, and additionally cleans up dangling, redundant and non-standard permission grants on the AccessControlManager (ACM). Refer to the community post for the full background and rationale.
Actions
On each of the eight chains the proposal executes the following sequence through that chain's ACMCommandsAggregator:
grantRole(DEFAULT_ADMIN_ROLE, aggregator)on the chain's AccessControlManager.executeGrantPermissions(index)on the aggregator (remote chains only; applies the wildcardsyncCash()grant to the NormalTimelock).executeRevokePermissions(index)on the aggregator, revoking all CriticalTimelock permissions plus the redundant and stale grants for that chain.revokeRole(DEFAULT_ADMIN_ROLE, aggregator)on the AccessControlManager.ACMCommandsAggregator addresses:
0x8b443Ea6726E56DF4C4F62f80F0556bB9B2a7c640xb78772bed6995551b64e54Cdb8e09800d86C73ee0x74AFeA28456a683b8fF907699Ff77138edef00f30xB2770DBD5146f7ee0766Dc9E3931433bb697Aa060x88B1452e512c8fcf83889DdCfe54dF37D561Db820xbbEBaF646e7a3E4064a899e68565B1b439eFdf700x904D11b00bdB2740d16176cc00DE139d0d6261150x6dB5e303289fea2E83F7d442470210045592AD93On BNB Chain, the proposal additionally issues direct
ACM.revokeRolecalls on the AccessControlManager (0x4788629ABc6cFCA10F9f969efdEAa1cF70c23555) to clear the CriticalTimelock's legacy 32-byte wildcard grants and the retired contracts' wildcard grants, which the aggregator cannot reach.Expected ACM event counts per chain (
RoleGranted/RoleRevoked): BNB Chain 1 / 279, Ethereum 2 / 136, Arbitrum One 2 / 122, Base 2 / 95, zkSync Era 2 / 81, OP Mainnet 2 / 74, Unichain 2 / 74, opBNB 2 / 56.The complete per-contract, per-chain list of every permission granted and revoked is in the GitHub pull request: #740
Voting options