Skip to content

Commit

Permalink
Merge pull request #130 from skalenetwork/enhancement/SKALE-2347-roun…
Browse files Browse the repository at this point in the history
…ding-errors

Enhancement/skale 2347 rounding errors
  • Loading branch information
DimaStebaev authored Apr 8, 2020
2 parents b02ced5 + 78b744d commit c703297
Show file tree
Hide file tree
Showing 66 changed files with 427 additions and 230 deletions.
2 changes: 1 addition & 1 deletion contracts/ContractManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pragma solidity 0.5.16;

import "@openzeppelin/contracts-ethereum-package/contracts/ownership/Ownable.sol";
import "@openzeppelin/upgrades/contracts/Initializable.sol";
import "./StringUtils.sol";
import "./utils/StringUtils.sol";


/**
Expand Down
1 change: 0 additions & 1 deletion contracts/SkaleManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ contract SkaleManager is IERC777Recipient, Permissions {

function getBounty(uint nodeIndex) external {
address nodesDataAddress = contractManager.getContract("NodesData");
ValidatorService validatorService = ValidatorService(contractManager.getContract("ValidatorService"));

require(INodesData(nodesDataAddress).isNodeExist(msg.sender, nodeIndex), "Node does not exist for Message sender");
require(INodesData(nodesDataAddress).isTimeForReward(nodeIndex), "Not time for bounty");
Expand Down
44 changes: 13 additions & 31 deletions contracts/delegation/DelegationController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import "@openzeppelin/contracts/math/SafeMath.sol";

import "../Permissions.sol";
import "../SkaleToken.sol";
import "../utils/MathUtils.sol";

import "./DelegationPeriodManager.sol";
import "./Punisher.sol";
Expand All @@ -34,6 +35,7 @@ import "./ValidatorService.sol";


contract DelegationController is Permissions, ILocker {
using MathUtils for uint;

enum State {
PROPOSED,
Expand Down Expand Up @@ -655,7 +657,7 @@ contract DelegationController is Permissions, ILocker {

if (sequence.firstUnprocessedMonth <= month) {
for (uint i = sequence.firstUnprocessedMonth; i <= month; ++i) {
sequence.value[i] = sequence.value[i - 1].add(sequence.addDiff[i]).sub(sequence.subtractDiff[i]);
sequence.value[i] = sequence.value[i - 1].add(sequence.addDiff[i]).boundedSub(sequence.subtractDiff[i]);
delete sequence.addDiff[i];
delete sequence.subtractDiff[i];
}
Expand All @@ -665,26 +667,6 @@ contract DelegationController is Permissions, ILocker {
return sequence.value[month];
}

function getValue(PartialDifferences storage sequence, uint month) internal view returns (uint) {
if (sequence.firstUnprocessedMonth == 0) {
return 0;
}
if (sequence.firstUnprocessedMonth <= month) {
uint value = sequence.value[sequence.firstUnprocessedMonth - 1];
for (uint i = sequence.firstUnprocessedMonth; i <= month; ++i) {
value = value.add(sequence.addDiff[i]).sub(sequence.subtractDiff[i]);
}
return value;
} else {
return sequence.value[month];
}
}

function getAndUpdateValueBeforeMonthAndGetAtMonth(PartialDifferences storage sequence, uint month) internal returns (uint) {
getAndUpdateValue(sequence, month.sub(1));
return getValue(sequence, month);
}

function add(PartialDifferencesValue storage sequence, uint diff, uint month) internal {
require(sequence.firstUnprocessedMonth <= month, "Cannot add to the past");
if (sequence.firstUnprocessedMonth == 0) {
Expand Down Expand Up @@ -715,7 +697,7 @@ contract DelegationController is Permissions, ILocker {
if (month >= sequence.firstUnprocessedMonth) {
sequence.subtractDiff[month] = sequence.subtractDiff[month].add(diff);
} else {
sequence.value = sequence.value.sub(diff);
sequence.value = sequence.value.boundedSub(diff);
}
}

Expand All @@ -727,7 +709,7 @@ contract DelegationController is Permissions, ILocker {

if (sequence.firstUnprocessedMonth <= month) {
for (uint i = sequence.firstUnprocessedMonth; i <= month; ++i) {
sequence.value = sequence.value.add(sequence.addDiff[i]).sub(sequence.subtractDiff[i]);
sequence.value = sequence.value.add(sequence.addDiff[i]).boundedSub(sequence.subtractDiff[i]);
delete sequence.addDiff[i];
delete sequence.subtractDiff[i];
}
Expand All @@ -743,7 +725,7 @@ contract DelegationController is Permissions, ILocker {
return createFraction(0);
}
uint value = getAndUpdateValue(sequence, month);
if (value == 0) {
if (value.approximatelyEqual(0)) {
return createFraction(0);
}

Expand All @@ -752,7 +734,7 @@ contract DelegationController is Permissions, ILocker {
_amount = value;
}

Fraction memory reducingCoefficient = createFraction(value.sub(_amount), value);
Fraction memory reducingCoefficient = createFraction(value.boundedSub(_amount), value);
reduce(sequence, reducingCoefficient, month);
return reducingCoefficient;
}
Expand Down Expand Up @@ -796,20 +778,20 @@ contract DelegationController is Permissions, ILocker {
return;
}
uint value = getAndUpdateValue(sequence, month);
if (value == 0) {
if (value.approximatelyEqual(0)) {
return;
}

uint newValue = sequence.value.mul(reducingCoefficient.numerator).div(reducingCoefficient.denominator);
if (hasSumSequence) {
subtract(sumSequence, sequence.value.sub(newValue), month);
subtract(sumSequence, sequence.value.boundedSub(newValue), month);
}
sequence.value = newValue;

for (uint i = month.add(1); i <= sequence.lastChangedMonth; ++i) {
uint newDiff = sequence.subtractDiff[i].mul(reducingCoefficient.numerator).div(reducingCoefficient.denominator);
if (hasSumSequence) {
sumSequence.subtractDiff[i] = sumSequence.subtractDiff[i].sub(sequence.subtractDiff[i].sub(newDiff));
sumSequence.subtractDiff[i] = sumSequence.subtractDiff[i].boundedSub(sequence.subtractDiff[i].boundedSub(newDiff));
}
sequence.subtractDiff[i] = newDiff;
}
Expand All @@ -826,7 +808,7 @@ contract DelegationController is Permissions, ILocker {
return;
}
uint value = getAndUpdateValue(sequence, month);
if (value == 0) {
if (value.approximatelyEqual(0)) {
return;
}

Expand Down Expand Up @@ -907,7 +889,7 @@ contract DelegationController is Permissions, ILocker {
uint validatorId = _slashes[index].validatorId;
uint month = _slashes[index].month;
uint oldValue = getAndUpdateDelegatedByHolderToValidator(holder, validatorId, month);
if (oldValue > 0) {
if (oldValue.muchGreater(0)) {
reduce(
_delegatedByHolderToValidator[holder][validatorId],
_delegatedByHolder[holder],
Expand All @@ -918,7 +900,7 @@ contract DelegationController is Permissions, ILocker {
_slashes[index].reducingCoefficient,
month);
slashingSignals[index.sub(begin)].holder = holder;
slashingSignals[index.sub(begin)].penalty = oldValue.sub(getAndUpdateDelegatedByHolderToValidator(holder, validatorId, month));
slashingSignals[index.sub(begin)].penalty = oldValue.boundedSub(getAndUpdateDelegatedByHolderToValidator(holder, validatorId, month));
}
}
_firstUnprocessedSlashByHolder[holder] = end;
Expand Down
5 changes: 4 additions & 1 deletion contracts/delegation/Distributor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import "@openzeppelin/contracts/math/SafeMath.sol";
import "../Permissions.sol";
import "../SkaleToken.sol";
import "../ConstantsHolder.sol";
import "../utils/MathUtils.sol";

import "./ValidatorService.sol";
import "./DelegationController.sol";
Expand All @@ -35,6 +36,8 @@ import "./TimeHelpers.sol";


contract Distributor is Permissions, IERC777Recipient {
using MathUtils for uint;

event WithdrawBounty(
address holder,
uint validatorId,
Expand Down Expand Up @@ -164,7 +167,7 @@ contract Distributor is Permissions, IERC777Recipient {
}
for (uint i = startMonth; i < endMonth; ++i) {
uint effectiveDelegatedToValidator = delegationController.getAndUpdateEffectiveDelegatedToValidator(validatorId, i);
if (effectiveDelegatedToValidator > 0) {
if (effectiveDelegatedToValidator.muchGreater(0)) {
earned = earned.add(
_bountyPaid[validatorId][i].mul(
delegationController.getAndUpdateEffectiveDelegatedByHolderToValidator(wallet, validatorId, i)).div(
Expand Down
11 changes: 7 additions & 4 deletions contracts/delegation/TokenLaunchLocker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,15 @@ import "@openzeppelin/contracts/math/SafeMath.sol";
import "../Permissions.sol";
import "../interfaces/delegation/ILocker.sol";
import "../ConstantsHolder.sol";
import "../utils/MathUtils.sol";

import "./DelegationController.sol";
import "./TimeHelpers.sol";


contract TokenLaunchLocker is Permissions, ILocker {
using MathUtils for uint;

event Unlocked(
address holder,
uint amount
Expand Down Expand Up @@ -82,7 +85,7 @@ contract TokenLaunchLocker is Permissions, ILocker {

uint currentMonth = timeHelpers.getCurrentMonth();
uint fromLocked = amount;
uint locked = _locked[holder].sub(getAndUpdateDelegatedAmount(holder, currentMonth));
uint locked = _locked[holder].boundedSub(getAndUpdateDelegatedAmount(holder, currentMonth));
if (fromLocked > locked) {
fromLocked = locked;
}
Expand Down Expand Up @@ -118,7 +121,7 @@ contract TokenLaunchLocker is Permissions, ILocker {
} else {
uint lockedByDelegationController = getAndUpdateDelegatedAmount(wallet, currentMonth).add(delegationController.getLockedInPendingDelegations(wallet));
if (_locked[wallet] > lockedByDelegationController) {
return _locked[wallet].sub(lockedByDelegationController);
return _locked[wallet].boundedSub(lockedByDelegationController);
} else {
return 0;
}
Expand Down Expand Up @@ -209,7 +212,7 @@ contract TokenLaunchLocker is Permissions, ILocker {
if (month >= sequence.firstUnprocessedMonth) {
sequence.subtractDiff[month] = sequence.subtractDiff[month].add(diff);
} else {
sequence.value = sequence.value.sub(diff);
sequence.value = sequence.value.boundedSub(diff);
}
}

Expand All @@ -221,7 +224,7 @@ contract TokenLaunchLocker is Permissions, ILocker {

if (sequence.firstUnprocessedMonth <= month) {
for (uint i = sequence.firstUnprocessedMonth; i <= month; ++i) {
sequence.value = sequence.value.add(sequence.addDiff[i]).sub(sequence.subtractDiff[i]);
sequence.value = sequence.value.add(sequence.addDiff[i]).boundedSub(sequence.subtractDiff[i]);
delete sequence.addDiff[i];
delete sequence.subtractDiff[i];
}
Expand Down
24 changes: 24 additions & 0 deletions contracts/test/MathUtilsTester.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
pragma solidity 0.5.16;

import "../utils/MathUtils.sol";


contract MathUtilsTester {
using MathUtils for uint;

function boundedSub(uint256 a, uint256 b) external returns (uint256) {
return a.boundedSub(b);
}

function boundedSubWithoutEvent(uint256 a, uint256 b) external pure returns (uint256) {
return a.boundedSubWithoutEvent(b);
}

function muchGreater(uint256 a, uint256 b) external pure returns (bool) {
return a.muchGreater(b);
}

function approximatelyEqual(uint256 a, uint256 b) external pure returns (bool) {
return a.approximatelyEqual(b);
}
}
60 changes: 60 additions & 0 deletions contracts/utils/MathUtils.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
StringUtils.sol - SKALE Manager
Copyright (C) 2018-Present SKALE Labs
@author DmytroStebaiev
SKALE Manager is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as published
by the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
SKALE Manager is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.
You should have received a copy of the GNU Affero General Public License
along with SKALE Manager. If not, see <https://www.gnu.org/licenses/>.
*/

pragma solidity 0.5.16;


library MathUtils {
event UnderflowError(
uint a,
uint b
);

uint constant EPS = 1e6;

function boundedSub(uint256 a, uint256 b) internal returns (uint256) {
if (a >= b) {
return a - b;
} else {
emit UnderflowError(a, b);
return 0;
}
}

function boundedSubWithoutEvent(uint256 a, uint256 b) internal pure returns (uint256) {
if (a >= b) {
return a - b;
} else {
return 0;
}
}

function muchGreater(uint256 a, uint256 b) internal pure returns (bool) {
assert(uint(-1) - EPS > b);
return a > b + EPS;
}

function approximatelyEqual(uint256 a, uint256 b) internal pure returns (bool) {
if (a > b) {
return a - b < EPS;
} else {
return b - a < EPS;
}
}
}
File renamed without changes.
11 changes: 3 additions & 8 deletions scripts/coverage.sh
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
for file in test/$TESTFOLDERS/*
do
file=${file:7}
if [ ! -f "test/$file" ]; then
file="delegation/$file"
fi
npx buidler coverage --testfiles test/$file --solcoverjs .solcover.js || exit 4
#!/bin/bash

npx buidler coverage --solcoverjs .solcover.js || exit $?
bash <(curl -s https://codecov.io/bash)
done
Loading

0 comments on commit c703297

Please sign in to comment.