Skip to content

Sip 196 remove internal oracle #1636

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
artdgn merged 34 commits into from
Dec 22, 2021
Merged

Sip 196 remove internal oracle #1636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
bec2040
remove oracle from ExchangeRates and tests
artdgn Dec 15, 2021
93c2adf
update exchanger.spec tests
artdgn Dec 15, 2021
f93aefa
remove reverting for unknown currencies
artdgn Dec 15, 2021
d23ec23
fix tests for Synthetix.js and BaseSynthetix.js
artdgn Dec 15, 2021
dc87881
fix collateral* tests
artdgn Dec 15, 2021
8b1e9c9
fix debt cache tests
artdgn Dec 15, 2021
d992ec5
fix depot tests
artdgn Dec 15, 2021
a947159
fix etherwrapper tests
artdgn Dec 15, 2021
e22fe0e
fix feepool tests
artdgn Dec 16, 2021
eb8ac83
fix liquidation tests
artdgn Dec 16, 2021
868e97f
fix multicollat synth
artdgn Dec 16, 2021
29ca3d1
fix issuer tests
artdgn Dec 16, 2021
60b308f
fix native eth wrapper tests
artdgn Dec 16, 2021
f993c02
fix purgeable tests
artdgn Dec 16, 2021
3372652
fix rewards integration tests
artdgn Dec 16, 2021
0ef9d6d
fix shorting rewards tests
artdgn Dec 16, 2021
7f8c941
fix staking rewards tests
artdgn Dec 16, 2021
d271506
fix synth tests
artdgn Dec 16, 2021
29b60d0
fix synths utils tests
artdgn Dec 16, 2021
5cd8673
fix trading rewards tests
artdgn Dec 16, 2021
27d0100
fix wrapper and wrapper factory tests
artdgn Dec 16, 2021
f2fa864
fix integration tests setup
artdgn Dec 16, 2021
c985a0c
move snx aggregator setup to common setup
artdgn Dec 16, 2021
9b16a16
only setup snx rate if exchangerates is needed
artdgn Dec 16, 2021
3b12a50
deploy missing aggregators in integration tests
artdgn Dec 16, 2021
f5dced0
fix exchange rates test setup
artdgn Dec 16, 2021
1ad9a01
fix integration tests
artdgn Dec 20, 2021
13ac92b
0 timestamp for sUSD rate last update
artdgn Dec 20, 2021
4da2c7f
fix deploy script test
artdgn Dec 20, 2021
17e4d86
add sip to releases.json
artdgn Dec 20, 2021
fd2c0c5
silence slither warnings
artdgn Dec 20, 2021
a0bfcf6
address some nits
artdgn Dec 20, 2021
a8fd5b0
add retries to tests to reduce flakiness
artdgn Dec 21, 2021
d7a4e15
fix fork tests: add rate for addedSynths sREDEEMER
artdgn Dec 21, 2021
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
213 changes: 54 additions & 159 deletions contracts/ExchangeRates.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,8 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
using SafeDecimalMath for uint;

bytes32 public constant CONTRACT_NAME = "ExchangeRates";

// Exchange rates and update times stored by currency code, e.g. 'SNX', or 'sUSD'
mapping(bytes32 => mapping(uint => RateAndUpdatedTime)) private _rates;

// The address of the oracle which pushes rate updates to this contract
address public oracle;
//slither-disable-next-line naming-convention
bytes32 internal constant sUSD = "sUSD";
Copy link
Copy Markdown
Contributor

@leckylao leckylao Dec 16, 2021

Choose a reason for hiding this comment

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

There's naming convention warning, maybe use SUSD for the constant?

Copy link
Copy Markdown
Contributor Author

@artdgn artdgn Dec 20, 2021

Choose a reason for hiding this comment

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

It's used this way in other places in the codebase, and to me seems more readable as sUSD. I'll add an ignore comment to silence the warning.

Copy link
Copy Markdown
Contributor

@jjgonecrypto jjgonecrypto Dec 20, 2021

Choose a reason for hiding this comment

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

please 🙏

Copy link
Copy Markdown
Contributor

@leckylao leckylao Dec 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

please 🙏

@artdgn Just saw merged with sUSD, I believe @justinjmoses please means please use SUSD not please use ignore 🙈


// Decentralized oracle networks that feed into pricing aggregators
mapping(bytes32 => AggregatorV2V3Interface) public aggregators;
Expand All @@ -37,58 +33,12 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
// List of aggregator keys for convenient iteration
bytes32[] public aggregatorKeys;

// Do not allow the oracle to submit times any further forward into the future than this constant.
uint private constant ORACLE_FUTURE_LIMIT = 10 minutes;

mapping(bytes32 => uint) public currentRoundForRate;

//
// ========== CONSTRUCTOR ==========

constructor(
address _owner,
address _oracle,
address _resolver,
bytes32[] memory _currencyKeys,
uint[] memory _newRates
) public Owned(_owner) MixinSystemSettings(_resolver) {
require(_currencyKeys.length == _newRates.length, "Currency key length and rate length must match.");

oracle = _oracle;

// The sUSD rate is always 1 and is never stale.
_setRate("sUSD", SafeDecimalMath.unit(), now);

internalUpdateRates(_currencyKeys, _newRates, now);
}

/* ========== SETTERS ========== */

function setOracle(address _oracle) external onlyOwner {
oracle = _oracle;
emit OracleUpdated(oracle);
}
constructor(address _owner, address _resolver) public Owned(_owner) MixinSystemSettings(_resolver) {}

/* ========== MUTATIVE FUNCTIONS ========== */

function updateRates(
bytes32[] calldata currencyKeys,
uint[] calldata newRates,
uint timeSent
) external onlyOracle returns (bool) {
return internalUpdateRates(currencyKeys, newRates, timeSent);
}

function deleteRate(bytes32 currencyKey) external onlyOracle {
require(_getRate(currencyKey) > 0, "Rate is zero");

delete _rates[currencyKey][currentRoundForRate[currencyKey]];

currentRoundForRate[currencyKey]--;

emit RateDeleted(currencyKey);
}

function addAggregator(bytes32 currencyKey, address aggregatorAddress) external onlyOwner {
AggregatorV2V3Interface aggregator = AggregatorV2V3Interface(aggregatorAddress);
// This check tries to make sure that a valid aggregator is being added.
Expand Down Expand Up @@ -288,7 +238,7 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
function rateAndInvalid(bytes32 currencyKey) external view returns (uint rate, bool isInvalid) {
RateAndUpdatedTime memory rateAndTime = _getRateAndUpdatedTime(currencyKey);

if (currencyKey == "sUSD") {
if (currencyKey == sUSD) {
return (rateAndTime.rate, false);
}
return (
Expand All @@ -314,7 +264,7 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
// do one lookup of the rate & time to minimize gas
RateAndUpdatedTime memory rateEntry = _getRateAndUpdatedTime(currencyKeys[i]);
rates[i] = rateEntry.rate;
if (!anyRateInvalid && currencyKeys[i] != "sUSD") {
if (!anyRateInvalid && currencyKeys[i] != sUSD) {
anyRateInvalid = flagList[i] || _rateIsStaleWithTime(_rateStalePeriod, rateEntry.time);
}
}
Expand Down Expand Up @@ -372,52 +322,6 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
}
}

function _setRate(
bytes32 currencyKey,
uint256 rate,
uint256 time
) internal {
// Note: this will effectively start the rounds at 1, which matches Chainlink's Agggregators
currentRoundForRate[currencyKey]++;

_rates[currencyKey][currentRoundForRate[currencyKey]] = RateAndUpdatedTime({
rate: uint216(rate),
time: uint40(time)
});
}

function internalUpdateRates(
bytes32[] memory currencyKeys,
uint[] memory newRates,
uint timeSent
) internal returns (bool) {
require(currencyKeys.length == newRates.length, "Currency key array length must match rates array length.");
require(timeSent < (now + ORACLE_FUTURE_LIMIT), "Time is too far into the future");

// Loop through each key and perform update.
for (uint i = 0; i < currencyKeys.length; i++) {
bytes32 currencyKey = currencyKeys[i];

// Should not set any rate to zero ever, as no asset will ever be
// truely worthless and still valid. In this scenario, we should
// delete the rate and remove it from the system.
require(newRates[i] != 0, "Zero is not a valid rate, please call deleteRate instead.");
require(currencyKey != "sUSD", "Rate of sUSD cannot be updated, it's always UNIT.");

// We should only update the rate if it's at least the same age as the last rate we've got.
if (timeSent < _getUpdatedTime(currencyKey)) {
continue;
}

// Ok, go ahead with the update.
_setRate(currencyKey, newRates[i], timeSent);
}

emit RatesUpdated(currencyKeys, newRates);

return true;
}

function removeFromArray(bytes32 entry, bytes32[] storage array) internal returns (bool) {
for (uint i = 0; i < array.length; i++) {
if (array[i] == entry) {
Expand Down Expand Up @@ -447,60 +351,64 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
}

function _getRateAndUpdatedTime(bytes32 currencyKey) internal view returns (RateAndUpdatedTime memory) {
AggregatorV2V3Interface aggregator = aggregators[currencyKey];

if (aggregator != AggregatorV2V3Interface(0)) {
// this view from the aggregator is the most gas efficient but it can throw when there's no data,
// so let's call it low-level to suppress any reverts
bytes memory payload = abi.encodeWithSignature("latestRoundData()");
// solhint-disable avoid-low-level-calls
(bool success, bytes memory returnData) = address(aggregator).staticcall(payload);

if (success) {
(, int256 answer, , uint256 updatedAt, ) =
abi.decode(returnData, (uint80, int256, uint256, uint256, uint80));
return
RateAndUpdatedTime({
rate: uint216(_formatAggregatorAnswer(currencyKey, answer)),
time: uint40(updatedAt)
});
}
// sUSD rate is 1.0
if (currencyKey == sUSD) {
return RateAndUpdatedTime({rate: uint216(SafeDecimalMath.unit()), time: 0});
} else {
uint roundId = currentRoundForRate[currencyKey];
RateAndUpdatedTime memory entry = _rates[currencyKey][roundId];

return RateAndUpdatedTime({rate: uint216(entry.rate), time: entry.time});
AggregatorV2V3Interface aggregator = aggregators[currencyKey];
if (aggregator != AggregatorV2V3Interface(0)) {
// this view from the aggregator is the most gas efficient but it can throw when there's no data,
// so let's call it low-level to suppress any reverts
bytes memory payload = abi.encodeWithSignature("latestRoundData()");
// solhint-disable avoid-low-level-calls
// slither-disable-next-line low-level-calls
(bool success, bytes memory returnData) = address(aggregator).staticcall(payload);

if (success) {
(, int256 answer, , uint256 updatedAt, ) =
abi.decode(returnData, (uint80, int256, uint256, uint256, uint80));
return
RateAndUpdatedTime({
rate: uint216(_formatAggregatorAnswer(currencyKey, answer)),
time: uint40(updatedAt)
});
} // else return defaults, to avoid reverting in views
} // else return defaults, to avoid reverting in views
}
}

function _getCurrentRoundId(bytes32 currencyKey) internal view returns (uint) {
if (currencyKey == sUSD) {
return 0; // no roundIds for sUSD
}
AggregatorV2V3Interface aggregator = aggregators[currencyKey];

if (aggregator != AggregatorV2V3Interface(0)) {
return aggregator.latestRound();
} else {
return currentRoundForRate[currencyKey];
}
} // else return defaults, to avoid reverting in views
}

function _getRateAndTimestampAtRound(bytes32 currencyKey, uint roundId) internal view returns (uint rate, uint time) {
AggregatorV2V3Interface aggregator = aggregators[currencyKey];

if (aggregator != AggregatorV2V3Interface(0)) {
// this view from the aggregator is the most gas efficient but it can throw when there's no data,
// so let's call it low-level to suppress any reverts
bytes memory payload = abi.encodeWithSignature("getRoundData(uint80)", roundId);
// solhint-disable avoid-low-level-calls
(bool success, bytes memory returnData) = address(aggregator).staticcall(payload);

if (success) {
(, int256 answer, , uint256 updatedAt, ) =
abi.decode(returnData, (uint80, int256, uint256, uint256, uint80));
return (_formatAggregatorAnswer(currencyKey, answer), updatedAt);
}
// short circuit sUSD
if (currencyKey == sUSD) {
// sUSD has no rounds, and 0 time is preferrable for "volatility" heuristics
// which are used in atomic swaps and fee reclamation
return (SafeDecimalMath.unit(), 0);
Copy link
Copy Markdown
Contributor

@jjgonecrypto jjgonecrypto Dec 16, 2021

Choose a reason for hiding this comment

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

you should return block.timestamp to be consistent instead of 0

Copy link
Copy Markdown
Contributor Author

@artdgn artdgn Dec 20, 2021

Choose a reason for hiding this comment

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

This screws up fee reclamation logic which looks at timestamps recency.

I will return 0 in both places for consistency.

Copy link
Copy Markdown
Contributor

@jjgonecrypto jjgonecrypto Dec 20, 2021

Choose a reason for hiding this comment

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

Thanku

} else {
RateAndUpdatedTime memory update = _rates[currencyKey][roundId];
return (update.rate, update.time);
AggregatorV2V3Interface aggregator = aggregators[currencyKey];

if (aggregator != AggregatorV2V3Interface(0)) {
// this view from the aggregator is the most gas efficient but it can throw when there's no data,
// so let's call it low-level to suppress any reverts
bytes memory payload = abi.encodeWithSignature("getRoundData(uint80)", roundId);
// solhint-disable avoid-low-level-calls
(bool success, bytes memory returnData) = address(aggregator).staticcall(payload);

if (success) {
(, int256 answer, , uint256 updatedAt, ) =
abi.decode(returnData, (uint80, int256, uint256, uint256, uint80));
return (_formatAggregatorAnswer(currencyKey, answer), updatedAt);
} // else return defaults, to avoid reverting in views
} // else return defaults, to avoid reverting in views
}
}

Expand Down Expand Up @@ -542,7 +450,7 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {

function _rateIsStale(bytes32 currencyKey, uint _rateStalePeriod) internal view returns (bool) {
// sUSD is a special case and is never stale (check before an SLOAD of getRateAndUpdatedTime)
if (currencyKey == "sUSD") return false;
if (currencyKey == sUSD) return false;

return _rateIsStaleWithTime(_rateStalePeriod, _getUpdatedTime(currencyKey));
}
Expand All @@ -553,7 +461,7 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {

function _rateIsFlagged(bytes32 currencyKey, FlagsInterface flags) internal view returns (bool) {
// sUSD is a special case and is never invalid
if (currencyKey == "sUSD") return false;
if (currencyKey == sUSD) return false;
address aggregator = address(aggregators[currencyKey]);
// when no aggregator or when the flags haven't been setup
if (aggregator == address(0) || flags == FlagsInterface(0)) {
Expand All @@ -563,25 +471,12 @@ contract ExchangeRates is Owned, MixinSystemSettings, IExchangeRates {
}

function _notImplemented() internal pure {
// slither-disable-next-line dead-code
revert("Cannot be run on this layer");
}

/* ========== MODIFIERS ========== */

modifier onlyOracle {
_onlyOracle();
_;
}

function _onlyOracle() internal view {
require(msg.sender == oracle, "Only the oracle can perform this action");
}

/* ========== EVENTS ========== */

event OracleUpdated(address newOracle);
event RatesUpdated(bytes32[] currencyKeys, uint[] newRates);
event RateDeleted(bytes32 currencyKey);
event AggregatorAdded(bytes32 currencyKey, address aggregator);
event AggregatorRemoved(bytes32 currencyKey, address aggregator);
}
8 changes: 1 addition & 7 deletions contracts/ExchangeRatesWithDexPricing.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@ contract ExchangeRatesWithDexPricing is ExchangeRates {

bytes32 internal constant SETTING_DEX_PRICE_AGGREGATOR = "dexPriceAggregator";

constructor(
address _owner,
address _oracle,
address _resolver,
bytes32[] memory _currencyKeys,
uint[] memory _newRates
) public ExchangeRates(_owner, _oracle, _resolver, _currencyKeys, _newRates) {}
constructor(address _owner, address _resolver) public ExchangeRates(_owner, _resolver) {}

/* ========== SETTERS ========== */

Expand Down
4 changes: 0 additions & 4 deletions contracts/interfaces/IExchangeRates.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ interface IExchangeRates {

function anyRateIsInvalid(bytes32[] calldata currencyKeys) external view returns (bool);

function currentRoundForRate(bytes32 currencyKey) external view returns (uint);

function currenciesUsingAggregator(address aggregator) external view returns (bytes32[] memory);

function effectiveValue(
Expand Down Expand Up @@ -71,8 +69,6 @@ interface IExchangeRates {

function lastRateUpdateTimes(bytes32 currencyKey) external view returns (uint256);

function oracle() external view returns (address);

function rateAndTimestampAtRound(bytes32 currencyKey, uint roundId) external view returns (uint rate, uint time);

function rateAndUpdatedTime(bytes32 currencyKey) external view returns (uint rate, uint time);
Expand Down
1 change: 1 addition & 0 deletions hardhat.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,6 @@ module.exports = {
},
mocha: {
timeout: 120e3, // 120s
retries: 3,
},
};
5 changes: 5 additions & 0 deletions publish/releases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,11 @@
"layer": "ovm",
"sources": ["CollateralEth"],
"released": "ovm"
},
{
"sip": 196,
"layer": "both",
"sources": ["ExchangeRates"]
}
],
"releases": [
Expand Down
3 changes: 1 addition & 2 deletions publish/src/commands/deploy/deploy-core.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ module.exports = async ({
currentSynthetixSupply,
currentWeekOfInflation,
deployer,
oracleAddress,
useOvm,
}) => {
console.log(gray(`\n------ DEPLOY LIBRARIES ------\n`));
Expand Down Expand Up @@ -59,7 +58,7 @@ module.exports = async ({
await deployer.deployContract({
name: 'ExchangeRates',
source: useOvm ? 'ExchangeRates' : 'ExchangeRatesWithDexPricing',
args: [account, oracleAddress, addressOf(readProxyForResolver), [], []],
args: [account, addressOf(readProxyForResolver)],
});

await deployer.deployContract({
Expand Down
4 changes: 0 additions & 4 deletions publish/src/commands/deploy/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ const deploy = async ({
ignoreSafetyChecks,
manageNonces,
network = DEFAULTS.network,
oracleExrates,
privateKey,
providerUrl,
skipFeedChecks = false,
Expand Down Expand Up @@ -212,7 +211,6 @@ const deploy = async ({
currentSynthetixSupply,
currentLastMintEvent,
currentWeekOfInflation,
oracleAddress,
systemSuspended,
} = await systemAndParameterCheck({
account,
Expand All @@ -229,7 +227,6 @@ const deploy = async ({
maxPriorityFeePerGas,
getDeployParameter,
network,
oracleExrates,
providerUrl,
skipFeedChecks,
standaloneFeeds,
Expand Down Expand Up @@ -276,7 +273,6 @@ const deploy = async ({
currentSynthetixSupply,
currentWeekOfInflation,
deployer,
oracleAddress,
useOvm,
});

Expand Down
Loading