Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Move proxy initialization to constructors #106

Merged
merged 9 commits into from
Sep 20, 2018
4 changes: 2 additions & 2 deletions examples/lib-simple/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ const MyContract_v1 = Contracts.getFromLocal('MyContract_v1');
async function main() {
const creatorAddress = web3.eth.accounts[1],
initializerAddress = web3.eth.accounts[2],
myProject = new SimpleProject('MyProject', { from: creatorAddress }, { from: initializerAddress });
myProject = new SimpleProject('MyProject', { from: creatorAddress });

log('Creating an upgradeable instance of v0...');
const instance = await myProject.createProxy(MyContract_v0, { initArgs: [42] })
log('Contract\'s storage value: ' + (await instance.value()).toString() + '\n');

log('Upgrading to v1...');
await myProject.upgradeProxy(instance, MyContract_v1, { initMethod: 'add', initArgs: [1] })
await myProject.upgradeProxy(instance, MyContract_v1, { initMethod: 'add', initArgs: [1], initFrom: initializerAddress })
log('Contract\'s storage new value: ' + (await instance.value()).toString() + '\n');

log('Wohoo! We\'ve upgraded our contract\'s behavior while preserving its storage, thus obtaining 43.');
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/models/status/StatusChecker.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export default class StatusChecker {
const implementationsInfo = await this._fetchOnChainImplementations()
const app = await this.app();
const filter = new EventsFilter()
const proxyEvents = await filter.call(app.factory, 'ProxyCreated')
const proxyEvents = await filter.call(app, 'ProxyCreated')
const proxiesInfo = []
await Promise.all(proxyEvents.map(async event => {
const address = event.args.proxy
Expand Down
34 changes: 12 additions & 22 deletions packages/lib/contracts/application/BaseApp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pragma solidity ^0.4.24;

import "./versioning/ImplementationProvider.sol";
import "../upgradeability/AdminUpgradeabilityProxy.sol";
import "../upgradeability/UpgradeabilityProxyFactory.sol";
import "openzeppelin-solidity/contracts/ownership/Ownable.sol";

/**
Expand All @@ -11,17 +10,16 @@ import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
* It handles the creation and upgrading of proxies.
*/
contract BaseApp is Ownable {
/// @dev Factory that creates proxies.
UpgradeabilityProxyFactory public factory;
/**
* @dev Emitted when a new proxy is created.
* @param proxy Address of the created proxy.
*/
event ProxyCreated(address indexed proxy);

/**
* @dev Constructor function
* @param _factory Proxy factory
*/
constructor(UpgradeabilityProxyFactory _factory) public {
require(address(_factory) != address(0), "Cannot set the proxy factory of an app to the zero address");
factory = _factory;
}
constructor() public {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitated about entirely removing the ProxyFactory, since the original idea was that we could have a single ProxyFactory shared by all Apps, so each user would only need to deploy the App. However, since we are now aiming at making these contracts upgradeable, gas cost for the end user is exactly the same, since they only need to deploy a proxy to App, regardless of where the factory code for proxies is. So +1 to this change.


/**
* @dev Abstract function to return the implementation provider for a given package name.
Expand Down Expand Up @@ -68,30 +66,22 @@ contract BaseApp is Ownable {
proxy.changeAdmin(newAdmin);
}

/**
* @dev Creates a new proxy for the given contract.
* @param packageName Name of the package where the contract is contained.
* @param contractName Name of the contract.
* @return Address of the new proxy.
*/
function create(string packageName, string contractName) public returns (AdminUpgradeabilityProxy) {
address implementation = getImplementation(packageName, contractName);
return factory.createProxy(this, implementation);
}

/**
* @dev Creates a new proxy for the given contract and forwards a function call to it.
* This is useful to initialize the proxied contract.
* @param packageName Name of the package where the contract is contained.
* @param contractName Name of the contract.
* @param data Data to send as msg.data in the low level call.
* @param data Data to send as msg.data to the corresponding implementation to initialize the proxied contract.
* It should include the signature and the parameters of the function to be called, as described in
* https://solidity.readthedocs.io/en/develop/abi-spec.html#function-selector-and-argument-encoding.
* This parameter is optional, if no data is given the initialization call to proxied contract will be skipped.
* @return Address of the new proxy.
*/
function createAndCall(string packageName, string contractName, bytes data) payable public returns (AdminUpgradeabilityProxy) {
function create(string packageName, string contractName, bytes data) payable public returns (AdminUpgradeabilityProxy) {
address implementation = getImplementation(packageName, contractName);
return factory.createProxyAndCall.value(msg.value)(this, implementation, data);
AdminUpgradeabilityProxy proxy = (new AdminUpgradeabilityProxy).value(msg.value)(implementation, data);
emit ProxyCreated(proxy);
return proxy;
}

/**
Expand Down
4 changes: 1 addition & 3 deletions packages/lib/contracts/application/UnversionedApp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pragma solidity ^0.4.24;

import "./BaseApp.sol";
import "./versioning/ImplementationProvider.sol";
import "../upgradeability/UpgradeabilityProxyFactory.sol";

/**
* @title UnversionedApp
Expand All @@ -23,9 +22,8 @@ contract UnversionedApp is BaseApp {

/**
* @dev Constructor function.
* @param _factory Proxy factory.
*/
constructor(UpgradeabilityProxyFactory _factory) BaseApp(_factory) public { }
constructor() BaseApp() public { }

/**
* @dev Returns the provider for a given package name, or zero if not set.
Expand Down
4 changes: 1 addition & 3 deletions packages/lib/contracts/application/VersionedApp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ pragma solidity ^0.4.24;

import "./BaseApp.sol";
import "./versioning/Package.sol";
import "../upgradeability/UpgradeabilityProxyFactory.sol";

/**
* @title VersionedApp
Expand All @@ -28,9 +27,8 @@ contract VersionedApp is BaseApp {

/**
* @dev Constructor function.
* @param _factory Proxy factory.
*/
constructor(UpgradeabilityProxyFactory _factory) BaseApp(_factory) public { }
constructor() BaseApp() public { }

/**
* @dev Returns the provider for a given package name, or zero if not set.
Expand Down
14 changes: 13 additions & 1 deletion packages/lib/contracts/mocks/DummyImplementation.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,19 @@ contract DummyImplementation {
string public text;
uint256[] public values;

function initialize(uint256 _value) payable public {
function initializeNonPayable() public {
value = 10;
}

function initializePayable() payable public {
value = 100;
}

function initializeNonPayable(uint256 _value) public {
value = _value;
}

function initializePayable(uint256 _value) payable public {
value = _value;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,12 @@ contract AdminUpgradeabilityProxy is UpgradeabilityProxy {
* Contract constructor.
* It sets the `msg.sender` as the proxy administrator.
* @param _implementation address of the initial implementation.
* @param _data Data to send as msg.data to the implementation to initialize the proxied contract.
* It should include the signature and the parameters of the function to be called, as described in
* https://solidity.readthedocs.io/en/develop/abi-spec.html#function-selector-and-argument-encoding.
* This parameter is optional, if no data is given the initialization call to proxied contract will be skipped.
*/
constructor(address _implementation) UpgradeabilityProxy(_implementation) public {
constructor(address _implementation, bytes _data) UpgradeabilityProxy(_implementation, _data) public payable {
assert(ADMIN_SLOT == keccak256("org.zeppelinos.proxy.admin"));

_setAdmin(msg.sender);
Expand Down Expand Up @@ -89,8 +93,7 @@ contract AdminUpgradeabilityProxy is UpgradeabilityProxy {
* This is useful to initialize the proxied contract.
* @param newImplementation Address of the new implementation.
* @param data Data to send as msg.data in the low level call.
* It should include the signature and the parameters of the function to be
* called, as described in
* It should include the signature and the parameters of the function to be called, as described in
* https://solidity.readthedocs.io/en/develop/abi-spec.html#function-selector-and-argument-encoding.
*/
function upgradeToAndCall(address newImplementation, bytes data) payable external ifAdmin {
Expand Down
10 changes: 8 additions & 2 deletions packages/lib/contracts/upgradeability/UpgradeabilityProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,17 @@ contract UpgradeabilityProxy is Proxy {
/**
* @dev Contract constructor.
* @param _implementation Address of the initial implementation.
* @param _data Data to send as msg.data to the implementation to initialize the proxied contract.
* It should include the signature and the parameters of the function to be called, as described in
* https://solidity.readthedocs.io/en/develop/abi-spec.html#function-selector-and-argument-encoding.
* This parameter is optional, if no data is given the initialization call to proxied contract will be skipped.
*/
constructor(address _implementation) public {
constructor(address _implementation, bytes _data) public payable {
assert(IMPLEMENTATION_SLOT == keccak256("org.zeppelinos.proxy.implementation"));

_setImplementation(_implementation);
if(_data.length > 0) {
require(_implementation.delegatecall(_data));
}
}

/**
Expand Down

This file was deleted.

13 changes: 5 additions & 8 deletions packages/lib/src/app/BaseApp.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
'use strict'

import Logger from '../utils/Logger'
import Contracts from '../utils/Contracts'
import decodeLogs from '../helpers/decodeLogs'
import copyContract from '../helpers/copyContract'
import { deploy as deployContract, sendTransaction, sendDataTransaction } from '../utils/Transactions'

import UpgradeabilityProxyFactory from '../factory/UpgradeabilityProxyFactory';
import FreezableImplementationDirectory from '../directory/FreezableImplementationDirectory';
import { isZeroAddress } from '../utils/Addresses';
import { buildCallData, callDescription } from '../utils/ABIs';
Expand All @@ -21,9 +19,8 @@ export default class BaseApp {
}

static async deploy(txParams = {}) {
const factory = await UpgradeabilityProxyFactory.deploy(txParams)
log.info('Deploying new App...')
const appContract = await deployContract(this.getContractClass(), [factory.address], txParams)
const appContract = await deployContract(this.getContractClass(), [], txParams)
log.info(`Deployed App at ${appContract.address}`)
return new this(appContract, txParams)
}
Expand Down Expand Up @@ -81,8 +78,7 @@ export default class BaseApp {
: await this._createProxyAndCall(contractClass, packageName, contractName, initMethodName, initArgs)

log.info(`TX receipt received: ${receipt.transactionHash}`)
const UpgradeabilityProxyFactory = Contracts.getFromLib('UpgradeabilityProxyFactory')
const logs = decodeLogs(receipt.logs, UpgradeabilityProxyFactory)
const logs = decodeLogs(receipt.logs, this.constructor.getContractClass())
const address = logs.find(l => l.event === 'ProxyCreated').args.proxy
log.info(`${packageName} ${contractName} proxy: ${address}`)
return new contractClass(address)
Expand All @@ -98,13 +94,14 @@ export default class BaseApp {

async _createProxy(packageName, contractName) {
log.info(`Creating ${packageName} ${contractName} proxy without initializing...`)
return sendTransaction(this.appContract.create, [packageName, contractName], this.txParams)
const initializeData = ''
return sendTransaction(this.appContract.create, [packageName, contractName, initializeData], this.txParams)
}

async _createProxyAndCall(contractClass, packageName, contractName, initMethodName, initArgs) {
const { method: initMethod, callData } = buildCallData(contractClass, initMethodName, initArgs)
log.info(`Creating ${packageName} ${contractName} proxy and calling ${callDescription(initMethod, initArgs)}`)
return sendTransaction(this.appContract.createAndCall, [packageName, contractName, callData], this.txParams)
return sendTransaction(this.appContract.create, [packageName, contractName, callData], this.txParams)
}

async _upgradeProxy(proxyAddress, packageName, contractName) {
Expand Down
35 changes: 0 additions & 35 deletions packages/lib/src/factory/UpgradeabilityProxyFactory.js

This file was deleted.

2 changes: 0 additions & 2 deletions packages/lib/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import VersionedApp from './app/VersionedApp'
import Package from './package/Package'
import ImplementationDirectory from './directory/ImplementationDirectory'
import FreezableImplementationDirectory from './directory/FreezableImplementationDirectory'
import UpgradeabilityProxyFactory from './factory/UpgradeabilityProxyFactory'
import BasePackageProject from './project/BasePackageProject'
import LibProject from './project/LibProject'
import AppProject from './project/AppProject'
Expand All @@ -46,7 +45,6 @@ export {
UnversionedApp,
ImplementationDirectory,
FreezableImplementationDirectory,
UpgradeabilityProxyFactory,
Package,
BasePackageProject,
LibProject,
Expand Down
Loading