diff --git a/.gitignore b/.gitignore index ef58a67d8..a98e6dc5f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ build flats .node* .idea +coverage diff --git a/.node-xmlhttprequest-sync-27493 b/.node-xmlhttprequest-sync-27493 deleted file mode 100644 index e69de29bb..000000000 diff --git a/contracts/IOwnedUpgradeabilityProxy.sol b/contracts/IOwnedUpgradeabilityProxy.sol index 7da3d5c65..53385a170 100644 --- a/contracts/IOwnedUpgradeabilityProxy.sol +++ b/contracts/IOwnedUpgradeabilityProxy.sol @@ -2,5 +2,5 @@ pragma solidity 0.4.24; interface IOwnedUpgradeabilityProxy { - function proxyOwner() public view returns (address); + function upgradeabilityOwner() public view returns (address); } diff --git a/contracts/upgradeability/OwnedUpgradeabilityProxy.sol b/contracts/upgradeability/OwnedUpgradeabilityProxy.sol index 8fc51a50f..e6d24af22 100644 --- a/contracts/upgradeability/OwnedUpgradeabilityProxy.sol +++ b/contracts/upgradeability/OwnedUpgradeabilityProxy.sol @@ -26,26 +26,18 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityOwnerStorage, UpgradeabilityP /** * @dev Throws if called by any account other than the owner. */ - modifier onlyProxyOwner() { - require(msg.sender == proxyOwner()); + modifier onlyUpgradeabilityOwner() { + require(msg.sender == upgradeabilityOwner()); _; } - /** - * @dev Tells the address of the proxy owner - * @return the address of the proxy owner - */ - function proxyOwner() public view returns (address) { - return upgradeabilityOwner(); - } - /** * @dev Allows the current owner to transfer control of the contract to a newOwner. * @param newOwner The address to transfer ownership to. */ - function transferProxyOwnership(address newOwner) public onlyProxyOwner { + function transferProxyOwnership(address newOwner) public onlyUpgradeabilityOwner { require(newOwner != address(0)); - emit ProxyOwnershipTransferred(proxyOwner(), newOwner); + emit ProxyOwnershipTransferred(upgradeabilityOwner(), newOwner); setUpgradeabilityOwner(newOwner); } @@ -54,7 +46,7 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityOwnerStorage, UpgradeabilityP * @param version representing the version name of the new implementation to be set. * @param implementation representing the address of the new implementation to be set. */ - function upgradeTo(uint256 version, address implementation) public onlyProxyOwner { + function upgradeTo(uint256 version, address implementation) public onlyUpgradeabilityOwner { _upgradeTo(version, implementation); } @@ -66,7 +58,7 @@ contract OwnedUpgradeabilityProxy is UpgradeabilityOwnerStorage, UpgradeabilityP * @param data represents the msg.data to bet sent in the low level call. This parameter may include the function * signature of the implementation to be called with the needed payload */ - function upgradeToAndCall(uint256 version, address implementation, bytes data) payable public onlyProxyOwner { + function upgradeToAndCall(uint256 version, address implementation, bytes data) payable public onlyUpgradeabilityOwner { upgradeTo(version, implementation); require(address(this).call.value(msg.value)(data)); } diff --git a/contracts/upgradeability/UpgradeabilityOwnerStorage.sol b/contracts/upgradeability/UpgradeabilityOwnerStorage.sol index e502e3249..a6a27cf02 100644 --- a/contracts/upgradeability/UpgradeabilityOwnerStorage.sol +++ b/contracts/upgradeability/UpgradeabilityOwnerStorage.sol @@ -7,7 +7,7 @@ pragma solidity 0.4.24; */ contract UpgradeabilityOwnerStorage { // Owner of the contract - address private _upgradeabilityOwner; + address internal _upgradeabilityOwner; /** * @dev Tells the address of the owner diff --git a/contracts/upgradeable_contracts/OwnedUpgradeability.sol b/contracts/upgradeable_contracts/OwnedUpgradeability.sol index c31b15ed2..165d4a179 100644 --- a/contracts/upgradeable_contracts/OwnedUpgradeability.sol +++ b/contracts/upgradeable_contracts/OwnedUpgradeability.sol @@ -4,14 +4,9 @@ import "../IOwnedUpgradeabilityProxy.sol"; contract OwnedUpgradeability { - - function upgradeabilityAdmin() public view returns (address) { - return IOwnedUpgradeabilityProxy(this).proxyOwner(); - } - // Avoid using onlyProxyOwner name to prevent issues with implementation from proxy contract modifier onlyIfOwnerOfProxy() { - require(msg.sender == upgradeabilityAdmin()); + require(msg.sender == IOwnedUpgradeabilityProxy(this).upgradeabilityOwner()); _; } } diff --git a/test/erc_to_erc/home_bridge.test.js b/test/erc_to_erc/home_bridge.test.js index ff610fb58..431de9943 100644 --- a/test/erc_to_erc/home_bridge.test.js +++ b/test/erc_to_erc/home_bridge.test.js @@ -993,19 +993,6 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => { await homeBridge.fixAssetsAboveLimits(transactionHash, true, { from: owner }).should.be.fulfilled }) }) - describe('#OwnedUpgradeability', async () => { - it('upgradeabilityAdmin should return the proxy owner', async () => { - const homeBridgeImpl = await HomeBridge.new() - const storageProxy = await EternalStorageProxy.new().should.be.fulfilled - await storageProxy.upgradeTo('1', homeBridgeImpl.address).should.be.fulfilled - const homeBridge = await HomeBridge.at(storageProxy.address) - - const proxyOwner = await storageProxy.proxyOwner() - const upgradeabilityAdmin = await homeBridge.upgradeabilityAdmin() - - upgradeabilityAdmin.should.be.equal(proxyOwner) - }) - }) describe('#rewardableInitialize', async () => { let homeFee diff --git a/test/erc_to_native/home_bridge.test.js b/test/erc_to_native/home_bridge.test.js index 108ab3a3e..7317a19f1 100644 --- a/test/erc_to_native/home_bridge.test.js +++ b/test/erc_to_native/home_bridge.test.js @@ -180,7 +180,6 @@ contract('HomeBridge_ERC20_to_Native', async accounts => { it('can be upgraded keeping the state', async () => { const homeOwner = accounts[8] const storageProxy = await EternalStorageProxy.new().should.be.fulfilled - const proxyOwner = await storageProxy.proxyOwner() const data = homeContract.contract.methods .initialize( validatorContract.address, @@ -205,7 +204,6 @@ contract('HomeBridge_ERC20_to_Native', async accounts => { expect(await finalContract.maxPerTx()).to.be.bignumber.equal('2') expect(await finalContract.minPerTx()).to.be.bignumber.equal('1') expect(await finalContract.blockRewardContract()).to.be.equal(blockRewardContract.address) - expect(await finalContract.upgradeabilityAdmin()).to.be.equal(proxyOwner) const homeContractV2 = await HomeBridge.new() await storageProxy.upgradeTo('2', homeContractV2.address).should.be.fulfilled @@ -217,7 +215,6 @@ contract('HomeBridge_ERC20_to_Native', async accounts => { expect(await finalContractV2.maxPerTx()).to.be.bignumber.equal('2') expect(await finalContractV2.minPerTx()).to.be.bignumber.equal('1') expect(await finalContractV2.blockRewardContract()).to.be.equal(blockRewardContract.address) - expect(await finalContractV2.upgradeabilityAdmin()).to.be.equal(proxyOwner) }) it('cant initialize with invalid arguments', async () => { false.should.be.equal(await homeContract.isInitialized()) @@ -1380,19 +1377,6 @@ contract('HomeBridge_ERC20_to_Native', async accounts => { await homeBridge.fixAssetsAboveLimits(transactionHash, true, { from: owner }).should.be.fulfilled }) }) - describe('#OwnedUpgradeability', async () => { - it('upgradeabilityAdmin should return the proxy owner', async () => { - const homeBridgeImpl = await HomeBridge.new() - const storageProxy = await EternalStorageProxy.new().should.be.fulfilled - await storageProxy.upgradeTo('1', homeBridgeImpl.address).should.be.fulfilled - const homeBridge = await HomeBridge.at(storageProxy.address) - - const proxyOwner = await storageProxy.proxyOwner() - const upgradeabilityAdmin = await homeBridge.upgradeabilityAdmin() - - upgradeabilityAdmin.should.be.equal(proxyOwner) - }) - }) describe('#feeManager', async () => { let homeBridge