From 4bd7310ff21cc19ad2dc8c2f97381df841e85842 Mon Sep 17 00:00:00 2001 From: Nenad Date: Mon, 18 Sep 2023 09:29:03 +0200 Subject: [PATCH] Move checkIfProxyAdmin to checkIfManagedByProxyAdmin --- tasks/manage/changeProxyAdmin.ts | 66 +------------- .../updateEndowmentProxiesAdmin.ts | 90 +++++++++++++++++-- 2 files changed, 84 insertions(+), 72 deletions(-) diff --git a/tasks/manage/changeProxyAdmin.ts b/tasks/manage/changeProxyAdmin.ts index e73683328..ebeae0775 100644 --- a/tasks/manage/changeProxyAdmin.ts +++ b/tasks/manage/changeProxyAdmin.ts @@ -1,14 +1,7 @@ import {task} from "hardhat/config"; -import { - ITransparentUpgradeableProxy__factory, - ProxyAdminMultiSig__factory, - ProxyContract__factory, -} from "typechain-types"; +import {ITransparentUpgradeableProxy__factory, ProxyContract__factory} from "typechain-types"; import {confirmAction, getAddresses, getProxyAdminOwner, logger} from "utils"; import {submitMultiSigTx} from "../helpers"; -import {HardhatRuntimeEnvironment} from "hardhat/types"; - -class CheckError extends Error {} type TaskArgs = { to?: string; @@ -51,8 +44,6 @@ task("manage:changeProxyAdmin", "Will update the proxy admin the target proxy co return logger.out(`"${targetAddress}" is already the proxy admin.`); } - await checkIfProxyAdmin(curAdmin, proxyAdminOwner.address, hre); - logger.out(`Current Admin: ${curAdmin}`); // submitting the Tx @@ -78,59 +69,6 @@ task("manage:changeProxyAdmin", "Will update the proxy admin the target proxy co } } } catch (error) { - if (error instanceof CheckError) { - logger.out(error, logger.Level.Warn); - } else { - logger.out(error, logger.Level.Error); - } + logger.out(error, logger.Level.Error); } }); - -/** - * It is possible the Endowment's proxy is not managed by our ProxyAdminMultiSig. - * In those cases we should skip submitting any Txs to avoid wasting gas. - * @param adminAddress Address of the current proxy admin - * @param adminOwner Address of one of the current proxy admin's owners - * @param hre HardhatRuntimeEnvironment - */ -async function checkIfProxyAdmin( - adminAddress: string, - adminOwner: string, - hre: HardhatRuntimeEnvironment -): Promise { - // this is just a rudimendaty check that will throw if `curAdmin` is not really a ProxyAdminMultiSig - // in which case it'll throw and stop the execution. We check this by assuming that if the address - // is connected to a contract that contains the `submitTransaction` function, then there's a high - // likelihood that the said contract is ProxyAdminMultiSig - const proxyAdminMS = ProxyAdminMultiSig__factory.connect(adminAddress, hre.ethers.provider); - const bytecode = await hre.ethers.provider.getCode(adminAddress); - - // No code : "0x" then functionA is definitely not there - if (bytecode.length <= 2) { - throw new CheckError("Skipping: admin address has no code"); - } - - // If the bytecode doesn't include the function selector submitTransaction() - // is definitely not present - const submitTransactionSelector = proxyAdminMS.interface.getSighash( - proxyAdminMS.interface.functions["submitTransaction(address,uint256,bytes,bytes)"] - ); - if (!bytecode.includes(submitTransactionSelector.slice(2))) { - throw new CheckError( - "Skipping: not a ProxyAdminMultiSig - no submitTransaction() function selector in bytecode" - ); - } - - const isOwnersSelector = proxyAdminMS.interface.getSighash( - proxyAdminMS.interface.functions["isOwner(address)"] - ); - if (!bytecode.includes(isOwnersSelector.slice(2))) { - throw new CheckError( - "Skipping: not a ProxyAdminMultiSig - no isOwner() function selector in bytecode" - ); - } - - if (!(await proxyAdminMS.isOwner(adminOwner))) { - throw new CheckError(`Skipping: "${adminOwner}" is not one of the target contract's owners`); - } -} diff --git a/tasks/manage/endowmentMultiSigFactory/updateProxyAdmin/updateEndowmentProxiesAdmin.ts b/tasks/manage/endowmentMultiSigFactory/updateProxyAdmin/updateEndowmentProxiesAdmin.ts index 629665e45..9d43aefec 100644 --- a/tasks/manage/endowmentMultiSigFactory/updateProxyAdmin/updateEndowmentProxiesAdmin.ts +++ b/tasks/manage/endowmentMultiSigFactory/updateProxyAdmin/updateEndowmentProxiesAdmin.ts @@ -1,6 +1,12 @@ import {HardhatRuntimeEnvironment} from "hardhat/types"; -import {IEndowmentMultiSigFactory__factory} from "typechain-types"; -import {getAddresses} from "utils"; +import { + IEndowmentMultiSigFactory__factory, + ProxyAdminMultiSig__factory, + ProxyContract__factory, +} from "typechain-types"; +import {getAddresses, getProxyAdminOwner, logger} from "utils"; + +class CheckError extends Error {} export default async function updateEndowmentProxiesAdmin( targetAddress: string, @@ -8,6 +14,7 @@ export default async function updateEndowmentProxiesAdmin( hre: HardhatRuntimeEnvironment ) { const addresses = await getAddresses(hre); + const proxyAdminOwner = await getProxyAdminOwner(hre, proxyAdminPkey); const endowmentMultiSigFactory = IEndowmentMultiSigFactory__factory.connect( addresses.multiSig.endowment.factory, @@ -16,11 +23,78 @@ export default async function updateEndowmentProxiesAdmin( const endowmentProxies = await endowmentMultiSigFactory.getInstantiations(); for (const endowmentProxy of endowmentProxies) { - await hre.run("manage:changeProxyAdmin", { - to: targetAddress, - proxy: endowmentProxy, - proxyAdminPkey: proxyAdminPkey, - yes: true, - }); + try { + await checkIfManagedByProxyAdmin(endowmentProxy, proxyAdminOwner.address, hre); + + await hre.run("manage:changeProxyAdmin", { + to: targetAddress, + proxy: endowmentProxy, + proxyAdminPkey: proxyAdminPkey, + yes: true, + }); + } catch (error) { + if (error instanceof CheckError) { + logger.out(error, logger.Level.Warn); + } else { + logger.out(error, logger.Level.Error); + } + } + } +} + +/** + * It is possible the Endowment's proxy is not managed by our ProxyAdminMultiSig. + * In those cases we should skip submitting any Txs to avoid wasting gas. + * + * This is just a rudimendaty check that will throw if `curAdmin` is not really a ProxyAdminMultiSig + * in which case it'll throw and stop the execution. We check this by assuming that if the address + * is connected to a contract that contains the `submitTransaction` and `isOwner` functions and also + * that the currently used proxy admin owner is really one of the owners of of the proxy's admin contract, + * then it's most likely that the said proxy admin contract is an instance of ProxyAdminMultiSig. + * + * @param proxy Address of the proxy to check + * @param proxyAdminOwner Address of one of the current proxy admin's owners + * @param hre HardhatRuntimeEnvironment + */ +async function checkIfManagedByProxyAdmin( + proxy: string, + proxyAdminOwner: string, + hre: HardhatRuntimeEnvironment +): Promise { + const proxyContract = ProxyContract__factory.connect(proxy, hre.ethers.provider); + const curAdmin = await proxyContract.getAdmin(); + + const proxyAdminMS = ProxyAdminMultiSig__factory.connect(curAdmin, hre.ethers.provider); + const bytecode = await hre.ethers.provider.getCode(curAdmin); + + // No code : "0x" then functionA is definitely not there + if (bytecode.length <= 2) { + throw new CheckError("Skipping: admin address has no code"); + } + + // If the bytecode doesn't include the function selector submitTransaction() + // is definitely not present + const submitTransactionSelector = proxyAdminMS.interface.getSighash( + proxyAdminMS.interface.functions["submitTransaction(address,uint256,bytes,bytes)"] + ); + if (!bytecode.includes(submitTransactionSelector.slice(2))) { + throw new CheckError( + "Skipping: not a ProxyAdminMultiSig - no submitTransaction() function selector in bytecode" + ); + } + + const isOwnersSelector = proxyAdminMS.interface.getSighash( + proxyAdminMS.interface.functions["isOwner(address)"] + ); + if (!bytecode.includes(isOwnersSelector.slice(2))) { + throw new CheckError( + "Skipping: not a ProxyAdminMultiSig - no isOwner() function selector in bytecode" + ); + } + + if (!(await proxyAdminMS.isOwner(proxyAdminOwner))) { + throw new CheckError( + `Skipping: "${proxyAdminOwner}" is not one of the target contract's owners` + ); } }