Added Initialize functions access control modifier (#333)

This commit is contained in:
Kirill Fedoseev 2019-12-24 21:41:06 +07:00 committed by Alexander Kolotov
parent fe132f0170
commit 1c8ee41fd4
16 changed files with 65 additions and 24 deletions

View File

@ -1,9 +1,8 @@
pragma solidity 0.4.24;
import "./EternalStorage.sol";
import "./OwnedUpgradeabilityProxy.sol";
import "./EternalStorageProxy.sol";
contract ClassicEternalStorageProxy is EternalStorage, OwnedUpgradeabilityProxy {
contract ClassicEternalStorageProxy is EternalStorageProxy {
// solhint-disable-next-line no-complex-fallback
function() public payable {
address _impl = implementation();

View File

@ -5,6 +5,7 @@ import "./BaseBridgeValidators.sol";
contract BridgeValidators is BaseBridgeValidators {
function initialize(uint256 _requiredSignatures, address[] _initialValidators, address _owner)
external
onlyRelevantSender
returns (bool)
{
require(!isInitialized());

View File

@ -1,12 +1,15 @@
pragma solidity 0.4.24;
import "../upgradeability/EternalStorage.sol";
import "../interfaces/IUpgradeabilityOwnerStorage.sol";
/**
* @title Ownable
* @dev This contract has an owner address providing basic authorization control
*/
contract Ownable is EternalStorage {
bytes4 internal constant UPGRADEABILITY_OWNER = 0x6fde8202; // upgradeabilityOwner()
/**
* @dev Event to show ownership has been transferred
* @param previousOwner representing the address of the previous owner
@ -23,6 +26,20 @@ contract Ownable is EternalStorage {
_;
}
/**
* @dev Throws if called by any account other than contract itself or owner.
*/
modifier onlyRelevantSender() {
// proxy owner if used through proxy, address(0) otherwise
require(
!address(this).call(abi.encodeWithSelector(UPGRADEABILITY_OWNER)) || // covers usage without calling through storage proxy
msg.sender == IUpgradeabilityOwnerStorage(this).upgradeabilityOwner() || // covers usage through regular proxy calls
msg.sender == address(this) // covers calls through upgradeAndCall proxy method
);
/* solcov ignore next */
_;
}
bytes32 internal constant OWNER = 0x02016836a56b71f0d02689e69e326f4f4c1b9057164ef592671cf0d37c8040c0; // keccak256(abi.encodePacked("owner"))
/**

View File

@ -8,7 +8,7 @@ contract RewardableValidators is BaseBridgeValidators {
address[] _initialValidators,
address[] _initialRewards,
address _owner
) public returns (bool) {
) external onlyRelevantSender returns (bool) {
require(!isInitialized());
require(_owner != address(0));
setOwner(_owner);

View File

@ -38,7 +38,7 @@ contract BasicAMBErc677ToErc677 is
uint256 _requestGasLimit,
uint256 _decimalShift,
address _owner
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
require(!isInitialized());
require(
_dailyLimitMaxPerTxMinPerTxArray[2] > 0 && // _minPerTx > 0

View File

@ -11,7 +11,7 @@ contract BasicAMB is BasicBridge {
uint256 _gasPrice,
uint256 _requiredBlockConfirmations,
address _owner
) public returns (bool) {
) external onlyRelevantSender returns (bool) {
require(!isInitialized());
require(_validatorContract != address(0) && AddressUtils.isContract(_validatorContract));
require(_gasPrice > 0);

View File

@ -13,7 +13,7 @@ contract ForeignBridgeErc677ToErc677 is ERC677Bridge, BasicForeignBridgeErcToErc
uint256[] _homeDailyLimitHomeMaxPerTxArray, // [ 0 = _homeDailyLimit, 1 = _homeMaxPerTx ]
address _owner,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_erc20token,

View File

@ -13,7 +13,7 @@ contract ForeignBridgeErcToErc is BasicForeignBridgeErcToErc, ERC20Bridge {
uint256[] _homeDailyLimitHomeMaxPerTxArray, // [ 0 = _homeDailyLimit, 1 = _homeMaxPerTx ]
address _owner,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_erc20token,

View File

@ -24,7 +24,7 @@ contract HomeBridgeErcToErc is
uint256[] _foreignDailyLimitForeignMaxPerTxArray, // [ 0 = _foreignDailyLimit, 1 = _foreignMaxPerTx ]
address _owner,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,
@ -51,7 +51,7 @@ contract HomeBridgeErcToErc is
address _feeManager,
uint256[] _homeFeeForeignFeeArray, // [ 0 = _homeFee, 1 = _foreignFee ]
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_rewardableInitialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,

View File

@ -18,7 +18,7 @@ contract HomeBridgeErcToErcPOSDAO is HomeBridgeErcToErc {
uint256[] _homeFeeForeignFeeArray, // [ 0 = _homeFee, 1 = _foreignFee ]
address _blockReward,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_rewardableInitialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,

View File

@ -21,7 +21,7 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge, OtherSideB
address _owner,
uint256 _decimalShift,
address _bridgeOnOtherSide
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
require(!isInitialized());
require(AddressUtils.isContract(_validatorContract));
require(_requiredBlockConfirmations != 0);

View File

@ -56,7 +56,7 @@ contract HomeBridgeErcToNative is
uint256[] _foreignDailyLimitForeignMaxPerTxArray, // [ 0 = _foreignDailyLimit, 1 = _foreignMaxPerTx ]
address _owner,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,
@ -83,7 +83,7 @@ contract HomeBridgeErcToNative is
address _feeManager,
uint256[] _homeFeeForeignFeeArray, // [ 0 = _homeFee, 1 = _foreignFee ]
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,

View File

@ -21,7 +21,7 @@ contract ForeignBridgeNativeToErc is
address _owner,
uint256 _decimalShift,
address _bridgeOnOtherSide
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_erc677token,
@ -49,7 +49,7 @@ contract ForeignBridgeNativeToErc is
uint256 _homeFee,
uint256 _decimalShift,
address _bridgeOnOtherSide
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_erc677token,

View File

@ -37,7 +37,7 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicHomeBridge, RewardableHom
uint256[] _foreignDailyLimitForeignMaxPerTxArray, // [ 0 = _foreignDailyLimit, 1 = _foreignMaxPerTx ]
address _owner,
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,
@ -61,7 +61,7 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicHomeBridge, RewardableHom
address _feeManager,
uint256[] _homeFeeForeignFeeArray, // [ 0 = _homeFee, 1 = _foreignFee ]
uint256 _decimalShift
) external returns (bool) {
) external onlyRelevantSender returns (bool) {
_initialize(
_validatorContract,
_dailyLimitMaxPerTxMinPerTxArray,

View File

@ -222,7 +222,9 @@ contract('RewardableValidators', async accounts => {
describe('#upgradable', async () => {
it('can be upgraded via upgradeToAndCall', async () => {
const storageProxy = await EternalStorageProxy.new().should.be.fulfilled
const storageProxy = await EternalStorageProxy.new({
from: accounts[0]
}).should.be.fulfilled
const requiredSignatures = '2'
const validators = [accounts[0], accounts[1]]
const rewards = accounts.slice(3, 5)
@ -230,7 +232,14 @@ contract('RewardableValidators', async accounts => {
const data = bridgeValidators.contract.methods
.initialize(requiredSignatures, validators, rewards, owner)
.encodeABI()
await storageProxy.upgradeToAndCall('1', bridgeValidators.address, data).should.be.fulfilled
await storageProxy
.upgradeToAndCall('1', bridgeValidators.address, data, {
from: accounts[1]
})
.should.be.rejectedWith(ERROR_MSG)
await storageProxy.upgradeToAndCall('1', bridgeValidators.address, data, {
from: accounts[0]
}).should.be.fulfilled
const finalContract = await BridgeValidators.at(storageProxy.address)
true.should.be.equal(await finalContract.isInitialized())
expect(await finalContract.requiredSignatures()).to.be.bignumber.equal(requiredSignatures)
@ -278,11 +287,14 @@ contract('RewardableValidators', async accounts => {
accounts.slice(0, 5).forEach(validator => {
it(`should remove ${validator} - with Proxy`, async () => {
// Given
const proxy = await EternalStorageProxy.new()
const proxy = await EternalStorageProxy.new({ from: owner })
const bridgeValidatorsImpl = await BridgeValidators.new()
await proxy.upgradeTo('1', bridgeValidatorsImpl.address)
bridgeValidators = await BridgeValidators.at(proxy.address)
const { initialize, isInitialized, removeValidator } = bridgeValidators
await initialize(1, accounts.slice(0, 5), accounts.slice(5, 10), owner, {
from: accounts[1]
}).should.be.rejectedWith(ERROR_MSG)
await initialize(1, accounts.slice(0, 5), accounts.slice(5, 10), owner, { from: owner }).should.be.fulfilled
true.should.be.equal(await isInitialized())

View File

@ -179,12 +179,21 @@ contract('BridgeValidators', async accounts => {
describe('#upgradable', async () => {
it('can be upgraded via upgradeToAndCall', async () => {
const storageProxy = await EternalStorageProxy.new().should.be.fulfilled
const storageProxy = await EternalStorageProxy.new({
from: accounts[0]
}).should.be.fulfilled
const requiredSignatures = '2'
const validators = [accounts[0], accounts[1]]
const owner = accounts[2]
const data = bridgeValidators.contract.methods.initialize(requiredSignatures, validators, owner).encodeABI()
await storageProxy.upgradeToAndCall('1', bridgeValidators.address, data).should.be.fulfilled
await storageProxy
.upgradeToAndCall('1', bridgeValidators.address, data, {
from: accounts[1]
})
.should.be.rejectedWith(ERROR_MSG)
await storageProxy.upgradeToAndCall('1', bridgeValidators.address, data, {
from: accounts[0]
}).should.be.fulfilled
const finalContract = await BridgeValidators.at(storageProxy.address)
true.should.be.equal(await finalContract.isInitialized())
expect(await finalContract.requiredSignatures()).to.be.bignumber.equal(requiredSignatures)
@ -232,11 +241,14 @@ contract('BridgeValidators', async accounts => {
accounts.slice(0, 5).forEach(validator => {
it(`should remove ${validator} - with Proxy`, async () => {
// Given
const proxy = await EternalStorageProxy.new()
const proxy = await EternalStorageProxy.new({
from: owner
})
const bridgeValidatorsImpl = await BridgeValidators.new()
await proxy.upgradeTo('1', bridgeValidatorsImpl.address)
bridgeValidators = await BridgeValidators.at(proxy.address)
const { initialize, isInitialized, removeValidator } = bridgeValidators
await initialize(1, accounts.slice(0, 5), owner, { from: accounts[1] }).should.be.rejectedWith(ERROR_MSG)
await initialize(1, accounts.slice(0, 5), owner, { from: owner }).should.be.fulfilled
true.should.be.equal(await isInitialized())