From 45a0e6781b18f61f3ce2027c355187a0790a098f Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 17:09:10 +0200 Subject: [PATCH 1/6] FixReputationAllocation - cannot redeem twice --- .../schemes/FixedReputationAllocation.sol | 1 + test/fixreputationallocation.js | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/contracts/schemes/FixedReputationAllocation.sol b/contracts/schemes/FixedReputationAllocation.sol index 15b7167c..7d0995a0 100644 --- a/contracts/schemes/FixedReputationAllocation.sol +++ b/contracts/schemes/FixedReputationAllocation.sol @@ -50,6 +50,7 @@ contract FixedReputationAllocation is Ownable { function redeem(address _beneficiary) public returns(bool) { require(isEnable, "require to be enable"); require(beneficiaries[_beneficiary], "require _beneficiary to exist in the beneficiaries map"); + beneficiaries[_beneficiary] = false; // solium-disable-next-line security/no-block-members require(now > redeemEnableTime, "require now > redeemEnableTime"); require(ControllerInterface(avatar.owner()).mintReputation(beneficiaryReward, _beneficiary, avatar), "mint reputation should success"); diff --git a/test/fixreputationallocation.js b/test/fixreputationallocation.js index 6cdeb5de..36756649 100644 --- a/test/fixreputationallocation.js +++ b/test/fixreputationallocation.js @@ -75,6 +75,35 @@ contract('FixedReputationAllocation', accounts => { reputation = await testSetup.org.reputation.balanceOf(accounts[i]); assert.equal(reputation.toNumber(),tx.logs[0].args._amount); } + + }); + + it("cannot redeem twice", async () => { + let testSetup = await setup(accounts); + let tx = await testSetup.fixedReputationAllocation.addBeneficiaries(accounts); + assert.equal(await testSetup.fixedReputationAllocation.numberOfBeneficiaries(),accounts.length); + assert.equal(await testSetup.fixedReputationAllocation.beneficiaryReward(),0); + await testSetup.fixedReputationAllocation.enable(); + assert.equal(await testSetup.fixedReputationAllocation.beneficiaryReward(),300/accounts.length); + var beneficiaryReward; + var reputation; + await helpers.increaseTime(3001); + for (var i = 0 ;i< accounts.length ;i++) { + tx = await testSetup.fixedReputationAllocation.redeem(accounts[i]); + assert.equal(tx.logs.length,1); + assert.equal(tx.logs[0].event,"Redeem"); + beneficiaryReward = await testSetup.fixedReputationAllocation.beneficiaryReward(); + assert.equal(tx.logs[0].args._amount,beneficiaryReward.toNumber()); + reputation = await testSetup.org.reputation.balanceOf(accounts[i]); + assert.equal(reputation.toNumber(),tx.logs[0].args._amount); + } + + try { + await testSetup.fixedReputationAllocation.redeem(accounts[0]); + assert(false, "cannot redeem twice"); + } catch(error) { + helpers.assertVMException(error); + } }); it("cannot redeem if not initialize", async () => { From cbcd97a9375a0307228ae04019d0e1a1df67222c Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 17:20:50 +0200 Subject: [PATCH 2/6] Use safe math addition at locking4rep --- contracts/schemes/Locking4Reputation.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/schemes/Locking4Reputation.sol b/contracts/schemes/Locking4Reputation.sol index 297a0bda..ab6fd509 100644 --- a/contracts/schemes/Locking4Reputation.sol +++ b/contracts/schemes/Locking4Reputation.sol @@ -104,7 +104,7 @@ contract Locking4Reputation { locker.amount = _amount; // solium-disable-next-line security/no-block-members locker.releaseTime = now + _period; - totalLocked += _amount; + totalLocked = totalLocked.add(_amount); totalLockedLeft = totalLocked; uint score = _period.mul(_amount).mul(_numerator).div(_denominator); require(score>0,"score must me > 0"); From 71e06e139a460651c4c43362ecb762006ec2d77f Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 17:28:13 +0200 Subject: [PATCH 3/6] use safe math for lockingsCounter increment --- contracts/schemes/Locking4Reputation.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/schemes/Locking4Reputation.sol b/contracts/schemes/Locking4Reputation.sol index ab6fd509..e73a5721 100644 --- a/contracts/schemes/Locking4Reputation.sol +++ b/contracts/schemes/Locking4Reputation.sol @@ -98,7 +98,7 @@ contract Locking4Reputation { require(now >= lockingStartTime, "lock should start after lockingStartTime"); lockingId = keccak256(abi.encodePacked(this, lockingsCounter)); - lockingsCounter++; + lockingsCounter = lockingsCounter.add(1); Locker storage locker = lockers[_locker][lockingId]; locker.amount = _amount; From 07fcd95eb83f5db4f05a8dcdffb9906de1804724 Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 17:48:14 +0200 Subject: [PATCH 4/6] Auction4Reputation : _reputationReward -> _auctionReputationReward --- contracts/schemes/Auction4Reputation.sol | 8 ++++---- test/auction4reputation.js | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/contracts/schemes/Auction4Reputation.sol b/contracts/schemes/Auction4Reputation.sol index bee36b62..77e9cf4e 100644 --- a/contracts/schemes/Auction4Reputation.sol +++ b/contracts/schemes/Auction4Reputation.sol @@ -41,7 +41,7 @@ contract Auction4Reputation is Ownable { /** * @dev initialize * @param _avatar the avatar to mint reputation from - * @param _reputationReward the total reputation this contract will reward + * @param _auctionReputationReward the reputation reward per auction this contract will reward * for the token locking * @param _auctionsStartTime auctions period start time * @param _auctionsEndTime auctions period end time. @@ -54,7 +54,7 @@ contract Auction4Reputation is Ownable { */ function initialize( Avatar _avatar, - uint _reputationReward, + uint _auctionReputationReward, uint _auctionsStartTime, uint _auctionsEndTime, uint _numberOfAuctions, @@ -77,8 +77,8 @@ contract Auction4Reputation is Ownable { auctionsEndTime = _auctionsEndTime; numberOfAuctions = _numberOfAuctions; wallet = _wallet; - auctionReputationReward = _reputationReward / _numberOfAuctions; - reputationRewardLeft = _reputationReward; + auctionReputationReward = _auctionReputationReward; + reputationRewardLeft = _auctionReputationReward.mul(_numberOfAuctions); redeemEnableTime = _redeemEnableTime; } diff --git a/test/auction4reputation.js b/test/auction4reputation.js index 2d2a7b37..7c446987 100644 --- a/test/auction4reputation.js +++ b/test/auction4reputation.js @@ -7,7 +7,7 @@ var Auction4Reputation = artifacts.require("./Auction4Reputation.sol"); const setup = async function (accounts, - _repAllocation = 300, + _auctionReputationReward = 100, _auctionsStartTime = 0, _auctionsEndTime = 3000, _numberOfAuctions = 3, @@ -24,7 +24,7 @@ const setup = async function (accounts, testSetup.auction4Reputation = await Auction4Reputation.new(); if (_initialize === true ) { await testSetup.auction4Reputation.initialize(testSetup.org.avatar.address, - _repAllocation, + _auctionReputationReward, testSetup.auctionsStartTime, testSetup.auctionsEndTime, _numberOfAuctions, @@ -76,7 +76,7 @@ contract('Auction4Reputation', accounts => { var auction4Reputation = await Auction4Reputation.new(); try { await auction4Reputation.initialize(accounts[0], - 300, + 100, 0, 3000, 1, @@ -94,7 +94,7 @@ contract('Auction4Reputation', accounts => { var auction4Reputation = await Auction4Reputation.new(); try { await auction4Reputation.initialize(accounts[0], - 300, + 100, 0, 3000, 1, @@ -107,7 +107,7 @@ contract('Auction4Reputation', accounts => { helpers.assertVMException(error); } await auction4Reputation.initialize(accounts[0], - 300, + 100, 0, 3000, 1, @@ -121,7 +121,7 @@ contract('Auction4Reputation', accounts => { var auction4Reputation = await Auction4Reputation.new(); try { await auction4Reputation.initialize(accounts[0], - 300, + 100, 300, 300, 1, @@ -138,7 +138,7 @@ contract('Auction4Reputation', accounts => { var auction4Reputation = await Auction4Reputation.new(); try { await auction4Reputation.initialize(accounts[0], - 300, + 100, 200, 100, 1, @@ -182,7 +182,7 @@ contract('Auction4Reputation', accounts => { }); it("bid without initialize should fail", async () => { - let testSetup = await setup(accounts,300,0,3000,3,3000,false); + let testSetup = await setup(accounts,100,0,3000,3,3000,false); try { await testSetup.auction4Reputation.bid(web3.utils.toWei('1', "ether")); assert(false, "bid without initialize should fail"); @@ -270,7 +270,7 @@ contract('Auction4Reputation', accounts => { }); it("redeem before redeemEnableTime should revert", async () => { - let testSetup = await setup(accounts,300,0,3000,3,4000,true); + let testSetup = await setup(accounts,100,0,3000,3,4000,true); var tx = await testSetup.auction4Reputation.bid(web3.utils.toWei('1', "ether")); var id = await helpers.getValueFromLogs(tx, '_auctionId',1); await helpers.increaseTime(3500); @@ -325,7 +325,7 @@ contract('Auction4Reputation', accounts => { let testSetup = await setup(accounts); try { await testSetup.auction4Reputation.initialize(accounts[0], - 300, + 100, 200, 100, 1, From c6a613748fefaca35c90e79f5a7060c1f502f256 Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 18:14:13 +0200 Subject: [PATCH 5/6] replace _auctionEndTime with auctionPeriod --- contracts/schemes/Auction4Reputation.sol | 18 +++++++++--------- test/auction4reputation.js | 7 ++++--- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/contracts/schemes/Auction4Reputation.sol b/contracts/schemes/Auction4Reputation.sol index 77e9cf4e..765b3a6b 100644 --- a/contracts/schemes/Auction4Reputation.sol +++ b/contracts/schemes/Auction4Reputation.sol @@ -44,8 +44,9 @@ contract Auction4Reputation is Ownable { * @param _auctionReputationReward the reputation reward per auction this contract will reward * for the token locking * @param _auctionsStartTime auctions period start time - * @param _auctionsEndTime auctions period end time. - * bidding is disable after this time. + * @param _auctionPeriod auctions period time. + * auctionsEndTime is set to _auctionsStartTime + _auctionPeriod*_numberOfAuctions + * bidding is disable after auctionsEndTime. * @param _numberOfAuctions number of auctions. * @param _redeemEnableTime redeem enable time . * redeem reputation can be done after this time. @@ -56,7 +57,7 @@ contract Auction4Reputation is Ownable { Avatar _avatar, uint _auctionReputationReward, uint _auctionsStartTime, - uint _auctionsEndTime, + uint _auctionPeriod, uint _numberOfAuctions, uint _redeemEnableTime, StandardToken _token, @@ -66,15 +67,14 @@ contract Auction4Reputation is Ownable { { require(avatar == Avatar(0), "can be called only one time"); require(_avatar != Avatar(0), "avatar cannot be zero"); - require(_redeemEnableTime >= _auctionsEndTime, "_redeemEnableTime >= _auctionsEndTime"); - // number of auctions cannot be zero - // auctionsEndTime should be greater than auctionsStartTime - auctionPeriod = (_auctionsEndTime.sub(_auctionsStartTime)).div(_numberOfAuctions); - require(auctionPeriod > 0, "auctionPeriod should be > 0"); + require(_numberOfAuctions > 0, "number of auctions cannot be zero"); + require(_auctionPeriod > 0, "auctionPeriod should be > 0"); + auctionPeriod = _auctionPeriod; + auctionsEndTime = _auctionsStartTime + _auctionPeriod.mul(_numberOfAuctions); + require(_redeemEnableTime >= auctionsEndTime, "_redeemEnableTime >= auctionsEndTime"); token = _token; avatar = _avatar; auctionsStartTime = _auctionsStartTime; - auctionsEndTime = _auctionsEndTime; numberOfAuctions = _numberOfAuctions; wallet = _wallet; auctionReputationReward = _auctionReputationReward; diff --git a/test/auction4reputation.js b/test/auction4reputation.js index 7c446987..6dcbe95e 100644 --- a/test/auction4reputation.js +++ b/test/auction4reputation.js @@ -22,11 +22,12 @@ const setup = async function (accounts, testSetup.auctionsStartTime = (await web3.eth.getBlock("latest")).timestamp + _auctionsStartTime; testSetup.redeemEnableTime = (await web3.eth.getBlock("latest")).timestamp + _redeemEnableTime; testSetup.auction4Reputation = await Auction4Reputation.new(); + testSetup.auctionPeriod = (testSetup.auctionsEndTime - testSetup.auctionsStartTime)/3; if (_initialize === true ) { await testSetup.auction4Reputation.initialize(testSetup.org.avatar.address, _auctionReputationReward, testSetup.auctionsStartTime, - testSetup.auctionsEndTime, + testSetup.auctionPeriod, _numberOfAuctions, testSetup.redeemEnableTime, testSetup.biddingToken.address, @@ -45,13 +46,13 @@ contract('Auction4Reputation', accounts => { let testSetup = await setup(accounts); assert.equal(await testSetup.auction4Reputation.reputationRewardLeft(),300); - assert.equal(await testSetup.auction4Reputation.auctionsEndTime(),testSetup.auctionsEndTime); + assert.equal(await testSetup.auction4Reputation.auctionsEndTime(),testSetup.auctionsStartTime + testSetup.auctionPeriod*3); assert.equal(await testSetup.auction4Reputation.auctionsStartTime(),testSetup.auctionsStartTime); assert.equal(await testSetup.auction4Reputation.redeemEnableTime(),testSetup.redeemEnableTime); assert.equal(await testSetup.auction4Reputation.token(),testSetup.biddingToken.address); assert.equal(await testSetup.auction4Reputation.numberOfAuctions(),3); assert.equal(await testSetup.auction4Reputation.wallet(),testSetup.org.avatar.address); - assert.equal(await testSetup.auction4Reputation.auctionPeriod(),(testSetup.auctionsEndTime-testSetup.auctionsStartTime)/3); + assert.equal(await testSetup.auction4Reputation.auctionPeriod(),testSetup.auctionPeriod); }); it("initialize numberOfAuctions = 0 is not allowed", async () => { From e72eec9459d3731fd6859f0591e7e07056587926 Mon Sep 17 00:00:00 2001 From: Oren Date: Sat, 22 Dec 2018 18:30:17 +0200 Subject: [PATCH 6/6] _auctionPeriod should be greater than block interval --- contracts/schemes/Auction4Reputation.sol | 3 ++- test/auction4reputation.js | 34 ++++++++++++++++++------ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/contracts/schemes/Auction4Reputation.sol b/contracts/schemes/Auction4Reputation.sol index 765b3a6b..204fe4e0 100644 --- a/contracts/schemes/Auction4Reputation.sol +++ b/contracts/schemes/Auction4Reputation.sol @@ -68,7 +68,8 @@ contract Auction4Reputation is Ownable { require(avatar == Avatar(0), "can be called only one time"); require(_avatar != Avatar(0), "avatar cannot be zero"); require(_numberOfAuctions > 0, "number of auctions cannot be zero"); - require(_auctionPeriod > 0, "auctionPeriod should be > 0"); + //_auctionPeriod should be greater than block interval + require(_auctionPeriod > 15, "auctionPeriod should be > 15"); auctionPeriod = _auctionPeriod; auctionsEndTime = _auctionsStartTime + _auctionPeriod.mul(_numberOfAuctions); require(_redeemEnableTime >= auctionsEndTime, "_redeemEnableTime >= auctionsEndTime"); diff --git a/test/auction4reputation.js b/test/auction4reputation.js index 6dcbe95e..9730d941 100644 --- a/test/auction4reputation.js +++ b/test/auction4reputation.js @@ -61,8 +61,26 @@ contract('Auction4Reputation', accounts => { await auction4Reputation.initialize(accounts[0], 300, 0, + 1000, + 0, 3000, + accounts[0], + accounts[0], + {gas :constants.ARC_GAS_LIMIT}); + assert(false, "numberOfAuctions = 0 is not allowed"); + } catch(error) { + helpers.assertVMException(error); + } + }); + + it("initialize auctionPeriod <= 15 seconds is not allowed", async () => { + var auction4Reputation = await Auction4Reputation.new(); + try { + await auction4Reputation.initialize(accounts[0], + 300, 0, + 15, + 3, 3000, accounts[0], accounts[0], @@ -73,19 +91,19 @@ contract('Auction4Reputation', accounts => { } }); - it("initialize _redeemEnableTime < _auctionsEndTime is not allowed", async () => { + it("initialize _redeemEnableTime < auctionsEndTime is not allowed", async () => { var auction4Reputation = await Auction4Reputation.new(); try { await auction4Reputation.initialize(accounts[0], 100, 0, - 3000, + 1000, 1, - 3000-1, + 1000-1, accounts[0], accounts[0], {gas :constants.ARC_GAS_LIMIT}); - assert(false, "numberOfAuctions = 0 is not allowed"); + assert(false, "_redeemEnableTime < auctionsEndTime is not allowed"); } catch(error) { helpers.assertVMException(error); } @@ -97,9 +115,9 @@ contract('Auction4Reputation', accounts => { await auction4Reputation.initialize(accounts[0], 100, 0, - 3000, + 1000, 1, - 3000, + 1000, accounts[0], accounts[0], {gas :constants.ARC_GAS_LIMIT,from:accounts[1]}); @@ -110,9 +128,9 @@ contract('Auction4Reputation', accounts => { await auction4Reputation.initialize(accounts[0], 100, 0, - 3000, + 1000, 1, - 3000, + 1000, accounts[0], accounts[0], {gas :constants.ARC_GAS_LIMIT,from:accounts[0]});