Merge pull request #26 from perpetual-protocol/hotfix/fix-update-rule

hotfix/fix update rule

Author: tienshaoku

contracts/ChainlinkPriceFeed.sol

@@ -22,7 +22,6 @@ contract ChainlinkPriceFeed is IPriceFeed, BlockContext, CachedTwap {
     /// @dev anyone can help update it.
     function update() external {
         (, uint256 latestPrice, uint256 latestTimestamp) = _getLatestRoundData();
-
         _update(latestPrice, latestTimestamp);
     }
 

contracts/twap/CumulativeTwap.sol

@@ -40,10 +40,8 @@ contract CumulativeTwap is BlockContext {
         }
 
         // CT_IT: invalid timestamp
-        // add `==` in the require statement in case that two or more price with the same timestamp
-        // this might happen on Optimism bcs their timestamp is not up-to-date
         Observation memory lastObservation = observations[currentObservationIndex];
-        require(lastUpdatedTimestamp >= lastObservation.timestamp, "CT_IT");
+        require(lastUpdatedTimestamp > lastObservation.timestamp, "CT_IT");
 
         // overflow of currentObservationIndex is desired since currentObservationIndex is uint8 (0 - 255),
         // so 255 + 1 will be 0

test/BandPriceFeed.spec.ts

@@ -88,9 +88,7 @@ describe("BandPriceFeed/CumulativeTwap Spec", () => {
             expect(observation.priceCumulative).to.eq(parseEther("6000"))
         })
 
-        // skip this test for being compatible with Chainlink aggregator
-        // Chainlink aggregator might have the same timestamp in different round
-        it.skip("force error, the second update is the same timestamp", async () => {
+        it("force error, the second update is the same timestamp", async () => {
             await updatePrice(400, false)
 
             roundData.push([parseEther("440"), currentTime, currentTime])

test/CachedTwap.spec.ts

@@ -86,6 +86,12 @@ describe("Cached Twap Spec", () => {
         await updatePrice(400)
         await updatePrice(405)
         await updatePrice(410)
+
+        await bandReference.setReferenceData({
+            rate: parseEther("415"),
+            lastUpdatedBase: currentTime,
+            lastUpdatedQuote: currentTime,
+        })
     })
 
     describe("cacheTwap should be exactly the same getPrice()", () => {
@@ -98,6 +104,8 @@ describe("Cached Twap Spec", () => {
         it("if cached twap found, twap price should equal cached twap", async () => {
             const price = await testPriceFeed.callStatic.getPrice(45)
             expect(price.twap).to.eq(price.cachedTwap)
+            // `getPrice` here is no a view function, it mocked function in TestPriceFeed
+            // and it will update the cache if necessary
             expect(price.twap).to.eq(await bandPriceFeed.getPrice(45))
         })
 
@@ -113,6 +121,8 @@ describe("Cached Twap Spec", () => {
 
             const price2 = await testPriceFeed.callStatic.getPrice(45)
             expect(price2.twap).to.eq(price2.cachedTwap)
+            // `getPrice` here is no a view function, it mocked function in TestPriceFeed
+            // and it will update the cache if necessary
             expect(price2.twap).to.eq(await bandPriceFeed.getPrice(45))
 
             expect(price1.cachedTwap).to.not.eq(price2.cachedTwap)

test/ChainlinkPriceFeed.spec.ts

@@ -48,50 +48,20 @@ describe("ChainlinkPriceFeed Spec", () => {
     })
 
     describe("edge cases, have the same timestamp for several rounds", () => {
-        beforeEach(async () => {
-            // `base` = now - _interval
-            // aggregator's answer
-            // timestamp(base + 0)  : 400
-            // timestamp(base + 15) : 405
-            // timestamp(base + 30) : 410
-            // now = base + 45
-            //
-            //  --+------+-----+-----+-----+-----+-----+
-            //          base                          now
-            const latestTimestamp = (await waffle.provider.getBlock("latest")).timestamp
-            currentTime = latestTimestamp
+        it("force error, can't update if timestamp is the same", async () => {
+            currentTime = (await waffle.provider.getBlock("latest")).timestamp
             roundData = [
                 // [roundId, answer, startedAt, updatedAt, answeredInRound]
             ]
+            // set first round data
+            roundData.push([0, parseEther("399"), currentTime, currentTime, 0])
+            aggregator.latestRoundData.returns(() => {
+                return roundData[roundData.length - 1]
+            })
 
+            // update without forward timestamp
             await updatePrice(0, 400, false)
-            await updatePrice(1, 405, false)
-            await updatePrice(2, 410, false)
-            // // have the same timestamp for rounds
-            // roundData.push([0, parseEther("400"), currentTime, currentTime, 0])
-            // roundData.push([1, parseEther("405"), currentTime, currentTime, 1])
-            // roundData.push([2, parseEther("410"), currentTime, currentTime, 2])
-
-            // aggregator.latestRoundData.returns(() => {
-            //     return roundData[roundData.length - 1]
-            // })
-            // aggregator.getRoundData.returns(round => {
-            //     return roundData[round]
-            // })
-
-            currentTime += 15
-            await ethers.provider.send("evm_setNextBlockTimestamp", [currentTime])
-            await ethers.provider.send("evm_mine", [])
-        })
-
-        it("get the latest price", async () => {
-            const price = await chainlinkPriceFeed.getPrice(45)
-            expect(price).to.eq(parseEther("410"))
-        })
-
-        it("asking interval more than aggregator has", async () => {
-            const price = await chainlinkPriceFeed.getPrice(46)
-            expect(price).to.eq(parseEther("410"))
+            await expect(chainlinkPriceFeed.update()).to.be.revertedWith("CT_IT")
         })
     })
 })