Skip to content

Commit fb1d5e5

Browse files
author
Joey Santoro
committed
optimistic timelock 2
1 parent f9d3a9a commit fb1d5e5

File tree

8 files changed

+155
-41
lines changed

8 files changed

+155
-41
lines changed
Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,36 @@
11
// SPDX-License-Identifier: GPL-3.0-or-later
22
pragma solidity ^0.8.4;
33

4-
import "./Timelock.sol";
4+
import "@openzeppelin/contracts/governance/TimelockController.sol";
55
import "../refs/CoreRef.sol";
66

77
// Timelock with veto admin roles
8-
contract OptimisticTimelock is Timelock, CoreRef {
8+
contract OptimisticTimelock is TimelockController, CoreRef {
99

10-
constructor(address core_, address admin_, uint delay_, uint minDelay_)
11-
Timelock(admin_, delay_, minDelay_)
10+
constructor(
11+
address core_,
12+
uint256 minDelay,
13+
address[] memory proposers,
14+
address[] memory executors
15+
)
16+
TimelockController(minDelay, proposers, executors)
1217
CoreRef(core_)
13-
{}
14-
15-
function queueTransaction(address target, uint value, string memory signature, bytes memory data, uint eta) public override whenNotPaused returns (bytes32) {
16-
return super.queueTransaction(target, value, signature, data, eta);
17-
}
18-
19-
function vetoTransactions(address[] memory targets, uint[] memory values, string[] memory signatures, bytes[] memory datas, uint[] memory etas) public onlyGuardianOrGovernor {
20-
for (uint i = 0; i < targets.length; i++) {
21-
_cancelTransaction(targets[i], values[i], signatures[i], datas[i], etas[i]);
22-
}
18+
{
19+
// Only guardians and governors are timelock admins
20+
revokeRole(TIMELOCK_ADMIN_ROLE, msg.sender);
2321
}
2422

25-
function executeTransaction(address target, uint value, string memory signature, bytes memory data, uint eta) public override whenNotPaused payable returns (bytes memory) {
26-
return super.executeTransaction(target, value, signature, data, eta);
27-
}
23+
/**
24+
@notice allow guardian or governor to assume timelock admin roles
25+
This more elegantly achieves optimistic timelock as follows:
26+
- veto: grant self PROPOSER_ROLE and cancel
27+
- pause proposals: revoke PROPOSER_ROLE from target
28+
- pause execution: revoke EXECUTOR_ROLE from target
29+
- set new proposer: revoke old proposer and add new one
2830
29-
function governorSetPendingAdmin(address newAdmin) public onlyGovernor {
30-
pendingAdmin = newAdmin;
31-
emit NewPendingAdmin(newAdmin);
31+
In addition it allows for much more granular and flexible access for multisig operators
32+
*/
33+
function becomeAdmin() public onlyGuardianOrGovernor {
34+
this.grantRole(TIMELOCK_ADMIN_ROLE, msg.sender);
3235
}
3336
}

package-lock.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"@nomiclabs/hardhat-truffle5": "^2.0.0",
3737
"@nomiclabs/hardhat-waffle": "^2.0.1",
3838
"@nomiclabs/hardhat-web3": "^2.0.0",
39-
"@openzeppelin/contracts": "^4.1.0",
39+
"@openzeppelin/contracts": "^4.3.2",
4040
"@openzeppelin/test-environment": "^0.1.7",
4141
"@openzeppelin/test-helpers": "^0.5.4",
4242
"@typechain/web3-v1": "^3.0.0",
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { ethers } from "hardhat";
2+
import { expect } from "chai";
3+
4+
async function setup(addresses, oldContracts, contracts, logging) {}
5+
6+
/*
7+
1. Revoke TribalChief role from old timelock
8+
2. Grant TribalChief admin to new timelock
9+
*/
10+
async function run(addresses, oldContracts, contracts, logging = false) {
11+
const {
12+
core,
13+
tribalChief,
14+
optimisticTimelock
15+
} = contracts;
16+
17+
const {
18+
tribalChiefOptimisticTimelockAddress
19+
} = addresses;
20+
21+
const role = await tribalChief.CONTRACT_ADMIN_ROLE();
22+
23+
// 1.
24+
await core.revokeRole(role, tribalChiefOptimisticTimelockAddress);
25+
26+
// 2.
27+
await core.grantRole(role, optimisticTimelock.address);
28+
}
29+
30+
async function teardown(addresses, oldContracts, contracts, logging) {}
31+
32+
async function validate(addresses, oldContracts, contracts) {
33+
const {
34+
core,
35+
tribalChief,
36+
optimisticTimelock
37+
} = contracts;
38+
39+
const {
40+
tribalChiefOptimisticTimelockAddress,
41+
tribalChiefOptimisticMultisigAddress
42+
} = addresses;
43+
44+
const proposerRole = await optimisticTimelock.PROPOSER_ROLE();
45+
const executorRole = await optimisticTimelock.EXECUTOR_ROLE();
46+
const role = await tribalChief.CONTRACT_ADMIN_ROLE();
47+
48+
expect(await optimisticTimelock.hasRole(proposerRole, tribalChiefOptimisticMultisigAddress)).to.be.true;
49+
expect(await optimisticTimelock.hasRole(executorRole, tribalChiefOptimisticMultisigAddress)).to.be.true;
50+
51+
expect(await core.hasRole(role, optimisticTimelock.address)).to.be.true;
52+
expect(await core.hasRole(role, tribalChiefOptimisticTimelockAddress)).to.be.false;
53+
}
54+
55+
module.exports = {
56+
setup, run, teardown, validate
57+
};
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
const OptimisticTimelock = artifacts.require('OptimisticTimelock');
2+
3+
const fourDays = 4 * 24 * 60 * 60;
4+
5+
async function deploy(deployAddress, addresses, logging = false) {
6+
const {
7+
tribalChiefOptimisticMultisigAddress,
8+
coreAddress
9+
} = addresses;
10+
11+
if (
12+
!tribalChiefOptimisticMultisigAddress || !coreAddress
13+
) {
14+
throw new Error('An environment variable contract address is not set');
15+
}
16+
17+
const optimisticTimelock = await OptimisticTimelock.new(
18+
coreAddress,
19+
fourDays,
20+
[tribalChiefOptimisticMultisigAddress],
21+
[tribalChiefOptimisticMultisigAddress]
22+
);
23+
24+
return {
25+
optimisticTimelock
26+
};
27+
}
28+
29+
module.exports = { deploy };

test/integration/e2e.spec.ts

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,6 @@ describe('e2e', function () {
556556
describe('Optimistic Approval', async () => {
557557
beforeEach(async function () {
558558
const { tribalChiefOptimisticMultisigAddress, timelockAddress } = contractAddresses;
559-
const { tribalChiefOptimisticTimelock } = contracts;
560559

561560
await hre.network.provider.request({
562561
method: 'hardhat_impersonateAccount',
@@ -570,35 +569,55 @@ describe('e2e', function () {
570569

571570
await web3.eth.sendTransaction({from: deployAddress, to: tribalChiefOptimisticMultisigAddress, value: '40000000000000000'});
572571

573-
});
574-
it('governor can cancel a proposal', async () => {
575-
const { tribalChiefOptimisticMultisigAddress, timelockAddress } = contractAddresses;
576-
const { tribalChiefOptimisticTimelock } = contracts;
572+
await web3.eth.sendTransaction({from: timelockAddress, to: tribalChiefOptimisticMultisigAddress, value: '40000000000000000'});
577573

578-
await tribalChiefOptimisticTimelock.queueTransaction(deployAddress, 0, 'sig()', '0x', '10000000000000000', {from: tribalChiefOptimisticMultisigAddress});
579-
const hash = await tribalChiefOptimisticTimelock.getTxHash(deployAddress, 0, 'sig()', '0x', '10000000000000000');
580-
expect(await tribalChiefOptimisticTimelock.queuedTransactions(hash)).to.be.true;
574+
});
575+
it('governor can assume timelock admin', async () => {
576+
const { timelockAddress } = contractAddresses;
577+
const { optimisticTimelock } = contracts;
581578

582-
await tribalChiefOptimisticTimelock.vetoTransactions([deployAddress], [0], ['sig()'], ['0x'], ['10000000000000000'], {from: timelockAddress});
579+
await optimisticTimelock.becomeAdmin({from: timelockAddress});
583580

584-
expect(await tribalChiefOptimisticTimelock.queuedTransactions(hash)).to.be.false;
581+
const admin = await optimisticTimelock.TIMELOCK_ADMIN_ROLE();
582+
expect(await optimisticTimelock.hasRole(admin, timelockAddress)).to.be.true;
585583
});
586584

587585
it('proposal can execute on tribalChief', async () => {
588586
const { tribalChiefOptimisticMultisigAddress } = contractAddresses;
589-
const { tribalChiefOptimisticTimelock, tribalChief } = contracts;
587+
const { optimisticTimelock, tribalChief } = contracts;
590588

591589
const oldBlockReward = await tribalChief.tribePerBlock();
590+
await optimisticTimelock.schedule(
591+
tribalChief.address,
592+
0,
593+
'0xf580ffcb0000000000000000000000000000000000000000000000000000000000000001',
594+
'0x0000000000000000000000000000000000000000000000000000000000000000',
595+
'0x0000000000000000000000000000000000000000000000000000000000000001',
596+
'500000',
597+
{from: tribalChiefOptimisticMultisigAddress}
598+
);
592599

593-
await tribalChiefOptimisticTimelock.queueTransaction(tribalChief.address, 0, 'updateBlockReward(uint256)', '0x0000000000000000000000000000000000000000000000000000000000000001', '100000000000', {from: tribalChiefOptimisticMultisigAddress});
594-
const hash = await tribalChiefOptimisticTimelock.getTxHash(tribalChief.address, 0, 'updateBlockReward(uint256)', '0x0000000000000000000000000000000000000000000000000000000000000001', '100000000000');
595-
expect(await tribalChiefOptimisticTimelock.queuedTransactions(hash)).to.be.true;
600+
const hash = await optimisticTimelock.hashOperation(
601+
tribalChief.address,
602+
0,
603+
'0xf580ffcb0000000000000000000000000000000000000000000000000000000000000001',
604+
'0x0000000000000000000000000000000000000000000000000000000000000000',
605+
'0x0000000000000000000000000000000000000000000000000000000000000001',
606+
);
607+
expect(await optimisticTimelock.isOperationPending(hash)).to.be.true;
596608

597-
await time.increaseTo('100000000000');
598-
await tribalChiefOptimisticTimelock.executeTransaction(tribalChief.address, 0, 'updateBlockReward(uint256)', '0x0000000000000000000000000000000000000000000000000000000000000001', '100000000000', {from: tribalChiefOptimisticMultisigAddress});
609+
await time.increase('500000');
610+
await optimisticTimelock.execute(
611+
tribalChief.address,
612+
0,
613+
'0xf580ffcb0000000000000000000000000000000000000000000000000000000000000001',
614+
'0x0000000000000000000000000000000000000000000000000000000000000000',
615+
'0x0000000000000000000000000000000000000000000000000000000000000001',
616+
{from: tribalChiefOptimisticMultisigAddress}
617+
);
599618

600619
expect(await tribalChief.tribePerBlock()).to.be.bignumber.equal('1');
601-
expect(await tribalChiefOptimisticTimelock.queuedTransactions(hash)).to.be.false;
620+
expect(await optimisticTimelock.isOperationDone(hash)).to.be.true;
602621

603622
await tribalChief.updateBlockReward(oldBlockReward);
604623
});

test/integration/proposals_config.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
"proposerAddress" : "0xe0ac4559739bD36f0913FB0A3f5bFC19BCBaCD52",
66
"voterAddress" : "0xB8f482539F2d3Ae2C9ea6076894df36D1f632775"
77
},
8+
"optimisticTimelock" : {
9+
"deploy" : true,
10+
"exec" : false,
11+
"proposerAddress" : "0xe0ac4559739bD36f0913FB0A3f5bFC19BCBaCD52"
12+
},
813
"fip_22": {
914
"deploy" : false,
1015
"exec" : true,

test/integration/setup/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export type MainnetContracts = {
7676
feiTribeLBPSwapper: typeof Contract,
7777
aaveLendingPool: typeof Contract,
7878
aaveTribeIncentivesController: typeof Contract,
79+
optimisticTimelock: typeof Contract,
7980
}
8081

8182
export type MainnetContractAddresses = {

0 commit comments

Comments
 (0)