Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing events in initialize methods #262

Merged
merged 7 commits into from
Aug 6, 2019
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ contract BasicForeignBridgeErcToErc is BasicForeignBridge {
require(_gasPrice > 0);
require(_homeMaxPerTx < _homeDailyLimit);
require(_owner != address(0));

addressStorage[VALIDATOR_CONTRACT] = _validatorContract;
setErc20token(_erc20token);
uintStorage[DEPLOYED_AT_BLOCK] = block.number;
Expand All @@ -30,6 +31,10 @@ contract BasicForeignBridgeErcToErc is BasicForeignBridge {
uintStorage[EXECUTION_MAX_PER_TX] = _homeMaxPerTx;
setOwner(_owner);
setInitialize();

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_gasPrice);
emit ExecutionDailyLimitChanged(_homeDailyLimit);
}

function getBridgeMode() external pure returns (bytes4 _data) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ contract ForeignBridgeErc677ToErc677 is ERC677Bridge, BasicForeignBridgeErcToErc
uintStorage[DAILY_LIMIT] = _dailyLimit;
uintStorage[MIN_PER_TX] = _minPerTx;

emit DailyLimitChanged(_dailyLimit);

return isInitialized();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ contract HomeBridgeErcToErc is
uintStorage[EXECUTION_MAX_PER_TX] = _foreignMaxPerTx;
setOwner(_owner);
setErc677token(_erc677token);

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_homeGasPrice);
emit DailyLimitChanged(_dailyLimit);
emit ExecutionDailyLimitChanged(_foreignDailyLimit);
}

function claimTokensFromErc677(address _token, address _to) external onlyIfUpgradeabilityOwner {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge {
require(_gasPrice > 0);
require(_homeMaxPerTx < _homeDailyLimit);
require(_owner != address(0));

addressStorage[VALIDATOR_CONTRACT] = _validatorContract;
setErc20token(_erc20token);
uintStorage[DEPLOYED_AT_BLOCK] = block.number;
Expand All @@ -32,6 +33,11 @@ contract ForeignBridgeErcToNative is BasicForeignBridge, ERC20Bridge {
uintStorage[EXECUTION_MAX_PER_TX] = _homeMaxPerTx;
setOwner(_owner);
setInitialize();

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_gasPrice);
emit ExecutionDailyLimitChanged(_homeDailyLimit);

return isInitialized();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ contract HomeBridgeErcToNative is
require(_blockReward == address(0) || AddressUtils.isContract(_blockReward));
require(_foreignMaxPerTx < _foreignDailyLimit);
require(_owner != address(0));

addressStorage[VALIDATOR_CONTRACT] = _validatorContract;
uintStorage[DEPLOYED_AT_BLOCK] = block.number;
uintStorage[DAILY_LIMIT] = _dailyLimit;
Expand All @@ -157,6 +158,11 @@ contract HomeBridgeErcToNative is
uintStorage[EXECUTION_DAILY_LIMIT] = _foreignDailyLimit;
uintStorage[EXECUTION_MAX_PER_TX] = _foreignMaxPerTx;
setOwner(_owner);

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_homeGasPrice);
emit DailyLimitChanged(_dailyLimit);
emit ExecutionDailyLimitChanged(_foreignDailyLimit);
}

function onExecuteAffirmation(address _recipient, uint256 _value, bytes32 txHash) internal returns (bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ contract ForeignBridgeNativeToErc is
require(_foreignGasPrice > 0);
require(_homeMaxPerTx < _homeDailyLimit);
require(_owner != address(0));

addressStorage[VALIDATOR_CONTRACT] = _validatorContract;
setErc677token(_erc677token);
uintStorage[DAILY_LIMIT] = _dailyLimit;
Expand All @@ -113,6 +114,11 @@ contract ForeignBridgeNativeToErc is
uintStorage[EXECUTION_DAILY_LIMIT] = _homeDailyLimit;
uintStorage[EXECUTION_MAX_PER_TX] = _homeMaxPerTx;
setOwner(_owner);

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_foreignGasPrice);
emit DailyLimitChanged(_dailyLimit);
emit ExecutionDailyLimitChanged(_homeDailyLimit);
}

function onExecuteMessage(address _recipient, uint256 _amount, bytes32 _txHash) internal returns (bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicHomeBridge, RewardableHom
require(_minPerTx > 0 && _maxPerTx > _minPerTx && _dailyLimit > _maxPerTx);
require(_foreignMaxPerTx < _foreignDailyLimit);
require(_owner != address(0));

addressStorage[VALIDATOR_CONTRACT] = _validatorContract;
uintStorage[DEPLOYED_AT_BLOCK] = block.number;
uintStorage[DAILY_LIMIT] = _dailyLimit;
Expand All @@ -116,6 +117,11 @@ contract HomeBridgeNativeToErc is EternalStorage, BasicHomeBridge, RewardableHom
uintStorage[EXECUTION_DAILY_LIMIT] = _foreignDailyLimit;
uintStorage[EXECUTION_MAX_PER_TX] = _foreignMaxPerTx;
setOwner(_owner);

emit RequiredBlockConfirmationChanged(_requiredBlockConfirmations);
emit GasPriceChanged(_homeGasPrice);
emit DailyLimitChanged(_dailyLimit);
emit ExecutionDailyLimitChanged(_foreignDailyLimit);
}

function onSignaturesCollected(bytes _message) internal {
Expand Down
34 changes: 32 additions & 2 deletions test/erc_to_erc/foreign_bridge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const ERC677BridgeToken = artifacts.require('ERC677BridgeToken.sol')

const { expect } = require('chai')
const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
const { createMessage, sign, signatureToVRS, ether, getEvents } = require('../helpers/helpers')
const { createMessage, sign, signatureToVRS, ether, getEvents, expectEventInLogs } = require('../helpers/helpers')

const oneEther = ether('1')
const halfEther = ether('0.5')
Expand Down Expand Up @@ -116,7 +116,7 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
)
.should.be.rejectedWith(ERROR_MSG)

await foreignBridge.initialize(
const { logs } = await foreignBridge.initialize(
validatorContract.address,
token.address,
requireBlockConfirmations,
Expand All @@ -141,6 +141,12 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: homeDailyLimit })
})
})

Expand Down Expand Up @@ -478,6 +484,30 @@ contract('ForeignBridge_ERC20_to_ERC20', async accounts => {
})
})
describe('#ForeignBridgeErc677ToErc677_onTokenTransfer', async () => {
it('should emit correct events on initialize', async () => {
const owner = accounts[3]
token = await ERC677BridgeToken.new('TEST', 'TST', 18, { from: owner })
const foreignBridge = await ForeignBridgeErc677ToErc677.new()
const { logs } = await foreignBridge.initialize(
validatorContract.address,
token.address,
requireBlockConfirmations,
gasPrice,
dailyLimit,
maxPerTx,
minPerTx,
homeDailyLimit,
homeMaxPerTx,
owner
)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: homeDailyLimit })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: dailyLimit })
})
it('can only be called from token contract', async () => {
const owner = accounts[3]
const user = accounts[4]
Expand Down
18 changes: 16 additions & 2 deletions test/erc_to_erc/home_bridge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => {
expect(await homeContract.maxPerTx()).to.be.bignumber.equal(ZERO)
expect(await homeContract.isInitialized()).to.be.equal(false)

await homeContract.initialize(
const { logs } = await homeContract.initialize(
validatorContract.address,
'3',
'2',
Expand All @@ -75,6 +75,13 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: '3' })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: foreignDailyLimit })
})
it('cant set maxPerTx > dailyLimit', async () => {
expect(await homeContract.isInitialized()).to.be.equal(false)
Expand Down Expand Up @@ -1264,7 +1271,7 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => {
foreignFee,
blockRewardContract.address
).should.be.rejected
await homeBridge.rewardableInitialize(
const { logs } = await homeBridge.rewardableInitialize(
rewardableValidators.address,
oneEther,
halfEther,
Expand Down Expand Up @@ -1302,6 +1309,13 @@ contract('HomeBridge_ERC20_to_ERC20', async accounts => {
bridgeForeignFee.should.be.bignumber.equal(foreignFee)
const blockReward = await homeBridge.blockRewardContract()
blockReward.should.be.equals(blockRewardContract.address)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: oneEther })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: foreignDailyLimit })
})
it('can update fee contract', async () => {
const feeManager = await FeeManagerErcToErcPOSDAO.new()
Expand Down
10 changes: 8 additions & 2 deletions test/erc_to_native/foreign_bridge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const ERC677BridgeToken = artifacts.require('ERC677BridgeToken.sol')

const { expect } = require('chai')
const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
const { createMessage, sign, signatureToVRS, ether } = require('../helpers/helpers')
const { createMessage, sign, signatureToVRS, ether, expectEventInLogs } = require('../helpers/helpers')

const halfEther = ether('0.5')
const requireBlockConfirmations = 8
Expand Down Expand Up @@ -125,7 +125,7 @@ contract('ForeignBridge_ERC20_to_Native', async accounts => {
)
.should.be.rejectedWith(ERROR_MSG)

await foreignBridge.initialize(
const { logs } = await foreignBridge.initialize(
validatorContract.address,
token.address,
requireBlockConfirmations,
Expand All @@ -150,6 +150,12 @@ contract('ForeignBridge_ERC20_to_Native', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: homeDailyLimit })
})
})

Expand Down
9 changes: 8 additions & 1 deletion test/erc_to_native/home_bridge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract('HomeBridge_ERC20_to_Native', async accounts => {
expect(await homeContract.isInitialized()).to.be.equal(false)
expect(await homeContract.blockRewardContract()).to.be.equal(ZERO_ADDRESS)

await homeContract.initialize(
const { logs } = await homeContract.initialize(
validatorContract.address,
'3',
'2',
Expand All @@ -75,6 +75,13 @@ contract('HomeBridge_ERC20_to_Native', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: foreignDailyLimit })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: '3' })
})

it('can update block reward contract', async () => {
Expand Down
11 changes: 9 additions & 2 deletions test/native_to_erc/foreign_bridge_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const NoReturnTransferTokenMock = artifacts.require('NoReturnTransferTokenMock.s

const { expect } = require('chai')
const { ERROR_MSG, ZERO_ADDRESS, toBN } = require('../setup')
const { createMessage, sign, signatureToVRS, getEvents, ether } = require('../helpers/helpers')
const { createMessage, sign, signatureToVRS, getEvents, ether, expectEventInLogs } = require('../helpers/helpers')

const oneEther = ether('1')
const halfEther = ether('0.5')
Expand Down Expand Up @@ -128,7 +128,7 @@ contract('ForeignBridge', async accounts => {
owner
)
.should.be.rejectedWith(ERROR_MSG)
await foreignBridge.initialize(
const { logs } = await foreignBridge.initialize(
validatorContract.address,
token.address,
oneEther,
Expand Down Expand Up @@ -157,6 +157,13 @@ contract('ForeignBridge', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: homeDailyLimit })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: oneEther })
})
})

Expand Down
9 changes: 8 additions & 1 deletion test/native_to_erc/home_bridge_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract('HomeBridge', async accounts => {
expect(await homeContract.maxPerTx()).to.be.bignumber.equal(ZERO)
expect(await homeContract.isInitialized()).to.be.equal(false)

await homeContract.initialize(
const { logs } = await homeContract.initialize(
validatorContract.address,
'3',
'2',
Expand All @@ -70,6 +70,13 @@ contract('HomeBridge', async accounts => {
expect(major).to.be.bignumber.gte(ZERO)
expect(minor).to.be.bignumber.gte(ZERO)
expect(patch).to.be.bignumber.gte(ZERO)

expectEventInLogs(logs, 'RequiredBlockConfirmationChanged', {
requiredBlockConfirmations: toBN(requireBlockConfirmations)
})
expectEventInLogs(logs, 'GasPriceChanged', { gasPrice })
expectEventInLogs(logs, 'ExecutionDailyLimitChanged', { newLimit: foreignDailyLimit })
expectEventInLogs(logs, 'DailyLimitChanged', { newLimit: '3' })
})
it('cant set maxPerTx > dailyLimit', async () => {
false.should.be.equal(await homeContract.isInitialized())
Expand Down