From 051bdd35d8fb8aefcf905c828f218a78ec304d74 Mon Sep 17 00:00:00 2001 From: bun919tw Date: Wed, 20 Oct 2021 19:26:51 +0800 Subject: [PATCH 1/2] contracts/PriceOracle/PriceOracleProxy.sol: handle stale price --- contracts/PriceOracle/PriceOracleProxy.sol | 35 +++++++++++++++------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/contracts/PriceOracle/PriceOracleProxy.sol b/contracts/PriceOracle/PriceOracleProxy.sol index 55fcaca3f..47fd1a246 100644 --- a/contracts/PriceOracle/PriceOracleProxy.sol +++ b/contracts/PriceOracle/PriceOracleProxy.sol @@ -172,13 +172,15 @@ contract PriceOracleProxy is PriceOracle, Exponential, Denominations { AggregatorInfo memory aggregatorInfo = aggregators[token]; if (aggregatorInfo.isUsed) { - uint256 price = getPriceFromChainlink(aggregatorInfo.base, aggregatorInfo.quote); - if (aggregatorInfo.quote == Denominations.USD) { - // Convert the price to ETH based if it's USD based. - price = mul_(price, Exp({mantissa: getUsdcEthPrice()})); + (bool success, uint256 price) = getPriceFromChainlink(aggregatorInfo.base, aggregatorInfo.quote); + if (success) { + if (aggregatorInfo.quote == Denominations.USD) { + // Convert the price to ETH based if it's USD based. + price = mul_(price, Exp({mantissa: getUsdcEthPrice()})); + } + uint256 underlyingDecimals = EIP20Interface(token).decimals(); + return mul_(price, 10**(18 - underlyingDecimals)); } - uint256 underlyingDecimals = EIP20Interface(token).decimals(); - return mul_(price, 10**(18 - underlyingDecimals)); } return getPriceFromV1(token); } @@ -187,14 +189,17 @@ contract PriceOracleProxy is PriceOracle, Exponential, Denominations { * @notice Get price from ChainLink * @param base The base token that ChainLink aggregator gets the price of * @param quote The quote token, currenlty support ETH and USD - * @return The price, scaled by 1e18 + * @return (Success, The price, scaled by 1e18) */ - function getPriceFromChainlink(address base, address quote) internal view returns (uint256) { - (, int256 price, , , ) = registry.latestRoundData(base, quote); - require(price > 0, "invalid price"); + function getPriceFromChainlink(address base, address quote) internal view returns (bool, uint256) { + (, int256 price, , uint256 updatedAt, ) = registry.latestRoundData(base, quote); + // If the price from ChainLink hasn't been updated for 1 day, we consider it stale. + if (price == 0 || add_(updatedAt, 1 days) < getBlockTimestamp()) { + return (false, 0); + } // Extend the decimals to 1e18. - return mul_(uint256(price), 10**(18 - uint256(registry.decimals(base, quote)))); + return (true, mul_(uint256(price), 10**(18 - uint256(registry.decimals(base, quote))))); } /** @@ -279,6 +284,14 @@ contract PriceOracleProxy is PriceOracle, Exponential, Denominations { return v1PriceOracle.assetPrices(token); } + /** + * @notice Get the curent block timestamp + * @return The curent block timestamp + */ + function getBlockTimestamp() internal view returns (uint256) { + return block.timestamp; + } + /*** Admin or guardian functions ***/ event AggregatorUpdated(address tokenAddress, address base, address quote, bool isUsed); From a7de728954660619ba64b8580f2d46aac5d0c86d Mon Sep 17 00:00:00 2001 From: bun919tw Date: Wed, 20 Oct 2021 19:27:09 +0800 Subject: [PATCH 2/2] tests/: add tests --- tests/Contracts/MockAggregator.sol | 18 ++++++++++---- tests/Contracts/PriceOracleProxyHarness.sol | 22 +++++++++++++++++ tests/PriceOracleProxyTest.js | 26 ++++++++++++++++++++- 3 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 tests/Contracts/PriceOracleProxyHarness.sol diff --git a/tests/Contracts/MockAggregator.sol b/tests/Contracts/MockAggregator.sol index b3c953e19..f753daeab 100644 --- a/tests/Contracts/MockAggregator.sol +++ b/tests/Contracts/MockAggregator.sol @@ -42,6 +42,7 @@ contract MockAggregator { uint8 public decimals = 18; int256 public answer; + uint256 public blockTimestamp = 100000; constructor(int256 _answer) public { answer = _answer; @@ -61,7 +62,7 @@ contract MockAggregator { // Shh _roundId; - return (roundId, answer, block.timestamp, block.timestamp, roundId); + return (roundId, answer, blockTimestamp, blockTimestamp, roundId); } function latestRoundData() @@ -75,7 +76,7 @@ contract MockAggregator { uint80 ) { - return (roundId, answer, block.timestamp, block.timestamp, roundId); + return (roundId, answer, blockTimestamp, blockTimestamp, roundId); } function setAnswer(int256 _answer) external { @@ -85,6 +86,10 @@ contract MockAggregator { function setDecimals(uint8 _decimals) external { decimals = _decimals; } + + function setBlockTimestamp(uint256 newBlockTimestamp) external { + blockTimestamp = newBlockTimestamp; + } } contract MockRegistry is FeedRegistryInterface { @@ -93,6 +98,7 @@ contract MockRegistry is FeedRegistryInterface { mapping(address => mapping(address => int256)) private answer; bool public getFeedFailed; bool public feedDisabled; + uint256 public blockTimestamp = 100000; function getRoundData( address base, @@ -109,7 +115,7 @@ contract MockRegistry is FeedRegistryInterface { uint80 ) { - return (roundId, answer[base][quote], block.timestamp, block.timestamp, _roundId); + return (roundId, answer[base][quote], blockTimestamp, blockTimestamp, _roundId); } function latestRoundData(address base, address quote) @@ -123,7 +129,7 @@ contract MockRegistry is FeedRegistryInterface { uint80 ) { - return (roundId, answer[base][quote], block.timestamp, block.timestamp, roundId); + return (roundId, answer[base][quote], blockTimestamp, blockTimestamp, roundId); } function decimals(address base, address quote) external view returns (uint8) { @@ -183,4 +189,8 @@ contract MockRegistry is FeedRegistryInterface { ) external { answer[base][quote] = _answer; } + + function setBlockTimestamp(uint256 newBlockTimestamp) external { + blockTimestamp = newBlockTimestamp; + } } diff --git a/tests/Contracts/PriceOracleProxyHarness.sol b/tests/Contracts/PriceOracleProxyHarness.sol new file mode 100644 index 000000000..98e83d688 --- /dev/null +++ b/tests/Contracts/PriceOracleProxyHarness.sol @@ -0,0 +1,22 @@ +pragma solidity ^0.5.16; + +import "../../contracts/PriceOracle/PriceOracleProxy.sol"; + +contract PriceOracleProxyHarness is PriceOracleProxy { + uint256 public blockTimestamp = 100000; + + constructor( + address admin_, + address v1PriceOracle_, + address cEthAddress_, + address registry_ + ) public PriceOracleProxy(admin_, v1PriceOracle_, cEthAddress_, registry_) {} + + function getBlockTimestamp() internal view returns (uint256) { + return blockTimestamp; + } + + function setBlockTimestamp(uint256 newBlockTimestamp) public { + blockTimestamp = newBlockTimestamp; + } +} diff --git a/tests/PriceOracleProxyTest.js b/tests/PriceOracleProxyTest.js index 23d58ca1e..f03a10ef0 100644 --- a/tests/PriceOracleProxyTest.js +++ b/tests/PriceOracleProxyTest.js @@ -47,7 +47,7 @@ describe('PriceOracleProxy', () => { cYv2CrvLP = await makeCToken({comptroller: cEth.comptroller, supportMarket: true, underlying: yv2CrvLP}); backingOracle = await makePriceOracle(); - oracle = await deploy('PriceOracleProxy', + oracle = await deploy('PriceOracleProxyHarness', [ root, backingOracle._address, @@ -196,6 +196,30 @@ describe('PriceOracleProxy', () => { let proxyPrice = await call(oracle, "getUnderlyingPrice", [cOther._address]); expect(Number(proxyPrice)).toEqual(0.0005 * 1e18); }); + + it("fallbacks to v1 if the price from chainlink is invalid", async () => { + const v1Price = 10; + const oraclePrice = '0'; + + await setAndVerifyBackingPrice(cOther, v1Price); + await setPrice(cOther.underlying._address, cOther.underlying._address, ethAddress, oraclePrice); + await readAndVerifyProxyPrice(cOther, v1Price); + }); + + it("fallbacks to v1 if the price from chainlink is stale", async () => { + const v1Price = 10; + const oraclePrice = '2'; + + const timestamp1 = 10000; + const timestamp2 = 96500; // greater than timestamp1 + 86400 (1 day) + await send(mockAggregator, "setBlockTimestamp", [timestamp1]); + await send(oracle, "setBlockTimestamp", [timestamp2]); + + await setAndVerifyBackingPrice(cOther, v1Price); + await setPrice(cOther.underlying._address, cOther.underlying._address, ethAddress, oraclePrice); + + await readAndVerifyProxyPrice(cOther, v1Price); + }); }); describe("_setAdmin", () => {