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

Use upgradeToAndCall depending on upgrade interface version #883

Merged
merged 15 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 2 additions & 0 deletions docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,8 @@ async function changeProxyAdmin(

Changes the admin for a specific proxy.

NOTE: This function is not supported with admins or proxies from OpenZeppelin Contracts 5.x.

*Parameters:*

* `proxyAddress` - the address of the proxy to change.
Expand Down
1 change: 1 addition & 0 deletions packages/core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- Support new upgrade interface in OpenZeppelin Contracts 5.0. ([883](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/883))
- Add validations for namespaced storage layout. ([#876](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/876))
- Deprecate low-level API. Use [CLI or high-level API](https://docs.openzeppelin.com/upgrades-plugins/1.x/api-core) instead.

Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openzeppelin/upgrades-core",
"version": "1.29.0",
"version": "1.30.0",
"description": "",
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core",
"license": "MIT",
Expand Down
22 changes: 22 additions & 0 deletions packages/core/src/call-optional-signature.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { keccak256 } from 'ethereumjs-util';
import { call, EthereumProvider } from './provider';

export async function callOptionalSignature(provider: EthereumProvider, address: string, signature: string) {
ericglau marked this conversation as resolved.
Show resolved Hide resolved
const data = '0x' + keccak256(Buffer.from(signature)).toString('hex').slice(0, 8);
try {
return await call(provider, address, data);
} catch (e: any) {
if (
!(
e.message.includes('function selector was not recognized') ||
e.message.includes('invalid opcode') ||
e.message.includes('revert') ||
e.message.includes('execution error')
)
) {
throw e;
} else {
return undefined;
}
ericglau marked this conversation as resolved.
Show resolved Hide resolved
}
}
29 changes: 9 additions & 20 deletions packages/core/src/impl-address.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { keccak256 } from 'ethereumjs-util';
import {
call,
EIP1967BeaconNotFound,
EIP1967ImplementationNotFound,
getBeaconAddress,
getImplementationAddress,
UpgradesError,
} from '.';
import { callOptionalSignature } from './call-optional-signature';

import { EthereumProvider } from './provider';
import { parseAddress } from './utils/address';
Expand All @@ -24,27 +23,17 @@ export async function getImplementationAddressFromBeacon(
provider: EthereumProvider,
beaconAddress: string,
): Promise<string> {
const implementationFunction = '0x' + keccak256(Buffer.from('implementation()')).toString('hex').slice(0, 8);
let result: string | undefined;
try {
const implAddress = await call(provider, beaconAddress, implementationFunction);
result = parseAddress(implAddress);
} catch (e: any) {
if (
!(
e.message.includes('function selector was not recognized') ||
e.message.includes('invalid opcode') ||
e.message.includes('revert') ||
e.message.includes('execution error')
)
) {
throw e;
} // otherwise fall through with no result
const impl = await callOptionalSignature(provider, beaconAddress, 'implementation()');
let parsedImplAddress;
if (impl !== undefined) {
parsedImplAddress = parseAddress(impl);
}
if (result === undefined) {

if (parsedImplAddress === undefined) {
throw new InvalidBeacon(`Contract at ${beaconAddress} doesn't look like a beacon`);
} else {
return parsedImplAddress;
}
return result;
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,5 @@ export {

export { ValidateUpgradeSafetyOptions, validateUpgradeSafety, ProjectReport, ReferenceContractNotFound } from './cli';

export { getUpgradeInterfaceVersion } from './upgrade-interface-version';
export { makeNamespacedInput } from './utils/make-namespaced';
37 changes: 37 additions & 0 deletions packages/core/src/upgrade-interface-version.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import test from 'ava';
import { EthereumProvider } from './provider';
import { getUpgradeInterfaceVersion } from './upgrade-interface-version';

const hash = '0x1234';

function makeProviderReturning(result: unknown): EthereumProvider {
return { send: (_method: string, _params: unknown[]) => Promise.resolve(result) } as EthereumProvider;
}

function makeProviderError(msg: string): EthereumProvider {
return {
send: (_method: string, _params: unknown[]) => {
throw new Error(msg);
},
} as EthereumProvider;
}

test('getUpgradeInterfaceVersion returns version', async t => {
// abi encoding of '5.0.0'
const provider = makeProviderReturning(
'0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000005352e302e30000000000000000000000000000000000000000000000000000000',
);
t.is(await getUpgradeInterfaceVersion(provider, hash), '5.0.0');
});

test('getUpgradeInterfaceVersion throws unrelated error', async t => {
const provider = makeProviderError('unrelated error');
await t.throwsAsync(() => getUpgradeInterfaceVersion(provider, hash), { message: 'unrelated error' });
});

test('getUpgradeInterfaceVersion returns undefined for invalid selector', async t => {
const provider = makeProviderError(
`Transaction reverted: function selector was not recognized and there's no fallback function`,
);
t.is(await getUpgradeInterfaceVersion(provider, hash), undefined);
});
27 changes: 27 additions & 0 deletions packages/core/src/upgrade-interface-version.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { callOptionalSignature } from './call-optional-signature';
import { EthereumProvider } from './provider';

export async function getUpgradeInterfaceVersion(
provider: EthereumProvider,
address: string,
): Promise<string | undefined> {
const encodedVersion = await callOptionalSignature(provider, address, 'UPGRADE_INTERFACE_VERSION()');
if (encodedVersion !== undefined) {
// Encoded string
const buf = Buffer.from(encodedVersion.replace(/^0x/, ''), 'hex');

// The first 32 bytes represent the offset, which should be 32 for a string
const offset = parseInt(buf.slice(0, 32).toString('hex'), 16);
if (offset !== 32) {
throw new Error(`Unexpected type for UPGRADE_INTERFACE_VERSION at address ${address}. Expected a string`);
}

// The next 32 bytes represent the length of the string
const length = parseInt(buf.slice(32, 64).toString('hex'), 16);

// The rest is the string itself
return buf.slice(64, 64 + length).toString('utf8');
} else {
return undefined;
}
}
2 changes: 1 addition & 1 deletion packages/core/src/validate/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export function isUpgradeSafe(data: ValidationData, version: Version): boolean {

export function inferUUPS(runValidation: ValidationRunData, fullContractName: string): boolean {
const methods = getAllMethods(runValidation, fullContractName);
return methods.includes(upgradeToSignature);
return methods.includes(upgradeToSignature) || methods.includes(upgradeToAndCallSignature);
}

export function inferProxyKind(data: ValidationData, version: Version): ProxyDeployment['kind'] {
Expand Down
3 changes: 3 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- Support new upgrade interface in OpenZeppelin Contracts 5.0. ([#883](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/883))
- Support importing and upgrading 5.0 proxies.
- **Note**: Deploying 5.0 proxies is not supported yet.
- Add validations for namespaced storage layout. ([#876](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/876))

## 2.2.1 (2023-08-18)
Expand Down
6 changes: 6 additions & 0 deletions packages/plugin-hardhat/contracts/Greeter50Proxiable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pragma solidity >= 0.4.22 <0.8.0;

import "./Greeter.sol";
import "./utils/Proxiable50.sol";

contract Greeter50Proxiable is Greeter, Proxiable50 {}
6 changes: 6 additions & 0 deletions packages/plugin-hardhat/contracts/Greeter50V2Proxiable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pragma solidity ^0.5.1;

import "./GreeterV2.sol";
import "./utils/Proxiable50.sol";

contract Greeter50V2Proxiable is GreeterV2, Proxiable50 {}
6 changes: 6 additions & 0 deletions packages/plugin-hardhat/contracts/Greeter50V3Proxiable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pragma solidity ^0.5.1;

import "./GreeterV3.sol";
import "./utils/Proxiable50.sol";

contract Greeter50V3Proxiable is GreeterV3, Proxiable50 {}
8 changes: 8 additions & 0 deletions packages/plugin-hardhat/contracts/Import50.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "@openzeppelin/contracts-5.0/proxy/beacon/BeaconProxy.sol";
import "@openzeppelin/contracts-5.0/proxy/beacon/UpgradeableBeacon.sol";
import "@openzeppelin/contracts-5.0/proxy/ERC1967/ERC1967Proxy.sol";
import "@openzeppelin/contracts-5.0/proxy/transparent/TransparentUpgradeableProxy.sol";
import "@openzeppelin/contracts-5.0/proxy/transparent/ProxyAdmin.sol";
46 changes: 46 additions & 0 deletions packages/plugin-hardhat/contracts/utils/Proxiable50.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
pragma solidity >= 0.4.22 <0.8.0;

// This contract is for testing only, it is not safe for use in production.

contract Proxiable50 {
bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;

string public constant UPGRADE_INTERFACE_VERSION = "5.0.0";

function upgradeToAndCall(address newImplementation, bytes calldata data) external {
_setImplementation(newImplementation);
if (data.length > 0) {
/**
* Using address(this).call is dangerous as the call can impersonate the proxy being upgraded.
* a better option is to use a delegate call with an oz-upgrades-unsafe-allow, but this is not
* supported by the early version of solidity used here.
*
* /// @custom:oz-upgrades-unsafe-allow delegatecall
* (bool success, ) = newImplementation.delegatecall(data);
*
* Note that using delegate call can make your implementation contract vulnerable if this function
* is not protected with the `onlyProxy` modifier. Again, This contract is for testing only, it is
* not safe for use in production. Instead, use the `UUPSUpgradeable` contract available in
* @openzeppelin/contracts-upgradeable
*/
(bool success, ) = address(this).call(data);
require(success, "upgrade call reverted");
} else {
_checkNonPayable();
}
}

function _checkNonPayable() private {
if (msg.value > 0) {
revert('non-payable upgrade call');
}
}

function _setImplementation(address newImplementation) private {
bytes32 slot = _IMPLEMENTATION_SLOT;
// solhint-disable-next-line no-inline-assembly
assembly {
sstore(slot, newImplementation)
}
}
}
5 changes: 3 additions & 2 deletions packages/plugin-hardhat/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openzeppelin/hardhat-upgrades",
"version": "2.2.1",
"version": "2.3.0",
"description": "",
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/plugin-hardhat",
"license": "MIT",
Expand All @@ -25,6 +25,7 @@
"@nomicfoundation/hardhat-verify": "^1.1.0",
"@openzeppelin/contracts": "4.8.3",
"@openzeppelin/contracts-upgradeable": "4.8.3",
"@openzeppelin/contracts-5.0": "npm:@openzeppelin/contracts@^5.0.0-rc.0",
"@types/mocha": "^7.0.2",
"ava": "^5.0.0",
"fgbg": "^0.1.4",
Expand All @@ -38,7 +39,7 @@
"@openzeppelin/defender-admin-client": "^1.48.0",
"@openzeppelin/defender-base-client": "^1.48.0",
"@openzeppelin/platform-deploy-client": "^0.10.0",
"@openzeppelin/upgrades-core": "^1.27.0",
"@openzeppelin/upgrades-core": "^1.30.0",
"chalk": "^4.1.0",
"debug": "^4.1.1",
"ethereumjs-util": "^7.1.5",
Expand Down
36 changes: 29 additions & 7 deletions packages/plugin-hardhat/src/upgrade-proxy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { HardhatRuntimeEnvironment } from 'hardhat/types';
import type { ethers, ContractFactory, Contract, Signer } from 'ethers';

import { getAdminAddress, getCode, isEmptySlot } from '@openzeppelin/upgrades-core';
import { getAdminAddress, getCode, getUpgradeInterfaceVersion, isEmptySlot } from '@openzeppelin/upgrades-core';

import {
UpgradeProxyOptions,
Expand Down Expand Up @@ -54,17 +54,39 @@ export function makeUpgradeProxy(hre: HardhatRuntimeEnvironment, defenderModule:
const ITransparentUpgradeableProxyFactory = await getITransparentUpgradeableProxyFactory(hre, signer);
const proxy = attach(ITransparentUpgradeableProxyFactory, proxyAddress);

return (nextImpl, call) =>
call ? proxy.upgradeToAndCall(nextImpl, call, ...overrides) : proxy.upgradeTo(nextImpl, ...overrides);
const upgradeInterfaceVersion = await getUpgradeInterfaceVersion(provider, proxyAddress);

return (nextImpl, call) => {
if (upgradeInterfaceVersion === undefined) {
return call ? proxy.upgradeToAndCall(nextImpl, call, ...overrides) : proxy.upgradeTo(nextImpl, ...overrides);
} else if (upgradeInterfaceVersion === '5.0.0') {
return proxy.upgradeToAndCall(nextImpl, call ?? '0x', ...overrides);
} else {
throw new Error(
`Unknown UPGRADE_INTERFACE_VERSION ${upgradeInterfaceVersion} for proxy at ${proxyAddress}. Expected 5.0.0`,
);
}
};
} else {
// Admin contract: redirect upgrade call through it
const AdminFactory = await getProxyAdminFactory(hre, signer);
const admin = attach(AdminFactory, adminAddress);

return (nextImpl, call) =>
call
? admin.upgradeAndCall(proxyAddress, nextImpl, call, ...overrides)
: admin.upgrade(proxyAddress, nextImpl, ...overrides);
const upgradeInterfaceVersion = await getUpgradeInterfaceVersion(provider, adminAddress);

return (nextImpl, call) => {
if (upgradeInterfaceVersion === undefined) {
return call
? admin.upgradeAndCall(proxyAddress, nextImpl, call, ...overrides)
: admin.upgrade(proxyAddress, nextImpl, ...overrides);
} else if (upgradeInterfaceVersion === '5.0.0') {
return admin.upgradeAndCall(proxyAddress, nextImpl, call ?? '0x', ...overrides);
} else {
throw new Error(
`Unknown UPGRADE_INTERFACE_VERSION ${upgradeInterfaceVersion} for proxy admin at ${adminAddress}. Expected 5.0.0`,
);
}
};
}
}
}
Expand Down
Loading