diff --git a/contracts/gateway/EIP20CoGateway.sol b/contracts/gateway/EIP20CoGateway.sol index 77638219..f32dfcc5 100644 --- a/contracts/gateway/EIP20CoGateway.sol +++ b/contracts/gateway/EIP20CoGateway.sol @@ -892,7 +892,6 @@ contract EIP20CoGateway is GatewayBase { ) public payable - isActive returns (bytes32 messageHash_) { require( diff --git a/contracts/gateway/EIP20Gateway.sol b/contracts/gateway/EIP20Gateway.sol index b58f07b5..d1055f50 100644 --- a/contracts/gateway/EIP20Gateway.sol +++ b/contracts/gateway/EIP20Gateway.sol @@ -180,6 +180,9 @@ contract EIP20Gateway is GatewayBase { /* public variables */ + /** Specifies if the Gateway is activated for any new process. */ + bool public activated; + /** Escrow address to lock staked fund. */ SimpleStake public stakeVault; @@ -198,6 +201,19 @@ contract EIP20Gateway is GatewayBase { /** Maps messageHash to the Unstake object. */ mapping(bytes32 /*messageHash*/ => Unstake) unstakes; + + /* Modifiers */ + + /** Checks that contract is active. */ + modifier isActive() { + require( + activated == true, + "Gateway is not activated." + ); + _; + } + + /* Constructor */ /** @@ -245,6 +261,7 @@ contract EIP20Gateway is GatewayBase { stakeVault = new SimpleStake(_token, address(this)); } + /* External functions */ /** @@ -778,12 +795,12 @@ contract EIP20Gateway is GatewayBase { bytes32 _messageHash, bytes32 _unlockSecret ) - external - returns ( - uint256 redeemAmount_, - uint256 unstakeAmount_, - uint256 rewardAmount_ - ) + external + returns ( + uint256 redeemAmount_, + uint256 unstakeAmount_, + uint256 rewardAmount_ + ) { // Get the inital gas uint256 initialGas = gasleft(); @@ -837,12 +854,12 @@ contract EIP20Gateway is GatewayBase { uint256 _blockHeight, uint256 _messageStatus ) - public - returns ( - uint256 redeemAmount_, - uint256 unstakeAmount_, - uint256 rewardAmount_ - ) + public + returns ( + uint256 redeemAmount_, + uint256 unstakeAmount_, + uint256 rewardAmount_ + ) { // Get the inital gas uint256 initialGas = gasleft(); @@ -900,12 +917,12 @@ contract EIP20Gateway is GatewayBase { uint256 _blockHeight, bytes calldata _rlpEncodedParentNodes ) - external - returns ( - address redeemer_, - uint256 redeemerNonce_, - uint256 amount_ - ) + external + returns ( + address redeemer_, + uint256 redeemerNonce_, + uint256 amount_ + ) { // Get the initial gas value uint256 initialGas = gasleft(); @@ -971,13 +988,17 @@ contract EIP20Gateway is GatewayBase { * @return success_ `true` if value is set */ function activateGateway( - address _coGatewayAddress + address _coGatewayAddress ) external onlyOrganization returns (bool success_) { + require( + _coGatewayAddress != address(0), + "Co-gateway address must not be zero." + ); require( remoteGateway == address(0), "Gateway was already activated once." @@ -1006,7 +1027,7 @@ contract EIP20Gateway is GatewayBase { { require( activated == true, - "Gateway is already deactivated" + "Gateway is already deactivated." ); activated = false; success_ = true; @@ -1102,6 +1123,7 @@ contract EIP20Gateway is GatewayBase { _unlockSecret ); } + /** * @notice This is internal method for process unstake called from external * methods which processUnstake(with hashlock) and diff --git a/contracts/gateway/GatewayBase.sol b/contracts/gateway/GatewayBase.sol index 553e082e..3f8df5ba 100644 --- a/contracts/gateway/GatewayBase.sol +++ b/contracts/gateway/GatewayBase.sol @@ -75,14 +75,11 @@ contract GatewayBase is Organized { */ MessageBus.MessageBox messageBox; - /** Specifies if the Gateway is activated for any new process. */ - bool public activated; - - /** Address of core contract. */ + /** address of core contract. */ CoreInterface public core; /** Path to make Merkle account proof for Gateway/CoGateway contract. */ - bytes internal encodedGatewayPath; + bytes public encodedGatewayPath; /** * Remote gateway contract address. If this is a gateway contract, then the @@ -127,18 +124,6 @@ contract GatewayBase is Organized { */ mapping(address => bytes32) outboxActiveProcess; - - /* Modifiers */ - - /** checks that contract is activated */ - modifier isActive() { - require( - activated == true, - "Gateway is not activated." - ); - _; - } - /* Constructor */ /** diff --git a/test/gateway/eip20_gateway/activate_gateway.js b/test/gateway/eip20_gateway/activate_gateway.js index 1082d5c6..a74318d9 100644 --- a/test/gateway/eip20_gateway/activate_gateway.js +++ b/test/gateway/eip20_gateway/activate_gateway.js @@ -3,9 +3,10 @@ const MockMembersManager = artifacts.require('MockMembersManager.sol'); const BN = require('bn.js'); const Utils = require('../../../test/test_lib/utils'); +const web3 = require('../../../test/test_lib/web3.js'); - -contract('EIP20Gateway.(de)activateGateway()', function (accounts) { +const zeroAddress = "0x0000000000000000000000000000000000000000"; +contract('EIP20Gateway.activateGateway()', function (accounts) { let gateway; let coGateway = accounts[5]; @@ -31,53 +32,68 @@ contract('EIP20Gateway.(de)activateGateway()', function (accounts) { ); }); - it('should deactivate if activated', async function () { + it('should activate if not already activated', async function () { + + let isSuccess = await gateway.activateGateway.call(coGateway, {from: owner}); - await gateway.activateGateway(coGateway, { from: owner }); - assert((await gateway.deactivateGateway.call({ from: owner }))); - await gateway.deactivateGateway({ from: owner }); - assert( - !(await gateway.activated.call()), - 'Activation flag is true but expected as false.' + assert.strictEqual( + isSuccess, + true, + "Gateway activation failed, activateGateway returned false.", ); - }); - it('should not deactivate if already deactivated', async function () { + await gateway.activateGateway(coGateway, {from: owner}); + let isActivated = await gateway.activated.call(); - await gateway.activateGateway(coGateway, { from: owner }); - await gateway.deactivateGateway({ from: owner }); - await Utils.expectThrow(gateway.deactivateGateway.call({ from: owner })); - }); + assert.strictEqual( + isActivated, + true, + 'Activation flag is false but expected as true.' + ); - it('should deactivated by organization only', async function () { + let actualCoGateway = await gateway.remoteGateway.call(); - await gateway.activateGateway(coGateway, { from: owner }); - await Utils.expectThrow(gateway.deactivateGateway.call({ from: accounts[0] })); - }); + assert.strictEqual( + coGateway, + actualCoGateway, + "Actual cogateway address is different from expected address." + ); - it('should activate if deActivated', async function () { + let actualEncodedGatewayPath = await gateway.encodedGatewayPath.call(); + let expectedEncodedGatewayPath = web3.utils.sha3(coGateway); - assert( - (await gateway.activateGateway.call(coGateway, { from: owner })), - "Gateway activation failed, activateGateway returned false.", + assert.strictEqual( + expectedEncodedGatewayPath, + actualEncodedGatewayPath, + "Actual encoded gateway path address is different from expected." ); - await gateway.activateGateway(coGateway, { from: owner }); - assert( - (await gateway.activated.call()), - 'Activation flag is false but expected as true.' - ); }); it('should not activate if already activated', async function () { - await gateway.activateGateway(coGateway, { from: owner }); - await Utils.expectThrow(gateway.activateGateway.call(coGateway, { from: owner })); + await gateway.activateGateway(coGateway, {from: owner}); + + await Utils.expectRevert( + gateway.activateGateway(coGateway, {from: owner}), + 'Gateway was already activated once.' + ); }); - it('should be activated by organization only', async function () { + it('should not activate with zero co-gateway address', async function () { - await Utils.expectThrow(gateway.activateGateway.call(coGateway, { from: accounts[0] })); + await Utils.expectRevert( + gateway.activateGateway(zeroAddress, {from: owner}), + 'Co-gateway address must not be zero.' + ); }); + it('should be activated by organization only', async function () { + + await Utils.expectRevert( + gateway.activateGateway(coGateway, {from: accounts[0]}), + 'Only the organization is allowed to call this method.' + ); + }); }); + diff --git a/test/gateway/eip20_gateway/deactivate_gateway.js b/test/gateway/eip20_gateway/deactivate_gateway.js new file mode 100644 index 00000000..1c6b3b29 --- /dev/null +++ b/test/gateway/eip20_gateway/deactivate_gateway.js @@ -0,0 +1,74 @@ +const Gateway = artifacts.require("./EIP20Gateway.sol") + , BN = require('bn.js'); +const MockMembersManager = artifacts.require('MockMembersManager.sol'); + +const Utils = require('../../../test/test_lib/utils'); + + +contract('EIP20Gateway.deactivateGateway()', function (accounts) { + + let gateway; + let owner = accounts[2]; + let worker = accounts[3]; + let coGateway = accounts[5]; + let membersManager; + + beforeEach(async function () { + + let mockToken = accounts[0], + baseToken = accounts[1], + coreAddress = accounts[2], + bountyAmount = new BN(100); + + membersManager = await MockMembersManager.new(owner, worker); + + gateway = await Gateway.new( + mockToken, + baseToken, + coreAddress, + bountyAmount, + membersManager.address + ); + + await gateway.activateGateway(coGateway, {from: owner}); + + }); + + it('should deactivate if activated', async function () { + + let isSuccess = await gateway.deactivateGateway.call({from: owner}); + + assert.strictEqual( + isSuccess, + true, + "Gateway deactivation failed, deactivateGateway returned false.", + ); + + await gateway.deactivateGateway({from: owner}); + let isActivated = await gateway.activated.call(); + + assert.strictEqual( + isActivated, + false, + 'Activation flag is true but expected as false.' + ); + }); + + it('should not deactivate if already deactivated', async function () { + + await gateway.deactivateGateway({from: owner}); + await Utils.expectRevert( + gateway.deactivateGateway.call({from: owner}), + 'Gateway is already deactivated.' + ); + }); + + it('should deactivated by organization only', async function () { + + await Utils.expectRevert( + gateway.deactivateGateway.call({from: accounts[0]}), + 'Only the organization is allowed to call this method.' + ); + }); + +}); diff --git a/test/gateway/eip20_gateway/stake.js b/test/gateway/eip20_gateway/stake.js index e03956a2..52908b13 100644 --- a/test/gateway/eip20_gateway/stake.js +++ b/test/gateway/eip20_gateway/stake.js @@ -52,19 +52,7 @@ let mockToken, errorMessage; -async function _setup(accounts) { - - mockToken = await MockToken.new(); - baseToken = await MockToken.new(); - - bountyAmount = new BN(100); - gateway = await Gateway.new( - mockToken.address, - baseToken.address, - accounts[3], //core address - bountyAmount, - accounts[4] // organization address - ); +async function _setup(accounts, gateway) { helper = new HelperKlass(gateway); gatewayTest = new EIP20GatewayKlass(gateway, mockToken, baseToken); @@ -159,7 +147,20 @@ contract('EIP20Gateway.stake() ', function (accounts) { beforeEach(async function () { - await _setup(accounts); + + mockToken = await MockToken.new(); + baseToken = await MockToken.new(); + + bountyAmount = new BN(100); + gateway = await Gateway.new( + mockToken.address, + baseToken.address, + accounts[1], //core address + bountyAmount, + accounts[2] // organisation address + ); + + await _setup(accounts, gateway); }); it('should fail to stake when stake amount is 0', async function () { @@ -304,5 +305,24 @@ contract('EIP20Gateway.stake() ', function (accounts) { errorMessage = "Previous process is not completed"; await _stake(utils.ResultType.FAIL); }); -}); + it('should fail stake if gateway is not activated.', async function () { + + let mockToken = await MockToken.new(); + let baseToken = await MockToken.new(); + let bountyAmount = new BN(100); + + gateway = await Gateway.new( + mockToken.address, + baseToken.address, + accounts[1], //core address + bountyAmount, + accounts[2] // organisation address + ); + + await _setup(accounts, gateway); + await _prepareData(); + await _stake(utils.ResultType.FAIL); + }); + +});