Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

Commit

Permalink
implement transparent proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
frangio committed Apr 12, 2018
1 parent c4c56dc commit 8f81183
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 30 deletions.
17 changes: 17 additions & 0 deletions contracts/ProjectController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,21 @@ contract ProjectController is ImplementationProvider, Ownable {
require(_implementation != address(0));
return _implementation;
}

/**
* @dev Gets the implementation for one of the owned proxies.
* @dev It's necessary to have this here because only the proxy owner can query it.
* @return the address of the current implemetation of the given proxy
*/
function getProxyImplementation(OwnedUpgradeabilityProxy proxy) public view returns (address) {
return proxy.implementation();
}

/**
* @dev Gets an owned proxy's owner. Necessary because only the owner can query it.
* @return the address of the current proxy owner of the given proxy
*/
function getProxyOwner(OwnedUpgradeabilityProxy proxy) public view returns (address) {
return proxy.proxyOwner();
}
}
22 changes: 20 additions & 2 deletions contracts/upgradeability/OwnedUpgradeabilityProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityProxy {
* @dev Throws if called by any account other than the owner.
*/
modifier onlyProxyOwner() {
require(msg.sender == proxyOwner());
_;
if (msg.sender == proxyOwner()) {
_;
} else {
_delegate();
}
}

/**
Expand All @@ -43,6 +46,13 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityProxy {
}
}

/**
* @return the address of the implementation
*/
function implementation() public view returns (address) {
return _implementation();
}

/**
* @dev Sets the address of the owner
*/
Expand Down Expand Up @@ -82,4 +92,12 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityProxy {
upgradeTo(implementation);
require(this.call.value(msg.value)(data));
}

/**
* @dev Redefines Proxy's fallback to disallow delegation when caller is the proxy owner.
*/
function () public payable {
require(msg.sender != proxyOwner());
_delegate();
}
}
25 changes: 16 additions & 9 deletions contracts/upgradeability/Proxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ pragma solidity ^0.4.21;
*/
contract Proxy {
/**
* @dev Tells the address of the implementation where every call will be delegated.
* @return address of the implementation to which it will be delegated
*/
function implementation() public view returns (address);
* @return address of the implementation to which all calls will be delegated.
*/
function _implementation() internal view returns (address);

/**
* @dev Fallback function allowing to perform a delegatecall to the given implementation.
* This function will return whatever the implementation call returns
*/
function () payable public {
address _impl = implementation();
* @dev Performs a delegatecall to the address returned by _implementation().
* @dev This is a low level function that doesn't return to its internal caller.
* @dev It will return to the external caller whatever the implementation returns.
*/
function _delegate() internal {
address _impl = _implementation();
require(_impl != address(0));

assembly {
Expand All @@ -31,4 +31,11 @@ contract Proxy {
default { return(ptr, size) }
}
}

/**
* @dev Delegates all incoming calls to the implementation.
*/
function () payable public {
_delegate();
}
}
8 changes: 4 additions & 4 deletions contracts/upgradeability/UpgradeabilityProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract UpgradeabilityProxy is Proxy {
* @dev Tells the address of the current implementation
* @return address of the current implementation
*/
function implementation() public view returns (address impl) {
function _implementation() internal view returns (address impl) {
bytes32 position = implementationPosition;
assembly {
impl := sload(position)
Expand All @@ -36,7 +36,7 @@ contract UpgradeabilityProxy is Proxy {
* @dev Sets the address of the current implementation
* @param newImplementation address representing the new implementation to be set
*/
function setImplementation(address newImplementation) internal {
function _setImplementation(address newImplementation) internal {
bytes32 position = implementationPosition;
assembly {
sstore(position, newImplementation)
Expand All @@ -48,9 +48,9 @@ contract UpgradeabilityProxy is Proxy {
* @param newImplementation representing the address of the new implementation to be set
*/
function _upgradeTo(address newImplementation) internal {
address currentImplementation = implementation();
address currentImplementation = _implementation();
require(currentImplementation != newImplementation);
setImplementation(newImplementation);
_setImplementation(newImplementation);
emit Upgraded(newImplementation);
}
}
12 changes: 6 additions & 6 deletions test/ProjectController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@ contract('ProjectController', ([_, controllerOwner, registryOwner, anAddress, an
})

it('creates a proxy pointing to the requested implementation', async function () {
const implementation = await this.proxy.implementation()
const implementation = await this.controller.getProxyImplementation(this.proxy.address)
assert.equal(implementation, implementation_v0)
})

it('transfers the ownership to the controller', async function () {
const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.controller.getProxyOwner(this.proxy.address)
assert.equal(proxyOwner, this.controller.address)
})
})
Expand Down Expand Up @@ -117,12 +117,12 @@ contract('ProjectController', ([_, controllerOwner, registryOwner, anAddress, an
})

it('creates a proxy pointing to the requested implementation', async function () {
const implementation = await this.proxy.implementation()
const implementation = await this.controller.getProxyImplementation(this.proxy.address)
assert.equal(implementation, this.behavior.address)
})

it('transfers the ownership to the controller', async function () {
const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.controller.getProxyOwner(this.proxy.address)
assert.equal(proxyOwner, this.controller.address)
})

Expand Down Expand Up @@ -165,7 +165,7 @@ contract('ProjectController', ([_, controllerOwner, registryOwner, anAddress, an
it('upgrades to the requested implementation', async function () {
await this.controller.upgradeTo(this.proxyAddress, projectName, version_1, contractName, { from })

const implementation = await this.proxy.implementation()
const implementation = await this.controller.getProxyImplementation(this.proxy.address)
assert.equal(implementation, implementation_v1)
})
})
Expand Down Expand Up @@ -211,7 +211,7 @@ contract('ProjectController', ([_, controllerOwner, registryOwner, anAddress, an
it('upgrades to the requested implementation', async function () {
await this.controller.upgradeToAndCall(this.proxyAddress, projectName, version_1, contractName, initializeData, { from, value })

const implementation = await this.proxy.implementation()
const implementation = await this.controller.getProxyImplementation(this.proxy.address)
assert.equal(implementation, this.behavior.address)
})

Expand Down
10 changes: 5 additions & 5 deletions test/upgradeability/OwnedUpgradeabilityProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ contract('OwnedUpgradeabilityProxy', ([_, owner, anotherAccount, implementation_

describe('owner', function () {
it('transfers the ownership to the requested owner', async function () {
const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.proxy.proxyOwner({ from: owner })

assert.equal(proxyOwner, owner)
})
})

describe('implementation', function () {
it('returns the current implementation address', async function () {
const implementation = await this.proxy.implementation()
const implementation = await this.proxy.implementation({ from: owner })

assert.equal(implementation, implementation_v0)
})
Expand All @@ -37,7 +37,7 @@ contract('OwnedUpgradeabilityProxy', ([_, owner, anotherAccount, implementation_
it('upgrades to the requested implementation', async function () {
await this.proxy.upgradeTo(implementation_v1, { from })

const implementation = await this.proxy.implementation()
const implementation = await this.proxy.implementation({ from: owner })
assert.equal(implementation, implementation_v1)
})

Expand Down Expand Up @@ -73,7 +73,7 @@ contract('OwnedUpgradeabilityProxy', ([_, owner, anotherAccount, implementation_
it('upgrades to the requested implementation', async function () {
await this.proxy.upgradeToAndCall(this.behavior.address, initializeData, { from, value })

const implementation = await this.proxy.implementation()
const implementation = await this.proxy.implementation({ from: owner })
assert.equal(implementation, this.behavior.address)
})

Expand Down Expand Up @@ -120,7 +120,7 @@ contract('OwnedUpgradeabilityProxy', ([_, owner, anotherAccount, implementation_
it('transfers the ownership', async function () {
await this.proxy.transferProxyOwnership(newOwner, { from })

const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.proxy.proxyOwner({ from: owner })
assert.equal(proxyOwner, anotherAccount)
})

Expand Down
8 changes: 4 additions & 4 deletions test/upgradeability/UpgradeabilityProxyFactory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ contract('UpgradeabilityProxyFactory', ([_, owner, implementation_v0]) => {
})

it('creates a proxy pointing to the requested implementation', async function () {
const implementation = await this.proxy.implementation()
const implementation = await this.proxy.implementation({ from: owner })
assert.equal(implementation, implementation_v0)
})

it('transfers the ownership to the requested owner', async function () {
const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.proxy.proxyOwner({ from: owner })
assert.equal(proxyOwner, owner)
})

Expand All @@ -48,12 +48,12 @@ contract('UpgradeabilityProxyFactory', ([_, owner, implementation_v0]) => {
})

it('creates a proxy pointing to the requested implementation', async function () {
const implementation = await this.proxy.implementation()
const implementation = await this.proxy.implementation({ from: owner })
assert.equal(implementation, this.behavior.address)
})

it('transfers the ownership to the requested owner', async function () {
const proxyOwner = await this.proxy.proxyOwner()
const proxyOwner = await this.proxy.proxyOwner({ from: owner })
assert.equal(proxyOwner, owner)
})

Expand Down

0 comments on commit 8f81183

Please sign in to comment.