Skip to content

Commit

Permalink
Check for non-zero admin address when importing transparent proxy (#887)
Browse files Browse the repository at this point in the history
Co-authored-by: Ernesto García <ernestognw@gmail.com>
  • Loading branch information
ericglau and ernestognw authored Sep 28, 2023
1 parent c160d1a commit ba541bf
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 14 deletions.
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

- Check for non-zero admin address when importing transparent proxy. ([#887](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/887))

## 2.3.0 (2023-09-27)

- Support new upgrade interface in OpenZeppelin Contracts 5.0. ([#883](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/883))
Expand Down
12 changes: 12 additions & 0 deletions packages/plugin-hardhat/src/force-import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
ProxyDeployment,
hasCode,
NoContractImportError,
isEmptySlot,
UpgradesError,
} from '@openzeppelin/upgrades-core';

import {
Expand Down Expand Up @@ -108,5 +110,15 @@ async function addAdminToManifest(
opts: ForceImportOptions,
) {
const adminAddress = await getAdminAddress(provider, proxyAddress);
if (isEmptySlot(adminAddress)) {
// Assert that the admin slot of a transparent proxy is not zero, otherwise the simulation below would fail due to no code at the address.
// Note: Transparent proxies should not have the zero address as the admin, according to TransparentUpgradeableProxy's _setAdmin function.
throw new UpgradesError(
`Proxy at ${proxyAddress} doesn't look like a transparent proxy`,
() =>
`The proxy doesn't look like a transparent proxy because its admin address slot is empty. ` +
`Set the \`kind\` option to the kind of proxy that was deployed at ${proxyAddress} (either 'uups' or 'beacon')`,
);
}
await simulateDeployAdmin(hre, ImplFactory, opts, adminAddress);
}
17 changes: 10 additions & 7 deletions packages/plugin-hardhat/test/import-50.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function getInitializerData(contractInterface, args) {
return contractInterface.encodeFunctionData(fragment, args);
}

const REQUESTED_UPGRADE_WRONG_KIND = 'Requested an upgrade of kind uups but proxy is transparent';
const NOT_TRANSPARENT_PROXY = `doesn't look like a transparent proxy`;

test('implementation happy path', async t => {
const { GreeterProxiable } = t.context;
Expand Down Expand Up @@ -186,13 +186,16 @@ test('wrong kind', async t => {
);
await proxy.waitForDeployment();

// specify wrong kind
const greeter = await upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'transparent' });
t.is(await greeter.greet(), 'Hello, Hardhat!');
// specify wrong kind.
// an error is expected since the admin adress is zero
const e = await t.throwsAsync(async () =>
upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'transparent' }),
);
t.true(e.message.includes(NOT_TRANSPARENT_PROXY), e.message);

// an error is expected since the user force imported the wrong kind
const e = await t.throwsAsync(() => upgrades.upgradeProxy(greeter, GreeterV2Proxiable));
t.true(e.message.startsWith(REQUESTED_UPGRADE_WRONG_KIND), e.message);
// import with correct kind
const greeter = await upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'uups' });
await upgrades.upgradeProxy(greeter, GreeterV2Proxiable);
});

test('import custom UUPS proxy', async t => {
Expand Down
17 changes: 10 additions & 7 deletions packages/plugin-hardhat/test/import.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function getInitializerData(contractInterface, args) {
return contractInterface.encodeFunctionData(fragment, args);
}

const REQUESTED_UPGRADE_WRONG_KIND = 'Requested an upgrade of kind uups but proxy is transparent';
const NOT_TRANSPARENT_PROXY = `doesn't look like a transparent proxy`;

test('implementation happy path', async t => {
const { GreeterProxiable } = t.context;
Expand Down Expand Up @@ -180,13 +180,16 @@ test('wrong kind', async t => {
);
await proxy.waitForDeployment();

// specify wrong kind
const greeter = await upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'transparent' });
t.is(await greeter.greet(), 'Hello, Hardhat!');
// specify wrong kind.
// an error is expected since the admin adress is zero
const e = await t.throwsAsync(async () =>
upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'transparent' }),
);
t.true(e.message.includes(NOT_TRANSPARENT_PROXY), e.message);

// an error is expected since the user force imported the wrong kind
const e = await t.throwsAsync(() => upgrades.upgradeProxy(greeter, GreeterV2Proxiable));
t.true(e.message.startsWith(REQUESTED_UPGRADE_WRONG_KIND), e.message);
// import with correct kind
const greeter = await upgrades.forceImport(await proxy.getAddress(), GreeterProxiable, { kind: 'uups' });
await upgrades.upgradeProxy(greeter, GreeterV2Proxiable);
});

test('import custom UUPS proxy', async t => {
Expand Down

0 comments on commit ba541bf

Please sign in to comment.