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

Co-authored-by: Ernesto García <ernestognw@gmail.com>
OpenZeppelin · Sep 28, 2023 · ba541bf · ba541bf
1 parent c160d1a
commit ba541bf
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 14 deletions.
diff --git a/packages/plugin-hardhat/CHANGELOG.md b/packages/plugin-hardhat/CHANGELOG.md
@@ -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))

diff --git a/packages/plugin-hardhat/src/force-import.ts b/packages/plugin-hardhat/src/force-import.ts
@@ -13,6 +13,8 @@ import {
   ProxyDeployment,
   hasCode,
   NoContractImportError,
+  isEmptySlot,
+  UpgradesError,
 } from '@openzeppelin/upgrades-core';
 
 import {
@@ -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);
 }
diff --git a/packages/plugin-hardhat/test/import-50.js b/packages/plugin-hardhat/test/import-50.js
@@ -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;
@@ -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 => {

diff --git a/packages/plugin-hardhat/test/import.js b/packages/plugin-hardhat/test/import.js
@@ -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;
@@ -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 => {