NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make TransparentUpgradeableProxy deploy its ProxyAdmin and optimize proxy interfaces (#4382)

Browse Source 
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Eric Lau <ericglau@outlook.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
This commit is contained in:
Ernesto García
2023-07-13 16:25:22 -06:00
committed by GitHub
parent 9cf873ea14
commit 121be5dd09
27 changed files with 521 additions and 356 deletions
Download Patch File Download Diff File Expand all files Collapse all files

203
test/proxy/transparent/TransparentUpgradeableProxy.behaviour.js
View File

@ -5,6 +5,8 @@ const { expectRevertCustomError } = require('../../helpers/customError');
const { expect } = require('chai');
const { web3 } = require('hardhat');
const { computeCreateAddress } = require('../../helpers/create');
const { impersonate } = require('../../helpers/account');
const Implementation1 = artifacts.require('Implementation1');
const Implementation2 = artifacts.require('Implementation2');
@ -16,9 +18,23 @@ const MigratableMockV3 = artifacts.require('MigratableMockV3');
const InitializableMock = artifacts.require('InitializableMock');
const DummyImplementation = artifacts.require('DummyImplementation');
const ClashingImplementation = artifacts.require('ClashingImplementation');
const Ownable = artifacts.require('Ownable');
module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProxy, accounts) {
const [proxyAdminAddress, proxyAdminOwner, anotherAccount] = accounts;
module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProxy, initialOwner, accounts) {
const [anotherAccount] = accounts;
async function createProxyWithImpersonatedProxyAdmin(logic, initData, opts = undefined) {
const proxy = await createProxy(logic, initData, opts);
// Expect proxy admin to be the first and only contract created by the proxy
const proxyAdminAddress = computeCreateAddress(proxy.address, 1);
await impersonate(proxyAdminAddress);
return {
proxy,
proxyAdminAddress,
};
}
before(async function () {
this.implementationV0 = (await DummyImplementation.new()).address;
@ -27,10 +43,12 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
beforeEach(async function () {
const initializeData = Buffer.from('');
this.proxy = await createProxy(this.implementationV0, proxyAdminAddress, initializeData, {
from: proxyAdminOwner,
});
this.proxyAddress = this.proxy.address;
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
this.implementationV0,
initializeData,
);
this.proxy = proxy;
this.proxyAdminAddress = proxyAdminAddress;
});
describe('implementation', function () {
@ -40,7 +58,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
});
it('delegates to the implementation', async function () {
const dummy = new DummyImplementation(this.proxyAddress);
const dummy = new DummyImplementation(this.proxy.address);
const value = await dummy.get();
expect(value).to.equal(true);
@ -49,64 +67,33 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
describe('proxy admin', function () {
it('emits AdminChanged event during construction', async function () {
expectEvent.inConstruction(this.proxy, 'AdminChanged', {
await expectEvent.inConstruction(this.proxy, 'AdminChanged', {
previousAdmin: ZERO_ADDRESS,
newAdmin: proxyAdminAddress,
newAdmin: this.proxyAdminAddress,
});
});
it('sets the admin in the storage', async function () {
expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(proxyAdminAddress);
it('sets the proxy admin in storage with the correct initial owner', async function () {
expect(await getAddressInSlot(this.proxy, AdminSlot)).to.be.equal(this.proxyAdminAddress);
const proxyAdmin = await Ownable.at(this.proxyAdminAddress);
expect(await proxyAdmin.owner()).to.be.equal(initialOwner);
});
it('can overwrite the admin by the implementation', async function () {
const dummy = new DummyImplementation(this.proxyAddress);
const dummy = new DummyImplementation(this.proxy.address);
await dummy.unsafeOverrideAdmin(anotherAccount);
const ERC1967AdminSlotValue = await getAddressInSlot(this.proxy, AdminSlot);
expect(ERC1967AdminSlotValue).to.be.equal(anotherAccount);
// Still allows previous admin to execute admin operations
expect(ERC1967AdminSlotValue).to.not.equal(proxyAdminAddress);
expectEvent(await this.proxy.upgradeTo(this.implementationV1, { from: proxyAdminAddress }), 'Upgraded', {
implementation: this.implementationV1,
});
});
});
describe('upgradeTo', function () {
describe('when the sender is the admin', function () {
const from = proxyAdminAddress;
describe('when the given implementation is different from the current one', function () {
it('upgrades to the requested implementation', async function () {
await this.proxy.upgradeTo(this.implementationV1, { from });
const implementationAddress = await getAddressInSlot(this.proxy, ImplementationSlot);
expect(implementationAddress).to.be.equal(this.implementationV1);
});
it('emits an event', async function () {
expectEvent(await this.proxy.upgradeTo(this.implementationV1, { from }), 'Upgraded', {
implementation: this.implementationV1,
});
});
});
describe('when the given implementation is the zero address', function () {
it('reverts', async function () {
await expectRevertCustomError(this.proxy.upgradeTo(ZERO_ADDRESS, { from }), 'ERC1967InvalidImplementation', [
ZERO_ADDRESS,
]);
});
});
});
describe('when the sender is not the admin', function () {
const from = anotherAccount;
it('reverts', async function () {
await expectRevert.unspecified(this.proxy.upgradeTo(this.implementationV1, { from }));
});
expect(ERC1967AdminSlotValue).to.not.equal(this.proxyAdminAddress);
expectEvent(
await this.proxy.upgradeToAndCall(this.implementationV1, '0x', { from: this.proxyAdminAddress }),
'Upgraded',
{
implementation: this.implementationV1,
},
);
});
});
@ -120,11 +107,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
const initializeData = new InitializableMock('').contract.methods['initializeWithX(uint256)'](42).encodeABI();
describe('when the sender is the admin', function () {
const from = proxyAdminAddress;
const value = 1e5;
beforeEach(async function () {
this.receipt = await this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from, value });
this.receipt = await this.proxy.upgradeToAndCall(this.behavior.address, initializeData, {
from: this.proxyAdminAddress,
value,
});
});
it('upgrades to the requested implementation', async function () {
@ -137,13 +126,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
});
it('calls the initializer function', async function () {
const migratable = new InitializableMock(this.proxyAddress);
const migratable = new InitializableMock(this.proxy.address);
const x = await migratable.x();
expect(x).to.be.bignumber.equal('42');
});
it('sends given value to the proxy', async function () {
const balance = await web3.eth.getBalance(this.proxyAddress);
const balance = await web3.eth.getBalance(this.proxy.address);
expect(balance.toString()).to.be.bignumber.equal(value.toString());
});
@ -151,7 +140,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
// storage layout should look as follows:
// - 0: Initializable storage ++ initializerRan ++ onlyInitializingRan
// - 1: x
const storedValue = await web3.eth.getStorageAt(this.proxyAddress, 1);
const storedValue = await web3.eth.getStorageAt(this.proxy.address, 1);
expect(parseInt(storedValue)).to.eq(42);
});
});
@ -170,7 +159,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('reverts', async function () {
await expectRevert.unspecified(
this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from: proxyAdminAddress }),
this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from: this.proxyAdminAddress }),
);
});
});
@ -178,7 +167,6 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
describe('with migrations', function () {
describe('when the sender is the admin', function () {
const from = proxyAdminAddress;
const value = 1e5;
describe('when upgrading to V1', function () {
@ -186,8 +174,11 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
beforeEach(async function () {
this.behaviorV1 = await MigratableMockV1.new();
this.balancePreviousV1 = new BN(await web3.eth.getBalance(this.proxyAddress));
this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV1.address, v1MigrationData, { from, value });
this.balancePreviousV1 = new BN(await web3.eth.getBalance(this.proxy.address));
this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV1.address, v1MigrationData, {
from: this.proxyAdminAddress,
value,
});
});
it('upgrades to the requested version and emits an event', async function () {
@ -197,12 +188,12 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
});
it("calls the 'initialize' function and sends given value to the proxy", async function () {
const migratable = new MigratableMockV1(this.proxyAddress);
const migratable = new MigratableMockV1(this.proxy.address);
const x = await migratable.x();
expect(x).to.be.bignumber.equal('42');
const balance = await web3.eth.getBalance(this.proxyAddress);
const balance = await web3.eth.getBalance(this.proxy.address);
expect(new BN(balance)).to.be.bignumber.equal(this.balancePreviousV1.addn(value));
});
@ -211,9 +202,9 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
beforeEach(async function () {
this.behaviorV2 = await MigratableMockV2.new();
this.balancePreviousV2 = new BN(await web3.eth.getBalance(this.proxyAddress));
this.balancePreviousV2 = new BN(await web3.eth.getBalance(this.proxy.address));
this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV2.address, v2MigrationData, {
from,
from: this.proxyAdminAddress,
value,
});
});
@ -225,7 +216,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
});
it("calls the 'migrate' function and sends given value to the proxy", async function () {
const migratable = new MigratableMockV2(this.proxyAddress);
const migratable = new MigratableMockV2(this.proxy.address);
const x = await migratable.x();
expect(x).to.be.bignumber.equal('10');
@ -233,7 +224,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
const y = await migratable.y();
expect(y).to.be.bignumber.equal('42');
const balance = new BN(await web3.eth.getBalance(this.proxyAddress));
const balance = new BN(await web3.eth.getBalance(this.proxy.address));
expect(balance).to.be.bignumber.equal(this.balancePreviousV2.addn(value));
});
@ -242,9 +233,9 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
beforeEach(async function () {
this.behaviorV3 = await MigratableMockV3.new();
this.balancePreviousV3 = new BN(await web3.eth.getBalance(this.proxyAddress));
this.balancePreviousV3 = new BN(await web3.eth.getBalance(this.proxy.address));
this.receipt = await this.proxy.upgradeToAndCall(this.behaviorV3.address, v3MigrationData, {
from,
from: this.proxyAdminAddress,
value,
});
});
@ -256,7 +247,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
});
it("calls the 'migrate' function and sends given value to the proxy", async function () {
const migratable = new MigratableMockV3(this.proxyAddress);
const migratable = new MigratableMockV3(this.proxy.address);
const x = await migratable.x();
expect(x).to.be.bignumber.equal('42');
@ -264,7 +255,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
const y = await migratable.y();
expect(y).to.be.bignumber.equal('10');
const balance = new BN(await web3.eth.getBalance(this.proxyAddress));
const balance = new BN(await web3.eth.getBalance(this.proxy.address));
expect(balance).to.be.bignumber.equal(this.balancePreviousV3.addn(value));
});
});
@ -289,15 +280,18 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
const initializeData = Buffer.from('');
this.clashingImplV0 = (await ClashingImplementation.new()).address;
this.clashingImplV1 = (await ClashingImplementation.new()).address;
this.proxy = await createProxy(this.clashingImplV0, proxyAdminAddress, initializeData, {
from: proxyAdminOwner,
});
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
this.clashingImplV0,
initializeData,
);
this.proxy = proxy;
this.proxyAdminAddress = proxyAdminAddress;
this.clashing = new ClashingImplementation(this.proxy.address);
});
it('proxy admin cannot call delegated functions', async function () {
await expectRevertCustomError(
this.clashing.delegatedFunction({ from: proxyAdminAddress }),
this.clashing.delegatedFunction({ from: this.proxyAdminAddress }),
'ProxyDeniedAdminAccess',
[],
);
@ -305,26 +299,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
describe('when function names clash', function () {
it('executes the proxy function if the sender is the admin', async function () {
const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: proxyAdminAddress, value: 0 });
const receipt = await this.proxy.upgradeToAndCall(this.clashingImplV1, '0x', {
from: this.proxyAdminAddress,
});
expectEvent(receipt, 'Upgraded', { implementation: this.clashingImplV1 });
});
it('delegates the call to implementation when sender is not the admin', async function () {
const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: anotherAccount, value: 0 });
expectEvent.notEmitted(receipt, 'Upgraded');
expectEvent.inTransaction(receipt.tx, this.clashing, 'ClashingImplementationCall');
});
it('requires 0 value calling upgradeTo by proxy admin', async function () {
await expectRevertCustomError(
this.proxy.upgradeTo(this.clashingImplV1, { from: proxyAdminAddress, value: 1 }),
'ProxyNonPayableFunction',
[],
);
});
it('allows calling with value if sender is not the admin', async function () {
const receipt = await this.proxy.upgradeTo(this.clashingImplV1, { from: anotherAccount, value: 1 });
const receipt = await this.proxy.upgradeToAndCall(this.clashingImplV1, '0x', {
from: anotherAccount,
});
expectEvent.notEmitted(receipt, 'Upgraded');
expectEvent.inTransaction(receipt.tx, this.clashing, 'ClashingImplementationCall');
});
@ -336,13 +320,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('should add new function', async () => {
const instance1 = await Implementation1.new();
const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
instance1.address,
initializeData,
);
const proxyInstance1 = new Implementation1(proxy.address);
await proxyInstance1.setValue(42);
const instance2 = await Implementation2.new();
await proxy.upgradeTo(instance2.address, { from: proxyAdminAddress });
await proxy.upgradeToAndCall(instance2.address, '0x', { from: proxyAdminAddress });
const proxyInstance2 = new Implementation2(proxy.address);
const res = await proxyInstance2.getValue();
@ -351,7 +338,10 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('should remove function', async () => {
const instance2 = await Implementation2.new();
const proxy = await createProxy(instance2.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
instance2.address,
initializeData,
);
const proxyInstance2 = new Implementation2(proxy.address);
await proxyInstance2.setValue(42);
@ -359,7 +349,7 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
expect(res.toString()).to.eq('42');
const instance1 = await Implementation1.new();
await proxy.upgradeTo(instance1.address, { from: proxyAdminAddress });
await proxy.upgradeToAndCall(instance1.address, '0x', { from: proxyAdminAddress });
const proxyInstance1 = new Implementation2(proxy.address);
await expectRevert.unspecified(proxyInstance1.getValue());
@ -367,13 +357,16 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('should change function signature', async () => {
const instance1 = await Implementation1.new();
const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
instance1.address,
initializeData,
);
const proxyInstance1 = new Implementation1(proxy.address);
await proxyInstance1.setValue(42);
const instance3 = await Implementation3.new();
await proxy.upgradeTo(instance3.address, { from: proxyAdminAddress });
await proxy.upgradeToAndCall(instance3.address, '0x', { from: proxyAdminAddress });
const proxyInstance3 = new Implementation3(proxy.address);
const res = await proxyInstance3.getValue(8);
@ -383,10 +376,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('should add fallback function', async () => {
const initializeData = Buffer.from('');
const instance1 = await Implementation1.new();
const proxy = await createProxy(instance1.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
instance1.address,
initializeData,
);
const instance4 = await Implementation4.new();
await proxy.upgradeTo(instance4.address, { from: proxyAdminAddress });
await proxy.upgradeToAndCall(instance4.address, '0x', { from: proxyAdminAddress });
const proxyInstance4 = new Implementation4(proxy.address);
const data = '0x';
@ -398,10 +394,13 @@ module.exports = function shouldBehaveLikeTransparentUpgradeableProxy(createProx
it('should remove fallback function', async () => {
const instance4 = await Implementation4.new();
const proxy = await createProxy(instance4.address, proxyAdminAddress, initializeData, { from: proxyAdminOwner });
const { proxy, proxyAdminAddress } = await createProxyWithImpersonatedProxyAdmin(
instance4.address,
initializeData,
);
const instance2 = await Implementation2.new();
await proxy.upgradeTo(instance2.address, { from: proxyAdminAddress });
await proxy.upgradeToAndCall(instance2.address, '0x', { from: proxyAdminAddress });
const data = '0x';
await expectRevert.unspecified(web3.eth.sendTransaction({ to: proxy.address, from: anotherAccount, data }));