From 13bd18a48c7a9cc24f3d928bdd5a09008d5a3d44 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Tue, 18 Dec 2018 20:55:50 +0530 Subject: [PATCH 1/9] Added mutex contract --- contracts/lib/Mutex.sol | 70 +++++++++++++++++++++++++++++ contracts/test/TestMutex.sol | 52 ++++++++++++++++++++++ test/lib/mutex/acquire.js | 86 ++++++++++++++++++++++++++++++++++++ test/lib/mutex/release.js | 73 ++++++++++++++++++++++++++++++ 4 files changed, 281 insertions(+) create mode 100644 contracts/lib/Mutex.sol create mode 100644 contracts/test/TestMutex.sol create mode 100644 test/lib/mutex/acquire.js create mode 100644 test/lib/mutex/release.js diff --git a/contracts/lib/Mutex.sol b/contracts/lib/Mutex.sol new file mode 100644 index 00000000..05f9c7fa --- /dev/null +++ b/contracts/lib/Mutex.sol @@ -0,0 +1,70 @@ +pragma solidity ^0.5.0; + +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** @title Mutex contract provide locking mechanism. */ +contract Mutex { + + /* Storage */ + + /** Mapping of address and lock status. */ + mapping(address => bool) private locks; + + + /* Constructor */ + + constructor() public + { } + + /** @notice This acquires lock for an address if not already acquired. + * This will revert the transaction if lock is already acquired. + * + * @param _address Address for which lock acquisition is required. + * + * @return success_ `true` on successful lock acquisition, `false` otherwise. + */ + function acquire(address _address) + internal + returns(bool success_) + { + require( + locks[_address] == false, + "Lock already acquired for the address." + ); + + locks[msg.sender] = true; + success_ = true; + } + + /** @notice This releases lock for an address if lock is acquired. + * This will revert the transaction if lock is not acquired. + * + * @param _address Address for which release lock is required. + * + * @return success_ `true` on successful lock release, `false` otherwise. + */ + function release(address _address) + internal + returns(bool success_) + { + require( + locks[_address] == true, + "Lock is not acquired for the address." + ); + + locks[_address] = false; + success_ = true; + } +} diff --git a/contracts/test/TestMutex.sol b/contracts/test/TestMutex.sol new file mode 100644 index 00000000..e45c4005 --- /dev/null +++ b/contracts/test/TestMutex.sol @@ -0,0 +1,52 @@ +pragma solidity ^0.5.0; + +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import "../lib/Mutex.sol"; + +/** @title TestMutex contract to test Mutex */ +contract TestMutex is Mutex{ + + constructor() public + { } + + /** @notice This acquires lock for an address if not already acquired. + * This will revert the transaction if lock is already acquired. + * + * @param _address Address for which lock acquisition is required. + * + * @return success_ `true` on successful lock acquisition, `false` otherwise. + */ + function acquireExternal(address _address) + external + returns(bool success_) + { + success_ = super.acquire(_address); + } + + /** @notice This releases lock for an address if lock is acquired. + * This will revert the transaction if lock is not acquired. + * + * @param _address Address for which release lock is required. + * + * @return success_ `true` on successful lock release, `false` otherwise. + */ + function releaseExternal(address _address) + external + returns(bool success_) + { + success_ = super.release(_address); + } +} diff --git a/test/lib/mutex/acquire.js b/test/lib/mutex/acquire.js new file mode 100644 index 00000000..0029a4d2 --- /dev/null +++ b/test/lib/mutex/acquire.js @@ -0,0 +1,86 @@ +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ---------------------------------------------------------------------------- +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- + +const Utils = require('../../test_lib/utils.js'); + +const Mutex = artifacts.require('TestMutex'); + +contract('Mutex.acquire()', async (accounts) => { + + it('should acquire lock for an address', async () => { + + let mutex = await Mutex.new(); + + let address = accounts[0]; + let result = await mutex.acquireExternal.call(address); + + assert.strictEqual( + result, + true, + 'Lock acquire should success.' + ); + await mutex.acquireExternal(address); + + Utils.expectRevert( + mutex.acquireExternal(address), + 'Lock already acquired for the address.' + ); + }); + + it('should not acquire lock for an address if already acquired', async () => { + + let mutex = await Mutex.new(); + + let address = accounts[0]; + await mutex.acquireExternal(address); + + Utils.expectRevert( + mutex.acquireExternal(address), + 'Lock already acquired for the address.' + ); + }); + + it('should allow lock acquisition for more than one account', async () => { + let mutex = await Mutex.new(); + + let firstAddress = accounts[0]; + let result = await mutex.acquireExternal.call(firstAddress); + + assert.strictEqual( + result, + true, + 'Lock acquire should success.' + ); + + await mutex.acquireExternal(firstAddress); + + let secondAddress = accounts[1]; + result = await mutex.acquireExternal.call(secondAddress); + + assert.strictEqual( + result, + true, + 'Lock acquire should success.' + ); + + await mutex.acquireExternal(secondAddress); + }); + +}); \ No newline at end of file diff --git a/test/lib/mutex/release.js b/test/lib/mutex/release.js new file mode 100644 index 00000000..d8730303 --- /dev/null +++ b/test/lib/mutex/release.js @@ -0,0 +1,73 @@ +// Copyright 2018 OpenST Ltd. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// ---------------------------------------------------------------------------- +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- + +const Utils = require('../../test_lib/utils.js'); + +const Mutex = artifacts.require('TestMutex'); + +contract('Mutex.release()', async (accounts) => { + + it('should release lock for an address', async () => { + + let mutex = await Mutex.new(); + + let address = accounts[0]; + await mutex.acquireExternal(address); + + let result = await mutex.releaseExternal.call(address); + assert.strictEqual( + result, + true, + 'Lock acquire should success.' + ); + await mutex.releaseExternal(address); + + Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + }); + + it('should not release lock for an address if already released', async () => { + + let mutex = await Mutex.new(); + + let address = accounts[0]; + await mutex.acquireExternal(address); + + await mutex.releaseExternal(address); + + Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + }); + + it('should not release lock if lock is not acquired', async () => { + let mutex = await Mutex.new(); + + let address = accounts[0]; + + Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + }); +}); \ No newline at end of file From 6f9b76799871c998cc3b7717db5da2a1dd5523d9 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Wed, 19 Dec 2018 00:01:58 +0530 Subject: [PATCH 2/9] OST prime now mints token in base token --- contracts/gateway/EIP20CoGateway.sol | 6 ++-- contracts/gateway/OSTPrime.sol | 12 +++++-- contracts/gateway/UtilityToken.sol | 2 +- contracts/gateway/UtilityTokenInterface.sol | 2 +- contracts/test/TestEIP20CoGateway.sol | 2 +- test/gateway/ost_prime/increase_supply.js | 38 +++++++-------------- 6 files changed, 28 insertions(+), 34 deletions(-) diff --git a/contracts/gateway/EIP20CoGateway.sol b/contracts/gateway/EIP20CoGateway.sol index 3d18d2d4..0821791f 100644 --- a/contracts/gateway/EIP20CoGateway.sol +++ b/contracts/gateway/EIP20CoGateway.sol @@ -173,7 +173,7 @@ contract EIP20CoGateway is GatewayBase { uint256 amount; /** Address for which the utility tokens will be minted */ - address beneficiary; + address payable beneficiary; } /* public Variables */ @@ -768,7 +768,7 @@ contract EIP20CoGateway is GatewayBase { function confirmStakeIntent( address _staker, uint256 _stakerNonce, - address _beneficiary, + address payable _beneficiary, uint256 _amount, uint256 _gasPrice, uint256 _gasLimit, @@ -1112,7 +1112,7 @@ contract EIP20CoGateway is GatewayBase { // Mint token after subtracting reward amount. UtilityTokenInterface(utilityToken).increaseSupply( - beneficiary_, + mint.beneficiary, mintedAmount_ ); diff --git a/contracts/gateway/OSTPrime.sol b/contracts/gateway/OSTPrime.sol index b10bc6a3..013eba2d 100644 --- a/contracts/gateway/OSTPrime.sol +++ b/contracts/gateway/OSTPrime.sol @@ -31,6 +31,7 @@ import "../lib/SafeMath.sol"; import "./UtilityToken.sol"; import "./OSTPrimeConfig.sol"; import "../lib/IsMemberInterface.sol"; +import "../lib/Mutex.sol"; /** * @title OSTPrime contract implements UtilityToken and @@ -42,7 +43,7 @@ import "../lib/IsMemberInterface.sol"; * @dev OSTPrime functions as the base token to pay for gas consumption on the * utility chain. */ -contract OSTPrime is UtilityToken, OSTPrimeConfig { +contract OSTPrime is UtilityToken, OSTPrimeConfig, Mutex { /* Usings */ @@ -231,7 +232,7 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig { * @return success_ `true` if increase supply is successful, false otherwise. */ function increaseSupply( - address _account, + address payable _account, uint256 _amount ) external @@ -239,7 +240,12 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig { onlyCoGateway returns (bool success_) { - success_ = increaseSupplyInternal(_account, _amount); + acquire(msg.sender); + + success_ = increaseSupplyInternal(address(this), _amount); + _account.transfer(_amount); + + release(msg.sender); } /** diff --git a/contracts/gateway/UtilityToken.sol b/contracts/gateway/UtilityToken.sol index f5a4008f..74fdba19 100644 --- a/contracts/gateway/UtilityToken.sol +++ b/contracts/gateway/UtilityToken.sol @@ -148,7 +148,7 @@ contract UtilityToken is EIP20Token, Organized, UtilityTokenInterface { * @return success_ `true` if increase supply is successful, false otherwise. */ function increaseSupply( - address _account, + address payable _account, uint256 _amount ) external diff --git a/contracts/gateway/UtilityTokenInterface.sol b/contracts/gateway/UtilityTokenInterface.sol index fb0a727c..a874ee64 100644 --- a/contracts/gateway/UtilityTokenInterface.sol +++ b/contracts/gateway/UtilityTokenInterface.sol @@ -43,7 +43,7 @@ contract UtilityTokenInterface { * @return success_ `true` if increase supply is successful, false otherwise. */ function increaseSupply( - address _account, + address payable _account, uint256 _amount ) external diff --git a/contracts/test/TestEIP20CoGateway.sol b/contracts/test/TestEIP20CoGateway.sol index 27999e40..99b13625 100644 --- a/contracts/test/TestEIP20CoGateway.sol +++ b/contracts/test/TestEIP20CoGateway.sol @@ -131,7 +131,7 @@ contract TestEIP20CoGateway is EIP20CoGateway { */ function setMints( bytes32 _messageHash, - address _beneficiary, + address payable _beneficiary, uint256 _amount ) public diff --git a/test/gateway/ost_prime/increase_supply.js b/test/gateway/ost_prime/increase_supply.js index 7a3b90d0..167e423b 100644 --- a/test/gateway/ost_prime/increase_supply.js +++ b/test/gateway/ost_prime/increase_supply.js @@ -64,23 +64,6 @@ contract('OSTPrime.increaseSupply()', function (accounts) { }); - it('should fail when account address is zero', async function () { - - await initialize(); - - beneficiary = NullAddress; - - await Utils.expectRevert( - ostPrime.increaseSupply( - beneficiary, - amount, - { from: coGatewayAddress }, - ), - 'Account address should not be zero.', - ); - - }); - it('should fail when amount is zero', async function () { await initialize(); @@ -141,18 +124,23 @@ contract('OSTPrime.increaseSupply()', function (accounts) { true, 'Contract should return true.', ); + let initialBeneficiaryBalance = await Utils.getBalance(beneficiary); await ostPrime.increaseSupply( - beneficiary, - amount, - { from: coGatewayAddress }, + beneficiary, + amount, + {from: coGatewayAddress}, ); - let beneficiaryBalance = await ostPrime.balanceOf.call(beneficiary); + let finalBeneficiaryBalance = await Utils.getBalance(beneficiary); + + initialBeneficiaryBalance = new BN(initialBeneficiaryBalance); + finalBeneficiaryBalance = new BN(finalBeneficiaryBalance); + assert.strictEqual( - amount.eq(beneficiaryBalance), + finalBeneficiaryBalance.eq(initialBeneficiaryBalance.add(amount)), true, - `Beneficiary address balance should be ${amount}.`, + `Beneficiary base token address balance should be ${amount}.`, ); let totalSupply = await ostPrime.totalSupply(); @@ -191,8 +179,8 @@ contract('OSTPrime.increaseSupply()', function (accounts) { assert.strictEqual( eventData._to, - beneficiary, - `The _to address in the event should be equal to ${beneficiary}.`, + ostPrime.address, + `The _to address in the event should be equal to ${ostPrime.address}.`, ); assert.strictEqual( From fc64716cdb6b3fea57a756b0079f52ee29937289 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Wed, 19 Dec 2018 12:39:43 +0530 Subject: [PATCH 3/9] Fixed broken unit tests , added await to expectRevert --- contracts/test/TestMutex.sol | 4 +++- test/lib/mutex/acquire.js | 4 ++-- test/lib/mutex/release.js | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/contracts/test/TestMutex.sol b/contracts/test/TestMutex.sol index e45c4005..86e72b9c 100644 --- a/contracts/test/TestMutex.sol +++ b/contracts/test/TestMutex.sol @@ -19,7 +19,9 @@ import "../lib/Mutex.sol"; /** @title TestMutex contract to test Mutex */ contract TestMutex is Mutex{ - constructor() public + constructor() + Mutex() + public { } /** @notice This acquires lock for an address if not already acquired. diff --git a/test/lib/mutex/acquire.js b/test/lib/mutex/acquire.js index 0029a4d2..95521216 100644 --- a/test/lib/mutex/acquire.js +++ b/test/lib/mutex/acquire.js @@ -38,7 +38,7 @@ contract('Mutex.acquire()', async (accounts) => { ); await mutex.acquireExternal(address); - Utils.expectRevert( + await Utils.expectRevert( mutex.acquireExternal(address), 'Lock already acquired for the address.' ); @@ -51,7 +51,7 @@ contract('Mutex.acquire()', async (accounts) => { let address = accounts[0]; await mutex.acquireExternal(address); - Utils.expectRevert( + await Utils.expectRevert( mutex.acquireExternal(address), 'Lock already acquired for the address.' ); diff --git a/test/lib/mutex/release.js b/test/lib/mutex/release.js index d8730303..9ed5c376 100644 --- a/test/lib/mutex/release.js +++ b/test/lib/mutex/release.js @@ -39,7 +39,7 @@ contract('Mutex.release()', async (accounts) => { ); await mutex.releaseExternal(address); - Utils.expectRevert( + await Utils.expectRevert( mutex.releaseExternal(address), 'Lock is not acquired for the address.' ); @@ -65,7 +65,7 @@ contract('Mutex.release()', async (accounts) => { let address = accounts[0]; - Utils.expectRevert( + await Utils.expectRevert( mutex.releaseExternal(address), 'Lock is not acquired for the address.' ); From 0a819600c6221999d69be7873cfe77dc6dd6cb15 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Wed, 19 Dec 2018 19:43:54 +0530 Subject: [PATCH 4/9] Added enhanced documentation --- contracts/gateway/EIP20CoGateway.sol | 4 +++- contracts/gateway/OSTPrime.sol | 9 ++++++++- contracts/gateway/UtilityToken.sol | 2 ++ contracts/gateway/UtilityTokenInterface.sol | 2 ++ contracts/test/TestMutex.sol | 8 +++++++- 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/contracts/gateway/EIP20CoGateway.sol b/contracts/gateway/EIP20CoGateway.sol index 0821791f..87531009 100644 --- a/contracts/gateway/EIP20CoGateway.sol +++ b/contracts/gateway/EIP20CoGateway.sol @@ -753,7 +753,9 @@ contract EIP20CoGateway is GatewayBase { * @param _staker Staker address. * @param _stakerNonce Nonce of the staker address. * @param _beneficiary The address in the auxiliary chain where the utility - * tokens will be minted. + * tokens will be minted. This is payable so that it + * provides flexibility of transferring base token + * to account on minting. * @param _amount Amount of utility token will be minted. * @param _gasPrice Gas price that staker is ready to pay to get the stake * and mint process done diff --git a/contracts/gateway/OSTPrime.sol b/contracts/gateway/OSTPrime.sol index 013eba2d..e7bf5276 100644 --- a/contracts/gateway/OSTPrime.sol +++ b/contracts/gateway/OSTPrime.sol @@ -226,7 +226,8 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig, Mutex { * is added in function increaseSupplyInternal. * * @param _account Account address for which the OST Prime balance will be - * increased. + * increased. This is payable so that base token can be + * transferred to the account. * @param _amount Amount of tokens. * * @return success_ `true` if increase supply is successful, false otherwise. @@ -240,11 +241,17 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig, Mutex { onlyCoGateway returns (bool success_) { + /* + * Acquire lock for msg.sender so that this function can only be + * executed once in a transaction. + */ + acquire(msg.sender); success_ = increaseSupplyInternal(address(this), _amount); _account.transfer(_amount); + // Release lock for msg.sender. release(msg.sender); } diff --git a/contracts/gateway/UtilityToken.sol b/contracts/gateway/UtilityToken.sol index 74fdba19..fb0e97fe 100644 --- a/contracts/gateway/UtilityToken.sol +++ b/contracts/gateway/UtilityToken.sol @@ -143,6 +143,8 @@ contract UtilityToken is EIP20Token, Organized, UtilityTokenInterface { * is added in function increaseSupplyInternal. * * @param _account Account address for which the balance will be increased. + This is payable so that it provides flexibility of + * transferring base token to account on increase supply. * @param _amount Amount of tokens. * * @return success_ `true` if increase supply is successful, false otherwise. diff --git a/contracts/gateway/UtilityTokenInterface.sol b/contracts/gateway/UtilityTokenInterface.sol index a874ee64..c54f9a80 100644 --- a/contracts/gateway/UtilityTokenInterface.sol +++ b/contracts/gateway/UtilityTokenInterface.sol @@ -38,6 +38,8 @@ contract UtilityTokenInterface { * total token supply. * * @param _account Account address for which the balance will be increased. + * This is payable so that it provides flexibility of + * transferring base token to account on increase supply. * @param _amount Amount of tokens. * * @return success_ `true` if increase supply is successful, false otherwise. diff --git a/contracts/test/TestMutex.sol b/contracts/test/TestMutex.sol index 86e72b9c..8dbde881 100644 --- a/contracts/test/TestMutex.sol +++ b/contracts/test/TestMutex.sol @@ -13,10 +13,16 @@ pragma solidity ^0.5.0; // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +// +// ---------------------------------------------------------------------------- +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- import "../lib/Mutex.sol"; -/** @title TestMutex contract to test Mutex */ +/** @title TestMutex contract to test Mutex. */ contract TestMutex is Mutex{ constructor() From 210d4cfd8668ac0995acabe9ae0c097107a2f5f3 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Wed, 19 Dec 2018 22:47:14 +0530 Subject: [PATCH 5/9] PR feedback changes Renamed Mutex -> MutexAddress , commenting and styling --- contracts/gateway/OSTPrime.sol | 10 +- contracts/lib/{Mutex.sol => MutexAddress.sol} | 22 ++-- .../{TestMutex.sol => TestMutexAddress.sol} | 8 +- test/gateway/ost_prime/increase_supply.js | 113 +++++++++--------- test/gateway/ost_prime/initialize.js | 6 +- test/gateway/ost_prime/unwrap.js | 17 +-- test/gateway/ost_prime/wrap.js | 16 +-- test/lib/mutex/acquire.js | 92 +++++++------- test/lib/mutex/release.js | 81 +++++++------ test/test_lib/utils.js | 2 +- 10 files changed, 178 insertions(+), 189 deletions(-) rename contracts/lib/{Mutex.sol => MutexAddress.sol} (74%) rename contracts/test/{TestMutex.sol => TestMutexAddress.sol} (92%) diff --git a/contracts/gateway/OSTPrime.sol b/contracts/gateway/OSTPrime.sol index e7bf5276..b8fd0246 100644 --- a/contracts/gateway/OSTPrime.sol +++ b/contracts/gateway/OSTPrime.sol @@ -31,7 +31,7 @@ import "../lib/SafeMath.sol"; import "./UtilityToken.sol"; import "./OSTPrimeConfig.sol"; import "../lib/IsMemberInterface.sol"; -import "../lib/Mutex.sol"; +import "../lib/MutexAddress.sol"; /** * @title OSTPrime contract implements UtilityToken and @@ -43,7 +43,7 @@ import "../lib/Mutex.sol"; * @dev OSTPrime functions as the base token to pay for gas consumption on the * utility chain. */ -contract OSTPrime is UtilityToken, OSTPrimeConfig, Mutex { +contract OSTPrime is UtilityToken, OSTPrimeConfig, MutexAddress { /* Usings */ @@ -245,14 +245,12 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig, Mutex { * Acquire lock for msg.sender so that this function can only be * executed once in a transaction. */ - - acquire(msg.sender); + MutexAddress.acquire(msg.sender); success_ = increaseSupplyInternal(address(this), _amount); _account.transfer(_amount); - // Release lock for msg.sender. - release(msg.sender); + MutexAddress.release(msg.sender); } /** diff --git a/contracts/lib/Mutex.sol b/contracts/lib/MutexAddress.sol similarity index 74% rename from contracts/lib/Mutex.sol rename to contracts/lib/MutexAddress.sol index 05f9c7fa..f110e93e 100644 --- a/contracts/lib/Mutex.sol +++ b/contracts/lib/MutexAddress.sol @@ -14,8 +14,8 @@ pragma solidity ^0.5.0; // See the License for the specific language governing permissions and // limitations under the License. -/** @title Mutex contract provide locking mechanism. */ -contract Mutex { +/** @title MutexAddress contract provide locking mechanism. */ +contract MutexAddress { /* Storage */ @@ -28,16 +28,17 @@ contract Mutex { constructor() public { } - /** @notice This acquires lock for an address if not already acquired. + /** + * @notice This acquires lock for an address if not already acquired. * This will revert the transaction if lock is already acquired. * * @param _address Address for which lock acquisition is required. * - * @return success_ `true` on successful lock acquisition, `false` otherwise. + * @return isAcquired_ `true` on successful lock acquisition, `false` otherwise. */ function acquire(address _address) internal - returns(bool success_) + returns(bool isAcquired_) { require( locks[_address] == false, @@ -45,19 +46,20 @@ contract Mutex { ); locks[msg.sender] = true; - success_ = true; + isAcquired_ = true; } - /** @notice This releases lock for an address if lock is acquired. + /** + * @notice This releases lock for an address if lock is acquired. * This will revert the transaction if lock is not acquired. * * @param _address Address for which release lock is required. * - * @return success_ `true` on successful lock release, `false` otherwise. + * @return isReleased_ `true` on successful lock release, `false` otherwise. */ function release(address _address) internal - returns(bool success_) + returns(bool isReleased_) { require( locks[_address] == true, @@ -65,6 +67,6 @@ contract Mutex { ); locks[_address] = false; - success_ = true; + isReleased_ = true; } } diff --git a/contracts/test/TestMutex.sol b/contracts/test/TestMutexAddress.sol similarity index 92% rename from contracts/test/TestMutex.sol rename to contracts/test/TestMutexAddress.sol index 8dbde881..ce4fa4ac 100644 --- a/contracts/test/TestMutex.sol +++ b/contracts/test/TestMutexAddress.sol @@ -20,13 +20,13 @@ pragma solidity ^0.5.0; // // ---------------------------------------------------------------------------- -import "../lib/Mutex.sol"; +import "../lib/MutexAddress.sol"; -/** @title TestMutex contract to test Mutex. */ -contract TestMutex is Mutex{ +/** @title TestMutexAddress contract to test Mutex. */ +contract TestMutexAddress is MutexAddress { constructor() - Mutex() + MutexAddress() public { } diff --git a/test/gateway/ost_prime/increase_supply.js b/test/gateway/ost_prime/increase_supply.js index 167e423b..0fa603f0 100644 --- a/test/gateway/ost_prime/increase_supply.js +++ b/test/gateway/ost_prime/increase_supply.js @@ -26,12 +26,12 @@ const EventDecoder = require('../../test_lib/event_decoder.js'); const NullAddress = "0x0000000000000000000000000000000000000000"; contract('OSTPrime.increaseSupply()', function (accounts) { - + const DECIMAL = new BN(10); const POW = new BN(18); const DECIMAL_FACTOR = DECIMAL.pow(POW); const TOKENS_MAX = new BN(800000000).mul(DECIMAL_FACTOR); - + let brandedTokenAddress, beneficiary, ostPrime, @@ -39,156 +39,153 @@ contract('OSTPrime.increaseSupply()', function (accounts) { amount, membersManager, coGatewayAddress; - - async function initialize(){ + + async function initialize() { await ostPrime.initialize( - {from: accounts[2], value:TOKENS_MAX} + {from: accounts[2], value: TOKENS_MAX} ); }; - + beforeEach(async function () { - + beneficiary = accounts[6]; membersManager = accounts[0]; brandedTokenAddress = accounts[2]; ostPrime = await OSTPrime.new(brandedTokenAddress, membersManager); - + callerAddress = accounts[3]; amount = new BN(1000); - + await ostPrime.setTokenBalance(callerAddress, amount); - + coGatewayAddress = accounts[1]; - + await ostPrime.setCoGatewayAddress(coGatewayAddress); - + }); - + it('should fail when amount is zero', async function () { - + await initialize(); - + amount = new BN(0); - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, amount, - { from: coGatewayAddress }, + {from: coGatewayAddress}, ), 'Amount should be greater than zero.', ); - + }); - + it('should fail when caller is not CoGateway address', async function () { - + await initialize(); - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, amount, - { from: accounts[7] }, + {from: accounts[7]}, ), 'Only CoGateway can call the function.', ); - + }); - + it('should fail when OST Prime is not initialized', async function () { - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, amount, - { from: accounts[7] }, + {from: accounts[7]}, ), 'Contract is not initialized.', ); - + }); - + it('should pass with correct params', async function () { - + await initialize(); - + let result = await ostPrime.increaseSupply.call( - beneficiary, - amount, - { from: coGatewayAddress }, - ); - + beneficiary, + amount, + {from: coGatewayAddress}, + ); + assert.strictEqual( result, true, 'Contract should return true.', ); let initialBeneficiaryBalance = await Utils.getBalance(beneficiary); - + await ostPrime.increaseSupply( - beneficiary, - amount, - {from: coGatewayAddress}, + beneficiary, + amount, + {from: coGatewayAddress}, ); - + let finalBeneficiaryBalance = await Utils.getBalance(beneficiary); - - initialBeneficiaryBalance = new BN(initialBeneficiaryBalance); - finalBeneficiaryBalance = new BN(finalBeneficiaryBalance); - + assert.strictEqual( finalBeneficiaryBalance.eq(initialBeneficiaryBalance.add(amount)), true, `Beneficiary base token address balance should be ${amount}.`, ); - + let totalSupply = await ostPrime.totalSupply(); assert.strictEqual( totalSupply.eq(amount), true, `Token total supply from contract must be equal to ${amount}.`, ); - + }); - + it('should emit Transfer event', async function () { - + await initialize(); - + let tx = await ostPrime.increaseSupply( beneficiary, amount, - { from: coGatewayAddress } + {from: coGatewayAddress} ); - + let event = EventDecoder.getEvents(tx, ostPrime); - + assert.isDefined( event.Transfer, 'Event Transfer must be emitted.', ); - + let eventData = event.Transfer; - + assert.strictEqual( eventData._from, NullAddress, 'The _from address in the event should be zero.', ); - + assert.strictEqual( eventData._to, ostPrime.address, `The _to address in the event should be equal to ${ostPrime.address}.`, ); - + assert.strictEqual( amount.eq(eventData._value), true, `The _value amount in the event should be equal to ${amount}.`, ); - + }); - + }); diff --git a/test/gateway/ost_prime/initialize.js b/test/gateway/ost_prime/initialize.js index 97d6c524..daf77e16 100644 --- a/test/gateway/ost_prime/initialize.js +++ b/test/gateway/ost_prime/initialize.js @@ -42,8 +42,8 @@ contract('OSTPrime.initialize()', function (accounts) { let balance = await Utils.getBalance(ostPrime.address); assert.strictEqual( - balance, - '0', + balance.eqn(0), + true, `The balance of contract must be zero.`, ); @@ -82,7 +82,7 @@ contract('OSTPrime.initialize()', function (accounts) { {from: accounts[2], value: TOKENS_MAX} ); - let balance = new BN(await Utils.getBalance(ostPrime.address)); + let balance = await Utils.getBalance(ostPrime.address); assert.strictEqual( TOKENS_MAX.eq(balance), true, diff --git a/test/gateway/ost_prime/unwrap.js b/test/gateway/ost_prime/unwrap.js index 98be120d..16968a71 100644 --- a/test/gateway/ost_prime/unwrap.js +++ b/test/gateway/ost_prime/unwrap.js @@ -102,13 +102,9 @@ contract('OSTPrime.unwrap()', function (accounts) { it('should pass with correct parameters', async function () { await initialize(); - let initialContractBalance = new BN( - await Utils.getBalance(ostPrime.address) - ); + let initialContractBalance = await Utils.getBalance(ostPrime.address); - let initialCallerBalance = new BN( - await Utils.getBalance(callerAddress) - ); + let initialCallerBalance = await Utils.getBalance(callerAddress); amount = new BN(400); @@ -136,13 +132,10 @@ contract('OSTPrime.unwrap()', function (accounts) { `The balance of OST prime contract should increase by ${amount}.` ); - let finalContractBalance = new BN( - await Utils.getBalance(ostPrime.address) - ); + let finalContractBalance = await Utils.getBalance(ostPrime.address); + - let finalCallerBalance = new BN( - await Utils.getBalance(callerAddress) - ); + let finalCallerBalance = await Utils.getBalance(callerAddress); assert.strictEqual( finalContractBalance.eq(initialContractBalance.sub(amount)), diff --git a/test/gateway/ost_prime/wrap.js b/test/gateway/ost_prime/wrap.js index 094da1f7..818b64d4 100644 --- a/test/gateway/ost_prime/wrap.js +++ b/test/gateway/ost_prime/wrap.js @@ -114,13 +114,9 @@ contract('OSTPrime.wrap()', function (accounts) { await initialize(); - let initialContractBalance = new BN( - await Utils.getBalance(ostPrime.address) - ); + let initialContractBalance = await Utils.getBalance(ostPrime.address); - let initialCallerBalance = new BN( - await Utils.getBalance(callerAddress) - ); + let initialCallerBalance = await Utils.getBalance(callerAddress); let result = await ostPrime.wrap.call( {from: callerAddress, value: amount} @@ -149,13 +145,9 @@ contract('OSTPrime.wrap()', function (accounts) { `The balance of OST prime contract should be zero.`, ); - let finalContractBalance = new BN( - await Utils.getBalance(ostPrime.address) - ); + let finalContractBalance = await Utils.getBalance(ostPrime.address); - let finalCallerBalance = new BN( - await Utils.getBalance(callerAddress) - ); + let finalCallerBalance = await Utils.getBalance(callerAddress); assert.strictEqual( finalContractBalance.eq(initialContractBalance.add(amount)), diff --git a/test/lib/mutex/acquire.js b/test/lib/mutex/acquire.js index 95521216..2f6dc645 100644 --- a/test/lib/mutex/acquire.js +++ b/test/lib/mutex/acquire.js @@ -20,67 +20,67 @@ const Utils = require('../../test_lib/utils.js'); -const Mutex = artifacts.require('TestMutex'); +const MutexAddress = artifacts.require('TestMutexAddress'); -contract('Mutex.acquire()', async (accounts) => { +contract('MutexAddress.acquire()', async (accounts) => { - it('should acquire lock for an address', async () => { + it('should acquire lock for an address', async () => { - let mutex = await Mutex.new(); + let mutex = await MutexAddress.new(); - let address = accounts[0]; - let result = await mutex.acquireExternal.call(address); + let address = accounts[0]; + let result = await mutex.acquireExternal.call(address); - assert.strictEqual( - result, - true, - 'Lock acquire should success.' - ); - await mutex.acquireExternal(address); + assert.strictEqual( + result, + true, + 'Lock acquire should succeed.' + ); + await mutex.acquireExternal(address); - await Utils.expectRevert( - mutex.acquireExternal(address), - 'Lock already acquired for the address.' - ); - }); + await Utils.expectRevert( + mutex.acquireExternal(address), + 'Lock already acquired for the address.' + ); + }); - it('should not acquire lock for an address if already acquired', async () => { + it('should not acquire lock for an address if already acquired', async () => { - let mutex = await Mutex.new(); + let mutex = await MutexAddress.new(); - let address = accounts[0]; - await mutex.acquireExternal(address); + let address = accounts[0]; + await mutex.acquireExternal(address); - await Utils.expectRevert( - mutex.acquireExternal(address), - 'Lock already acquired for the address.' - ); - }); + await Utils.expectRevert( + mutex.acquireExternal(address), + 'Lock already acquired for the address.' + ); + }); - it('should allow lock acquisition for more than one account', async () => { - let mutex = await Mutex.new(); + it('should allow lock acquisition for more than one account', async () => { + let mutex = await MutexAddress.new(); - let firstAddress = accounts[0]; - let result = await mutex.acquireExternal.call(firstAddress); + let firstAddress = accounts[0]; + let result = await mutex.acquireExternal.call(firstAddress); - assert.strictEqual( - result, - true, - 'Lock acquire should success.' - ); + assert.strictEqual( + result, + true, + 'Lock acquire should succeed.' + ); - await mutex.acquireExternal(firstAddress); + await mutex.acquireExternal(firstAddress); - let secondAddress = accounts[1]; - result = await mutex.acquireExternal.call(secondAddress); + let secondAddress = accounts[1]; + result = await mutex.acquireExternal.call(secondAddress); - assert.strictEqual( - result, - true, - 'Lock acquire should success.' - ); + assert.strictEqual( + result, + true, + 'Lock acquire should succeed.' + ); - await mutex.acquireExternal(secondAddress); - }); + await mutex.acquireExternal(secondAddress); + }); -}); \ No newline at end of file +}); diff --git a/test/lib/mutex/release.js b/test/lib/mutex/release.js index 9ed5c376..febdb34c 100644 --- a/test/lib/mutex/release.js +++ b/test/lib/mutex/release.js @@ -20,54 +20,61 @@ const Utils = require('../../test_lib/utils.js'); -const Mutex = artifacts.require('TestMutex'); +const MutexAddress = artifacts.require('TestMutexAddress'); -contract('Mutex.release()', async (accounts) => { +contract('MutexAddress.release()', async (accounts) => { - it('should release lock for an address', async () => { + it('should release lock for an address', async () => { - let mutex = await Mutex.new(); + let mutex = await MutexAddress.new(); - let address = accounts[0]; - await mutex.acquireExternal(address); + let address = accounts[0]; + await mutex.acquireExternal(address); - let result = await mutex.releaseExternal.call(address); - assert.strictEqual( - result, - true, - 'Lock acquire should success.' - ); - await mutex.releaseExternal(address); + let result = await mutex.releaseExternal.call(address); + assert.strictEqual( + result, + true, + 'Lock acquire should succeed.' + ); + await mutex.releaseExternal(address); - await Utils.expectRevert( - mutex.releaseExternal(address), - 'Lock is not acquired for the address.' - ); - }); + await Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + result = await mutex.acquireExternal.call(address); - it('should not release lock for an address if already released', async () => { + assert.strictEqual( + result, + true, + 'Lock acquire should succeed.' + ); + }); - let mutex = await Mutex.new(); + it('should not release lock for an address if already released', async () => { - let address = accounts[0]; - await mutex.acquireExternal(address); + let mutex = await MutexAddress.new(); - await mutex.releaseExternal(address); + let address = accounts[0]; + await mutex.acquireExternal(address); - Utils.expectRevert( - mutex.releaseExternal(address), - 'Lock is not acquired for the address.' - ); - }); + await mutex.releaseExternal(address); - it('should not release lock if lock is not acquired', async () => { - let mutex = await Mutex.new(); + Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + }); - let address = accounts[0]; + it('should not release lock if lock is not acquired', async () => { + let mutex = await MutexAddress.new(); - await Utils.expectRevert( - mutex.releaseExternal(address), - 'Lock is not acquired for the address.' - ); - }); -}); \ No newline at end of file + let address = accounts[0]; + + await Utils.expectRevert( + mutex.releaseExternal(address), + 'Lock is not acquired for the address.' + ); + }); +}); diff --git a/test/test_lib/utils.js b/test/test_lib/utils.js index 66f34c5b..51577c70 100644 --- a/test/test_lib/utils.js +++ b/test/test_lib/utils.js @@ -191,7 +191,7 @@ Utils.prototype = { if (error) { reject(error); } else { - resolve(result); + resolve(new BN(result)); } }) }) From 97dbd207089d933eaa92a7388ad893f7a0297101 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Thu, 20 Dec 2018 10:50:52 +0530 Subject: [PATCH 6/9] Feedback changes , Rename address -> account , unit test refacotring and contract licence --- contracts/lib/MutexAddress.sol | 29 ++++--- contracts/test/TestMutexAddress.sol | 22 +++--- test/gateway/ost_prime/decrease_supply.js | 16 ++-- test/gateway/ost_prime/increase_supply.js | 92 ++++++++++++----------- test/lib/mutex/acquire.js | 11 +-- test/lib/mutex/release.js | 10 +-- 6 files changed, 99 insertions(+), 81 deletions(-) diff --git a/contracts/lib/MutexAddress.sol b/contracts/lib/MutexAddress.sol index f110e93e..638c2826 100644 --- a/contracts/lib/MutexAddress.sol +++ b/contracts/lib/MutexAddress.sol @@ -13,6 +13,13 @@ pragma solidity ^0.5.0; // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. +// +// ---------------------------------------------------------------------------- +// Origin Chain: Gateway Contract +// +// http://www.simpletoken.org/ +// +// ---------------------------------------------------------------------------- /** @title MutexAddress contract provide locking mechanism. */ contract MutexAddress { @@ -29,44 +36,44 @@ contract MutexAddress { { } /** - * @notice This acquires lock for an address if not already acquired. + * @notice This acquires lock for an account if not already acquired. * This will revert the transaction if lock is already acquired. * - * @param _address Address for which lock acquisition is required. + * @param _account Account for which lock acquisition is required. * * @return isAcquired_ `true` on successful lock acquisition, `false` otherwise. */ - function acquire(address _address) + function acquire(address _account) internal returns(bool isAcquired_) { require( - locks[_address] == false, - "Lock already acquired for the address." + locks[_account] == false, + "Lock already acquired for the account." ); - locks[msg.sender] = true; + locks[_account] = true; isAcquired_ = true; } /** - * @notice This releases lock for an address if lock is acquired. + * @notice This releases lock for an account if lock is acquired. * This will revert the transaction if lock is not acquired. * - * @param _address Address for which release lock is required. + * @param _account Account for which release lock is required. * * @return isReleased_ `true` on successful lock release, `false` otherwise. */ - function release(address _address) + function release(address _account) internal returns(bool isReleased_) { require( - locks[_address] == true, + locks[_account] == true, "Lock is not acquired for the address." ); - locks[_address] = false; + locks[_account] = false; isReleased_ = true; } } diff --git a/contracts/test/TestMutexAddress.sol b/contracts/test/TestMutexAddress.sol index ce4fa4ac..bfd3dfdb 100644 --- a/contracts/test/TestMutexAddress.sol +++ b/contracts/test/TestMutexAddress.sol @@ -30,31 +30,33 @@ contract TestMutexAddress is MutexAddress { public { } - /** @notice This acquires lock for an address if not already acquired. + /** + * @notice This acquires lock for an account if not already acquired. * This will revert the transaction if lock is already acquired. * - * @param _address Address for which lock acquisition is required. + * @param _account Account for which lock acquisition is required. * - * @return success_ `true` on successful lock acquisition, `false` otherwise. + * @return isAcquired_ `true` on successful lock acquisition, `false` otherwise. */ - function acquireExternal(address _address) + function acquireExternal(address _account) external returns(bool success_) { - success_ = super.acquire(_address); + success_ = super.acquire(_account); } - /** @notice This releases lock for an address if lock is acquired. + /** + * @notice This releases lock for an account if lock is acquired. * This will revert the transaction if lock is not acquired. * - * @param _address Address for which release lock is required. + * @param _account Account for which release lock is required. * - * @return success_ `true` on successful lock release, `false` otherwise. + * @return isReleased_ `true` on successful lock release, `false` otherwise. */ - function releaseExternal(address _address) + function releaseExternal(address _account) external returns(bool success_) { - success_ = super.release(_address); + success_ = super.release(_account); } } diff --git a/test/gateway/ost_prime/decrease_supply.js b/test/gateway/ost_prime/decrease_supply.js index 4ed8c34d..c0062d63 100644 --- a/test/gateway/ost_prime/decrease_supply.js +++ b/test/gateway/ost_prime/decrease_supply.js @@ -39,7 +39,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { membersManager, coGatewayAddress; - async function initialize(){ + async function initialize() { await ostPrime.initialize( {from: accounts[2], value: TOKENS_MAX} ); @@ -71,7 +71,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { amount = new BN(0); await Utils.expectRevert( - ostPrime.decreaseSupply(amount, { from: coGatewayAddress }), + ostPrime.decreaseSupply(amount, {from: coGatewayAddress}), 'Amount should be greater than zero.', ); @@ -82,7 +82,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { await initialize(); await Utils.expectRevert( - ostPrime.decreaseSupply(amount, { from: accounts[5] }), + ostPrime.decreaseSupply(amount, {from: accounts[5]}), 'Only CoGateway can call the function.', ); @@ -91,7 +91,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { it('should fail when OST Prime contract is not initialized', async function () { await Utils.expectRevert( - ostPrime.decreaseSupply(amount, { from: coGatewayAddress }), + ostPrime.decreaseSupply(amount, {from: coGatewayAddress}), 'Contract is not initialized.', ); @@ -105,7 +105,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { amount = new BN(2000); await Utils.expectRevert( - ostPrime.decreaseSupply(amount, { from: coGatewayAddress }), + ostPrime.decreaseSupply(amount, {from: coGatewayAddress}), 'Insufficient balance.', ); @@ -118,7 +118,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { amount = new BN(500); let result = await ostPrime.decreaseSupply.call( - amount, { from: coGatewayAddress } + amount, {from: coGatewayAddress} ); assert.strictEqual( @@ -127,7 +127,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { 'Contract should return true.', ); - await ostPrime.decreaseSupply(amount, { from: coGatewayAddress }); + await ostPrime.decreaseSupply(amount, {from: coGatewayAddress}); let coGatewayBalance = await ostPrime.balanceOf.call(coGatewayAddress); @@ -152,7 +152,7 @@ contract('OSTPrime.decreaseSupply()', function (accounts) { let tx = await ostPrime.decreaseSupply( amount, - { from: coGatewayAddress } + {from: coGatewayAddress} ); let event = EventDecoder.getEvents(tx, ostPrime); diff --git a/test/gateway/ost_prime/increase_supply.js b/test/gateway/ost_prime/increase_supply.js index 0fa603f0..4bf20bce 100644 --- a/test/gateway/ost_prime/increase_supply.js +++ b/test/gateway/ost_prime/increase_supply.js @@ -26,12 +26,12 @@ const EventDecoder = require('../../test_lib/event_decoder.js'); const NullAddress = "0x0000000000000000000000000000000000000000"; contract('OSTPrime.increaseSupply()', function (accounts) { - + const DECIMAL = new BN(10); const POW = new BN(18); const DECIMAL_FACTOR = DECIMAL.pow(POW); const TOKENS_MAX = new BN(800000000).mul(DECIMAL_FACTOR); - + let brandedTokenAddress, beneficiary, ostPrime, @@ -39,37 +39,37 @@ contract('OSTPrime.increaseSupply()', function (accounts) { amount, membersManager, coGatewayAddress; - + async function initialize() { await ostPrime.initialize( {from: accounts[2], value: TOKENS_MAX} ); }; - + beforeEach(async function () { - + beneficiary = accounts[6]; membersManager = accounts[0]; brandedTokenAddress = accounts[2]; ostPrime = await OSTPrime.new(brandedTokenAddress, membersManager); - + callerAddress = accounts[3]; amount = new BN(1000); - + await ostPrime.setTokenBalance(callerAddress, amount); - + coGatewayAddress = accounts[1]; - + await ostPrime.setCoGatewayAddress(coGatewayAddress); - + }); - + it('should fail when amount is zero', async function () { - + await initialize(); - + amount = new BN(0); - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, @@ -78,13 +78,13 @@ contract('OSTPrime.increaseSupply()', function (accounts) { ), 'Amount should be greater than zero.', ); - + }); - + it('should fail when caller is not CoGateway address', async function () { - + await initialize(); - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, @@ -93,11 +93,11 @@ contract('OSTPrime.increaseSupply()', function (accounts) { ), 'Only CoGateway can call the function.', ); - + }); - + it('should fail when OST Prime is not initialized', async function () { - + await Utils.expectRevert( ostPrime.increaseSupply( beneficiary, @@ -106,86 +106,94 @@ contract('OSTPrime.increaseSupply()', function (accounts) { ), 'Contract is not initialized.', ); - + }); - + it('should pass with correct params', async function () { - + await initialize(); - + let result = await ostPrime.increaseSupply.call( beneficiary, amount, {from: coGatewayAddress}, ); - + assert.strictEqual( result, true, 'Contract should return true.', ); let initialBeneficiaryBalance = await Utils.getBalance(beneficiary); - + let initialOSTPrimeBalance = await Utils.getBalance(ostPrime.address); + await ostPrime.increaseSupply( beneficiary, amount, {from: coGatewayAddress}, ); - + let finalBeneficiaryBalance = await Utils.getBalance(beneficiary); - + let finalOSTPrimeBalance = await Utils.getBalance(ostPrime.address); + assert.strictEqual( finalBeneficiaryBalance.eq(initialBeneficiaryBalance.add(amount)), true, `Beneficiary base token address balance should be ${amount}.`, ); - + + assert.strictEqual( + finalOSTPrimeBalance.eq(initialOSTPrimeBalance.sub(amount)), + true, + `OST Prime base token balance should be reduced by ${amount}.`, + ); + let totalSupply = await ostPrime.totalSupply(); assert.strictEqual( totalSupply.eq(amount), true, `Token total supply from contract must be equal to ${amount}.`, ); - + }); - + it('should emit Transfer event', async function () { - + await initialize(); - + let tx = await ostPrime.increaseSupply( beneficiary, amount, {from: coGatewayAddress} ); - + let event = EventDecoder.getEvents(tx, ostPrime); - + assert.isDefined( event.Transfer, 'Event Transfer must be emitted.', ); - + let eventData = event.Transfer; - + assert.strictEqual( eventData._from, NullAddress, 'The _from address in the event should be zero.', ); - + assert.strictEqual( eventData._to, ostPrime.address, `The _to address in the event should be equal to ${ostPrime.address}.`, ); - + assert.strictEqual( amount.eq(eventData._value), true, `The _value amount in the event should be equal to ${amount}.`, ); - + }); - + }); diff --git a/test/lib/mutex/acquire.js b/test/lib/mutex/acquire.js index 2f6dc645..e45913c8 100644 --- a/test/lib/mutex/acquire.js +++ b/test/lib/mutex/acquire.js @@ -34,13 +34,14 @@ contract('MutexAddress.acquire()', async (accounts) => { assert.strictEqual( result, true, - 'Lock acquire should succeed.' + 'Lock acquire should succeed.', ); + await mutex.acquireExternal(address); await Utils.expectRevert( mutex.acquireExternal(address), - 'Lock already acquired for the address.' + 'Lock already acquired for the account.', ); }); @@ -53,7 +54,7 @@ contract('MutexAddress.acquire()', async (accounts) => { await Utils.expectRevert( mutex.acquireExternal(address), - 'Lock already acquired for the address.' + 'Lock already acquired for the account.', ); }); @@ -66,7 +67,7 @@ contract('MutexAddress.acquire()', async (accounts) => { assert.strictEqual( result, true, - 'Lock acquire should succeed.' + 'Lock acquire should succeed.', ); await mutex.acquireExternal(firstAddress); @@ -77,7 +78,7 @@ contract('MutexAddress.acquire()', async (accounts) => { assert.strictEqual( result, true, - 'Lock acquire should succeed.' + 'Lock acquire should succeed.', ); await mutex.acquireExternal(secondAddress); diff --git a/test/lib/mutex/release.js b/test/lib/mutex/release.js index febdb34c..781a7ab1 100644 --- a/test/lib/mutex/release.js +++ b/test/lib/mutex/release.js @@ -35,20 +35,20 @@ contract('MutexAddress.release()', async (accounts) => { assert.strictEqual( result, true, - 'Lock acquire should succeed.' + 'Lock acquire should succeed.', ); await mutex.releaseExternal(address); await Utils.expectRevert( mutex.releaseExternal(address), - 'Lock is not acquired for the address.' + 'Lock is not acquired for the address.', ); result = await mutex.acquireExternal.call(address); assert.strictEqual( result, true, - 'Lock acquire should succeed.' + 'Lock acquire should succeed.', ); }); @@ -63,7 +63,7 @@ contract('MutexAddress.release()', async (accounts) => { Utils.expectRevert( mutex.releaseExternal(address), - 'Lock is not acquired for the address.' + 'Lock is not acquired for the address.', ); }); @@ -74,7 +74,7 @@ contract('MutexAddress.release()', async (accounts) => { await Utils.expectRevert( mutex.releaseExternal(address), - 'Lock is not acquired for the address.' + 'Lock is not acquired for the address.', ); }); }); From b2c1582dfacc86df9a5e390ce10097886be1f43a Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Thu, 20 Dec 2018 10:53:32 +0530 Subject: [PATCH 7/9] Corrected licence of MutexAddress contract --- contracts/lib/MutexAddress.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/lib/MutexAddress.sol b/contracts/lib/MutexAddress.sol index 638c2826..0757e8da 100644 --- a/contracts/lib/MutexAddress.sol +++ b/contracts/lib/MutexAddress.sol @@ -15,7 +15,6 @@ pragma solidity ^0.5.0; // limitations under the License. // // ---------------------------------------------------------------------------- -// Origin Chain: Gateway Contract // // http://www.simpletoken.org/ // From 3f9d1f886e9612856f86539b10ea8ed6faa9b861 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Fri, 21 Dec 2018 14:58:53 +0530 Subject: [PATCH 8/9] base token should not be transferred to zero account --- contracts/gateway/OSTPrime.sol | 5 +++++ test/gateway/ost_prime/increase_supply.js | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/contracts/gateway/OSTPrime.sol b/contracts/gateway/OSTPrime.sol index b8fd0246..edfebcc0 100644 --- a/contracts/gateway/OSTPrime.sol +++ b/contracts/gateway/OSTPrime.sol @@ -241,6 +241,11 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig, MutexAddress { onlyCoGateway returns (bool success_) { + require( + _account != address(0), + "Account address should not be zero." + ); + /* * Acquire lock for msg.sender so that this function can only be * executed once in a transaction. diff --git a/test/gateway/ost_prime/increase_supply.js b/test/gateway/ost_prime/increase_supply.js index 4bf20bce..79611cbd 100644 --- a/test/gateway/ost_prime/increase_supply.js +++ b/test/gateway/ost_prime/increase_supply.js @@ -64,6 +64,23 @@ contract('OSTPrime.increaseSupply()', function (accounts) { }); + it('should fail when account address is zero', async function () { + + await initialize(); + + beneficiary = NullAddress; + + await Utils.expectRevert( + ostPrime.increaseSupply( + beneficiary, + amount, + { from: coGatewayAddress }, + ), + 'Account address should not be zero.', + ); + + }); + it('should fail when amount is zero', async function () { await initialize(); From 89154762875bc214227f5cd5175c74a084420258 Mon Sep 17 00:00:00 2001 From: Sarvesh Jain Date: Fri, 21 Dec 2018 15:33:57 +0530 Subject: [PATCH 9/9] Added documentation of increase supply --- contracts/gateway/OSTPrime.sol | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/contracts/gateway/OSTPrime.sol b/contracts/gateway/OSTPrime.sol index edfebcc0..782f1cca 100644 --- a/contracts/gateway/OSTPrime.sol +++ b/contracts/gateway/OSTPrime.sol @@ -218,11 +218,14 @@ contract OSTPrime is UtilityToken, OSTPrimeConfig, MutexAddress { } /** - * @notice Adds number of OST Prime tokens to account balance and increases - * the total token supply. Can be called only when contract is - * initialized and only by CoGateway address. + * @notice This method performs following operations: + * - Adds number of OST Prime EIP20 tokens to this contract address. + * - Increases the total token supply. + * - Transfers base token to the beneficiary address. + * It can be called by CoGateway address and when contract is + * initialized. * - * @dev The parameters _account and _amount should not be zero. This check + * @dev The parameter _amount should not be zero. This check * is added in function increaseSupplyInternal. * * @param _account Account address for which the OST Prime balance will be