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.

Sign up for GitHub

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

Check for non-zero admin address when importing transparent proxy #887

Merged
merged 4 commits into from
Sep 28, 2023
Merged

Check for non-zero admin address when importing transparent proxy #887

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bd2e37e
Assert non-zero admin address when import transparent proxy
ericglau Sep 27, 2023
6fcf4e6
Update changelog
ericglau Sep 27, 2023
1eaed00
Update packages/plugin-hardhat/src/force-import.ts
ericglau Sep 27, 2023
6d58e19
Add more context to error message
ericglau Sep 28, 2023
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
4 changes: 4 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
View file
Open in desktop
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
11 changes: 11 additions & 0 deletions packages/plugin-hardhat/src/force-import.ts
View file
Open in desktop
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,14 @@ async function addAdminToManifest(
opts: ForceImportOptions,
) {
const adminAddress = await getAdminAddress(provider, proxyAddress);
if (isEmptySlot(adminAddress)) {
// Asser that the admin slot of a transparent proxy is not zero, otherwise the simulation below would fail due to no code at the address.
ericglau marked this conversation as resolved.
Show resolved Hide resolved
// Note: Transparent proxies should not have the zero address as the admin, according to TransparentUpgradeableProxy's _setAdmin function.
ernestognw marked this conversation as resolved.
Show resolved Hide resolved
throw new UpgradesError(
`Proxy at ${proxyAddress} doesn't look like a transparent proxy`,
() =>
`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
View file
Open in desktop
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
View file
Open in desktop
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