Conversation
xklob
left a comment
xklob
left a comment
There was a problem hiding this comment.
Beautiful unit tests. Left some comments; only thing that really needs changing imo is the this thing.
|
Didn't mean to actually approve the PR since integration tests haven't been written yet, oops. |
|
Pretty sure the proposals plugin is compatible with OZ Gov the way we use it: it uses a static call to get the pid, which would work for OZ gov. |
| override(IGovernor, Governor, GovernorCompatibilityBravo) | ||
| returns (uint256) | ||
| { | ||
| return super.propose(targets, values, calldatas, description); |
There was a problem hiding this comment.
Does this function need to be permissioned or does the function we call here enforce that?
There was a problem hiding this comment.
the supercall checks proposal threshold
test/unit/dao/FeiDao.test.ts
Outdated
|
|
||
| // Deploy timelock with vanilla user admin | ||
| const timelockDeployer = await ethers.getContractFactory(Timelock.abi, Timelock.bytecode); | ||
| this.timelock = await timelockDeployer.deploy(userAddress, 1, 1); // delay and min delay 1s |
There was a problem hiding this comment.
Since you already wrote this I hate to ask, but can you remove all references to this given the importance of the upgrade?
There was a problem hiding this comment.
Fixed. I didn't type Core because it had some upstream stuff I didn't want to mess with
3 participants
This PR upgrades the Fei governor to OZ Governor with the following changes:
TODO: