Skip to content
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.

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

Adding a parameter to limit variation to the median price #1490

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
fd5de76
added a parameter to limit variation to the median price
mrsmkl Oct 25, 2019
e7438ef
recompute rate when reports are removed
mrsmkl Oct 25, 2019
457ad7b
Merge branch 'master' into limit-median-rate-change-529
diminator Nov 7, 2019
e216d8b
Merge branch 'master' into limit-median-rate-change-529
diminator Nov 7, 2019
576c41c
Merge branch 'master' of github.com:celo-org/celo-monorepo into limit…
mrsmkl Nov 7, 2019
6b31e9b
review comments
mrsmkl Nov 7, 2019
426787b
Merge branch 'limit-median-rate-change-529' of github.com:mrsmkl/celo…
mrsmkl Nov 7, 2019
3a15df8
fixing test
mrsmkl Nov 7, 2019
807e877
seems to work
mrsmkl Nov 8, 2019
9bbbb15
merge
mrsmkl Nov 8, 2019
04d01be
fixing lint
mrsmkl Nov 8, 2019
db21ef1
Merge branch 'master' of github.com:celo-org/celo-monorepo into limit…
mrsmkl Nov 8, 2019
f778894
prettify
mrsmkl Nov 8, 2019
1ca69c2
review changes
mrsmkl Nov 8, 2019
293bdc3
added overflow checks
mrsmkl Nov 11, 2019
f78aa7f
prettify
mrsmkl Nov 11, 2019
0976726
Merge branch 'master' into limit-median-rate-change-529
timmoreton Dec 12, 2019
24240f2
Merge branch 'master' of github.com:celo-org/celo-monorepo into limit…
mrsmkl Dec 19, 2019
dcf5e2b
Merge branch 'limit-median-rate-change-529' of github.com:mrsmkl/celo…
mrsmkl Dec 19, 2019
13541ce
lint
mrsmkl Dec 19, 2019
0b45384
Merge branch 'master' into limit-median-rate-change-529
mrsmkl Dec 27, 2019
592fb53
if long time has passed, do not limit median rate change
mrsmkl Dec 27, 2019
16a5be0
small fixes
mrsmkl Dec 27, 2019
e522cc2
merge
mrsmkl Dec 30, 2019
e5c373f
Merge branch 'master' of github.com:celo-org/celo-monorepo into limit…
mrsmkl Dec 31, 2019
25533c9
changed max time from year to day
mrsmkl Dec 31, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 14 additions & 14 deletions packages/protocol/contracts/common/FixidityLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ library FixidityLib {
* Test newFixed(maxNewFixed()+1) fails
*/
function newFixed(uint256 x) internal pure returns (Fraction memory) {
require(x <= maxNewFixed());
require(x <= maxNewFixed(), "newFixed overflow");
return Fraction(x * FIXED1_UINT);
}

Expand Down Expand Up @@ -148,9 +148,9 @@ library FixidityLib {
pure
returns (Fraction memory)
{
require(numerator <= maxNewFixed());
require(denominator <= maxNewFixed());
require(denominator != 0);
require(numerator <= maxNewFixed(), "numerator too large");
require(denominator <= maxNewFixed(), "denominator too large");
require(denominator != 0, "denominator zero");
Fraction memory convertedNumerator = newFixed(numerator);
Fraction memory convertedDenominator = newFixed(denominator);
return divide(convertedNumerator, convertedDenominator);
Expand Down Expand Up @@ -189,7 +189,7 @@ library FixidityLib {
*/
function add(Fraction memory x, Fraction memory y) internal pure returns (Fraction memory) {
uint256 z = x.value + y.value;
require(z >= x.value);
require(z >= x.value, "addition overflow");
return Fraction(z);
}

Expand All @@ -199,7 +199,7 @@ library FixidityLib {
* Test subtract(6, 10) fails
*/
function subtract(Fraction memory x, Fraction memory y) internal pure returns (Fraction memory) {
require(x.value >= y.value);
require(x.value >= y.value, "subtract");
return Fraction(x.value - y.value);
}

Expand Down Expand Up @@ -228,24 +228,24 @@ library FixidityLib {

// (x1 + x2) * (y1 + y2) = (x1 * y1) + (x1 * y2) + (x2 * y1) + (x2 * y2)
uint256 x1y1 = x1 * y1;
if (x1 != 0) require(x1y1 / x1 == y1); // Overflow x1y1
if (x1 != 0) require(x1y1 / x1 == y1, "Overflow x1y1");

// x1y1 needs to be multiplied back by fixed1
// solium-disable-next-line mixedcase
uint256 fixed_x1y1 = x1y1 * FIXED1_UINT;
if (x1y1 != 0) require(fixed_x1y1 / x1y1 == FIXED1_UINT); // Overflow x1y1 * fixed1
if (x1y1 != 0) require(fixed_x1y1 / x1y1 == FIXED1_UINT, "Overflow x1y1 * fixed1");
x1y1 = fixed_x1y1;

uint256 x2y1 = x2 * y1;
if (x2 != 0) require(x2y1 / x2 == y1); // Overflow x2y1
if (x2 != 0) require(x2y1 / x2 == y1, "Overflow x2y1");

uint256 x1y2 = x1 * y2;
if (x1 != 0) require(x1y2 / x1 == y2); // Overflow x1y2
if (x1 != 0) require(x1y2 / x1 == y2, "Overflow x1y2");

x2 = x2 / mulPrecision();
y2 = y2 / mulPrecision();
uint256 x2y2 = x2 * y2;
if (x2 != 0) require(x2y2 / x2 == y2); // Overflow x2y2
if (x2 != 0) require(x2y2 / x2 == y2, "Overflow x2y2");

// result = fixed1() * x1 * y1 + x1 * y2 + x2 * y1 + x2 * y2 / fixed1();
Fraction memory result = Fraction(x1y1);
Expand All @@ -264,7 +264,7 @@ library FixidityLib {
* Test reciprocal(1+fixed1()*fixed1()) returns 0 // Testing how the fractional is truncated
*/
function reciprocal(Fraction memory x) internal pure returns (Fraction memory) {
require(x.value != 0);
require(x.value != 0, "reciprocal of zero");
return Fraction((FIXED1_UINT * FIXED1_UINT) / x.value); // Can't overflow
}

Expand All @@ -277,9 +277,9 @@ library FixidityLib {
* Test divide(maxFixedDividend()+1,1) throws
*/
function divide(Fraction memory x, Fraction memory y) internal pure returns (Fraction memory) {
require(y.value != 0);
require(y.value != 0, "division by zero");
uint256 X = x.value * FIXED1_UINT;
require(X / FIXED1_UINT == x.value);
require(X / FIXED1_UINT == x.value, "division overflow");
return Fraction(X / y.value);
}

Expand Down
106 changes: 96 additions & 10 deletions packages/protocol/contracts/stability/SortedOracles.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,20 @@ import "./interfaces/ISortedOracles.sol";
import "../common/Initializable.sol";
import "../common/linkedlists/AddressSortedLinkedListWithMedian.sol";
import "../common/linkedlists/SortedLinkedListWithMedian.sol";
import "../common/FractionUtil.sol";
import "../common/FixidityLib.sol";
import "../common/UsingPrecompiles.sol";

// TODO: Move SortedOracles to Fixidity
/**
* @title Maintains a sorted list of oracle exchange rates between Celo Gold and other currencies.
*/
contract SortedOracles is ISortedOracles, Ownable, Initializable {
contract SortedOracles is ISortedOracles, Ownable, Initializable, UsingPrecompiles {
using SafeMath for uint256;
using AddressSortedLinkedListWithMedian for SortedLinkedListWithMedian.List;
using FractionUtil for FractionUtil.Fraction;
using FixidityLib for FixidityLib.Fraction;

// All oracle rates are assumed to have a denominator of 2 ^ 64.
uint256 public constant DENOMINATOR = 0x10000000000000000;

Expand All @@ -23,8 +29,11 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
mapping(address => SortedLinkedListWithMedian.List) private timestamps;
mapping(address => mapping(address => bool)) public isOracle;
mapping(address => address[]) public oracles;
mapping(address => uint256) private limitedMedianRate;
mapping(address => uint256) private limitedMedianRateTimestamp;

uint256 public reportExpirySeconds;
FixidityLib.Fraction public maxMedianChangeRatePerSecond;

event OracleAdded(address indexed token, address indexed oracleAddress);

Expand All @@ -44,14 +53,17 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {

event ReportExpirySet(uint256 reportExpiry);

event MaxMedianChangeRatePerSecondSet(uint256 rate);

modifier onlyOracle(address token) {
require(isOracle[token][msg.sender], "sender was not an oracle for token addr");
_;
}

function initialize(uint256 _reportExpirySeconds) external initializer {
function initialize(uint256 _reportExpirySeconds, uint256 _maxChangeRate) external initializer {
_transferOwnership(msg.sender);
reportExpirySeconds = _reportExpirySeconds;
setMaxMedianChangeRatePerSecond(_maxChangeRate);
}

/**
Expand All @@ -63,6 +75,26 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
emit ReportExpirySet(_reportExpirySeconds);
}

/**
* @notice Sets the maximum change rate for median.
* @param rate Maximum median change rate as unwrapped Fraction.
* @return True upon success.
*/
function setMaxMedianChangeRatePerSecond(uint256 rate) public onlyOwner returns (bool) {
maxMedianChangeRatePerSecond = FixidityLib.wrap(rate);
require(rate < 500000000000000000000, "Rate must be smaller than 0.0005");
emit MaxMedianChangeRatePerSecondSet(rate);
return true;
}

/**
* @notice Get maximum median change rate.
* @return Max median change rate as unwrapped fraction.
*/
function getMaxMedianChangeRatePerSecond() public view returns (uint256) {
return maxMedianChangeRatePerSecond.unwrap();
}

/**
* @notice Adds a new Oracle.
* @param token The address of the token.
Expand Down Expand Up @@ -137,7 +169,6 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
address lesserKey,
address greaterKey
) external onlyOracle(token) {
uint256 originalMedian = rates[token].getMedianValue();
uint256 value = numerator.mul(DENOMINATOR).div(denominator);
if (rates[token].contains(msg.sender)) {
rates[token].update(msg.sender, value, lesserKey, greaterKey);
Expand All @@ -164,9 +195,68 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
address(0)
);
emit OracleReported(token, msg.sender, now, value, DENOMINATOR);
recomputeRate(token);
}

/**
* @notice a * b^exp
*/
function fractionMulExp(FixidityLib.Fraction memory a, FixidityLib.Fraction memory b, uint256 exp)
internal
view
returns (FixidityLib.Fraction memory)
{
uint256 numerator;
uint256 denominator;
(numerator, denominator) = fractionMulExp(
a.unwrap(),
FixidityLib.fixed1().unwrap(),
b.unwrap(),
FixidityLib.fixed1().unwrap(),
exp,
18
);
return FixidityLib.newFixedFraction(numerator, denominator);
}

function recomputeRate(address token) internal {
uint256 originalMedian = limitedMedianRate[token];
uint256 newMedian = rates[token].getMedianValue();
uint256 td = now.sub(limitedMedianRateTimestamp[token]);
if (
limitedMedianRateTimestamp[token] != 0 &&
maxMedianChangeRatePerSecond.unwrap() != 0 &&
td < 1 days
) {
FixidityLib.Fraction memory maxChangeRate = fractionMulExp(
FixidityLib.fixed1(),
maxMedianChangeRatePerSecond.add(FixidityLib.fixed1()),
td
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel safe to do as the exponents could grow very large...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So need to add something to make sure it won't overflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some checks, now it shouldn't be able to overflow.

)
.subtract(FixidityLib.fixed1());
// Check if it is safe to multiply
if (maxChangeRate.unwrap() < FixidityLib.maxFixedMul() * FixidityLib.fixed1().unwrap()) {
uint256 maxChange = maxChangeRate.multiply(FixidityLib.wrap(originalMedian)).unwrap();
if (newMedian > originalMedian) {
if (maxChange < newMedian.sub(originalMedian)) {
newMedian = originalMedian.add(maxChange);
}
} else {
if (maxChange < originalMedian.sub(newMedian)) {
newMedian = originalMedian.sub(maxChange);
}
}
}
}
// Cannot become too large, or might overflow multiplications
require(
newMedian < FixidityLib.maxFixedMul() * FixidityLib.fixed1().unwrap(),
"Median rate exceeds limit"
);
limitedMedianRateTimestamp[token] = now;
limitedMedianRate[token] = newMedian;
if (newMedian != originalMedian) {
emit MedianUpdated(token, newMedian, DENOMINATOR);
emit MedianUpdated(token, newMedian, numRates(token) == 0 ? 0 : DENOMINATOR);
}
}

Expand All @@ -185,7 +275,7 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
* @return The median exchange rate for `token`.
*/
function medianRate(address token) external view returns (uint256, uint256) {
return (rates[token].getMedianValue(), numRates(token) == 0 ? 0 : DENOMINATOR);
return (limitedMedianRate[token], numRates(token) == 0 ? 0 : DENOMINATOR);
}

/**
Expand Down Expand Up @@ -250,13 +340,9 @@ contract SortedOracles is ISortedOracles, Ownable, Initializable {
*/
function removeReport(address token, address oracle) private {
if (numTimestamps(token) == 1 && reportExists(token, oracle)) return;
uint256 originalMedian = rates[token].getMedianValue();
rates[token].remove(oracle);
timestamps[token].remove(oracle);
emit OracleReportRemoved(token, oracle);
uint256 newMedian = rates[token].getMedianValue();
if (newMedian != originalMedian) {
emit MedianUpdated(token, newMedian, numRates(token) == 0 ? 0 : DENOMINATOR);
}
recomputeRate(token);
}
}
2 changes: 1 addition & 1 deletion packages/protocol/migrations/05_sortedoracles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ module.exports = deploymentForCoreContract<SortedOraclesInstance>(
web3,
artifacts,
CeloContractName.SortedOracles,
async () => [config.oracles.reportExpiry]
async () => [config.oracles.reportExpiry, config.oracles.maxChangeRatePerSecond]
)
1 change: 1 addition & 0 deletions packages/protocol/migrationsConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ const DefaultConfig = {
},
oracles: {
reportExpiry: 10 * 60, // 10 minutes
maxChangeRatePerSecond: 0, // not checked
},
random: {
randomnessBlockRetentionWindow: (60 * 60) / 5, // 1 hour to match attestationExpiryBlocks
Expand Down
61 changes: 59 additions & 2 deletions packages/protocol/test/stability/sortedoracles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
NULL_ADDRESS,
timeTravel,
} from '@celo/protocol/lib/test-utils'
import { toFixed } from '@celo/utils/lib/fixidity'
import BigNumber from 'bignumber.js'
import * as _ from 'lodash'
import { SortedOraclesContract, SortedOraclesInstance } from 'types'
Expand All @@ -25,7 +26,7 @@ contract('SortedOracles', (accounts: string[]) => {

beforeEach(async () => {
sortedOracles = await SortedOracles.new()
await sortedOracles.initialize(aReportExpiry)
await sortedOracles.initialize(aReportExpiry, 0)
})

describe('#initialize()', () => {
Expand All @@ -39,7 +40,63 @@ contract('SortedOracles', (accounts: string[]) => {
})

it('should not be callable again', async () => {
await assertRevert(sortedOracles.initialize(aReportExpiry))
await assertRevert(sortedOracles.initialize(aReportExpiry, 0))
})
})

describe('#setMaxMedianChangeRatePerSecond', () => {
const newMaxMedianChangeRatePerSecond = toFixed(0.000001)
const denominator = new BigNumber(2).pow(64)
const smallChange = new BigNumber(2).pow(64).plus(1)
const largeChange = new BigNumber(2).pow(64).multipliedBy(2)
beforeEach(async () => {
await sortedOracles.addOracle(aToken, anOracle)
await sortedOracles.report(aToken, denominator, denominator, NULL_ADDRESS, NULL_ADDRESS, {
from: anOracle,
})
})
it('should update maxMedianChangeRatePerSecond', async () => {
await sortedOracles.setMaxMedianChangeRatePerSecond(newMaxMedianChangeRatePerSecond)
assertEqualBN(
await sortedOracles.getMaxMedianChangeRatePerSecond(),
newMaxMedianChangeRatePerSecond
)
})
it('should revert when called by a non-owner', async () => {
await assertRevert(
sortedOracles.setMaxMedianChangeRatePerSecond(newMaxMedianChangeRatePerSecond, {
from: accounts[1],
})
)
})
it('should emit the MaxMedianChangeRatePerSecondSet event', async () => {
const resp = await sortedOracles.setMaxMedianChangeRatePerSecond(
newMaxMedianChangeRatePerSecond
)
assert.equal(resp.logs.length, 1)
const log = resp.logs[0]
assertLogMatches2(log, {
event: 'MaxMedianChangeRatePerSecondSet',
args: {
rate: new BigNumber(newMaxMedianChangeRatePerSecond),
},
})
})
it('should allow a small change', async () => {
await sortedOracles.setMaxMedianChangeRatePerSecond(newMaxMedianChangeRatePerSecond)
await timeTravel(10, web3)
await sortedOracles.report(aToken, smallChange, denominator, NULL_ADDRESS, NULL_ADDRESS, {
from: anOracle,
})
assertEqualBN((await sortedOracles.medianRate(aToken))[0], smallChange)
})
it('should limit a large change', async () => {
await sortedOracles.setMaxMedianChangeRatePerSecond(newMaxMedianChangeRatePerSecond)
await timeTravel(10, web3)
await sortedOracles.report(aToken, largeChange, denominator, NULL_ADDRESS, NULL_ADDRESS, {
from: anOracle,
})
assert((await sortedOracles.medianRate(aToken))[0].lt(largeChange))
})
})

Expand Down