Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add txOverrides option for overriding transaction parameters #852

Merged
merged 20 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ The following options are common to some functions.
** If set to `"always"`, the implementation contract is always redeployed even if it was previously deployed with the same bytecode. This can be used with the `salt` option when deploying a proxy through OpenZeppelin Platform to ensure that the implementation contract is deployed with the same salt as the proxy.
** If set to `"never"`, the implementation contract is never redeployed. If the implementation contract was not previously deployed or is not found in the network file, an error will be thrown.
** If set to `"onchange"`, the implementation contract is redeployed only if the bytecode has changed from previous deployments.
* `txOverrides`: (`ethers.Overrides`) An ethers.js https://docs.ethers.org/v6/api/contract/#Overrides[Overrides] object to override transaction parameters, such as `gasLimit` and `gasPrice`. Applies to all transactions sent by a function with this option, even if the function sends multiple transactions.
* `usePlatformDeploy`: (`boolean`) Deploy contracts using the OpenZeppelin Platform instead of ethers.js. See xref:platform-deploy.adoc[Using with OpenZeppelin Platform]. **Note**: OpenZeppelin Platform is in beta and functionality related to Platform is subject to change.
* `verifySourceCode`: (`boolean`) When using Platform, whether to verify source code on block explorers. Defaults to `true`.
* `relayerId`: (`string`) When using Platform, the ID of the relayer to use for the deployment. Defaults to the relayer configured for your deployment environment on Platform.
Expand Down Expand Up @@ -53,6 +54,7 @@ async function deployProxy(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
kind?: 'uups' | 'transparent',
usePlatformDeploy?: boolean,
},
Expand Down Expand Up @@ -92,6 +94,7 @@ async function upgradeProxy(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
kind?: 'uups' | 'transparent',
},
): Promise<ethers.Contract>
Expand Down Expand Up @@ -124,6 +127,7 @@ async function deployBeacon(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
},
): Promise<ethers.Contract>
----
Expand Down Expand Up @@ -160,6 +164,7 @@ async function upgradeBeacon(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
},
): Promise<ethers.Contract>
----
Expand Down Expand Up @@ -192,6 +197,7 @@ async function deployBeaconProxy(
args: unknown[] = [],
opts?: {
initializer?: string | false,
txOverrides?: ethers.Overrides,
usePlatformDeploy?: boolean,
},
): Promise<ethers.Contract>
Expand Down Expand Up @@ -290,6 +296,7 @@ async function deployImplementation(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
getTxResponse?: boolean,
kind?: 'uups' | 'transparent' | 'beacon',
usePlatformDeploy?: boolean,
Expand Down Expand Up @@ -381,6 +388,7 @@ async function prepareUpgrade(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: ethers.Overrides,
getTxResponse?: boolean,
kind?: 'uups' | 'transparent' | 'beacon',
usePlatformDeploy?: boolean,
Expand Down Expand Up @@ -566,6 +574,7 @@ async function deployProxyAdmin(
opts?: {
timeout?: number,
pollingInterval?: number,
txOverrides?: ethers.Overrides,
},
): Promise<string>
----
Expand Down Expand Up @@ -595,6 +604,9 @@ async function changeProxyAdmin(
proxyAddress: string,
newAdmin: string,
signer?: ethers.Signer,
opts?: {
txOverrides?: ethers.Overrides,
}
): Promise<void>
----

Expand All @@ -605,6 +617,8 @@ Changes the admin for a specific proxy.
* `proxyAddress` - the address of the proxy to change.
* `newAdmin` - the new admin address.
* `signer` - the signer to use for the transaction.
* `opts` - an object with options:
** additional options as described in <<common-options>>.

[[admin-transfer-proxy-admin-ownership]]
== admin.transferProxyAdminOwnership
Expand All @@ -614,6 +628,9 @@ Changes the admin for a specific proxy.
async function transferProxyAdminOwnership(
newAdmin: string,
signer?: ethers.Signer,
opts?: {
txOverrides?: ethers.Overrides,
}
): Promise<void>
----

Expand All @@ -623,6 +640,8 @@ Changes the owner of the proxy admin contract, which is the default admin for up

* `newAdmin` - the new admin address.
* `signer` - the signer to use for the transaction.
* `opts` - an object with options:
** additional options as described in <<common-options>>.

[[erc1967]]
== erc1967
Expand Down
22 changes: 22 additions & 0 deletions docs/modules/ROOT/pages/api-truffle-upgrades.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ The following options are common to some functions.
** If set to `"always"`, the implementation contract is always redeployed even if it was previously deployed with the same bytecode.
** If set to `"never"`, the implementation contract is never redeployed. If the implementation contract was not previously deployed or is not found in the network file, an error will be thrown.
** If set to `"onchange"`, the implementation contract is redeployed only if the bytecode has changed from previous deployments.
* `txOverrides`: (`TruffleTxOptions`) A Truffle https://trufflesuite.com/docs/truffle/how-to/contracts/run-migrations/#deployerdeploycontract-args-options[options] object to override transaction parameters, such as `gas` and `gasPrice`. Applies to all transactions sent by a function with this option, even if the function sends multiple transactions.

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?]

Expand All @@ -49,6 +50,7 @@ async function deployProxy(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
kind?: 'uups' | 'transparent',
},
): Promise<ContractInstance>
Expand Down Expand Up @@ -90,6 +92,7 @@ async function upgradeProxy(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
kind?: 'uups' | 'transparent',
},
): Promise<ContractInstance>
Expand Down Expand Up @@ -123,6 +126,7 @@ async function deployBeacon(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
},
): Promise<ContractInstance>
----
Expand Down Expand Up @@ -160,6 +164,7 @@ async function upgradeBeacon(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
},
): Promise<ContractInstance>
----
Expand Down Expand Up @@ -193,6 +198,7 @@ async function deployBeaconProxy(
opts?: {
deployer?: Deployer,
initializer?: string | false,
txOverrides?: TruffleTxOptions,
},
): Promise<ContractInstance>
----
Expand Down Expand Up @@ -290,6 +296,7 @@ async function deployImplementation(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
kind?: 'uups' | 'transparent' | 'beacon',
},
): Promise<string>
Expand Down Expand Up @@ -379,6 +386,7 @@ async function prepareUpgrade(
timeout?: number,
pollingInterval?: number,
redeployImplementation?: 'always' | 'never' | 'onchange',
txOverrides?: TruffleTxOptions,
kind?: 'uups' | 'transparent' | 'beacon',
},
): Promise<string>
Expand Down Expand Up @@ -407,6 +415,7 @@ async function deployProxyAdmin(
deployer?: Deployer,
timeout?: number,
pollingInterval?: number,
txOverrides?: TruffleTxOptions,
},
): Promise<string>
----
Expand Down Expand Up @@ -434,6 +443,10 @@ Deploys a https://docs.openzeppelin.com/contracts/4.x/api/proxy#ProxyAdmin[proxy
async function changeProxyAdmin(
proxyAddress: string,
newAdmin: string,
opts?: {
deployer?: Deployer,
txOverrides?: TruffleTxOptions,
},
): Promise<void>
----

Expand All @@ -443,6 +456,8 @@ Changes the admin for a specific proxy.

* `proxyAddress` - the address of the proxy to change.
* `newAdmin` - the new admin address.
* `opts` - an object with options:
** See <<common-options>>.

[[admin-transfer-proxy-admin-ownership]]
== admin.transferProxyAdminOwnership
Expand All @@ -451,6 +466,11 @@ Changes the admin for a specific proxy.
----
async function transferProxyAdminOwnership(
newAdmin: string,
opts?: {
deployer?: Deployer,
txOverrides?: TruffleTxOptions,
},

): Promise<void>
----

Expand All @@ -459,6 +479,8 @@ Changes the owner of the proxy admin contract, which is the default admin for up
*Parameters:*

* `newAdmin` - the new admin address.
* `opts` - an object with options:
** See <<common-options>>.

[[erc1967]]
== erc1967
Expand Down
4 changes: 4 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- Add `txOverrides` option for overriding transaction parameters. ([#852](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/852))

## 2.0.2 (2023-07-26)

- Enable storage layout for overrides from Hardhat config. ([#851](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/851))
Expand Down
31 changes: 24 additions & 7 deletions packages/plugin-hardhat/src/admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,33 @@ import chalk from 'chalk';
import type { HardhatRuntimeEnvironment } from 'hardhat/types';
import { Manifest, getAdminAddress } from '@openzeppelin/upgrades-core';
import { Contract, Signer } from 'ethers';
import { getProxyAdminFactory } from './utils';
import { EthersDeployOptions, getProxyAdminFactory } from './utils';
import { disablePlatform } from './platform/utils';
import { attach } from './utils/ethers';

const SUCCESS_CHECK = chalk.green('✔') + ' ';
const FAILURE_CROSS = chalk.red('✘') + ' ';

export type ChangeAdminFunction = (proxyAddress: string, newAdmin: string, signer?: Signer) => Promise<void>;
export type TransferProxyAdminOwnershipFunction = (newOwner: string, signer?: Signer) => Promise<void>;
export type ChangeAdminFunction = (
proxyAddress: string,
newAdmin: string,
signer?: Signer,
opts?: EthersDeployOptions,
) => Promise<void>;
export type TransferProxyAdminOwnershipFunction = (
newOwner: string,
signer?: Signer,
opts?: EthersDeployOptions,
) => Promise<void>;
export type GetInstanceFunction = (signer?: Signer) => Promise<Contract>;

export function makeChangeProxyAdmin(hre: HardhatRuntimeEnvironment, platformModule: boolean): ChangeAdminFunction {
return async function changeProxyAdmin(proxyAddress, newAdmin, signer?: Signer) {
return async function changeProxyAdmin(
proxyAddress: string,
newAdmin: string,
signer?: Signer,
opts: EthersDeployOptions = {},
) {
disablePlatform(hre, platformModule, {}, changeProxyAdmin.name);

const admin = await getManifestAdmin(hre, signer);
Expand All @@ -24,7 +38,8 @@ export function makeChangeProxyAdmin(hre: HardhatRuntimeEnvironment, platformMod
if (manifestAdminAddress !== proxyAdminAddress) {
throw new Error('Proxy admin is not the one registered in the network manifest');
} else if (manifestAdminAddress !== newAdmin) {
await admin.changeProxyAdmin(proxyAddress, newAdmin);
const overrides = opts.txOverrides ? [opts.txOverrides] : [];
await admin.changeProxyAdmin(proxyAddress, newAdmin, ...overrides);
}
};
}
Expand All @@ -33,11 +48,13 @@ export function makeTransferProxyAdminOwnership(
hre: HardhatRuntimeEnvironment,
platformModule: boolean,
): TransferProxyAdminOwnershipFunction {
return async function transferProxyAdminOwnership(newOwner, signer?: Signer) {
return async function transferProxyAdminOwnership(newOwner: string, signer?: Signer, opts: EthersDeployOptions = {}) {
disablePlatform(hre, platformModule, {}, transferProxyAdminOwnership.name);

const admin = await getManifestAdmin(hre, signer);
await admin.transferOwnership(newOwner);

const overrides = opts.txOverrides ? [opts.txOverrides] : [];
await admin.transferOwnership(newOwner, ...overrides);

const { provider } = hre.network;
const manifest = await Manifest.forNetwork(provider);
Expand Down
4 changes: 3 additions & 1 deletion packages/plugin-hardhat/src/upgrade-beacon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ export function makeUpgradeBeacon(hre: HardhatRuntimeEnvironment, platformModule

const UpgradeableBeaconFactory = await getUpgradeableBeaconFactory(hre, getSigner(ImplFactory.runner));
const beaconContract = attach(UpgradeableBeaconFactory, beaconAddress);
const upgradeTx = await beaconContract.upgradeTo(nextImpl);

const overrides = opts.txOverrides ? [opts.txOverrides] : [];
const upgradeTx = await beaconContract.upgradeTo(nextImpl, ...overrides);
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you just write upgradeTo(nextImpl, overrides)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would cause an error because then overrides appears as an array and Ethers tries to use it as part of the actual function arguments.

We need to spread the array because its element opts.txOverrides is the actual overrides object, and if that is not present, then it will be excluded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my question was actually if we write upgradeTo(nextImpl, opts.txOverrides) 🙂

Copy link
Member

@ericglau ericglau Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work, but we could use either:
upgradeTo(nextImpl, opts.txOverrides ?? {})
or
upgradeTo(nextImpl, withDefaults(opts).txOverrides)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that these options will work with Truffle, I recall it working weird if you pass an empty object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, these don't work for Truffle. Will leave them as-is for both Hardhat and Truffle, for consistency.


// @ts-ignore Won't be readonly because beaconContract was created through attach.
beaconContract.deployTransaction = upgradeTx;
Expand Down
13 changes: 9 additions & 4 deletions packages/plugin-hardhat/src/upgrade-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function makeUpgradeProxy(hre: HardhatRuntimeEnvironment, platformModule:

const { impl: nextImpl } = await deployProxyImpl(hre, ImplFactory, opts, proxyAddress);
// upgrade kind is inferred above
const upgradeTo = await getUpgrader(proxyAddress, getSigner(ImplFactory.runner));
const upgradeTo = await getUpgrader(proxyAddress, opts, getSigner(ImplFactory.runner));
const call = encodeCall(ImplFactory, opts.call);
const upgradeTx = await upgradeTo(nextImpl, call);

Expand All @@ -41,18 +41,21 @@ export function makeUpgradeProxy(hre: HardhatRuntimeEnvironment, platformModule:

type Upgrader = (nextImpl: string, call?: string) => Promise<ethers.TransactionResponse>;

async function getUpgrader(proxyAddress: string, signer?: Signer): Promise<Upgrader> {
async function getUpgrader(proxyAddress: string, opts: UpgradeProxyOptions, signer?: Signer): Promise<Upgrader> {
const { provider } = hre.network;

const adminAddress = await getAdminAddress(provider, proxyAddress);
const adminBytecode = await getCode(provider, adminAddress);

const overrides = opts.txOverrides ? [opts.txOverrides] : [];

if (isEmptySlot(adminAddress) || adminBytecode === '0x') {
// No admin contract: use ITransparentUpgradeableProxyFactory to get proxiable interface
const ITransparentUpgradeableProxyFactory = await getITransparentUpgradeableProxyFactory(hre, signer);
const proxy = attach(ITransparentUpgradeableProxyFactory, proxyAddress);

return (nextImpl, call) => (call ? proxy.upgradeToAndCall(nextImpl, call) : proxy.upgradeTo(nextImpl));
return (nextImpl, call) =>
call ? proxy.upgradeToAndCall(nextImpl, call, ...overrides) : proxy.upgradeTo(nextImpl, ...overrides);
} else {
// Admin contract: redirect upgrade call through it
const manifest = await Manifest.forNetwork(provider);
Expand All @@ -65,7 +68,9 @@ export function makeUpgradeProxy(hre: HardhatRuntimeEnvironment, platformModule:
}

return (nextImpl, call) =>
call ? admin.upgradeAndCall(proxyAddress, nextImpl, call) : admin.upgrade(proxyAddress, nextImpl);
call
? admin.upgradeAndCall(proxyAddress, nextImpl, call, ...overrides)
: admin.upgrade(proxyAddress, nextImpl, ...overrides);
}
}
}
Expand Down
11 changes: 7 additions & 4 deletions packages/plugin-hardhat/src/utils/deploy.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,33 @@
import type { Deployment, RemoteDeploymentId } from '@openzeppelin/upgrades-core';
import type { ethers, ContractFactory } from 'ethers';
import type { ethers, ContractFactory, ContractMethodArgs } from 'ethers';
import { HardhatRuntimeEnvironment } from 'hardhat/types';
import { platformDeploy } from '../platform/deploy';
import { PlatformDeployOptions, UpgradeOptions } from './options';
import { EthersDeployOptions, PlatformDeployOptions, UpgradeOptions } from './options';

export interface DeployTransaction {
deployTransaction?: ethers.TransactionResponse;
}

export async function deploy(
hre: HardhatRuntimeEnvironment,
opts: UpgradeOptions & PlatformDeployOptions,
opts: UpgradeOptions & EthersDeployOptions & PlatformDeployOptions,
factory: ContractFactory,
...args: unknown[]
): Promise<Required<Deployment> & DeployTransaction & RemoteDeploymentId> {
// platform always includes RemoteDeploymentId, while ethers always includes DeployTransaction
if (opts?.usePlatformDeploy) {
return await platformDeploy(hre, factory, opts, ...args);
} else {
if (opts.txOverrides !== undefined) {
args.push(opts.txOverrides);
}
return await ethersDeploy(factory, ...args);
}
}

async function ethersDeploy(
factory: ContractFactory,
...args: unknown[]
...args: ContractMethodArgs<unknown[]>
): Promise<Required<Deployment & DeployTransaction> & RemoteDeploymentId> {
const contractInstance = await factory.deploy(...args);

Expand Down
Loading