Skip to content

Commit

Permalink
Defender: Fix proxy deployments when using constructorArgs (#1029)
Browse files Browse the repository at this point in the history 
Co-authored-by: Hadrien Croubois <[email protected]>
  • Loading branch information
@ericglau @Amxx
ericglau and Amxx authored Jun 3, 2024
1 parent 9badcc6 commit af383ff
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 27 deletions.
4 changes: 4 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 3.1.1 (2024-06-03)

- Defender: Fix proxy deployments when using `constructorArgs` option, support arbitrary constructor arguments. ([#1029](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1029))

## 3.1.0 (2024-04-22)

- Defender: Fix handling of license types for block explorer verification, support `licenseType` and `skipLicenseType` options. ([#1013](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1013))
Expand Down
9 changes: 9 additions & 0 deletions packages/plugin-hardhat/contracts/Constructor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,13 @@ contract WithConstructor {
constructor(uint256 args) {
value = args;
}
}

contract WithConstructorArray {
uint256[] public x;

/// @custom:oz-upgrades-unsafe-allow constructor
constructor(uint256[] memory args) {
x = args;
}
}
2 changes: 1 addition & 1 deletion packages/plugin-hardhat/package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openzeppelin/hardhat-upgrades",
"version": "3.1.0",
"version": "3.1.1",
"description": "",
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/plugin-hardhat",
"license": "MIT",
Expand Down
22 changes: 16 additions & 6 deletions packages/plugin-hardhat/src/defender/deploy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ interface ContractInfo {
contractName: string;
buildInfo: ReducedBuildInfo;
libraries?: DeployRequestLibraries;
constructorBytecode: string;
}

type CompilerOutputWithMetadata = CompilerOutputContract & {
Expand All @@ -62,8 +63,10 @@ export async function defenderDeploy(
): Promise<Required<Deployment & RemoteDeploymentId> & DeployTransaction> {
const client = getDeployClient(hre);

const constructorArgs = [...args] as (string | number | boolean)[];
const contractInfo = await getContractInfo(hre, factory, { constructorArgs, ...opts });
// Override constructor arguments in options with the ones passed as arguments to this function.
// The ones in the options are for implementation contracts only, while this function
// can be used to deploy proxies as well.
const contractInfo = await getContractInfo(hre, factory, { ...opts, constructorArgs: args });
const network = await getNetwork(hre);
debug(`Network ${network}`);

Expand Down Expand Up @@ -103,7 +106,7 @@ export async function defenderDeploy(
network: network,
artifactPayload: JSON.stringify(contractInfo.buildInfo),
licenseType: licenseType,
constructorInputs: constructorArgs,
constructorBytecode: contractInfo.constructorBytecode,
verifySourceCode: verifySourceCode,
relayerId: opts.relayerId,
salt: opts.salt,
Expand Down Expand Up @@ -159,13 +162,15 @@ export async function defenderDeploy(
async function getContractInfo(
hre: HardhatRuntimeEnvironment,
factory: ethers.ContractFactory,
opts: UpgradeOptions,
opts: UpgradeOptions & Required<Pick<UpgradeOptions, 'constructorArgs'>>,
): Promise<ContractInfo> {
let fullContractName, runValidation;
let libraries: DeployRequestLibraries | undefined;
let constructorBytecode: string;
try {
// Get fully qualified contract name and link references from validations
const deployData = await getDeployData(hre, factory, opts);
constructorBytecode = deployData.encodedArgs;
[fullContractName, runValidation] = getContractNameAndRunValidation(deployData.validations, deployData.version);
debug(`Contract ${fullContractName}`);

Expand Down Expand Up @@ -193,7 +198,12 @@ async function getContractInfo(
const contractName = artifact.contractName;
const buildInfo = artifactsBuildInfo;
debug(`Proxy contract ${sourceName}:${contractName}`);
return { sourceName, contractName, buildInfo };
return {
sourceName,
contractName,
buildInfo,
constructorBytecode: factory.interface.encodeDeploy(opts.constructorArgs),
};
}
}
}
Expand All @@ -211,7 +221,7 @@ async function getContractInfo(
);
}

return { sourceName, contractName, buildInfo, libraries };
return { sourceName, contractName, buildInfo, libraries, constructorBytecode };
}

/**
Expand Down
125 changes: 105 additions & 20 deletions packages/plugin-hardhat/test/defender-deploy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const {
} = require('../dist/utils/factories');
const artifactsBuildInfo = require('@openzeppelin/upgrades-core/artifacts/build-info-v5.json');

const { AbiCoder } = require('ethers');

const TX_HASH = '0x1';
const DEPLOYMENT_ID = 'abc';
const ADDRESS = '0x2';
Expand Down Expand Up @@ -100,7 +102,7 @@ test('calls defender deploy', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -128,7 +130,7 @@ test('calls defender deploy with relayerId', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: RELAYER_ID,
salt: undefined,
Expand Down Expand Up @@ -156,7 +158,7 @@ test('calls defender deploy with salt', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: SALT,
Expand Down Expand Up @@ -184,7 +186,7 @@ test('calls defender deploy with createFactoryAddress', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -212,7 +214,7 @@ test('calls defender deploy with license', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: 'MIT',
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -242,7 +244,7 @@ test('calls defender deploy - licenseType', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: 'My License Type',
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -272,7 +274,7 @@ test('calls defender deploy - verifySourceCode false', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: false,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -302,7 +304,7 @@ test('calls defender deploy - skipLicenseType', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -383,7 +385,7 @@ test('calls defender deploy - no contract license', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -411,7 +413,7 @@ test('calls defender deploy - unlicensed', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: 'None',
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -439,7 +441,35 @@ test('calls defender deploy with constructor args', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: 'MIT',
constructorInputs: [10],
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['uint256'], [10]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
createFactoryAddress: undefined,
txOverrides: undefined,
libraries: undefined,
});

assertResult(t, result);
});

test('calls defender deploy with constructor args with array', async t => {
const { spy, deploy, fakeHre, fakeChainId } = t.context;

const contractPath = 'contracts/Constructor.sol';
const contractName = 'WithConstructorArray';

const factory = await ethers.getContractFactory(contractName);
const result = await deploy.defenderDeploy(fakeHre, factory, {}, [1, 2, 3]);

const buildInfo = await hre.artifacts.getBuildInfo(`${contractPath}:${contractName}`);
sinon.assert.calledWithExactly(spy, {
contractName: contractName,
contractPath: contractPath,
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: 'MIT',
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['uint256[]'], [[1, 2, 3]]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -467,7 +497,7 @@ test('calls defender deploy with verify false', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: false,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -495,7 +525,59 @@ test('calls defender deploy with ERC1967Proxy', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(artifactsBuildInfo),
licenseType: 'MIT',
constructorInputs: [LOGIC_ADDRESS, DATA],
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['address', 'bytes'], [LOGIC_ADDRESS, DATA]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
createFactoryAddress: undefined,
txOverrides: undefined,
libraries: undefined,
});
});

test('calls defender deploy with ERC1967Proxy - ignores constructorArgs', async t => {
const { spy, deploy, fakeHre, fakeChainId } = t.context;

const contractPath = '@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol';
const contractName = 'ERC1967Proxy';
const factory = await getProxyFactory(hre);

const result = await deploy.defenderDeploy(fakeHre, factory, { constructorArgs: ['foo'] }, LOGIC_ADDRESS, DATA);
assertResult(t, result);

sinon.assert.calledWithExactly(spy, {
contractName: contractName,
contractPath: contractPath,
network: fakeChainId,
artifactPayload: JSON.stringify(artifactsBuildInfo),
licenseType: 'MIT',
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['address', 'bytes'], [LOGIC_ADDRESS, DATA]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
createFactoryAddress: undefined,
txOverrides: undefined,
libraries: undefined,
});
});

test('calls defender deploy with ERC1967Proxy - ignores empty constructorArgs', async t => {
const { spy, deploy, fakeHre, fakeChainId } = t.context;

const contractPath = '@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol';
const contractName = 'ERC1967Proxy';
const factory = await getProxyFactory(hre);

const result = await deploy.defenderDeploy(fakeHre, factory, { constructorArgs: [] }, LOGIC_ADDRESS, DATA);
assertResult(t, result);

sinon.assert.calledWithExactly(spy, {
contractName: contractName,
contractPath: contractPath,
network: fakeChainId,
artifactPayload: JSON.stringify(artifactsBuildInfo),
licenseType: 'MIT',
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['address', 'bytes'], [LOGIC_ADDRESS, DATA]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand All @@ -521,7 +603,7 @@ test('calls defender deploy with BeaconProxy', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(artifactsBuildInfo),
licenseType: 'MIT',
constructorInputs: [LOGIC_ADDRESS, DATA],
constructorBytecode: AbiCoder.defaultAbiCoder().encode(['address', 'bytes'], [LOGIC_ADDRESS, DATA]),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand All @@ -547,7 +629,10 @@ test('calls defender deploy with TransparentUpgradeableProxy', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(artifactsBuildInfo),
licenseType: 'MIT',
constructorInputs: [LOGIC_ADDRESS, INITIAL_OWNER_ADDRESS, DATA],
constructorBytecode: AbiCoder.defaultAbiCoder().encode(
['address', 'address', 'bytes'],
[LOGIC_ADDRESS, INITIAL_OWNER_ADDRESS, DATA],
),
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand All @@ -573,7 +658,7 @@ test('calls defender deploy with txOverrides.gasLimit', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -606,7 +691,7 @@ test('calls defender deploy with txOverrides.gasPrice', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -641,7 +726,7 @@ test('calls defender deploy with txOverrides.maxFeePerGas and txOverrides.maxPri
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -678,7 +763,7 @@ test('calls defender deploy with external library', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down Expand Up @@ -713,7 +798,7 @@ test('calls defender deploy with multiple external libraries', async t => {
network: fakeChainId,
artifactPayload: JSON.stringify(buildInfo),
licenseType: undefined,
constructorInputs: [],
constructorBytecode: '0x',
verifySourceCode: true,
relayerId: undefined,
salt: undefined,
Expand Down

0 comments on commit af383ff

Please sign in to comment.