diff --git a/contracts/PushBravoAdmin.sol b/contracts/PushBravoAdmin.sol new file mode 100644 index 0000000..d79f3a8 --- /dev/null +++ b/contracts/PushBravoAdmin.sol @@ -0,0 +1,5 @@ +pragma solidity >=0.6.0 <0.7.0; + +import "@openzeppelin/contracts/proxy/ProxyAdmin.sol"; + +contract PushBravoAdmin is ProxyAdmin {} \ No newline at end of file diff --git a/contracts/PushBravoProxy.sol b/contracts/PushBravoProxy.sol index a85db2c..6c98fc1 100644 --- a/contracts/PushBravoProxy.sol +++ b/contracts/PushBravoProxy.sol @@ -7,6 +7,7 @@ import "@openzeppelin/contracts/proxy/TransparentUpgradeableProxy.sol"; contract PushBravoProxy is TransparentUpgradeableProxy { constructor( address _logic, + address _proxyAdmin, address _admin, address _timelock, address _push, @@ -18,7 +19,7 @@ contract PushBravoProxy is TransparentUpgradeableProxy { payable TransparentUpgradeableProxy( _logic, - _admin, + _proxyAdmin, abi.encodeWithSignature( "initialize(address,address,address,uint256,uint256,uint256)", _admin, diff --git a/scripts/deployGovernance.js b/scripts/deployGovernance.js index 266076c..b7ced1a 100644 --- a/scripts/deployGovernance.js +++ b/scripts/deployGovernance.js @@ -16,7 +16,7 @@ async function main() { await logic.waitForDeployment(); console.log("logic deployed on", logic.target); console.log("verifying logic"); - await logic.deploymentTransaction().wait(6); + await logic.deploymentTransaction().wait(4); try { await hre.run("verify:verify", { @@ -34,7 +34,7 @@ async function main() { console.log("timelock deployed to:", timelock.target); console.log("verifying timelock"); - await timelock.deploymentTransaction().wait(6); + await timelock.deploymentTransaction().wait(4); try { await hre.run("verify:verify", { @@ -45,9 +45,29 @@ async function main() { console.log("Verification failed :", error); } + console.log("deploying proxy Admin"); + + const proxyAdmin = await ethers.deployContract("PushBravoAdmin"); + console.log("proxyAdmin deployed to:", proxyAdmin.target); + + console.log("verifying proxy Admin"); + await proxyAdmin.deploymentTransaction().wait(4); + try { + await hre.run("verify:verify", { + address: proxyAdmin.target, + constructorArguments: [], + contract:"contracts/PushBravoAdmin.sol:PushBravoAdmin" + }); + } catch (error) { + console.log("Verification failed :", error); + } + + console.log("deploying proxy"); + const proxy = await ethers.deployContract("PushBravoProxy", [ logic.target, + proxyAdmin.target, _admin, timelock.target, _push, @@ -60,12 +80,13 @@ async function main() { console.log("proxy deployed to:", proxy.target); console.log("verifying proxy"); - await proxy.deploymentTransaction().wait(6); + await proxy.deploymentTransaction().wait(4); try { await hre.run("verify:verify", { address: proxy.target, constructorArguments: [ logic.target, + proxyAdmin.target, _admin, timelock.target, _push, diff --git a/test/GovernorBravo.js b/test/GovernorBravo.js index 83474b6..89ceba3 100644 --- a/test/GovernorBravo.js +++ b/test/GovernorBravo.js @@ -23,7 +23,7 @@ const { describe("Governor Bravo", function () { async function deployFixtures() { const [owner, otherAccount, owner2] = await ethers.getSigners(); - const { governorBravo, timelock, pushToken, governorBravoProxy } = + const { governorBravo, timelock, pushToken, proxyAdmin } = await setupGovernorBravo(); return { @@ -33,7 +33,7 @@ describe("Governor Bravo", function () { governorBravo, timelock, pushToken, - governorBravoProxy, + proxyAdmin, }; } @@ -45,11 +45,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -65,11 +70,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -83,6 +93,7 @@ describe("Governor Bravo", function () { await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -101,11 +112,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -119,6 +135,7 @@ describe("Governor Bravo", function () { await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -137,11 +154,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -157,6 +179,7 @@ describe("Governor Bravo", function () { await expect( GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, addresses[0], addresses[1], addresses[2], @@ -197,11 +220,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await expect( GovernorBravoDelegator.deploy( governorBravoDelegate, + proxyAdmin.target, addresses[0], addresses[2], ethers.zeroPadBytes("0x", 20), @@ -220,11 +248,16 @@ describe("Governor Bravo", function () { const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); const addresses = (await ethers.getSigners()).slice(3); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); await expect( GovernorBravoDelegator.deploy( governorBravoDelegate, + proxyAdmin.target, addresses[0], ethers.zeroPadBytes("0x", 20), addresses[2], @@ -416,7 +449,6 @@ describe("Governor Bravo", function () { governorBravo.connect(otherAccount).cancel(proposalId) ).to.be.revertedWith("GovernorBravo::cancel: proposer above threshold"); }); - }); describe("Vote", function () { @@ -774,7 +806,9 @@ describe("Governor Bravo", function () { }); it("Invalid address", async function () { - const { governorBravo, owner2 } = await loadFixture(deployFixtures); + const { governorBravo, owner, proxyAdmin } = await loadFixture( + deployFixtures + ); const GovernorBravoDelegator = await ethers.getContractFactory( "PushBravoProxy" ); @@ -782,25 +816,28 @@ describe("Governor Bravo", function () { await governorBravo.getAddress() ); await expect( - governorBravoDelegator.connect(owner2).upgradeTo(ethers.ZeroAddress) + proxyAdmin + .connect(owner) + .upgrade(governorBravoDelegator, ethers.ZeroAddress) ).to.be.revertedWith( "UpgradeableProxy: new implementation is not a contract" ); }); it("Happy path", async function () { - const { governorBravo, owner2 } = await loadFixture(deployFixtures); + const { governorBravo, owner, proxyAdmin } = await loadFixture( + deployFixtures + ); const GovernorBravoDelegator = await ethers.getContractFactory( "PushBravoProxy" ); const governorBravoDelegator = GovernorBravoDelegator.attach( await governorBravo.getAddress() ); - await governorBravoDelegator - .connect(owner2) - .upgradeTo(governorBravo.target); await expect( - governorBravoDelegator.connect(owner2).upgradeTo(governorBravo.target) + proxyAdmin + .connect(owner) + .upgrade(governorBravoDelegator.target, governorBravo.target) ).to.be.fulfilled; }); }); diff --git a/test/governanceHelpers.js b/test/governanceHelpers.js index 7994bf5..a35e65d 100644 --- a/test/governanceHelpers.js +++ b/test/governanceHelpers.js @@ -114,6 +114,9 @@ const setupGovernorBravo = async function setupGovernorBravo() { const GovernorBravoDelegator = await ethers.getContractFactory( "PushBravoProxy" ); + const GovernorBravoProxyAdmin = await ethers.getContractFactory( + "PushBravoAdmin" + ); const GovernorBravoDelegate = await ethers.getContractFactory( "GovernorBravoDelegate" ); @@ -126,8 +129,10 @@ const setupGovernorBravo = async function setupGovernorBravo() { await pushToken.delegate(owner); const governorBravoDelegate = await GovernorBravoDelegate.deploy(); + const proxyAdmin = await GovernorBravoProxyAdmin.deploy(); let governorBravo = await GovernorBravoDelegator.deploy( governorBravoDelegate.target, + proxyAdmin.target, owner, timelock, pushToken, @@ -135,7 +140,6 @@ const setupGovernorBravo = async function setupGovernorBravo() { 100, 500000n * 10n ** 18n ); - await governorBravo.connect(owner).changeAdmin(owner2.address); governorBravo = GovernorBravoDelegate.attach( await governorBravo.getAddress() @@ -161,7 +165,7 @@ const setupGovernorBravo = async function setupGovernorBravo() { await timelock.executeTransaction(timelock, 0, "", txData, eta); await governorBravo.acceptTimelockOwnership(); - return { governorBravo, timelock, pushToken }; + return { governorBravo, timelock, pushToken,proxyAdmin }; }; const getTypedDomain = async function getTypedDomain(address, chainId) {