Remove proxyOwner method from proxy contract (#198)

* Remove proxyOwner method from proxy
* Rename onlyProxyOwner
* Remove upgradeabilityAdmin
* Rename onlyIfOwnerOfProxy to onlyIfUpgradeabilityOwner
This commit is contained in:
Gerardo Nardelli 2019-06-27 17:44:09 -03:00 committed by Alexander Kolotov
parent dcec59bdda
commit a1f64ca6c5
13 changed files with 17 additions and 58 deletions

1
.gitignore vendored
View File

@ -3,3 +3,4 @@ build
flats
.node*
.idea
coverage

View File

@ -2,5 +2,5 @@ pragma solidity 0.4.24;
interface IOwnedUpgradeabilityProxy {
function proxyOwner() public view returns (address);
function upgradeabilityOwner() public view returns (address);
}

View File

@ -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));
}

View File

@ -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

View File

@ -127,7 +127,7 @@ contract BasicBridge is EternalStorage, Validatable, Ownable, OwnedUpgradeabilit
return executionDailyLimit() >= nextLimit && _amount <= executionMaxPerTx();
}
function claimTokens(address _token, address _to) public onlyIfOwnerOfProxy {
function claimTokens(address _token, address _to) public onlyIfUpgradeabilityOwner {
require(_to != address(0));
if (_token == address(0)) {
_to.transfer(address(this).balance);

View File

@ -10,7 +10,7 @@ contract OverdrawManagement is EternalStorage, OwnedUpgradeability {
event UserRequestForSignature(address recipient, uint256 value);
function fixAssetsAboveLimits(bytes32 txHash, bool unlockOnForeign) external onlyIfOwnerOfProxy {
function fixAssetsAboveLimits(bytes32 txHash, bool unlockOnForeign) external onlyIfUpgradeabilityOwner {
require(!fixedAssets(txHash));
address recipient;
uint256 value;

View File

@ -4,14 +4,9 @@ import "../interfaces/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());
// Avoid using onlyUpgradeabilityOwner name to prevent issues with implementation from proxy contract
modifier onlyIfUpgradeabilityOwner() {
require(msg.sender == IOwnedUpgradeabilityProxy(this).upgradeabilityOwner());
_;
}
}

View File

@ -39,7 +39,7 @@ contract BasicForeignBridgeErcToErc is BasicBridge, BasicForeignBridge {
return bytes4(keccak256(abi.encodePacked("erc-to-erc-core")));
}
function claimTokens(address _token, address _to) public onlyIfOwnerOfProxy {
function claimTokens(address _token, address _to) public {
require(_token != address(erc20token()));
super.claimTokens(_token, _to);
}

View File

@ -43,7 +43,7 @@ contract ForeignBridgeErcToNative is BasicBridge, BasicForeignBridge {
return bytes4(keccak256(abi.encodePacked("erc-to-native-core")));
}
function claimTokens(address _token, address _to) public onlyIfOwnerOfProxy {
function claimTokens(address _token, address _to) public {
require(_token != address(erc20token()));
super.claimTokens(_token, _to);
}

View File

@ -79,7 +79,7 @@ contract ForeignBridgeNativeToErc is ERC677Receiver, BasicBridge, BasicForeignBr
return bytes4(keccak256(abi.encodePacked("native-to-erc-core")));
}
function claimTokensFromErc677(address _token, address _to) external onlyIfOwnerOfProxy {
function claimTokensFromErc677(address _token, address _to) external onlyIfUpgradeabilityOwner {
IBurnableMintableERC677Token(erc677token()).claimTokens(_token, _to);
}

View File

@ -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

View File

@ -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