Skip to content

FixReputationAllocation - prevent redeem twice for the same beneficiary . #571

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.

Sign up for GitHub

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

Merged
orenyodfat merged 6 commits into from
Dec 23, 2018
Merged

FixReputationAllocation - prevent redeem twice for the same beneficiary . #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
45a0e67
FixReputationAllocation - cannot redeem twice
orenyodfat Dec 22, 2018
cbcd97a
Use safe math addition at locking4rep
orenyodfat Dec 22, 2018
71e06e1
use safe math for lockingsCounter increment
orenyodfat Dec 22, 2018
07fcd95
Auction4Reputation : _reputationReward -> _auctionReputationReward
orenyodfat Dec 22, 2018
c6a6137
replace _auctionEndTime with auctionPeriod
orenyodfat Dec 22, 2018
e72eec9
_auctionPeriod should be greater than block interval
orenyodfat Dec 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions contracts/schemes/Auction4Reputation.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ 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.
* 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.
Expand All @@ -54,9 +55,9 @@ contract Auction4Reputation is Ownable {
*/
function initialize(
Avatar _avatar,
uint _reputationReward,
uint _auctionReputationReward,
uint _auctionsStartTime,
uint _auctionsEndTime,
uint _auctionPeriod,
uint _numberOfAuctions,
uint _redeemEnableTime,
StandardToken _token,
Expand All @@ -66,19 +67,19 @@ 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");
//_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");
token = _token;
avatar = _avatar;
auctionsStartTime = _auctionsStartTime;
auctionsEndTime = _auctionsEndTime;
numberOfAuctions = _numberOfAuctions;
wallet = _wallet;
auctionReputationReward = _reputationReward / _numberOfAuctions;
reputationRewardLeft = _reputationReward;
auctionReputationReward = _auctionReputationReward;
reputationRewardLeft = _auctionReputationReward.mul(_numberOfAuctions);
redeemEnableTime = _redeemEnableTime;
}

Expand Down
1 change: 1 addition & 0 deletions contracts/schemes/FixedReputationAllocation.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
4 changes: 2 additions & 2 deletions contracts/schemes/Locking4Reputation.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ 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;
// 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");
Expand Down
59 changes: 39 additions & 20 deletions test/auction4reputation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
_repAllocation,
_auctionReputationReward,
testSetup.auctionsStartTime,
testSetup.auctionsEndTime,
testSetup.auctionPeriod,
_numberOfAuctions,
testSetup.redeemEnableTime,
testSetup.biddingToken.address,
Expand All @@ -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 () => {
Expand All @@ -60,7 +61,7 @@ contract('Auction4Reputation', accounts => {
await auction4Reputation.initialize(accounts[0],
300,
0,
3000,
1000,
0,
3000,
accounts[0],
Expand All @@ -72,15 +73,15 @@ contract('Auction4Reputation', accounts => {
}
});

it("initialize _redeemEnableTime < _auctionsEndTime is not allowed", async () => {
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,
1,
3000-1,
accounts[0],
accounts[0],
{gas :constants.ARC_GAS_LIMIT});
Expand All @@ -90,15 +91,33 @@ contract('Auction4Reputation', accounts => {
}
});

it("initialize _redeemEnableTime < auctionsEndTime is not allowed", async () => {
var auction4Reputation = await Auction4Reputation.new();
try {
await auction4Reputation.initialize(accounts[0],
100,
0,
1000,
1,
1000-1,
accounts[0],
accounts[0],
{gas :constants.ARC_GAS_LIMIT});
assert(false, "_redeemEnableTime < auctionsEndTime is not allowed");
} catch(error) {
helpers.assertVMException(error);
}
});

it("initialize is onlyOwner", async () => {
var auction4Reputation = await Auction4Reputation.new();
try {
await auction4Reputation.initialize(accounts[0],
300,
100,
0,
3000,
1000,
1,
3000,
1000,
accounts[0],
accounts[0],
{gas :constants.ARC_GAS_LIMIT,from:accounts[1]});
Expand All @@ -107,11 +126,11 @@ contract('Auction4Reputation', accounts => {
helpers.assertVMException(error);
}
await auction4Reputation.initialize(accounts[0],
300,
100,
0,
3000,
1000,
1,
3000,
1000,
accounts[0],
accounts[0],
{gas :constants.ARC_GAS_LIMIT,from:accounts[0]});
Expand All @@ -121,7 +140,7 @@ contract('Auction4Reputation', accounts => {
var auction4Reputation = await Auction4Reputation.new();
try {
await auction4Reputation.initialize(accounts[0],
300,
100,
300,
300,
1,
Expand All @@ -138,7 +157,7 @@ contract('Auction4Reputation', accounts => {
var auction4Reputation = await Auction4Reputation.new();
try {
await auction4Reputation.initialize(accounts[0],
300,
100,
200,
100,
1,
Expand Down Expand Up @@ -182,7 +201,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");
Expand Down Expand Up @@ -270,7 +289,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);
Expand Down Expand Up @@ -325,7 +344,7 @@ contract('Auction4Reputation', accounts => {
let testSetup = await setup(accounts);
try {
await testSetup.auction4Reputation.initialize(accounts[0],
300,
100,
200,
100,
1,
Expand Down
29 changes: 29 additions & 0 deletions test/fixreputationallocation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down