From 1824f94831765852e6c11944d483924ee3153ec3 Mon Sep 17 00:00:00 2001 From: "Guillermo M. Narvaja" Date: Thu, 5 Dec 2024 17:23:21 -0300 Subject: [PATCH] Options to change deploy function and proxy contract factory (#1104) Co-authored-by: Eric Lau --- .../ROOT/pages/api-hardhat-upgrades.adoc | 8 +++ packages/plugin-hardhat/CHANGELOG.md | 4 ++ .../contracts/AccessManagedProxy.sol | 30 ++++++++++ .../contracts/CustomBeaconProxy.sol | 24 ++++++++ packages/plugin-hardhat/package.json | 2 +- .../plugin-hardhat/src/defender/deploy.ts | 11 +--- .../plugin-hardhat/src/deploy-beacon-proxy.ts | 4 +- .../plugin-hardhat/src/deploy-contract.ts | 6 +- packages/plugin-hardhat/src/deploy-proxy.ts | 10 ++-- packages/plugin-hardhat/src/utils/deploy.ts | 10 +++- packages/plugin-hardhat/src/utils/options.ts | 25 ++++++++- .../test/beacon-custom-beacon-proxy.js | 56 +++++++++++++++++++ .../plugin-hardhat/test/uups-custom-proxy.js | 53 ++++++++++++++++++ 13 files changed, 219 insertions(+), 24 deletions(-) create mode 100644 packages/plugin-hardhat/contracts/AccessManagedProxy.sol create mode 100644 packages/plugin-hardhat/contracts/CustomBeaconProxy.sol create mode 100644 packages/plugin-hardhat/test/beacon-custom-beacon-proxy.js create mode 100644 packages/plugin-hardhat/test/uups-custom-proxy.js diff --git a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc index 11831b4e0..651b809e2 100644 --- a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc +++ b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc @@ -36,6 +36,10 @@ The following options are common to some functions. * `relayerId`: (`string`) When using OpenZeppelin Defender deployments, the ID of the relayer to use for the deployment. Defaults to the relayer configured for your deployment environment on Defender. * `salt`: (`string`) When using OpenZeppelin Defender deployments, if this is not set, deployments will be performed using the CREATE opcode. If this is set, deployments will be performed using the CREATE2 opcode with the provided salt. Note that deployments using a Safe are done using CREATE2 and require a salt. **Warning:** CREATE2 affects `msg.sender` behavior. See https://docs.openzeppelin.com/defender/tutorial/deploy#deploy-caveat[Caveats] for more information. * `metadata`: (`{ commitHash?: string; tag?: string; [k: string]: any; }`) When using OpenZeppelin Defender deployments, you can use this to identify, tag, or classify deployments. See https://docs.openzeppelin.com/defender/module/deploy#metadata[Metadata]. +* `proxyFactory`: (`ethers.ContractFactory`) Customizes the ethers contract factory to use for deploying the proxy, allowing a custom proxy contract to be deployed. See https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/master/packages/plugin-hardhat/src/utils/factories.ts[factories.ts] for the default contract factory for each kind of proxy. +** *Since:* `@openzeppelin/hardhat-upgrades@3.7.0` +* `deployFunction`: (`(hre, opts, factory, ...args) => Promise`) Customizes the function used to deploy the proxy. Can be used along with the `proxyFactory` option to override constructor parameters for custom proxy deployments. See https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/master/packages/plugin-hardhat/src/utils/deploy.ts[deploy.ts] for the default deploy function. +** *Since:* `@openzeppelin/hardhat-upgrades@3.7.0` Note that the options `unsafeAllow` can also be specified in a more granular way directly in the source code if using Solidity >=0.8.2. See xref:faq.adoc#how-can-i-disable-checks[How can I disable some of the checks?] @@ -65,6 +69,8 @@ async function deployProxy( txOverrides?: ethers.Overrides, kind?: 'uups' | 'transparent', useDefenderDeploy?: boolean, + proxyFactory?: ethers.ContractFactory, + deployFunction?: () => Promise, }, ): Promise ---- @@ -208,6 +214,8 @@ async function deployBeaconProxy( initializer?: string | false, txOverrides?: ethers.Overrides, useDefenderDeploy?: boolean, + proxyFactory?: ethers.ContractFactory, + deployFunction?: () => Promise, }, ): Promise ---- diff --git a/packages/plugin-hardhat/CHANGELOG.md b/packages/plugin-hardhat/CHANGELOG.md index 9f0b7efdd..97f40e6c4 100644 --- a/packages/plugin-hardhat/CHANGELOG.md +++ b/packages/plugin-hardhat/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 3.7.0 (2024-12-04) + +- Add `proxyFactory` and `deployFunction` options which can be used to deploy a custom proxy contract. ([#1104](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1104)) + ## 3.6.0 (2024-11-25) - Update Slang dependency to 0.18.3. ([#1102](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1102)) diff --git a/packages/plugin-hardhat/contracts/AccessManagedProxy.sol b/packages/plugin-hardhat/contracts/AccessManagedProxy.sol new file mode 100644 index 000000000..9cc234ec1 --- /dev/null +++ b/packages/plugin-hardhat/contracts/AccessManagedProxy.sol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +// Example of a custom proxy. +// This contract is for testing only. + +import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import {AccessManager} from "@openzeppelin/contracts/access/manager/AccessManager.sol"; +import {IAccessManager} from "@openzeppelin/contracts/access/manager/IAccessManager.sol"; +import {IAccessManaged} from "@openzeppelin/contracts/access/manager/IAccessManaged.sol"; + +contract AccessManagedProxy is ERC1967Proxy { + IAccessManager public immutable ACCESS_MANAGER; + + constructor(address implementation, bytes memory _data, IAccessManager manager) payable ERC1967Proxy(implementation, _data) { + ACCESS_MANAGER = manager; + } + + /** + * @dev Checks with the ACCESS_MANAGER if msg.sender is authorized to call the current call's function, + * and if so, delegates the current call to `implementation`. + * + * This function does not return to its internal call site, it will return directly to the external caller. + */ + function _delegate(address implementation) internal virtual override { + (bool immediate, ) = ACCESS_MANAGER.canCall(msg.sender, address(this), bytes4(msg.data[0:4])); + if (!immediate) revert IAccessManaged.AccessManagedUnauthorized(msg.sender); + super._delegate(implementation); + } +} diff --git a/packages/plugin-hardhat/contracts/CustomBeaconProxy.sol b/packages/plugin-hardhat/contracts/CustomBeaconProxy.sol new file mode 100644 index 000000000..656157ba6 --- /dev/null +++ b/packages/plugin-hardhat/contracts/CustomBeaconProxy.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol"; + +// Example of a custom beacon proxy. +// This contract is for testing only, it is not safe for use in production. + +contract CustomBeaconProxy is BeaconProxy { + address private immutable _deployer; + // The beacon that will be used on calls by the deployer address + address private immutable _deployerBeacon; + + constructor(address beacon, bytes memory data, address deployerBeacon) payable BeaconProxy(beacon, data) { + _deployer = msg.sender; + _deployerBeacon = deployerBeacon; + } + + function _getBeacon() internal view override returns (address) { + if (msg.sender == _deployer) return _deployerBeacon; + return super._getBeacon(); + } +} diff --git a/packages/plugin-hardhat/package.json b/packages/plugin-hardhat/package.json index 0489675d6..298f8dec9 100644 --- a/packages/plugin-hardhat/package.json +++ b/packages/plugin-hardhat/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/hardhat-upgrades", - "version": "3.6.0", + "version": "3.7.0", "description": "", "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/plugin-hardhat", "license": "MIT", diff --git a/packages/plugin-hardhat/src/defender/deploy.ts b/packages/plugin-hardhat/src/defender/deploy.ts index aad4699f9..1bc1e933b 100644 --- a/packages/plugin-hardhat/src/defender/deploy.ts +++ b/packages/plugin-hardhat/src/defender/deploy.ts @@ -9,12 +9,7 @@ import { DeployContractRequest, DeployRequestLibraries, } from '@openzeppelin/defender-sdk-deploy-client'; -import { - Deployment, - RemoteDeploymentId, - getContractNameAndRunValidation, - UpgradesError, -} from '@openzeppelin/upgrades-core'; +import { getContractNameAndRunValidation, UpgradesError } from '@openzeppelin/upgrades-core'; import artifactsBuildInfo from '@openzeppelin/upgrades-core/artifacts/build-info-v5.json'; @@ -24,7 +19,7 @@ import UpgradeableBeacon from '@openzeppelin/upgrades-core/artifacts/@openzeppel import TransparentUpgradeableProxy from '@openzeppelin/upgrades-core/artifacts/@openzeppelin/contracts-v5/proxy/transparent/TransparentUpgradeableProxy.sol/TransparentUpgradeableProxy.json'; import { getNetwork, parseTxOverrides } from './utils'; -import { DeployTransaction, DefenderDeployOptions, UpgradeOptions, EthersDeployOptions } from '../utils'; +import { DefenderDeployOptions, UpgradeOptions, EthersDeployOptions, DefenderDeployment } from '../utils'; import debug from '../utils/debug'; import { getDeployData } from '../utils/deploy-impl'; import { ContractSourceNotFoundError } from '@openzeppelin/upgrades-core'; @@ -60,7 +55,7 @@ export async function defenderDeploy( factory: ContractFactory, opts: UpgradeOptions & EthersDeployOptions & DefenderDeployOptions, ...args: unknown[] -): Promise & DeployTransaction> { +): Promise { const client = getDeployClient(hre); // Override constructor arguments in options with the ones passed as arguments to this function. diff --git a/packages/plugin-hardhat/src/deploy-beacon-proxy.ts b/packages/plugin-hardhat/src/deploy-beacon-proxy.ts index 68514532f..63686dbed 100644 --- a/packages/plugin-hardhat/src/deploy-beacon-proxy.ts +++ b/packages/plugin-hardhat/src/deploy-beacon-proxy.ts @@ -80,10 +80,10 @@ export function makeDeployBeaconProxy( ]); } - const BeaconProxyFactory = await getBeaconProxyFactory(hre, getSigner(attachTo.runner)); + const BeaconProxyFactory = opts.proxyFactory || (await getBeaconProxyFactory(hre, getSigner(attachTo.runner))); const proxyDeployment: Required & DeployTransaction & RemoteDeploymentId = Object.assign( { kind: opts.kind }, - await deploy(hre, opts, BeaconProxyFactory, beaconAddress, data), + await (opts.deployFunction || deploy)(hre, opts, BeaconProxyFactory, beaconAddress, data), ); await manifest.addProxy(proxyDeployment); diff --git a/packages/plugin-hardhat/src/deploy-contract.ts b/packages/plugin-hardhat/src/deploy-contract.ts index e29fa4962..080d7e677 100644 --- a/packages/plugin-hardhat/src/deploy-contract.ts +++ b/packages/plugin-hardhat/src/deploy-contract.ts @@ -1,12 +1,10 @@ import { HardhatRuntimeEnvironment } from 'hardhat/types'; import type { ContractFactory, Contract } from 'ethers'; -import { deploy, DeployContractOptions, DeployTransaction } from './utils'; +import { deploy, DeployContractOptions, EthersOrDefenderDeployment } from './utils'; import { DeployData, getDeployData } from './utils/deploy-impl'; import { enableDefender } from './defender/utils'; import { - Deployment, - RemoteDeploymentId, getContractNameAndRunValidation, inferProxyKind, UpgradesError, @@ -30,7 +28,7 @@ async function deployNonUpgradeableContract( assertNonUpgradeable(deployData); } - const deployment: Required & DeployTransaction & RemoteDeploymentId = await deploy( + const deployment: EthersOrDefenderDeployment = await deploy( hre, opts, Contract, diff --git a/packages/plugin-hardhat/src/deploy-proxy.ts b/packages/plugin-hardhat/src/deploy-proxy.ts index 1fdf68b0b..370464f56 100644 --- a/packages/plugin-hardhat/src/deploy-proxy.ts +++ b/packages/plugin-hardhat/src/deploy-proxy.ts @@ -51,6 +51,7 @@ export function makeDeployProxy(hre: HardhatRuntimeEnvironment, defenderModule: const contractInterface = ImplFactory.interface; const data = getInitializerData(contractInterface, args, opts.initializer); + const deployFn = opts.deployFunction || deploy; if (await manifest.getAdmin()) { if (kind === 'uups') { @@ -79,8 +80,8 @@ export function makeDeployProxy(hre: HardhatRuntimeEnvironment, defenderModule: throw new InitialOwnerUnsupportedKindError(kind); } - const ProxyFactory = await getProxyFactory(hre, signer); - proxyDeployment = Object.assign({ kind }, await deploy(hre, opts, ProxyFactory, impl, data)); + const ProxyFactory = opts.proxyFactory || (await getProxyFactory(hre, signer)); + proxyDeployment = Object.assign({ kind }, await deployFn(hre, opts, ProxyFactory, impl, data)); break; } @@ -95,10 +96,11 @@ export function makeDeployProxy(hre: HardhatRuntimeEnvironment, defenderModule: ); } - const TransparentUpgradeableProxyFactory = await getTransparentUpgradeableProxyFactory(hre, signer); + const TransparentUpgradeableProxyFactory = + opts.proxyFactory || (await getTransparentUpgradeableProxyFactory(hre, signer)); proxyDeployment = Object.assign( { kind }, - await deploy(hre, opts, TransparentUpgradeableProxyFactory, impl, initialOwner, data), + await deployFn(hre, opts, TransparentUpgradeableProxyFactory, impl, initialOwner, data), ); break; } diff --git a/packages/plugin-hardhat/src/utils/deploy.ts b/packages/plugin-hardhat/src/utils/deploy.ts index fab957310..954331726 100644 --- a/packages/plugin-hardhat/src/utils/deploy.ts +++ b/packages/plugin-hardhat/src/utils/deploy.ts @@ -8,13 +8,17 @@ export interface DeployTransaction { deployTransaction?: ethers.TransactionResponse; } +// Defender always includes RemoteDeploymentId, while ethers always includes DeployTransaction +export type EthersOrDefenderDeployment = Required & DeployTransaction & RemoteDeploymentId; +export type DefenderDeployment = Required & DeployTransaction; +export type EthersDeployment = Required & RemoteDeploymentId; + export async function deploy( hre: HardhatRuntimeEnvironment, opts: UpgradeOptions & EthersDeployOptions & DefenderDeployOptions, factory: ContractFactory, ...args: unknown[] -): Promise & DeployTransaction & RemoteDeploymentId> { - // defender always includes RemoteDeploymentId, while ethers always includes DeployTransaction +): Promise { if (opts?.useDefenderDeploy) { return await defenderDeploy(hre, factory, opts, ...args); } else { @@ -28,7 +32,7 @@ export async function deploy( async function ethersDeploy( factory: ContractFactory, ...args: ContractMethodArgs -): Promise & RemoteDeploymentId> { +): Promise { const contractInstance = await factory.deploy(...args); const deployTransaction = contractInstance.deploymentTransaction(); diff --git a/packages/plugin-hardhat/src/utils/options.ts b/packages/plugin-hardhat/src/utils/options.ts index cd2ba413c..3ee7954a1 100644 --- a/packages/plugin-hardhat/src/utils/options.ts +++ b/packages/plugin-hardhat/src/utils/options.ts @@ -6,7 +6,23 @@ import { ValidationOptions, withValidationDefaults, } from '@openzeppelin/upgrades-core'; -import { Overrides } from 'ethers'; +import { ContractFactory, Overrides } from 'ethers'; +import { EthersOrDefenderDeployment } from './deploy'; + +/** + * Options for customizing the factory or deploy functions + */ +export type DeployFactoryOpts = { + /** + * Allows to customize the ethers ContractFactory of the proxy to deploy, instead of using the ones defined in utils/factories.ts + */ + proxyFactory?: ContractFactory; + + /** + * Allows to customize the deploy function used instead of utils/deploy.ts:deploy + */ + deployFunction?: () => Promise; +}; /** * Options for functions that can deploy an implementation contract. @@ -91,6 +107,7 @@ export type InitialOwner = { export type DeployBeaconProxyOptions = EthersDeployOptions & DeployOpts & + DeployFactoryOpts & ProxyKindOption & Initializer & DefenderDeployOptions; @@ -101,7 +118,11 @@ export type DeployContractOptions = Omit & // DefenderDeployOptions & { unsafeAllowDeployContract?: boolean; }; -export type DeployProxyOptions = StandaloneOptions & Initializer & InitialOwner & DefenderDeployOptions; +export type DeployProxyOptions = StandaloneOptions & + DeployFactoryOpts & + Initializer & + InitialOwner & + DefenderDeployOptions; export type ForceImportOptions = ProxyKindOption; export type PrepareUpgradeOptions = UpgradeOptions & GetTxResponse & DefenderDeployOptions; export type UpgradeBeaconOptions = UpgradeOptions & DefenderDeploy; diff --git a/packages/plugin-hardhat/test/beacon-custom-beacon-proxy.js b/packages/plugin-hardhat/test/beacon-custom-beacon-proxy.js new file mode 100644 index 000000000..9a77472e5 --- /dev/null +++ b/packages/plugin-hardhat/test/beacon-custom-beacon-proxy.js @@ -0,0 +1,56 @@ +const test = require('ava'); + +const { ethers, upgrades } = require('hardhat'); +const { deploy } = require('../dist/utils/deploy'); + +test.before(async t => { + t.context.Greeter = await ethers.getContractFactory('Greeter'); + t.context.GreeterV2 = await ethers.getContractFactory('GreeterV2'); + t.context.GreeterV3 = await ethers.getContractFactory('GreeterV3'); + t.context.CustomBeaconProxy = await ethers.getContractFactory('CustomBeaconProxy'); + const [deployer, anon] = await ethers.getSigners(); + t.context.deployer = deployer; + t.context.anon = anon; +}); + +async function deployWithExtraProxyArgs(hre, opts, factory, ...args) { + const allArgs = [...args, ...(opts.proxyExtraConstructorArgs || [])]; + return deploy(hre, opts, factory, ...allArgs); +} + +test('custom beacon proxy factory and deploy function', async t => { + const { Greeter, GreeterV2, GreeterV3, CustomBeaconProxy, deployer, anon } = t.context; + + const greeterBeacon = await upgrades.deployBeacon(Greeter); + const greeterBeaconDeployer = await upgrades.deployBeacon(GreeterV2); + const greeter = await upgrades.deployBeaconProxy(greeterBeacon, Greeter, ['Hello, Hardhat!'], { + proxyFactory: CustomBeaconProxy, + deployFunction: deployWithExtraProxyArgs, + proxyExtraConstructorArgs: [await greeterBeaconDeployer.getAddress()], + }); + await greeter.waitForDeployment(); + t.is(await greeter.greet(), 'Hello, Hardhat!'); + + const greeterAsV2 = GreeterV2.attach(await greeter.getAddress()); + + // When calling from anon, uses Greeter as implementation and doesn't have resetGreeting method + let e = await t.throwsAsync(() => greeterAsV2.connect(anon).resetGreeting()); + t.true(e.message.includes('Transaction reverted: function selector was not recognized'), e.message); + + // When calling from deployer address, uses Greeter as implementation and doesn't have resetGreeting method + await greeterAsV2.connect(deployer).resetGreeting(); + + // For both changes the greet, because even when using different implementations, they share the storage + t.is(await greeterAsV2.connect(anon).greet(), 'Hello World'); + t.is(await greeterAsV2.connect(deployer).greet(), 'Hello World'); + + // Upgrade only the deployer beacon + await upgrades.upgradeBeacon(greeterBeaconDeployer, GreeterV3); + + const greeterAsV3 = GreeterV3.attach(await greeter.getAddress()); + + // When calling from anon, still uses Greeter as implementation and doesn't have version() method + e = await t.throwsAsync(() => greeterAsV3.connect(anon).version()); + t.true(e.message.includes('Transaction reverted: function selector was not recognized'), e.message); + t.is(await greeterAsV3.connect(deployer).version(), 'V3'); +}); diff --git a/packages/plugin-hardhat/test/uups-custom-proxy.js b/packages/plugin-hardhat/test/uups-custom-proxy.js new file mode 100644 index 000000000..fd1dd1441 --- /dev/null +++ b/packages/plugin-hardhat/test/uups-custom-proxy.js @@ -0,0 +1,53 @@ +const test = require('ava'); + +const { ethers, upgrades } = require('hardhat'); +const { deploy } = require('../dist/utils/deploy'); + +test.before(async t => { + t.context.Greeter = await ethers.getContractFactory('GreeterProxiable'); + t.context.GreeterV2 = await ethers.getContractFactory('GreeterV2Proxiable'); + t.context.GreeterV3 = await ethers.getContractFactory('GreeterV3Proxiable'); + t.context.AccessManagedProxy = await ethers.getContractFactory('AccessManagedProxy'); + const AccessManager = await ethers.getContractFactory('AccessManager'); + const [admin, anon] = await ethers.getSigners(); + t.context.admin = admin; + t.context.anon = anon; + t.context.acMgr = await AccessManager.deploy(admin); +}); + +async function deployWithExtraProxyArgs(hre, opts, factory, ...args) { + const allArgs = [...args, ...(opts.proxyExtraConstructorArgs || [])]; + return deploy(hre, opts, factory, ...allArgs); +} + +test('custom uups proxy factory and deploy function', async t => { + const { Greeter, GreeterV2, GreeterV3, AccessManagedProxy, acMgr, admin, anon } = t.context; + + const greeter = await upgrades.deployProxy(Greeter, ['Hello, Hardhat!'], { + kind: 'uups', + proxyExtraConstructorArgs: [await acMgr.getAddress()], + deployFunction: deployWithExtraProxyArgs, + proxyFactory: AccessManagedProxy, + }); + + // By default it calls from admin address, so, it works fine + let greet = await greeter.connect(admin).greet(); + t.is(greet, 'Hello, Hardhat!'); + // But fails when called from other user + let e = await t.throwsAsync(() => greeter.connect(anon).greet()); + t.true(e.message.includes('AccessManagedUnauthorized'), e.message); + + // Upgrades work well, because the call executed from the default signer that is the admin + const greeter2 = await upgrades.upgradeProxy(greeter, GreeterV2); + await greeter2.waitForDeployment(); + await greeter2.resetGreeting(); + + // Upgrades don't break the access control + e = await t.throwsAsync(() => greeter2.connect(anon).resetGreeting()); + t.true(e.message.includes('AccessManagedUnauthorized'), e.message); + + const greeter3ImplAddr = await upgrades.prepareUpgrade(await greeter.getAddress(), GreeterV3); + const greeter3 = GreeterV3.attach(greeter3ImplAddr); + const version3 = await greeter3.version(); + t.is(version3, 'V3'); +});