Skip to content

Commit

Permalink
fix UUPSProxy and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Feb 24, 2021
1 parent ae64ea7 commit 7812d1d
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 15 deletions.
18 changes: 18 additions & 0 deletions contracts/mocks/DummyImplementationProxiable.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "./DummyImplementation.sol";
import "../proxy/uups/UUPSProxiable.sol";

contract DummyImplementationProxiable is DummyImplementation, UUPSProxiable {
function updateCodeAddress(address newImplementation) public {
_updateCodeAddress(newImplementation);
}
}

contract DummyImplementationV2Proxiable is DummyImplementationV2, UUPSProxiable {
function updateCodeAddress(address newImplementation) public {
_updateCodeAddress(newImplementation);
}
}
2 changes: 1 addition & 1 deletion contracts/proxy/uups/UUPSProxiable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.0;

import "./UUPS.sol";

contract UUPSProxiable {
abstract contract UUPSProxiable {
function proxiableUUID() public pure returns (bytes32) {
return UUPS.uuid();
}
Expand Down
4 changes: 3 additions & 1 deletion contracts/proxy/uups/UUPSProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
pragma solidity ^0.8.0;

import "./UUPS.sol";
import "./UUPSProxiable.sol";
import "../Proxy.sol";
import "../../utils/Address.sol";

contract UUPSProxy is Proxy {
constructor(address _logic, bytes memory _data) payable {
require(UUPS.uuid() == UUPSProxiable(_logic).proxiableUUID(), "Not compatible");
UUPS.instance().implementation = _logic;
if (_data.length > 0) {
Address.functionCall(_logic, _data);
Address.functionDelegateCall(_logic, _data);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ function toChecksumAddress (address) {
return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0'));
}

module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAdminAddress, proxyCreator) {
module.exports = function shouldBehaveLikeProxy (createProxy, proxyAdminAddress, proxyCreator, opts = {}) {
const { artefact, slot } = Object.assign({
artefact: DummyImplementation,
slot: '0x' + new BN(ethereumjsUtil.keccak256(Buffer.from(IMPLEMENTATION_LABEL))).subn(1).toString(16),
}, opts);


it('cannot be initialized with a non-contract address', async function () {
const nonContractAddress = proxyCreator;
const initializeData = Buffer.from('');
Expand All @@ -23,18 +29,17 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd
});

before('deploy implementation', async function () {
this.implementation = web3.utils.toChecksumAddress((await DummyImplementation.new()).address);
this.implementation = web3.utils.toChecksumAddress((await artefact.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);
});

it('initializes the proxy', async function () {
const dummy = new DummyImplementation(this.proxy);
const dummy = new artefact(this.proxy);
expect(await dummy.value()).to.be.bignumber.equal(value.toString());
});

Expand Down Expand Up @@ -77,7 +82,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd
describe('initialization without parameters', function () {
describe('non payable', function () {
const expectedInitializedValue = 10;
const initializeData = new DummyImplementation('').contract.methods['initializeNonPayable()']().encodeABI();
const initializeData = new artefact('').contract.methods['initializeNonPayable()']().encodeABI();

describe('when not sending balance', function () {
beforeEach('creating proxy', async function () {
Expand Down Expand Up @@ -107,7 +112,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd

describe('payable', function () {
const expectedInitializedValue = 100;
const initializeData = new DummyImplementation('').contract.methods['initializePayable()']().encodeABI();
const initializeData = new artefact('').contract.methods['initializePayable()']().encodeABI();

describe('when not sending balance', function () {
beforeEach('creating proxy', async function () {
Expand Down Expand Up @@ -147,7 +152,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd
describe('initialization with parameters', function () {
describe('non payable', function () {
const expectedInitializedValue = 10;
const initializeData = new DummyImplementation('').contract
const initializeData = new artefact('').contract
.methods.initializeNonPayableWithValue(expectedInitializedValue).encodeABI();

describe('when not sending balance', function () {
Expand Down Expand Up @@ -178,7 +183,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd

describe('payable', function () {
const expectedInitializedValue = 42;
const initializeData = new DummyImplementation('').contract
const initializeData = new artefact('').contract
.methods.initializePayableWithValue(expectedInitializedValue).encodeABI();

describe('when not sending balance', function () {
Expand Down Expand Up @@ -216,7 +221,7 @@ module.exports = function shouldBehaveLikeUpgradeableProxy (createProxy, proxyAd
});

describe('reverting initialization', function () {
const initializeData = new DummyImplementation('').contract
const initializeData = new artefact('').contract
.methods.reverts().encodeABI();

it('reverts', async function () {
Expand Down
4 changes: 2 additions & 2 deletions test/proxy/UpgradeableProxy.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const shouldBehaveLikeUpgradeableProxy = require('./UpgradeableProxy.behaviour');
const shouldBehaveLikeProxy = require('./Proxy.behaviour');

const UpgradeableProxy = artifacts.require('UpgradeableProxy');

Expand All @@ -9,5 +9,5 @@ contract('UpgradeableProxy', function (accounts) {
return UpgradeableProxy.new(implementation, initData, opts);
};

shouldBehaveLikeUpgradeableProxy(createProxy, undefined, proxyAdminOwner);
shouldBehaveLikeProxy(createProxy, undefined, proxyAdminOwner);
});
File renamed without changes.
4 changes: 2 additions & 2 deletions test/proxy/transparent/TransparentUpgradeableProxy.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const shouldBehaveLikeUpgradeableProxy = require('../UpgradeableProxy.behaviour');
const shouldBehaveLikeProxy = require('../Proxy.behaviour');
const shouldBehaveLikeTransparentUpgradeableProxy = require('./TransparentUpgradeableProxy.behaviour');

const TransparentUpgradeableProxy = artifacts.require('TransparentUpgradeableProxy');
Expand All @@ -10,6 +10,6 @@ contract('TransparentUpgradeableProxy', function (accounts) {
return TransparentUpgradeableProxy.new(logic, admin, initData, opts);
};

shouldBehaveLikeUpgradeableProxy(createProxy, proxyAdminAddress, proxyAdminOwner);
shouldBehaveLikeProxy(createProxy, proxyAdminAddress, proxyAdminOwner);
shouldBehaveLikeTransparentUpgradeableProxy(createProxy, accounts);
});
56 changes: 56 additions & 0 deletions test/proxy/uups/UUPSProxy.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
const { expectRevert } = require('@openzeppelin/test-helpers');
const ethereumjsUtil = require('ethereumjs-util');

const shouldBehaveLikeProxy = require('../Proxy.behaviour');

const DummyImplementation = artifacts.require('DummyImplementation')
const DummyImplementationProxiable = artifacts.require('DummyImplementationProxiable')
const DummyImplementationV2Proxiable = artifacts.require('DummyImplementationV2Proxiable')
const UUPSProxy = artifacts.require('UUPSProxy');

const UUPS_UUID = '0xc5f16f0fcc639fa48a6947836d9850f504798523bf8c9a3a87d5876cf622bcf7';

function toChecksumAddress (address) {
return ethereumjsUtil.toChecksumAddress('0x' + address.replace(/^0x/, '').padStart(40, '0'));
}

contract('UUPSProxy', function (accounts) {
const [ proxyCreator ] = accounts;

const createProxy = async function (implementation, _admin, initData, opts) {
return UUPSProxy.new(implementation, initData, opts);
};

shouldBehaveLikeProxy(createProxy, undefined, proxyCreator, {
artefact: DummyImplementationProxiable,
slot: UUPS_UUID,
});

context('UUPS', function () {
beforeEach(async function () {
this.implementation = await DummyImplementation.new();
this.implementationproxiable = await DummyImplementationProxiable.new();
this.implementationproxiablev2 = await DummyImplementationV2Proxiable.new();
const { address } = await createProxy(this.implementationproxiable.address, undefined, '0x', { from: proxyCreator });
this.dummy = new DummyImplementationProxiable(address);
});

it('check uuid', async function() {
expect(await this.dummy.proxiableUUID()).to.be.equal(UUPS_UUID);
});

it('can update to proxiable', async function() {
await this.dummy.updateCodeAddress(this.implementationproxiablev2.address);
const implementation = toChecksumAddress((await web3.eth.getStorageAt(this.dummy.address, UUPS_UUID)).substr(-40));
expect(implementation).to.be.equal(this.implementationproxiablev2.address);
});

it('does not deploy with non-proxiable', async function() {
await expectRevert.unspecified(createProxy(this.implementation.address, undefined, '0x', { from: proxyCreator }));
});

it('does not update to non-proxiable', async function() {
await expectRevert.unspecified(this.dummy.updateCodeAddress(this.implementation.address));
});
});
});

0 comments on commit 7812d1d

Please sign in to comment.