From c94b5e709ead11896015753c30f671b750340371 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 13 Dec 2021 21:42:31 +0100 Subject: [PATCH 01/21] ERC1822 implementation of UUPSUpgradeable --- contracts/proxy/ERC1822/IProxiable.sol | 8 ++++++ contracts/proxy/ERC1967/ERC1967Upgrade.sol | 29 ++++++---------------- contracts/proxy/utils/UUPSUpgradeable.sol | 20 ++++++++++++++- test/proxy/utils/UUPSUpgradeable.test.js | 6 ++--- 4 files changed, 38 insertions(+), 25 deletions(-) create mode 100644 contracts/proxy/ERC1822/IProxiable.sol diff --git a/contracts/proxy/ERC1822/IProxiable.sol b/contracts/proxy/ERC1822/IProxiable.sol new file mode 100644 index 00000000000..265f09e10aa --- /dev/null +++ b/contracts/proxy/ERC1822/IProxiable.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts v4.x.0 (proxy/ERC1822/IProxiable.sol) + +pragma solidity ^0.8.0; + +interface IProxiable { + function proxiableUUID() external view returns (bytes32); +} diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 04394aeeec7..adc8254bdbe 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.2; import "../beacon/IBeacon.sol"; +import "../ERC1822/IProxiable.sol"; import "../../utils/Address.sol"; import "../../utils/StorageSlot.sol"; @@ -82,28 +83,14 @@ abstract contract ERC1967Upgrade { bytes memory data, bool forceCall ) internal { - address oldImplementation = _getImplementation(); - - // Initial upgrade and setup call - _setImplementation(newImplementation); - if (data.length > 0 || forceCall) { - Address.functionDelegateCall(newImplementation, data); - } - - // Perform rollback test if not already in progress - StorageSlot.BooleanSlot storage rollbackTesting = StorageSlot.getBooleanSlot(_ROLLBACK_SLOT); - if (!rollbackTesting.value) { - // Trigger rollback using upgradeTo from the new implementation - rollbackTesting.value = true; - Address.functionDelegateCall( - newImplementation, - abi.encodeWithSignature("upgradeTo(address)", oldImplementation) + if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) { + _setImplementation(newImplementation); + } else { + require( + IProxiable(newImplementation).proxiableUUID() == _IMPLEMENTATION_SLOT, + "Invalid proxiableUUID value" ); - rollbackTesting.value = false; - // Check rollback was effective - require(oldImplementation == _getImplementation(), "ERC1967Upgrade: upgrade breaks further upgrades"); - // Finally reset to the new implementation and log the upgrade - _upgradeTo(newImplementation); + _upgradeToAndCall(newImplementation, data, forceCall); } } diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index 89ceccced48..a26df02edb0 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.0; +import "../ERC1822/IProxiable.sol"; import "../ERC1967/ERC1967Upgrade.sol"; /** @@ -17,7 +18,7 @@ import "../ERC1967/ERC1967Upgrade.sol"; * * _Available since v4.1._ */ -abstract contract UUPSUpgradeable is ERC1967Upgrade { +abstract contract UUPSUpgradeable is IProxiable, ERC1967Upgrade { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment address private immutable __self = address(this); @@ -34,6 +35,23 @@ abstract contract UUPSUpgradeable is ERC1967Upgrade { _; } + /** + * @dev Check that the execution is not being performed through a delegate call. This allows a function to be + * callable on the implementing contract but not through proxies. + */ + modifier notDelegated() { + require(address(this) == __self, "Function must not be called through delegatecall"); + _; + } + + /** + * @dev Implementation of the ERC1822 {proxiableUUID} function. This returns the storage slot used by the + * implementation. It is used to validate that the this implementation remains valid after an upgrade. + */ + function proxiableUUID() external view virtual override notDelegated returns (bytes32) { + return _IMPLEMENTATION_SLOT; + } + /** * @dev Upgrade the implementation of the proxy to `newImplementation`. * diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 3351ddcb872..91686bb16ee 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -44,7 +44,7 @@ contract('UUPSUpgradeable', function (accounts) { expectEvent(receipt, 'Upgraded', { implementation: this.implUpgradeUnsafe.address }); }); - it('reject upgrade to broken upgradeable implementation', async function () { + it.skip('reject upgrade to broken upgradeable implementation (no longer supported)', async function () { await expectRevert( this.instance.upgradeTo(this.implUpgradeBroken.address), 'ERC1967Upgrade: upgrade breaks further upgrades', @@ -55,7 +55,7 @@ contract('UUPSUpgradeable', function (accounts) { it('reject upgrade to non uups implementation', async function () { await expectRevert( this.instance.upgradeTo(this.implUpgradeNonUUPS.address), - 'Address: low-level delegate call failed', + 'Transaction reverted: function selector was not recognized and there\'s no fallback function', ); }); @@ -66,7 +66,7 @@ contract('UUPSUpgradeable', function (accounts) { // infinite loop reverts when a nested call is out-of-gas await expectRevert( this.instance.upgradeTo(otherInstance.address), - 'Address: low-level delegate call failed', + 'Function must not be called through delegatecall', ); }); }); From 4d5e5172f78f67323c5d26335c91231a2433f924 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 14 Dec 2021 16:52:04 +0100 Subject: [PATCH 02/21] ship legacy version of UUPSUpgradeable for tests --- test/proxy/utils/UUPSUpgradeable.test.js | 22 ++++ .../utils/legacy/UUPSUpgradeableMockV1.json | 121 ++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 test/proxy/utils/legacy/UUPSUpgradeableMockV1.json diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 91686bb16ee..1d081fc3585 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -6,6 +6,8 @@ const UUPSUpgradeableUnsafeMock = artifacts.require('UUPSUpgradeableUnsafeMock') const UUPSUpgradeableBrokenMock = artifacts.require('UUPSUpgradeableBrokenMock'); const CountersImpl = artifacts.require('CountersImpl'); +const Legacy = [ './legacy/UUPSUpgradeableMockV1' ]; + contract('UUPSUpgradeable', function (accounts) { before(async function () { this.implInitial = await UUPSUpgradeableMock.new(); @@ -69,4 +71,24 @@ contract('UUPSUpgradeable', function (accounts) { 'Function must not be called through delegatecall', ); }); + + describe(`compatibility with legacy version of UUPSUpgradeable`, function () { + // This could possibly be cleaner/simpler with ethers.js + for (const path of Legacy) { + it(`can upgrade from ${path}`, async function () { + const artefact = require(path); + // deploy legacy implementation + const legacy = new web3.eth.Contract(artefact.abi); + const { _address: implAddr } = await legacy.deploy({ data: artefact.bytecode }).send({ from: accounts[0] }); + + // deploy proxy + const { address: proxyAddr } = await ERC1967Proxy.new(implAddr, '0x'); + const proxy = new web3.eth.Contract(artefact.abi, proxyAddr); + + // perform upgrade + const receipt = await proxy.methods.upgradeTo(this.implInitial.address).send({ from: accounts[0] }); + expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); + }); + } + }); }); diff --git a/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json b/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json new file mode 100644 index 00000000000..5bab5511635 --- /dev/null +++ b/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json @@ -0,0 +1,121 @@ +{ + "_format": "hh-sol-artifact-1", + "contractName": "UUPSUpgradeableMockV1", + "sourceName": "contracts/mocks/UUPS/TestInProd.sol", + "abi": [ + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "previousAdmin", + "type": "address" + }, + { + "indexed": false, + "internalType": "address", + "name": "newAdmin", + "type": "address" + } + ], + "name": "AdminChanged", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "beacon", + "type": "address" + } + ], + "name": "BeaconUpgraded", + "type": "event" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "implementation", + "type": "address" + } + ], + "name": "Upgraded", + "type": "event" + }, + { + "inputs": [], + "name": "current", + "outputs": [ + { + "internalType": "uint256", + "name": "", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "inputs": [], + "name": "decrement", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "increment", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [], + "name": "reset", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "newImplementation", + "type": "address" + } + ], + "name": "upgradeTo", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, + { + "inputs": [ + { + "internalType": "address", + "name": "newImplementation", + "type": "address" + }, + { + "internalType": "bytes", + "name": "data", + "type": "bytes" + } + ], + "name": "upgradeToAndCall", + "outputs": [], + "stateMutability": "payable", + "type": "function" + } + ], + "bytecode": "0x60a06040523073ffffffffffffffffffffffffffffffffffffffff1660809073ffffffffffffffffffffffffffffffffffffffff1660601b81525034801561004657600080fd5b5060805160601c61109a61007a6000396000818161011d015281816101ac015281816102cc015261035b015261109a6000f3fe6080604052600436106100555760003560e01c80632baeceb71461005a5780633659cfe6146100715780634f1ef2861461009a5780639fa6a6e3146100b6578063d09de08a146100e1578063d826f88f146100f8575b600080fd5b34801561006657600080fd5b5061006f61010f565b005b34801561007d57600080fd5b50610098600480360381019061009391906109f5565b61011b565b005b6100b460048036038101906100af9190610a1e565b6102ca565b005b3480156100c257600080fd5b506100cb610407565b6040516100d89190610ce0565b60405180910390f35b3480156100ed57600080fd5b506100f6610418565b005b34801561010457600080fd5b5061010d610424565b005b6101196000610430565b565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff1614156101aa576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016101a190610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff166101e961048c565b73ffffffffffffffffffffffffffffffffffffffff161461023f576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161023690610c80565b60405180910390fd5b610248816104e3565b6102c781600067ffffffffffffffff81111561028d577f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6040519080825280601f01601f1916602001820160405280156102bf5781602001600182028036833780820191505090505b5060006104e6565b50565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff161415610359576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161035090610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff1661039861048c565b73ffffffffffffffffffffffffffffffffffffffff16146103ee576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016103e590610c80565b60405180910390fd5b6103f7826104e3565b610403828260016104e6565b5050565b600061041360006106b7565b905090565b61042260006106c5565b565b61042e60006106db565b565b6000816000015490506000811161047c576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161047390610c20565b60405180910390fd5b6001810382600001819055505050565b60006104ba7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b50565b60006104f061048c565b90506104fb846106f2565b6000835111806105085750815b156105195761051784846107ab565b505b60006105477f4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd914360001b6107d8565b90508060000160009054906101000a900460ff166106b05760018160000160006101000a81548160ff02191690831515021790555061061385836040516024016105919190610be3565b6040516020818303038152906040527f3659cfe6000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506107ab565b5060008160000160006101000a81548160ff02191690831515021790555061063961048c565b73ffffffffffffffffffffffffffffffffffffffff168273ffffffffffffffffffffffffffffffffffffffff16146106a6576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161069d90610c40565b60405180910390fd5b6106af856107e2565b5b5050505050565b600081600001549050919050565b6001816000016000828254019250508190555050565b6000816000018190555050565b6000819050919050565b6106fb81610831565b61073a576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161073190610ca0565b60405180910390fd5b806107677f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff16021790555050565b60606107d0838360405180606001604052806027815260200161103e60279139610844565b905092915050565b6000819050919050565b6107eb816106f2565b8073ffffffffffffffffffffffffffffffffffffffff167fbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b60405160405180910390a250565b600080823b905060008111915050919050565b606061084f84610831565b61088e576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161088590610cc0565b60405180910390fd5b6000808573ffffffffffffffffffffffffffffffffffffffff16856040516108b69190610bcc565b600060405180830381855af49150503d80600081146108f1576040519150601f19603f3d011682016040523d82523d6000602084013e6108f6565b606091505b5091509150610906828286610911565b925050509392505050565b6060831561092157829050610971565b6000835111156109345782518084602001fd5b816040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016109689190610bfe565b60405180910390fd5b9392505050565b600061098b61098684610d20565b610cfb565b9050828152602081018484840111156109a357600080fd5b6109ae848285610dbf565b509392505050565b6000813590506109c581611026565b92915050565b600082601f8301126109dc57600080fd5b81356109ec848260208601610978565b91505092915050565b600060208284031215610a0757600080fd5b6000610a15848285016109b6565b91505092915050565b60008060408385031215610a3157600080fd5b6000610a3f858286016109b6565b925050602083013567ffffffffffffffff811115610a5c57600080fd5b610a68858286016109cb565b9150509250929050565b610a7b81610d83565b82525050565b6000610a8c82610d51565b610a968185610d67565b9350610aa6818560208601610dce565b80840191505092915050565b6000610abd82610d5c565b610ac78185610d72565b9350610ad7818560208601610dce565b610ae081610e61565b840191505092915050565b6000610af8601b83610d72565b9150610b0382610e72565b602082019050919050565b6000610b1b602f83610d72565b9150610b2682610e9b565b604082019050919050565b6000610b3e602c83610d72565b9150610b4982610eea565b604082019050919050565b6000610b61602c83610d72565b9150610b6c82610f39565b604082019050919050565b6000610b84602d83610d72565b9150610b8f82610f88565b604082019050919050565b6000610ba7602683610d72565b9150610bb282610fd7565b604082019050919050565b610bc681610db5565b82525050565b6000610bd88284610a81565b915081905092915050565b6000602082019050610bf86000830184610a72565b92915050565b60006020820190508181036000830152610c188184610ab2565b905092915050565b60006020820190508181036000830152610c3981610aeb565b9050919050565b60006020820190508181036000830152610c5981610b0e565b9050919050565b60006020820190508181036000830152610c7981610b31565b9050919050565b60006020820190508181036000830152610c9981610b54565b9050919050565b60006020820190508181036000830152610cb981610b77565b9050919050565b60006020820190508181036000830152610cd981610b9a565b9050919050565b6000602082019050610cf56000830184610bbd565b92915050565b6000610d05610d16565b9050610d118282610e01565b919050565b6000604051905090565b600067ffffffffffffffff821115610d3b57610d3a610e32565b5b610d4482610e61565b9050602081019050919050565b600081519050919050565b600081519050919050565b600081905092915050565b600082825260208201905092915050565b6000610d8e82610d95565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b6000819050919050565b82818337600083830152505050565b60005b83811015610dec578082015181840152602081019050610dd1565b83811115610dfb576000848401525b50505050565b610e0a82610e61565b810181811067ffffffffffffffff82111715610e2957610e28610e32565b5b80604052505050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6000601f19601f8301169050919050565b7f436f756e7465723a2064656372656d656e74206f766572666c6f770000000000600082015250565b7f45524331393637557067726164653a207570677261646520627265616b73206660008201527f7572746865722075706772616465730000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f64656c656761746563616c6c0000000000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f6163746976652070726f78790000000000000000000000000000000000000000602082015250565b7f455243313936373a206e657720696d706c656d656e746174696f6e206973206e60008201527f6f74206120636f6e747261637400000000000000000000000000000000000000602082015250565b7f416464726573733a2064656c65676174652063616c6c20746f206e6f6e2d636f60008201527f6e74726163740000000000000000000000000000000000000000000000000000602082015250565b61102f81610d83565b811461103a57600080fd5b5056fe416464726573733a206c6f772d6c6576656c2064656c65676174652063616c6c206661696c6564a2646970667358221220a095e223f72f7004a2c1ad6ca22791a4e71300ad963839f11dcacba09398d9da64736f6c63430008030033", + "deployedBytecode": "0x6080604052600436106100555760003560e01c80632baeceb71461005a5780633659cfe6146100715780634f1ef2861461009a5780639fa6a6e3146100b6578063d09de08a146100e1578063d826f88f146100f8575b600080fd5b34801561006657600080fd5b5061006f61010f565b005b34801561007d57600080fd5b50610098600480360381019061009391906109f5565b61011b565b005b6100b460048036038101906100af9190610a1e565b6102ca565b005b3480156100c257600080fd5b506100cb610407565b6040516100d89190610ce0565b60405180910390f35b3480156100ed57600080fd5b506100f6610418565b005b34801561010457600080fd5b5061010d610424565b005b6101196000610430565b565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff1614156101aa576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016101a190610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff166101e961048c565b73ffffffffffffffffffffffffffffffffffffffff161461023f576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161023690610c80565b60405180910390fd5b610248816104e3565b6102c781600067ffffffffffffffff81111561028d577f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6040519080825280601f01601f1916602001820160405280156102bf5781602001600182028036833780820191505090505b5060006104e6565b50565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff161415610359576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161035090610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff1661039861048c565b73ffffffffffffffffffffffffffffffffffffffff16146103ee576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016103e590610c80565b60405180910390fd5b6103f7826104e3565b610403828260016104e6565b5050565b600061041360006106b7565b905090565b61042260006106c5565b565b61042e60006106db565b565b6000816000015490506000811161047c576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161047390610c20565b60405180910390fd5b6001810382600001819055505050565b60006104ba7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b50565b60006104f061048c565b90506104fb846106f2565b6000835111806105085750815b156105195761051784846107ab565b505b60006105477f4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd914360001b6107d8565b90508060000160009054906101000a900460ff166106b05760018160000160006101000a81548160ff02191690831515021790555061061385836040516024016105919190610be3565b6040516020818303038152906040527f3659cfe6000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506107ab565b5060008160000160006101000a81548160ff02191690831515021790555061063961048c565b73ffffffffffffffffffffffffffffffffffffffff168273ffffffffffffffffffffffffffffffffffffffff16146106a6576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161069d90610c40565b60405180910390fd5b6106af856107e2565b5b5050505050565b600081600001549050919050565b6001816000016000828254019250508190555050565b6000816000018190555050565b6000819050919050565b6106fb81610831565b61073a576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161073190610ca0565b60405180910390fd5b806107677f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff16021790555050565b60606107d0838360405180606001604052806027815260200161103e60279139610844565b905092915050565b6000819050919050565b6107eb816106f2565b8073ffffffffffffffffffffffffffffffffffffffff167fbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b60405160405180910390a250565b600080823b905060008111915050919050565b606061084f84610831565b61088e576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161088590610cc0565b60405180910390fd5b6000808573ffffffffffffffffffffffffffffffffffffffff16856040516108b69190610bcc565b600060405180830381855af49150503d80600081146108f1576040519150601f19603f3d011682016040523d82523d6000602084013e6108f6565b606091505b5091509150610906828286610911565b925050509392505050565b6060831561092157829050610971565b6000835111156109345782518084602001fd5b816040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016109689190610bfe565b60405180910390fd5b9392505050565b600061098b61098684610d20565b610cfb565b9050828152602081018484840111156109a357600080fd5b6109ae848285610dbf565b509392505050565b6000813590506109c581611026565b92915050565b600082601f8301126109dc57600080fd5b81356109ec848260208601610978565b91505092915050565b600060208284031215610a0757600080fd5b6000610a15848285016109b6565b91505092915050565b60008060408385031215610a3157600080fd5b6000610a3f858286016109b6565b925050602083013567ffffffffffffffff811115610a5c57600080fd5b610a68858286016109cb565b9150509250929050565b610a7b81610d83565b82525050565b6000610a8c82610d51565b610a968185610d67565b9350610aa6818560208601610dce565b80840191505092915050565b6000610abd82610d5c565b610ac78185610d72565b9350610ad7818560208601610dce565b610ae081610e61565b840191505092915050565b6000610af8601b83610d72565b9150610b0382610e72565b602082019050919050565b6000610b1b602f83610d72565b9150610b2682610e9b565b604082019050919050565b6000610b3e602c83610d72565b9150610b4982610eea565b604082019050919050565b6000610b61602c83610d72565b9150610b6c82610f39565b604082019050919050565b6000610b84602d83610d72565b9150610b8f82610f88565b604082019050919050565b6000610ba7602683610d72565b9150610bb282610fd7565b604082019050919050565b610bc681610db5565b82525050565b6000610bd88284610a81565b915081905092915050565b6000602082019050610bf86000830184610a72565b92915050565b60006020820190508181036000830152610c188184610ab2565b905092915050565b60006020820190508181036000830152610c3981610aeb565b9050919050565b60006020820190508181036000830152610c5981610b0e565b9050919050565b60006020820190508181036000830152610c7981610b31565b9050919050565b60006020820190508181036000830152610c9981610b54565b9050919050565b60006020820190508181036000830152610cb981610b77565b9050919050565b60006020820190508181036000830152610cd981610b9a565b9050919050565b6000602082019050610cf56000830184610bbd565b92915050565b6000610d05610d16565b9050610d118282610e01565b919050565b6000604051905090565b600067ffffffffffffffff821115610d3b57610d3a610e32565b5b610d4482610e61565b9050602081019050919050565b600081519050919050565b600081519050919050565b600081905092915050565b600082825260208201905092915050565b6000610d8e82610d95565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b6000819050919050565b82818337600083830152505050565b60005b83811015610dec578082015181840152602081019050610dd1565b83811115610dfb576000848401525b50505050565b610e0a82610e61565b810181811067ffffffffffffffff82111715610e2957610e28610e32565b5b80604052505050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6000601f19601f8301169050919050565b7f436f756e7465723a2064656372656d656e74206f766572666c6f770000000000600082015250565b7f45524331393637557067726164653a207570677261646520627265616b73206660008201527f7572746865722075706772616465730000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f64656c656761746563616c6c0000000000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f6163746976652070726f78790000000000000000000000000000000000000000602082015250565b7f455243313936373a206e657720696d706c656d656e746174696f6e206973206e60008201527f6f74206120636f6e747261637400000000000000000000000000000000000000602082015250565b7f416464726573733a2064656c65676174652063616c6c20746f206e6f6e2d636f60008201527f6e74726163740000000000000000000000000000000000000000000000000000602082015250565b61102f81610d83565b811461103a57600080fd5b5056fe416464726573733a206c6f772d6c6576656c2064656c65676174652063616c6c206661696c6564a2646970667358221220a095e223f72f7004a2c1ad6ca22791a4e71300ad963839f11dcacba09398d9da64736f6c63430008030033", + "linkReferences": {}, + "deployedLinkReferences": {} +} From a791e5ae83941dbe8dd1d89623cb070b64679f26 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 14 Dec 2021 20:48:30 +0100 Subject: [PATCH 03/21] fix lint --- test/proxy/utils/UUPSUpgradeable.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 1d081fc3585..665314019a2 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -72,7 +72,7 @@ contract('UUPSUpgradeable', function (accounts) { ); }); - describe(`compatibility with legacy version of UUPSUpgradeable`, function () { + describe('compatibility with legacy version of UUPSUpgradeable', function () { // This could possibly be cleaner/simpler with ethers.js for (const path of Legacy) { it(`can upgrade from ${path}`, async function () { From 03a29da51d11d70257c076c6e2a8c4b6c671b1ff Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 21 Dec 2021 10:21:46 +0100 Subject: [PATCH 04/21] use solidity mock for legacy tests --- contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol | 54 ++++++++ test/proxy/utils/UUPSUpgradeable.test.js | 28 ++-- .../utils/legacy/UUPSUpgradeableMockV1.json | 121 ------------------ 3 files changed, 66 insertions(+), 137 deletions(-) create mode 100644 contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol delete mode 100644 test/proxy/utils/legacy/UUPSUpgradeableMockV1.json diff --git a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol new file mode 100644 index 00000000000..5624f202b2f --- /dev/null +++ b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +import "../TestInProd.sol"; + +contract UUPSLegacyV1 is UUPSUpgradeableMock { + // Inlined from ERC1967Upgrade + bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143; + + function __setImplementation(address newImplementation) private { + require(Address.isContract(newImplementation), "ERC1967: new implementation is not a contract"); + StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; + } + + function _upgradeToAndCallSecureLegacyV1( + address newImplementation, + bytes memory data, + bool forceCall + ) internal { + address oldImplementation = _getImplementation(); + + // Initial upgrade and setup call + __setImplementation(newImplementation); + if (data.length > 0 || forceCall) { + Address.functionDelegateCall(newImplementation, data); + } + + // Perform rollback test if not already in progress + StorageSlot.BooleanSlot storage rollbackTesting = StorageSlot.getBooleanSlot(_ROLLBACK_SLOT); + if (!rollbackTesting.value) { + // Trigger rollback using upgradeTo from the new implementation + rollbackTesting.value = true; + Address.functionDelegateCall( + newImplementation, + abi.encodeWithSignature("upgradeTo(address)", oldImplementation) + ); + rollbackTesting.value = false; + // Check rollback was effective + require(oldImplementation == _getImplementation(), "ERC1967Upgrade: upgrade breaks further upgrades"); + // Finally reset to the new implementation and log the upgrade + _upgradeTo(newImplementation); + } + } + + // hooking into the old mechanism + function upgradeTo(address newImplementation) external virtual override { + _upgradeToAndCallSecureLegacyV1(newImplementation, bytes(""), false); + } + + function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual override { + _upgradeToAndCallSecureLegacyV1(newImplementation, data, false); + } +} \ No newline at end of file diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 665314019a2..c0b2b8131a3 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -6,7 +6,7 @@ const UUPSUpgradeableUnsafeMock = artifacts.require('UUPSUpgradeableUnsafeMock') const UUPSUpgradeableBrokenMock = artifacts.require('UUPSUpgradeableBrokenMock'); const CountersImpl = artifacts.require('CountersImpl'); -const Legacy = [ './legacy/UUPSUpgradeableMockV1' ]; +const Legacy = [ 'UUPSLegacyV1' ].map(artifacts.require); contract('UUPSUpgradeable', function (accounts) { before(async function () { @@ -73,22 +73,18 @@ contract('UUPSUpgradeable', function (accounts) { }); describe('compatibility with legacy version of UUPSUpgradeable', function () { - // This could possibly be cleaner/simpler with ethers.js - for (const path of Legacy) { - it(`can upgrade from ${path}`, async function () { - const artefact = require(path); - // deploy legacy implementation - const legacy = new web3.eth.Contract(artefact.abi); - const { _address: implAddr } = await legacy.deploy({ data: artefact.bytecode }).send({ from: accounts[0] }); - - // deploy proxy - const { address: proxyAddr } = await ERC1967Proxy.new(implAddr, '0x'); - const proxy = new web3.eth.Contract(artefact.abi, proxyAddr); - - // perform upgrade - const receipt = await proxy.methods.upgradeTo(this.implInitial.address).send({ from: accounts[0] }); + for (const artefact of Legacy) { + it(`can upgrade from ${artefact._json.contractName}`, async function () { + const legacyImpl = await artefact.new(); + const legacyInstance = await ERC1967Proxy.new(legacyImpl.address, '0x').then(({ address }) => artefact.at(address)); + + const receipt = await legacyInstance.upgradeTo(this.implInitial.address); + + // only produce a single upgrade event + expect(receipt.logs.filter(({ address, event }) => address == legacyInstance.address && event == 'Upgraded' ).length).to.be.equal(1); + // produces the right event expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); }); - } + }; }); }); diff --git a/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json b/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json deleted file mode 100644 index 5bab5511635..00000000000 --- a/test/proxy/utils/legacy/UUPSUpgradeableMockV1.json +++ /dev/null @@ -1,121 +0,0 @@ -{ - "_format": "hh-sol-artifact-1", - "contractName": "UUPSUpgradeableMockV1", - "sourceName": "contracts/mocks/UUPS/TestInProd.sol", - "abi": [ - { - "anonymous": false, - "inputs": [ - { - "indexed": false, - "internalType": "address", - "name": "previousAdmin", - "type": "address" - }, - { - "indexed": false, - "internalType": "address", - "name": "newAdmin", - "type": "address" - } - ], - "name": "AdminChanged", - "type": "event" - }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "address", - "name": "beacon", - "type": "address" - } - ], - "name": "BeaconUpgraded", - "type": "event" - }, - { - "anonymous": false, - "inputs": [ - { - "indexed": true, - "internalType": "address", - "name": "implementation", - "type": "address" - } - ], - "name": "Upgraded", - "type": "event" - }, - { - "inputs": [], - "name": "current", - "outputs": [ - { - "internalType": "uint256", - "name": "", - "type": "uint256" - } - ], - "stateMutability": "view", - "type": "function" - }, - { - "inputs": [], - "name": "decrement", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [], - "name": "increment", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [], - "name": "reset", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [ - { - "internalType": "address", - "name": "newImplementation", - "type": "address" - } - ], - "name": "upgradeTo", - "outputs": [], - "stateMutability": "nonpayable", - "type": "function" - }, - { - "inputs": [ - { - "internalType": "address", - "name": "newImplementation", - "type": "address" - }, - { - "internalType": "bytes", - "name": "data", - "type": "bytes" - } - ], - "name": "upgradeToAndCall", - "outputs": [], - "stateMutability": "payable", - "type": "function" - } - ], - "bytecode": "0x60a06040523073ffffffffffffffffffffffffffffffffffffffff1660809073ffffffffffffffffffffffffffffffffffffffff1660601b81525034801561004657600080fd5b5060805160601c61109a61007a6000396000818161011d015281816101ac015281816102cc015261035b015261109a6000f3fe6080604052600436106100555760003560e01c80632baeceb71461005a5780633659cfe6146100715780634f1ef2861461009a5780639fa6a6e3146100b6578063d09de08a146100e1578063d826f88f146100f8575b600080fd5b34801561006657600080fd5b5061006f61010f565b005b34801561007d57600080fd5b50610098600480360381019061009391906109f5565b61011b565b005b6100b460048036038101906100af9190610a1e565b6102ca565b005b3480156100c257600080fd5b506100cb610407565b6040516100d89190610ce0565b60405180910390f35b3480156100ed57600080fd5b506100f6610418565b005b34801561010457600080fd5b5061010d610424565b005b6101196000610430565b565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff1614156101aa576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016101a190610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff166101e961048c565b73ffffffffffffffffffffffffffffffffffffffff161461023f576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161023690610c80565b60405180910390fd5b610248816104e3565b6102c781600067ffffffffffffffff81111561028d577f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6040519080825280601f01601f1916602001820160405280156102bf5781602001600182028036833780820191505090505b5060006104e6565b50565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff161415610359576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161035090610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff1661039861048c565b73ffffffffffffffffffffffffffffffffffffffff16146103ee576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016103e590610c80565b60405180910390fd5b6103f7826104e3565b610403828260016104e6565b5050565b600061041360006106b7565b905090565b61042260006106c5565b565b61042e60006106db565b565b6000816000015490506000811161047c576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161047390610c20565b60405180910390fd5b6001810382600001819055505050565b60006104ba7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b50565b60006104f061048c565b90506104fb846106f2565b6000835111806105085750815b156105195761051784846107ab565b505b60006105477f4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd914360001b6107d8565b90508060000160009054906101000a900460ff166106b05760018160000160006101000a81548160ff02191690831515021790555061061385836040516024016105919190610be3565b6040516020818303038152906040527f3659cfe6000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506107ab565b5060008160000160006101000a81548160ff02191690831515021790555061063961048c565b73ffffffffffffffffffffffffffffffffffffffff168273ffffffffffffffffffffffffffffffffffffffff16146106a6576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161069d90610c40565b60405180910390fd5b6106af856107e2565b5b5050505050565b600081600001549050919050565b6001816000016000828254019250508190555050565b6000816000018190555050565b6000819050919050565b6106fb81610831565b61073a576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161073190610ca0565b60405180910390fd5b806107677f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff16021790555050565b60606107d0838360405180606001604052806027815260200161103e60279139610844565b905092915050565b6000819050919050565b6107eb816106f2565b8073ffffffffffffffffffffffffffffffffffffffff167fbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b60405160405180910390a250565b600080823b905060008111915050919050565b606061084f84610831565b61088e576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161088590610cc0565b60405180910390fd5b6000808573ffffffffffffffffffffffffffffffffffffffff16856040516108b69190610bcc565b600060405180830381855af49150503d80600081146108f1576040519150601f19603f3d011682016040523d82523d6000602084013e6108f6565b606091505b5091509150610906828286610911565b925050509392505050565b6060831561092157829050610971565b6000835111156109345782518084602001fd5b816040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016109689190610bfe565b60405180910390fd5b9392505050565b600061098b61098684610d20565b610cfb565b9050828152602081018484840111156109a357600080fd5b6109ae848285610dbf565b509392505050565b6000813590506109c581611026565b92915050565b600082601f8301126109dc57600080fd5b81356109ec848260208601610978565b91505092915050565b600060208284031215610a0757600080fd5b6000610a15848285016109b6565b91505092915050565b60008060408385031215610a3157600080fd5b6000610a3f858286016109b6565b925050602083013567ffffffffffffffff811115610a5c57600080fd5b610a68858286016109cb565b9150509250929050565b610a7b81610d83565b82525050565b6000610a8c82610d51565b610a968185610d67565b9350610aa6818560208601610dce565b80840191505092915050565b6000610abd82610d5c565b610ac78185610d72565b9350610ad7818560208601610dce565b610ae081610e61565b840191505092915050565b6000610af8601b83610d72565b9150610b0382610e72565b602082019050919050565b6000610b1b602f83610d72565b9150610b2682610e9b565b604082019050919050565b6000610b3e602c83610d72565b9150610b4982610eea565b604082019050919050565b6000610b61602c83610d72565b9150610b6c82610f39565b604082019050919050565b6000610b84602d83610d72565b9150610b8f82610f88565b604082019050919050565b6000610ba7602683610d72565b9150610bb282610fd7565b604082019050919050565b610bc681610db5565b82525050565b6000610bd88284610a81565b915081905092915050565b6000602082019050610bf86000830184610a72565b92915050565b60006020820190508181036000830152610c188184610ab2565b905092915050565b60006020820190508181036000830152610c3981610aeb565b9050919050565b60006020820190508181036000830152610c5981610b0e565b9050919050565b60006020820190508181036000830152610c7981610b31565b9050919050565b60006020820190508181036000830152610c9981610b54565b9050919050565b60006020820190508181036000830152610cb981610b77565b9050919050565b60006020820190508181036000830152610cd981610b9a565b9050919050565b6000602082019050610cf56000830184610bbd565b92915050565b6000610d05610d16565b9050610d118282610e01565b919050565b6000604051905090565b600067ffffffffffffffff821115610d3b57610d3a610e32565b5b610d4482610e61565b9050602081019050919050565b600081519050919050565b600081519050919050565b600081905092915050565b600082825260208201905092915050565b6000610d8e82610d95565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b6000819050919050565b82818337600083830152505050565b60005b83811015610dec578082015181840152602081019050610dd1565b83811115610dfb576000848401525b50505050565b610e0a82610e61565b810181811067ffffffffffffffff82111715610e2957610e28610e32565b5b80604052505050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6000601f19601f8301169050919050565b7f436f756e7465723a2064656372656d656e74206f766572666c6f770000000000600082015250565b7f45524331393637557067726164653a207570677261646520627265616b73206660008201527f7572746865722075706772616465730000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f64656c656761746563616c6c0000000000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f6163746976652070726f78790000000000000000000000000000000000000000602082015250565b7f455243313936373a206e657720696d706c656d656e746174696f6e206973206e60008201527f6f74206120636f6e747261637400000000000000000000000000000000000000602082015250565b7f416464726573733a2064656c65676174652063616c6c20746f206e6f6e2d636f60008201527f6e74726163740000000000000000000000000000000000000000000000000000602082015250565b61102f81610d83565b811461103a57600080fd5b5056fe416464726573733a206c6f772d6c6576656c2064656c65676174652063616c6c206661696c6564a2646970667358221220a095e223f72f7004a2c1ad6ca22791a4e71300ad963839f11dcacba09398d9da64736f6c63430008030033", - "deployedBytecode": "0x6080604052600436106100555760003560e01c80632baeceb71461005a5780633659cfe6146100715780634f1ef2861461009a5780639fa6a6e3146100b6578063d09de08a146100e1578063d826f88f146100f8575b600080fd5b34801561006657600080fd5b5061006f61010f565b005b34801561007d57600080fd5b50610098600480360381019061009391906109f5565b61011b565b005b6100b460048036038101906100af9190610a1e565b6102ca565b005b3480156100c257600080fd5b506100cb610407565b6040516100d89190610ce0565b60405180910390f35b3480156100ed57600080fd5b506100f6610418565b005b34801561010457600080fd5b5061010d610424565b005b6101196000610430565b565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff1614156101aa576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016101a190610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff166101e961048c565b73ffffffffffffffffffffffffffffffffffffffff161461023f576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161023690610c80565b60405180910390fd5b610248816104e3565b6102c781600067ffffffffffffffff81111561028d577f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6040519080825280601f01601f1916602001820160405280156102bf5781602001600182028036833780820191505090505b5060006104e6565b50565b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff163073ffffffffffffffffffffffffffffffffffffffff161415610359576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161035090610c60565b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff1661039861048c565b73ffffffffffffffffffffffffffffffffffffffff16146103ee576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016103e590610c80565b60405180910390fd5b6103f7826104e3565b610403828260016104e6565b5050565b600061041360006106b7565b905090565b61042260006106c5565b565b61042e60006106db565b565b6000816000015490506000811161047c576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161047390610c20565b60405180910390fd5b6001810382600001819055505050565b60006104ba7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff16905090565b50565b60006104f061048c565b90506104fb846106f2565b6000835111806105085750815b156105195761051784846107ab565b505b60006105477f4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd914360001b6107d8565b90508060000160009054906101000a900460ff166106b05760018160000160006101000a81548160ff02191690831515021790555061061385836040516024016105919190610be3565b6040516020818303038152906040527f3659cfe6000000000000000000000000000000000000000000000000000000007bffffffffffffffffffffffffffffffffffffffffffffffffffffffff19166020820180517bffffffffffffffffffffffffffffffffffffffffffffffffffffffff83818316178352505050506107ab565b5060008160000160006101000a81548160ff02191690831515021790555061063961048c565b73ffffffffffffffffffffffffffffffffffffffff168273ffffffffffffffffffffffffffffffffffffffff16146106a6576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161069d90610c40565b60405180910390fd5b6106af856107e2565b5b5050505050565b600081600001549050919050565b6001816000016000828254019250508190555050565b6000816000018190555050565b6000819050919050565b6106fb81610831565b61073a576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161073190610ca0565b60405180910390fd5b806107677f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc60001b6106e8565b60000160006101000a81548173ffffffffffffffffffffffffffffffffffffffff021916908373ffffffffffffffffffffffffffffffffffffffff16021790555050565b60606107d0838360405180606001604052806027815260200161103e60279139610844565b905092915050565b6000819050919050565b6107eb816106f2565b8073ffffffffffffffffffffffffffffffffffffffff167fbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b60405160405180910390a250565b600080823b905060008111915050919050565b606061084f84610831565b61088e576040517f08c379a000000000000000000000000000000000000000000000000000000000815260040161088590610cc0565b60405180910390fd5b6000808573ffffffffffffffffffffffffffffffffffffffff16856040516108b69190610bcc565b600060405180830381855af49150503d80600081146108f1576040519150601f19603f3d011682016040523d82523d6000602084013e6108f6565b606091505b5091509150610906828286610911565b925050509392505050565b6060831561092157829050610971565b6000835111156109345782518084602001fd5b816040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016109689190610bfe565b60405180910390fd5b9392505050565b600061098b61098684610d20565b610cfb565b9050828152602081018484840111156109a357600080fd5b6109ae848285610dbf565b509392505050565b6000813590506109c581611026565b92915050565b600082601f8301126109dc57600080fd5b81356109ec848260208601610978565b91505092915050565b600060208284031215610a0757600080fd5b6000610a15848285016109b6565b91505092915050565b60008060408385031215610a3157600080fd5b6000610a3f858286016109b6565b925050602083013567ffffffffffffffff811115610a5c57600080fd5b610a68858286016109cb565b9150509250929050565b610a7b81610d83565b82525050565b6000610a8c82610d51565b610a968185610d67565b9350610aa6818560208601610dce565b80840191505092915050565b6000610abd82610d5c565b610ac78185610d72565b9350610ad7818560208601610dce565b610ae081610e61565b840191505092915050565b6000610af8601b83610d72565b9150610b0382610e72565b602082019050919050565b6000610b1b602f83610d72565b9150610b2682610e9b565b604082019050919050565b6000610b3e602c83610d72565b9150610b4982610eea565b604082019050919050565b6000610b61602c83610d72565b9150610b6c82610f39565b604082019050919050565b6000610b84602d83610d72565b9150610b8f82610f88565b604082019050919050565b6000610ba7602683610d72565b9150610bb282610fd7565b604082019050919050565b610bc681610db5565b82525050565b6000610bd88284610a81565b915081905092915050565b6000602082019050610bf86000830184610a72565b92915050565b60006020820190508181036000830152610c188184610ab2565b905092915050565b60006020820190508181036000830152610c3981610aeb565b9050919050565b60006020820190508181036000830152610c5981610b0e565b9050919050565b60006020820190508181036000830152610c7981610b31565b9050919050565b60006020820190508181036000830152610c9981610b54565b9050919050565b60006020820190508181036000830152610cb981610b77565b9050919050565b60006020820190508181036000830152610cd981610b9a565b9050919050565b6000602082019050610cf56000830184610bbd565b92915050565b6000610d05610d16565b9050610d118282610e01565b919050565b6000604051905090565b600067ffffffffffffffff821115610d3b57610d3a610e32565b5b610d4482610e61565b9050602081019050919050565b600081519050919050565b600081519050919050565b600081905092915050565b600082825260208201905092915050565b6000610d8e82610d95565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b6000819050919050565b82818337600083830152505050565b60005b83811015610dec578082015181840152602081019050610dd1565b83811115610dfb576000848401525b50505050565b610e0a82610e61565b810181811067ffffffffffffffff82111715610e2957610e28610e32565b5b80604052505050565b7f4e487b7100000000000000000000000000000000000000000000000000000000600052604160045260246000fd5b6000601f19601f8301169050919050565b7f436f756e7465723a2064656372656d656e74206f766572666c6f770000000000600082015250565b7f45524331393637557067726164653a207570677261646520627265616b73206660008201527f7572746865722075706772616465730000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f64656c656761746563616c6c0000000000000000000000000000000000000000602082015250565b7f46756e6374696f6e206d7573742062652063616c6c6564207468726f7567682060008201527f6163746976652070726f78790000000000000000000000000000000000000000602082015250565b7f455243313936373a206e657720696d706c656d656e746174696f6e206973206e60008201527f6f74206120636f6e747261637400000000000000000000000000000000000000602082015250565b7f416464726573733a2064656c65676174652063616c6c20746f206e6f6e2d636f60008201527f6e74726163740000000000000000000000000000000000000000000000000000602082015250565b61102f81610d83565b811461103a57600080fd5b5056fe416464726573733a206c6f772d6c6576656c2064656c65676174652063616c6c206661696c6564a2646970667358221220a095e223f72f7004a2c1ad6ca22791a4e71300ad963839f11dcacba09398d9da64736f6c63430008030033", - "linkReferences": {}, - "deployedLinkReferences": {} -} From f62bd71d7b4900d800e226791fe99a8fb7748643 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 22 Dec 2021 11:28:37 +0100 Subject: [PATCH 05/21] fix lint --- contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol | 2 +- test/proxy/utils/UUPSUpgradeable.test.js | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol index 5624f202b2f..7b32441ff75 100644 --- a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol +++ b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol @@ -51,4 +51,4 @@ contract UUPSLegacyV1 is UUPSUpgradeableMock { function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual override { _upgradeToAndCallSecureLegacyV1(newImplementation, data, false); } -} \ No newline at end of file +} diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index c0b2b8131a3..8885e0b8330 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -76,13 +76,17 @@ contract('UUPSUpgradeable', function (accounts) { for (const artefact of Legacy) { it(`can upgrade from ${artefact._json.contractName}`, async function () { const legacyImpl = await artefact.new(); - const legacyInstance = await ERC1967Proxy.new(legacyImpl.address, '0x').then(({ address }) => artefact.at(address)); + const legacyInstance = await ERC1967Proxy.new(legacyImpl.address, '0x') + .then(({ address }) => artefact.at(address)); const receipt = await legacyInstance.upgradeTo(this.implInitial.address); - // only produce a single upgrade event - expect(receipt.logs.filter(({ address, event }) => address == legacyInstance.address && event == 'Upgraded' ).length).to.be.equal(1); - // produces the right event + const UpgradedEvents = receipt.logs.filter(({ address, event }) => + address === legacyInstance.address && + event === 'Upgraded', + ); + expect(UpgradedEvents.length).to.be.equal(1); + expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); }); }; From 190286ff398299ca67c2da1fac1ea61cc7e1ae58 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 30 Dec 2021 11:10:32 +0100 Subject: [PATCH 06/21] address comments from PR --- .../draft-IERC1822.sol} | 2 +- contracts/proxy/ERC1967/ERC1967Upgrade.sol | 7 ++++-- contracts/proxy/utils/UUPSUpgradeable.sol | 4 ++-- test/helpers/erc1967.js | 24 +++++++++++++++++++ test/proxy/Proxy.behaviour.js | 18 +++++--------- test/proxy/beacon/BeaconProxy.test.js | 16 ++++--------- .../TransparentUpgradeableProxy.behaviour.js | 21 ++++++---------- test/proxy/utils/UUPSUpgradeable.test.js | 9 +++++-- 8 files changed, 57 insertions(+), 44 deletions(-) rename contracts/{proxy/ERC1822/IProxiable.sol => interfaces/draft-IERC1822.sol} (86%) create mode 100644 test/helpers/erc1967.js diff --git a/contracts/proxy/ERC1822/IProxiable.sol b/contracts/interfaces/draft-IERC1822.sol similarity index 86% rename from contracts/proxy/ERC1822/IProxiable.sol rename to contracts/interfaces/draft-IERC1822.sol index 265f09e10aa..32d14e89915 100644 --- a/contracts/proxy/ERC1822/IProxiable.sol +++ b/contracts/interfaces/draft-IERC1822.sol @@ -3,6 +3,6 @@ pragma solidity ^0.8.0; -interface IProxiable { +interface IERC1822Proxiable { function proxiableUUID() external view returns (bytes32); } diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index adc8254bdbe..256336cde11 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.2; import "../beacon/IBeacon.sol"; -import "../ERC1822/IProxiable.sol"; +import "../../interfaces/draft-IERC1822.sol"; import "../../utils/Address.sol"; import "../../utils/StorageSlot.sol"; @@ -83,11 +83,14 @@ abstract contract ERC1967Upgrade { bytes memory data, bool forceCall ) internal { + // Upgrades from old implementations will perform a rollback test. This test requires this + // new implementation to upgrade to an old, non-ERC1822 compliant, implementation. Removing + // this special case will break upgrade paths from old UUPS implementation to new ones. if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) { _setImplementation(newImplementation); } else { require( - IProxiable(newImplementation).proxiableUUID() == _IMPLEMENTATION_SLOT, + IERC1822Proxiable(newImplementation).proxiableUUID() == _IMPLEMENTATION_SLOT, "Invalid proxiableUUID value" ); _upgradeToAndCall(newImplementation, data, forceCall); diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index a26df02edb0..9ef067a3975 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; -import "../ERC1822/IProxiable.sol"; +import "../../interfaces/draft-IERC1822.sol"; import "../ERC1967/ERC1967Upgrade.sol"; /** @@ -18,7 +18,7 @@ import "../ERC1967/ERC1967Upgrade.sol"; * * _Available since v4.1._ */ -abstract contract UUPSUpgradeable is IProxiable, ERC1967Upgrade { +abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { /// @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment address private immutable __self = address(this); diff --git a/test/helpers/erc1967.js b/test/helpers/erc1967.js new file mode 100644 index 00000000000..592ad7234ef --- /dev/null +++ b/test/helpers/erc1967.js @@ -0,0 +1,24 @@ +const { BN } = require('@openzeppelin/test-helpers'); +const ethereumjsUtil = require('ethereumjs-util'); + +const ImplementationLabel = 'eip1967.proxy.implementation'; +const AdminLabel = 'eip1967.proxy.admin'; +const BeaconLabel = 'eip1967.proxy.beacon'; + +function labelToSlot (label) { + return '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(label))).subn(1).toString(16); +} + +function toChecksumAddress (address) { + return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); +} + +module.exports = { + ImplementationLabel, + AdminLabel, + BeaconLabel, + ImplementationSlot: labelToSlot(ImplementationLabel), + AdminSlot: labelToSlot(AdminLabel), + BeaconSlot: labelToSlot(BeaconLabel), + toChecksumAddress, +}; diff --git a/test/proxy/Proxy.behaviour.js b/test/proxy/Proxy.behaviour.js index 8cb457b06c6..01ae8315c6e 100644 --- a/test/proxy/Proxy.behaviour.js +++ b/test/proxy/Proxy.behaviour.js @@ -1,16 +1,10 @@ -const { BN, expectRevert } = require('@openzeppelin/test-helpers'); -const ethereumjsUtil = require('ethereumjs-util'); +const { expectRevert } = require('@openzeppelin/test-helpers'); +const { ImplementationSlot, toChecksumAddress } = require('../helpers/erc1967'); const { expect } = require('chai'); const DummyImplementation = artifacts.require('DummyImplementation'); -const IMPLEMENTATION_LABEL = 'eip1967.proxy.implementation'; - -function toChecksumAddress (address) { - return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); -} - module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator) { it('cannot be initialized with a non-contract address', async function () { const nonContractAddress = proxyCreator; @@ -23,14 +17,14 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, }); before('deploy implementation', async function () { - this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address); + this.implementation = toChecksumAddress((await DummyImplementation.new()).address); }); const assertProxyInitialization = function ({ value, balance }) { it('sets the implementation address', async function () { - const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); - const implementation = toChecksumAddress((await web3.eth.getStorageAt(this.proxy, slot)).substr(-40)); - expect(implementation).to.be.equal(this.implementation); + const implementationSlot = await web3.eth.getStorageAt(this.proxy, ImplementationSlot); + const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + expect(implementationAddress).to.be.equal(this.implementation); }); it('initializes the proxy', async function () { diff --git a/test/proxy/beacon/BeaconProxy.test.js b/test/proxy/beacon/BeaconProxy.test.js index e02bb3b297a..e5f7b07ffca 100644 --- a/test/proxy/beacon/BeaconProxy.test.js +++ b/test/proxy/beacon/BeaconProxy.test.js @@ -1,6 +1,6 @@ -const { BN, expectRevert } = require('@openzeppelin/test-helpers'); -const ethereumjsUtil = require('ethereumjs-util'); -const { keccak256 } = ethereumjsUtil; +const { expectRevert } = require('@openzeppelin/test-helpers'); +const { keccak256 } = require('ethereumjs-util'); +const { BeaconSlot, toChecksumAddress } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -11,13 +11,6 @@ const DummyImplementationV2 = artifacts.require('DummyImplementationV2'); const BadBeaconNoImpl = artifacts.require('BadBeaconNoImpl'); const BadBeaconNotContract = artifacts.require('BadBeaconNotContract'); -function toChecksumAddress (address) { - return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0').substr(-40)); -} - -const BEACON_LABEL = 'eip1967.proxy.beacon'; -const BEACON_SLOT = '0x' + new BN(keccak256(Buffer.from(BEACON_LABEL))).subn(1).toString(16); - contract('BeaconProxy', function (accounts) { const [anotherAccount] = accounts; @@ -53,7 +46,8 @@ contract('BeaconProxy', function (accounts) { describe('initialization', function () { before(function () { this.assertInitialized = async ({ value, balance }) => { - const beaconAddress = toChecksumAddress(await web3.eth.getStorageAt(this.proxy.address, BEACON_SLOT)); + const beaconSlot = await web3.eth.getStorageAt(this.proxy.address, BeaconSlot); + const beaconAddress = toChecksumAddress(beaconSlot.substr(-40)); expect(beaconAddress).to.equal(this.beacon.address); const dummy = new DummyImplementation(this.proxy.address); diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js index 54b78e06420..d2b96166947 100644 --- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js +++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js @@ -1,6 +1,6 @@ const { BN, expectRevert, expectEvent, constants } = require('@openzeppelin/test-helpers'); const { ZERO_ADDRESS } = constants; -const ethereumjsUtil = require('ethereumjs-util'); +const { ImplementationSlot, AdminSlot, toChecksumAddress } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -16,13 +16,6 @@ const InitializableMock = artifacts.require('InitializableMock'); const DummyImplementation = artifacts.require('DummyImplementation'); const ClashingImplementation = artifacts.require('ClashingImplementation'); -const IMPLEMENTATION_LABEL = 'eip1967.proxy.implementation'; -const ADMIN_LABEL = 'eip1967.proxy.admin'; - -function toChecksumAddress (address) { - return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0').substr(-40)); -} - module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createProxy, accounts) { const [proxyAdminAddress, proxyAdminOwner, anotherAccount] = accounts; @@ -312,15 +305,15 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro describe('storage', function () { it('should store the implementation address in specified location', async function () { - const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16); - const implementation = toChecksumAddress(await web3.eth.getStorageAt(this.proxyAddress, slot)); - expect(implementation).to.be.equal(this.implementationV0); + const implementationSlot = await web3.eth.getStorageAt(this.proxyAddress, ImplementationSlot); + const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + expect(implementationAddress).to.be.equal(this.implementationV0); }); it('should store the admin proxy in specified location', async function () { - const slot = '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(ADMIN_LABEL))).subn(1).toString(16); - const proxyAdmin = toChecksumAddress(await web3.eth.getStorageAt(this.proxyAddress, slot)); - expect(proxyAdmin).to.be.equal(proxyAdminAddress); + const proxyAdminSlot = await web3.eth.getStorageAt(this.proxyAddress, AdminSlot); + const proxyAdminAddress = toChecksumAddress(proxyAdminSlot.substr(-40)); + expect(proxyAdminAddress).to.be.equal(proxyAdminAddress); }); }); diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 8885e0b8330..5d4115d655f 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -1,4 +1,6 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); +const { web3 } = require('@openzeppelin/test-helpers/src/setup'); +const { ImplementationSlot, toChecksumAddress } = require('../../helpers/erc1967'); const ERC1967Proxy = artifacts.require('ERC1967Proxy'); const UUPSUpgradeableMock = artifacts.require('UUPSUpgradeableMock'); @@ -55,9 +57,8 @@ contract('UUPSUpgradeable', function (accounts) { // delegate to a non existing upgradeTo function causes a low level revert it('reject upgrade to non uups implementation', async function () { - await expectRevert( + await expectRevert.unspecified( this.instance.upgradeTo(this.implUpgradeNonUUPS.address), - 'Transaction reverted: function selector was not recognized and there\'s no fallback function', ); }); @@ -88,6 +89,10 @@ contract('UUPSUpgradeable', function (accounts) { expect(UpgradedEvents.length).to.be.equal(1); expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); + + const implementationSlot = await web3.eth.getStorageAt(legacyInstance.address, ImplementationSlot); + const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + expect(implementationAddress).to.be.equal(this.implInitial.address); }); }; }); From e61fec08767a6f886f78621087a113014c045b2a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 30 Dec 2021 11:24:37 +0100 Subject: [PATCH 07/21] Apply suggestions from code review Co-authored-by: Francisco Giordano --- contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol | 2 ++ contracts/proxy/utils/UUPSUpgradeable.sol | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol index 7b32441ff75..292920c5803 100644 --- a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol +++ b/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.0; import "../TestInProd.sol"; +// This contract implements the pre-4.5 UUPS upgrade function with a rollback test. +// It's used to test that newer UUPS contracts are considered valid upgrades by older UUPS contracts. contract UUPSLegacyV1 is UUPSUpgradeableMock { // Inlined from ERC1967Upgrade bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143; diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index 9ef067a3975..3d85249a8f8 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -40,7 +40,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { * callable on the implementing contract but not through proxies. */ modifier notDelegated() { - require(address(this) == __self, "Function must not be called through delegatecall"); + require(address(this) == __self, "UUPSUpgradeable: must not be called through delegatecall"); _; } From ed716652df5e3ed46596a6e28be56d3b05cd62a9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 30 Dec 2021 11:27:58 +0100 Subject: [PATCH 08/21] fix lint & test --- test/proxy/beacon/BeaconProxy.test.js | 1 - test/proxy/utils/UUPSUpgradeable.test.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/proxy/beacon/BeaconProxy.test.js b/test/proxy/beacon/BeaconProxy.test.js index e5f7b07ffca..06847157f2b 100644 --- a/test/proxy/beacon/BeaconProxy.test.js +++ b/test/proxy/beacon/BeaconProxy.test.js @@ -1,5 +1,4 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { keccak256 } = require('ethereumjs-util'); const { BeaconSlot, toChecksumAddress } = require('../../helpers/erc1967'); const { expect } = require('chai'); diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 5d4115d655f..0dba80693cb 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -69,7 +69,7 @@ contract('UUPSUpgradeable', function (accounts) { // infinite loop reverts when a nested call is out-of-gas await expectRevert( this.instance.upgradeTo(otherInstance.address), - 'Function must not be called through delegatecall', + 'UUPSUpgradeable: must not be called through delegatecall', ); }); From 6402acece6fb12218206cbffeaa0197630501879 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 30 Dec 2021 11:48:31 +0100 Subject: [PATCH 09/21] reduce test dependency on ethereumjs-util --- test/helpers/erc1967.js | 10 +--------- test/proxy/Proxy.behaviour.js | 6 +++--- test/proxy/beacon/BeaconProxy.test.js | 4 ++-- .../TransparentUpgradeableProxy.behaviour.js | 6 +++--- test/proxy/utils/UUPSUpgradeable.test.js | 4 ++-- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/test/helpers/erc1967.js b/test/helpers/erc1967.js index 592ad7234ef..163ad3d73a4 100644 --- a/test/helpers/erc1967.js +++ b/test/helpers/erc1967.js @@ -1,16 +1,9 @@ -const { BN } = require('@openzeppelin/test-helpers'); -const ethereumjsUtil = require('ethereumjs-util'); - const ImplementationLabel = 'eip1967.proxy.implementation'; const AdminLabel = 'eip1967.proxy.admin'; const BeaconLabel = 'eip1967.proxy.beacon'; function labelToSlot (label) { - return '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(label))).subn(1).toString(16); -} - -function toChecksumAddress (address) { - return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0')); + return '0x' + web3.utils.toBN(web3.utils.keccak256(label)).subn(1).toString(16); } module.exports = { @@ -20,5 +13,4 @@ module.exports = { ImplementationSlot: labelToSlot(ImplementationLabel), AdminSlot: labelToSlot(AdminLabel), BeaconSlot: labelToSlot(BeaconLabel), - toChecksumAddress, }; diff --git a/test/proxy/Proxy.behaviour.js b/test/proxy/Proxy.behaviour.js index 01ae8315c6e..4ee708c599a 100644 --- a/test/proxy/Proxy.behaviour.js +++ b/test/proxy/Proxy.behaviour.js @@ -1,5 +1,5 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { ImplementationSlot, toChecksumAddress } = require('../helpers/erc1967'); +const { ImplementationSlot } = require('../helpers/erc1967'); const { expect } = require('chai'); @@ -17,13 +17,13 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, }); before('deploy implementation', async function () { - this.implementation = toChecksumAddress((await DummyImplementation.new()).address); + this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address); }); const assertProxyInitialization = function ({ value, balance }) { it('sets the implementation address', async function () { const implementationSlot = await web3.eth.getStorageAt(this.proxy, ImplementationSlot); - const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implementation); }); diff --git a/test/proxy/beacon/BeaconProxy.test.js b/test/proxy/beacon/BeaconProxy.test.js index 06847157f2b..99aa9f29aa6 100644 --- a/test/proxy/beacon/BeaconProxy.test.js +++ b/test/proxy/beacon/BeaconProxy.test.js @@ -1,5 +1,5 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { BeaconSlot, toChecksumAddress } = require('../../helpers/erc1967'); +const { BeaconSlot } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -46,7 +46,7 @@ contract('BeaconProxy', function (accounts) { before(function () { this.assertInitialized = async ({ value, balance }) => { const beaconSlot = await web3.eth.getStorageAt(this.proxy.address, BeaconSlot); - const beaconAddress = toChecksumAddress(beaconSlot.substr(-40)); + const beaconAddress = web3.utils.toChecksumAddress(beaconSlot.substr(-40)); expect(beaconAddress).to.equal(this.beacon.address); const dummy = new DummyImplementation(this.proxy.address); diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js index d2b96166947..bb48927cadd 100644 --- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js +++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js @@ -1,6 +1,6 @@ const { BN, expectRevert, expectEvent, constants } = require('@openzeppelin/test-helpers'); const { ZERO_ADDRESS } = constants; -const { ImplementationSlot, AdminSlot, toChecksumAddress } = require('../../helpers/erc1967'); +const { ImplementationSlot, AdminSlot } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -306,13 +306,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro describe('storage', function () { it('should store the implementation address in specified location', async function () { const implementationSlot = await web3.eth.getStorageAt(this.proxyAddress, ImplementationSlot); - const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implementationV0); }); it('should store the admin proxy in specified location', async function () { const proxyAdminSlot = await web3.eth.getStorageAt(this.proxyAddress, AdminSlot); - const proxyAdminAddress = toChecksumAddress(proxyAdminSlot.substr(-40)); + const proxyAdminAddress = web3.utils.toChecksumAddress(proxyAdminSlot.substr(-40)); expect(proxyAdminAddress).to.be.equal(proxyAdminAddress); }); }); diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 0dba80693cb..5837f30b450 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -1,6 +1,6 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); const { web3 } = require('@openzeppelin/test-helpers/src/setup'); -const { ImplementationSlot, toChecksumAddress } = require('../../helpers/erc1967'); +const { ImplementationSlot } = require('../../helpers/erc1967'); const ERC1967Proxy = artifacts.require('ERC1967Proxy'); const UUPSUpgradeableMock = artifacts.require('UUPSUpgradeableMock'); @@ -91,7 +91,7 @@ contract('UUPSUpgradeable', function (accounts) { expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); const implementationSlot = await web3.eth.getStorageAt(legacyInstance.address, ImplementationSlot); - const implementationAddress = toChecksumAddress(implementationSlot.substr(-40)); + const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implInitial.address); }); }; From 02a91e27c95ff7d13740b3f5cea805a337ee6c2d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 30 Dec 2021 13:49:01 +0100 Subject: [PATCH 10/21] getSlot utils --- test/helpers/erc1967.js | 8 ++++++++ test/proxy/Proxy.behaviour.js | 4 ++-- test/proxy/beacon/BeaconProxy.test.js | 4 ++-- .../transparent/TransparentUpgradeableProxy.behaviour.js | 6 +++--- test/proxy/utils/UUPSUpgradeable.test.js | 4 ++-- 5 files changed, 17 insertions(+), 9 deletions(-) diff --git a/test/helpers/erc1967.js b/test/helpers/erc1967.js index 163ad3d73a4..4b06e618ade 100644 --- a/test/helpers/erc1967.js +++ b/test/helpers/erc1967.js @@ -6,6 +6,13 @@ function labelToSlot (label) { return '0x' + web3.utils.toBN(web3.utils.keccak256(label)).subn(1).toString(16); } +function getSlot (address, slot) { + return web3.eth.getStorageAt( + web3.utils.isAddress(address) ? address : address.address, + web3.utils.isHex(slot) ? slot : labelToSlot(slot), + ); +} + module.exports = { ImplementationLabel, AdminLabel, @@ -13,4 +20,5 @@ module.exports = { ImplementationSlot: labelToSlot(ImplementationLabel), AdminSlot: labelToSlot(AdminLabel), BeaconSlot: labelToSlot(BeaconLabel), + getSlot }; diff --git a/test/proxy/Proxy.behaviour.js b/test/proxy/Proxy.behaviour.js index 4ee708c599a..435792f2302 100644 --- a/test/proxy/Proxy.behaviour.js +++ b/test/proxy/Proxy.behaviour.js @@ -1,5 +1,5 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { ImplementationSlot } = require('../helpers/erc1967'); +const { getSlot, ImplementationSlot } = require('../helpers/erc1967'); const { expect } = require('chai'); @@ -22,7 +22,7 @@ module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, const assertProxyInitialization = function ({ value, balance }) { it('sets the implementation address', async function () { - const implementationSlot = await web3.eth.getStorageAt(this.proxy, ImplementationSlot); + const implementationSlot = await getSlot(this.proxy, ImplementationSlot); const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implementation); }); diff --git a/test/proxy/beacon/BeaconProxy.test.js b/test/proxy/beacon/BeaconProxy.test.js index 99aa9f29aa6..0a4a8d0cfb9 100644 --- a/test/proxy/beacon/BeaconProxy.test.js +++ b/test/proxy/beacon/BeaconProxy.test.js @@ -1,5 +1,5 @@ const { expectRevert } = require('@openzeppelin/test-helpers'); -const { BeaconSlot } = require('../../helpers/erc1967'); +const { getSlot, BeaconSlot } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -45,7 +45,7 @@ contract('BeaconProxy', function (accounts) { describe('initialization', function () { before(function () { this.assertInitialized = async ({ value, balance }) => { - const beaconSlot = await web3.eth.getStorageAt(this.proxy.address, BeaconSlot); + const beaconSlot = await getSlot(this.proxy, BeaconSlot); const beaconAddress = web3.utils.toChecksumAddress(beaconSlot.substr(-40)); expect(beaconAddress).to.equal(this.beacon.address); diff --git a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js index bb48927cadd..33fef6f4119 100644 --- a/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js +++ b/test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js @@ -1,6 +1,6 @@ const { BN, expectRevert, expectEvent, constants } = require('@openzeppelin/test-helpers'); const { ZERO_ADDRESS } = constants; -const { ImplementationSlot, AdminSlot } = require('../../helpers/erc1967'); +const { getSlot, ImplementationSlot, AdminSlot } = require('../../helpers/erc1967'); const { expect } = require('chai'); @@ -305,13 +305,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy (createPro describe('storage', function () { it('should store the implementation address in specified location', async function () { - const implementationSlot = await web3.eth.getStorageAt(this.proxyAddress, ImplementationSlot); + const implementationSlot = await getSlot(this.proxy, ImplementationSlot); const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implementationV0); }); it('should store the admin proxy in specified location', async function () { - const proxyAdminSlot = await web3.eth.getStorageAt(this.proxyAddress, AdminSlot); + const proxyAdminSlot = await getSlot(this.proxy, AdminSlot); const proxyAdminAddress = web3.utils.toChecksumAddress(proxyAdminSlot.substr(-40)); expect(proxyAdminAddress).to.be.equal(proxyAdminAddress); }); diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 5837f30b450..58529171871 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -1,6 +1,6 @@ const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); const { web3 } = require('@openzeppelin/test-helpers/src/setup'); -const { ImplementationSlot } = require('../../helpers/erc1967'); +const { getSlot, ImplementationSlot } = require('../../helpers/erc1967'); const ERC1967Proxy = artifacts.require('ERC1967Proxy'); const UUPSUpgradeableMock = artifacts.require('UUPSUpgradeableMock'); @@ -90,7 +90,7 @@ contract('UUPSUpgradeable', function (accounts) { expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); - const implementationSlot = await web3.eth.getStorageAt(legacyInstance.address, ImplementationSlot); + const implementationSlot = await getSlot(legacyInstance, ImplementationSlot); const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); expect(implementationAddress).to.be.equal(this.implInitial.address); }); From 032da70f1ae3181ef6dd54c071b47e8e98704f32 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jan 2022 14:14:48 +0100 Subject: [PATCH 11/21] changelog and documentation for the new UUPSUpgrade --- CHANGELOG.md | 2 ++ contracts/proxy/ERC1967/ERC1967Upgrade.sol | 4 ++-- contracts/proxy/README.adoc | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb6a58dce1e..fd62b0709f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ * `ERC721`: improved revert reason when transferring from wrong owner. ([#2975](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2975)) * `Votes`: Added a base contract for vote tracking with delegation. ([#2944](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2944)) * `ERC721Votes`: Added an extension of ERC721 enabled with vote tracking and delegation. ([#2944](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2944)) + * `ERC1967Upgrade`: Refactor the secure upgrade to use `ERC1822` instead of the previous rollback mechanism. This reduces code complexity and attack surface with similar security guarantees. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) + * `UUPSUpgradeable`: Add `ERC1822` compliance to support the updated secure upgrade mechanism. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) ## 4.4.1 (2021-12-10) diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 256336cde11..272a0c02148 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -83,8 +83,8 @@ abstract contract ERC1967Upgrade { bytes memory data, bool forceCall ) internal { - // Upgrades from old implementations will perform a rollback test. This test requires this - // new implementation to upgrade to an old, non-ERC1822 compliant, implementation. Removing + // Upgrades from old implementations will perform a rollback test. This test requires the + // new implementation to upgrade to the old, non-ERC1822 compliant, implementation. Removing // this special case will break upgrade paths from old UUPS implementation to new ones. if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) { _setImplementation(newImplementation); diff --git a/contracts/proxy/README.adoc b/contracts/proxy/README.adoc index ae278b083ce..9f37b267f90 100644 --- a/contracts/proxy/README.adoc +++ b/contracts/proxy/README.adoc @@ -17,14 +17,14 @@ In order to avoid clashes with the storage variables of the implementation contr There are two alternative ways to add upgradeability to an ERC1967 proxy. Their differences are explained below in <>. - {TransparentUpgradeableProxy}: A proxy with a built in admin and upgrade interface. -- {UUPSUpgradeable}: An upgradeability mechanism to be included in the implementation for an ERC1967 proxy. +- {UUPSUpgradeable}: An upgradeability mechanism to be included in the implementation contract. CAUTION: Using upgradeable proxies correctly and securely is a difficult task that requires deep knowledge of the proxy pattern, Solidity, and the EVM. Unless you want a lot of low level control, we recommend using the xref:upgrades-plugins::index.adoc[OpenZeppelin Upgrades Plugins] for Truffle and Hardhat. A different family of proxies are beacon proxies. This pattern, popularized by Dharma, allows multiple proxies to be upgraded to a different implementation in a single transaction. - {BeaconProxy}: A proxy that retreives its implementation from a beacon contract. -- {UpgradeableBeacon}: A beacon contract that can be upgraded. +- {UpgradeableBeacon}: A beacon contract with a built in admin that can upgrade the {BeaconProxy} pointing to it. In this pattern, the proxy contract doesn't hold the implementation address in storage like an ERC1967 proxy, instead the address is stored in a separate beacon contract. The `upgrade` operations that are sent to the beacon instead of to the proxy contract, and all proxies that follow that beacon are automatically upgraded. @@ -48,6 +48,8 @@ By default, the upgrade functionality included in {UUPSUpgradeable} contains a s - Adding a flag mechanism in the implementation that will disable the upgrade function when triggered. - Upgrading to an implementation that features an upgrade mechanism without the additional security check, and then upgrading again to another implementation without the upgrade mechanism. +The current implementation of this security mechanism uses https://eips.ethereum.org/EIPS/eip-1822[EIP1822] to detect the storage slot used by the implementation. A previous implementation, now deprecated, relied on a rollback check. It is possible to upgrade from a contract using the old mechanism to a new one. The inverse is however not possible, as old implementations (before version 4.5) did not include the `ERC1822` interface. + == Core {{Proxy}} From 86c77d88eda6759a3916e3c27563e8ef467df855 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jan 2022 14:15:45 +0100 Subject: [PATCH 12/21] fix lint --- test/helpers/erc1967.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers/erc1967.js b/test/helpers/erc1967.js index 4b06e618ade..aab0e2288a3 100644 --- a/test/helpers/erc1967.js +++ b/test/helpers/erc1967.js @@ -20,5 +20,5 @@ module.exports = { ImplementationSlot: labelToSlot(ImplementationLabel), AdminSlot: labelToSlot(AdminLabel), BeaconSlot: labelToSlot(BeaconLabel), - getSlot + getSlot, }; From 828d7847cf5dea0bd09a7849de830d40d18b9f50 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jan 2022 14:20:15 +0100 Subject: [PATCH 13/21] minor wording change --- contracts/proxy/ERC1967/ERC1967Upgrade.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 272a0c02148..b3202c37eb2 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -83,8 +83,8 @@ abstract contract ERC1967Upgrade { bytes memory data, bool forceCall ) internal { - // Upgrades from old implementations will perform a rollback test. This test requires the - // new implementation to upgrade to the old, non-ERC1822 compliant, implementation. Removing + // Upgrades from old implementations will perform a rollback test. This test requires the new + // implementation to upgrade back to the old, non-ERC1822 compliant, implementation. Removing // this special case will break upgrade paths from old UUPS implementation to new ones. if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) { _setImplementation(newImplementation); From 7d77e9bfe29846d2ce7117746bf21b96fcf743fd Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 10 Jan 2022 21:38:32 +0100 Subject: [PATCH 14/21] address comments for the PR --- .../UUPSLegacyV1.sol => UUPSLegacy.sol} | 4 +- ...TestInProd.sol => UUPSUpgradeableMock.sol} | 0 test/proxy/utils/UUPSUpgradeable.test.js | 42 +++++++------------ 3 files changed, 17 insertions(+), 29 deletions(-) rename contracts/mocks/UUPS/{legacy/UUPSLegacyV1.sol => UUPSLegacy.sol} (95%) rename contracts/mocks/UUPS/{TestInProd.sol => UUPSUpgradeableMock.sol} (100%) diff --git a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol b/contracts/mocks/UUPS/UUPSLegacy.sol similarity index 95% rename from contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol rename to contracts/mocks/UUPS/UUPSLegacy.sol index 292920c5803..3250a331195 100644 --- a/contracts/mocks/UUPS/legacy/UUPSLegacyV1.sol +++ b/contracts/mocks/UUPS/UUPSLegacy.sol @@ -2,11 +2,11 @@ pragma solidity ^0.8.0; -import "../TestInProd.sol"; +import "./UUPSUpgradeableMock.sol"; // This contract implements the pre-4.5 UUPS upgrade function with a rollback test. // It's used to test that newer UUPS contracts are considered valid upgrades by older UUPS contracts. -contract UUPSLegacyV1 is UUPSUpgradeableMock { +contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock { // Inlined from ERC1967Upgrade bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143; diff --git a/contracts/mocks/UUPS/TestInProd.sol b/contracts/mocks/UUPS/UUPSUpgradeableMock.sol similarity index 100% rename from contracts/mocks/UUPS/TestInProd.sol rename to contracts/mocks/UUPS/UUPSUpgradeableMock.sol diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 58529171871..3fd588f155d 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -6,10 +6,9 @@ const ERC1967Proxy = artifacts.require('ERC1967Proxy'); const UUPSUpgradeableMock = artifacts.require('UUPSUpgradeableMock'); const UUPSUpgradeableUnsafeMock = artifacts.require('UUPSUpgradeableUnsafeMock'); const UUPSUpgradeableBrokenMock = artifacts.require('UUPSUpgradeableBrokenMock'); +const UUPSUpgradeableLegacyMock = artifacts.require('UUPSUpgradeableLegacyMock'); const CountersImpl = artifacts.require('CountersImpl'); -const Legacy = [ 'UUPSLegacyV1' ].map(artifacts.require); - contract('UUPSUpgradeable', function (accounts) { before(async function () { this.implInitial = await UUPSUpgradeableMock.new(); @@ -48,13 +47,6 @@ contract('UUPSUpgradeable', function (accounts) { expectEvent(receipt, 'Upgraded', { implementation: this.implUpgradeUnsafe.address }); }); - it.skip('reject upgrade to broken upgradeable implementation (no longer supported)', async function () { - await expectRevert( - this.instance.upgradeTo(this.implUpgradeBroken.address), - 'ERC1967Upgrade: upgrade breaks further upgrades', - ); - }); - // delegate to a non existing upgradeTo function causes a low level revert it('reject upgrade to non uups implementation', async function () { await expectRevert.unspecified( @@ -73,27 +65,23 @@ contract('UUPSUpgradeable', function (accounts) { ); }); - describe('compatibility with legacy version of UUPSUpgradeable', function () { - for (const artefact of Legacy) { - it(`can upgrade from ${artefact._json.contractName}`, async function () { - const legacyImpl = await artefact.new(); - const legacyInstance = await ERC1967Proxy.new(legacyImpl.address, '0x') - .then(({ address }) => artefact.at(address)); + it('can upgrade from legacy implementations', async function () { + const legacyImpl = await UUPSUpgradeableLegacyMock.new(); + const legacyInstance = await ERC1967Proxy.new(legacyImpl.address, '0x') + .then(({ address }) => UUPSUpgradeableLegacyMock.at(address)); - const receipt = await legacyInstance.upgradeTo(this.implInitial.address); + const receipt = await legacyInstance.upgradeTo(this.implInitial.address); - const UpgradedEvents = receipt.logs.filter(({ address, event }) => - address === legacyInstance.address && - event === 'Upgraded', - ); - expect(UpgradedEvents.length).to.be.equal(1); + const UpgradedEvents = receipt.logs.filter(({ address, event }) => + address === legacyInstance.address && + event === 'Upgraded', + ); + expect(UpgradedEvents.length).to.be.equal(1); - expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); + expectEvent(receipt, 'Upgraded', { implementation: this.implInitial.address }); - const implementationSlot = await getSlot(legacyInstance, ImplementationSlot); - const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); - expect(implementationAddress).to.be.equal(this.implInitial.address); - }); - }; + const implementationSlot = await getSlot(legacyInstance, ImplementationSlot); + const implementationAddress = web3.utils.toChecksumAddress(implementationSlot.substr(-40)); + expect(implementationAddress).to.be.equal(this.implInitial.address); }); }); From c39457e411816f112a49d61bfbd3abdf4d3ed9e0 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 10 Jan 2022 19:57:38 -0300 Subject: [PATCH 15/21] add docs to IERC1822Proxiable --- contracts/interfaces/draft-IERC1822.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contracts/interfaces/draft-IERC1822.sol b/contracts/interfaces/draft-IERC1822.sol index 32d14e89915..46dd05ba399 100644 --- a/contracts/interfaces/draft-IERC1822.sol +++ b/contracts/interfaces/draft-IERC1822.sol @@ -3,6 +3,15 @@ pragma solidity ^0.8.0; +/** + * @dev ERC1822: Universal Upgradeable Proxy Standard (UUPS) documents a method for upgradeability through a simplified + * proxy whose upgrades are fully controlled by the current implementation. + */ interface IERC1822Proxiable { + /** + * @dev Returns the storage slot that the proxiable contract assumes is being used to store the implementation + * address. Must revert if invoked through delegatecall, i.e. through a proxy, otherwise risks bricking a contract + * that upgrades to it. + */ function proxiableUUID() external view returns (bytes32); } From 1ac5e9ec6f84e32bdd10af24556914c468115cae Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jan 2022 23:06:46 -0300 Subject: [PATCH 16/21] remove unused broken uups contract --- contracts/mocks/UUPS/UUPSUpgradeableMock.sol | 10 ---------- test/proxy/utils/UUPSUpgradeable.test.js | 2 -- 2 files changed, 12 deletions(-) diff --git a/contracts/mocks/UUPS/UUPSUpgradeableMock.sol b/contracts/mocks/UUPS/UUPSUpgradeableMock.sol index bbb610300fa..367303ec0b0 100644 --- a/contracts/mocks/UUPS/UUPSUpgradeableMock.sol +++ b/contracts/mocks/UUPS/UUPSUpgradeableMock.sol @@ -19,13 +19,3 @@ contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock { ERC1967Upgrade._upgradeToAndCall(newImplementation, data, false); } } - -contract UUPSUpgradeableBrokenMock is UUPSUpgradeableMock { - function upgradeTo(address) external virtual override { - // pass - } - - function upgradeToAndCall(address, bytes memory) external payable virtual override { - // pass - } -} diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 3fd588f155d..83a368fabd7 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -5,7 +5,6 @@ const { getSlot, ImplementationSlot } = require('../../helpers/erc1967'); const ERC1967Proxy = artifacts.require('ERC1967Proxy'); const UUPSUpgradeableMock = artifacts.require('UUPSUpgradeableMock'); const UUPSUpgradeableUnsafeMock = artifacts.require('UUPSUpgradeableUnsafeMock'); -const UUPSUpgradeableBrokenMock = artifacts.require('UUPSUpgradeableBrokenMock'); const UUPSUpgradeableLegacyMock = artifacts.require('UUPSUpgradeableLegacyMock'); const CountersImpl = artifacts.require('CountersImpl'); @@ -14,7 +13,6 @@ contract('UUPSUpgradeable', function (accounts) { this.implInitial = await UUPSUpgradeableMock.new(); this.implUpgradeOk = await UUPSUpgradeableMock.new(); this.implUpgradeUnsafe = await UUPSUpgradeableUnsafeMock.new(); - this.implUpgradeBroken = await UUPSUpgradeableBrokenMock.new(); this.implUpgradeNonUUPS = await CountersImpl.new(); }); From dc3a456ded3b223632590c97fd2485692b938623 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jan 2022 23:09:37 -0300 Subject: [PATCH 17/21] rename _upgradeToAndCallSecure -> _upgradeToAndCallUUPS --- contracts/proxy/ERC1967/ERC1967Upgrade.sol | 2 +- contracts/proxy/utils/UUPSUpgradeable.sol | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 8cd5693743c..8d19b3def96 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -78,7 +78,7 @@ abstract contract ERC1967Upgrade { * * Emits an {Upgraded} event. */ - function _upgradeToAndCallSecure( + function _upgradeToAndCallUUPS( address newImplementation, bytes memory data, bool forceCall diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index 5dde8fa2aaf..f74fa23a9e0 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -61,7 +61,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { */ function upgradeTo(address newImplementation) external virtual onlyProxy { _authorizeUpgrade(newImplementation); - _upgradeToAndCallSecure(newImplementation, new bytes(0), false); + _upgradeToAndCallUUPS(newImplementation, new bytes(0), false); } /** @@ -74,7 +74,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { */ function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual onlyProxy { _authorizeUpgrade(newImplementation); - _upgradeToAndCallSecure(newImplementation, data, true); + _upgradeToAndCallUUPS(newImplementation, data, true); } /** From cd162be1db250d02748dfe77a26a03e8976ea313 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 12 Jan 2022 23:48:07 -0300 Subject: [PATCH 18/21] improve revert reasons --- contracts/proxy/ERC1967/ERC1967Upgrade.sol | 9 +++++---- test/proxy/utils/UUPSUpgradeable.test.js | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 8d19b3def96..45a45991676 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -89,10 +89,11 @@ abstract contract ERC1967Upgrade { if (StorageSlot.getBooleanSlot(_ROLLBACK_SLOT).value) { _setImplementation(newImplementation); } else { - require( - IERC1822Proxiable(newImplementation).proxiableUUID() == _IMPLEMENTATION_SLOT, - "Invalid proxiableUUID value" - ); + try IERC1822Proxiable(newImplementation).proxiableUUID() returns (bytes32 slot) { + require(slot == _IMPLEMENTATION_SLOT, "ERC1967Upgrade: unsupported proxiableUUID"); + } catch { + revert("ERC1967Upgrade: new implementation is not UUPS"); + } _upgradeToAndCall(newImplementation, data, forceCall); } } diff --git a/test/proxy/utils/UUPSUpgradeable.test.js b/test/proxy/utils/UUPSUpgradeable.test.js index 83a368fabd7..466081d2050 100644 --- a/test/proxy/utils/UUPSUpgradeable.test.js +++ b/test/proxy/utils/UUPSUpgradeable.test.js @@ -47,8 +47,9 @@ contract('UUPSUpgradeable', function (accounts) { // delegate to a non existing upgradeTo function causes a low level revert it('reject upgrade to non uups implementation', async function () { - await expectRevert.unspecified( + await expectRevert( this.instance.upgradeTo(this.implUpgradeNonUUPS.address), + 'ERC1967Upgrade: new implementation is not UUPS', ); }); @@ -56,10 +57,9 @@ contract('UUPSUpgradeable', function (accounts) { const { address } = await ERC1967Proxy.new(this.implInitial.address, '0x'); const otherInstance = await UUPSUpgradeableMock.at(address); - // infinite loop reverts when a nested call is out-of-gas await expectRevert( this.instance.upgradeTo(otherInstance.address), - 'UUPSUpgradeable: must not be called through delegatecall', + 'ERC1967Upgrade: new implementation is not UUPS', ); }); From fb8f23cd6ba4ffcb7bcbbac3f3bc3ab98bdae97a Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 13 Jan 2022 14:58:06 -0300 Subject: [PATCH 19/21] add comment for __setImplementation in mock --- contracts/mocks/UUPS/UUPSLegacy.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/mocks/UUPS/UUPSLegacy.sol b/contracts/mocks/UUPS/UUPSLegacy.sol index 3250a331195..550a6221b48 100644 --- a/contracts/mocks/UUPS/UUPSLegacy.sol +++ b/contracts/mocks/UUPS/UUPSLegacy.sol @@ -10,6 +10,8 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock { // Inlined from ERC1967Upgrade bytes32 private constant _ROLLBACK_SLOT = 0x4910fdfa16fed3260ed0e7147f7cc6da11a60208b5b9406d12a635614ffd9143; + // ERC1967Upgrade._setImplementation is private so we reproduce it here. + // An extra underscore prevents a name clash error. function __setImplementation(address newImplementation) private { require(Address.isContract(newImplementation), "ERC1967: new implementation is not a contract"); StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; From d5e8dd06bfab0b4ecdff5663b8c5e3f27405f152 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 13 Jan 2022 15:26:16 -0300 Subject: [PATCH 20/21] write more prominent warning about proxiable bricking risk --- contracts/interfaces/draft-IERC1822.sol | 7 +++++-- contracts/proxy/utils/UUPSUpgradeable.sol | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/contracts/interfaces/draft-IERC1822.sol b/contracts/interfaces/draft-IERC1822.sol index 46dd05ba399..51d8f41ddcc 100644 --- a/contracts/interfaces/draft-IERC1822.sol +++ b/contracts/interfaces/draft-IERC1822.sol @@ -10,8 +10,11 @@ pragma solidity ^0.8.0; interface IERC1822Proxiable { /** * @dev Returns the storage slot that the proxiable contract assumes is being used to store the implementation - * address. Must revert if invoked through delegatecall, i.e. through a proxy, otherwise risks bricking a contract - * that upgrades to it. + * address. + * + * IMPORTANT: A proxy pointing at a proxiable contract should not be considered proxiable itself, because this risks + * bricking a proxy that upgrades to it, by delegating to itself until out of gas. Thus it is critical that this + * function revert if invoked through a proxy. */ function proxiableUUID() external view returns (bytes32); } diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index f74fa23a9e0..5d544c4f016 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -47,6 +47,10 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { /** * @dev Implementation of the ERC1822 {proxiableUUID} function. This returns the storage slot used by the * implementation. It is used to validate that the this implementation remains valid after an upgrade. + * + * IMPORTANT: A proxy pointing at a proxiable contract should not be considered proxiable itself, because this risks + * bricking a proxy that upgrades to it, by delegating to itself until out of gas. Thus it is critical that this + * function revert if invoked through a proxy. This is guaranteed by the `notDelegated` modifier. */ function proxiableUUID() external view virtual override notDelegated returns (bytes32) { return _IMPLEMENTATION_SLOT; From efe04e3724dce00dc7cd7f32505905b596d6440a Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Thu, 13 Jan 2022 15:36:20 -0300 Subject: [PATCH 21/21] document rename as breaking change --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6d794b335b..7fdf8dd9c69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,13 +16,14 @@ * `ERC20`: reduce allowance before triggering transfer. ([#3056](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3056)) * `ERC20`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085)) * `ERC777`: do not update allowance on `transferFrom` when allowance is `type(uint256).max`. ([#3085](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#3085)) - * `ERC1967Upgrade`: Refactor the secure upgrade to use `ERC1822` instead of the previous rollback mechanism. This reduces code complexity and attack surface with similar security guarantees. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) * `SignedMath`: a new signed version of the Math library with `max`, `min`, and `average`. ([#2686](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2686)) + * `ERC1967Upgrade`: Refactor the secure upgrade to use `ERC1822` instead of the previous rollback mechanism. This reduces code complexity and attack surface with similar security guarantees. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) * `UUPSUpgradeable`: Add `ERC1822` compliance to support the updated secure upgrade mechanism. ([#3021](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3021)) -### Breaking change +### Breaking changes -Solidity pragma in `utils/Address.sol` is increased from `^0.8.0` to `^0.8.1`. This is required by the `account.code.length` syntax that replaces inline assembly. This may require users to bump their compiler version from `0.8.0` to `0.8.1` or later. Note that other parts of the code already include stricter requirements. +* `ERC1967Upgrade`: The function `_upgradeToAndCallSecure` was renamed to `_upgradeToAndCallUUPS`, along with the change in security mechanism described above. +* `Address`: The Solidity pragma is increased from `^0.8.0` to `^0.8.1`. This is required by the `account.code.length` syntax that replaces inline assembly. This may require users to bump their compiler version from `0.8.0` to `0.8.1` or later. Note that other parts of the code already include stricter requirements. ## 4.4.2 (2022-01-11)