Make validTimePeriod configurable (#249)

ali-behjati · web-flow · commit f09c46b1cd79 · 2022-08-22T20:13:13.000+02:00
* Make validTimePeriod configurable
diff --git a/ethereum/.env.prod.aurora b/ethereum/.env.prod.aurora
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=aurora
 WORMHOLE_BRIDGE_ADDRESS=0xa321448d90d4e5b0A732867c18eA198e75CAC48E
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.aurora_testnet b/ethereum/.env.prod.aurora_testnet
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=aurora_testnet
 WORMHOLE_BRIDGE_ADDRESS=0xBd07292de7b505a4E803CEe286184f7Acf908F5e
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.avalanche b/ethereum/.env.prod.avalanche
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=avalanche
 WORMHOLE_BRIDGE_ADDRESS=0x54a8e5f9c4CbA08F9943965859F6c34eAF03E26c
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.bnb b/ethereum/.env.prod.bnb
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=bnb
 WORMHOLE_BRIDGE_ADDRESS=0x98f3c9e6E3fAce36bAAd05FE09d375Ef1464288B
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.bnb_testnet b/ethereum/.env.prod.bnb_testnet
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=bnb_testnet
 WORMHOLE_BRIDGE_ADDRESS=0x68605AD7b15c732a30b1BbC62BE8F2A509D74b4D
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.celo b/ethereum/.env.prod.celo
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=celo
 WORMHOLE_BRIDGE_ADDRESS=0xa321448d90d4e5b0A732867c18eA198e75CAC48E
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.celo_alfajores_testnet b/ethereum/.env.prod.celo_alfajores_testnet
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=celo_alfajores_testnet
 WORMHOLE_BRIDGE_ADDRESS=0x88505117CA88e7dd2eC6EA1E13f0948db2D50D56
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.development b/ethereum/.env.prod.development
@@ -6,3 +6,5 @@ MIGRATIONS_NETWORK=development
 WORMHOLE_BRIDGE_ADDRESS=0x68605AD7b15c732a30b1BbC62BE8F2A509D74b4D
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.fantom b/ethereum/.env.prod.fantom
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=fantom
 WORMHOLE_BRIDGE_ADDRESS=0x126783A6Cb203a3E35344528B26ca3a0489a1485
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.fantom_testnet b/ethereum/.env.prod.fantom_testnet
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=fantom_testnet
 WORMHOLE_BRIDGE_ADDRESS=0x1BB3B4119b7BA9dfad76B0545fb3F531383c3bB7
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.fuji b/ethereum/.env.prod.fuji
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=fuji
 WORMHOLE_BRIDGE_ADDRESS=0x7bbcE28e64B3F8b84d876Ab298393c38ad7aac4C
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.goerli b/ethereum/.env.prod.goerli
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=goerli
 WORMHOLE_BRIDGE_ADDRESS=0x706abc4E45D419950511e474C7B9Ed348A4a716c
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=120
diff --git a/ethereum/.env.prod.mainnet b/ethereum/.env.prod.mainnet
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=mainnet
 WORMHOLE_BRIDGE_ADDRESS=0x98f3c9e6E3fAce36bAAd05FE09d375Ef1464288B
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=120
diff --git a/ethereum/.env.prod.mumbai b/ethereum/.env.prod.mumbai
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=mumbai
 WORMHOLE_BRIDGE_ADDRESS=0x0CBE91CF822c73C2315FB05100C2F714765d5c20
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.polygon b/ethereum/.env.prod.polygon
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=polygon
 WORMHOLE_BRIDGE_ADDRESS=0x7A4B5a56256163F07b2C80A7cA55aBE66c4ec4d7
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x6bb14509a612f01fbbc4cffeebd4bbfb492a86df717ebe92eb6df432a3f00a25
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/.env.prod.ropsten b/ethereum/.env.prod.ropsten
@@ -5,3 +5,5 @@ MIGRATIONS_NETWORK=ropsten
 WORMHOLE_BRIDGE_ADDRESS=0x210c5F5e2AF958B4defFe715Dc621b7a3BA888c5
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0xf346195ac02f37d60d4db8ffa6ef74cb1be3550047543a4a9ee9acf4d78697b0
+
+VALID_TIME_PERIOD_SECONDS=120
diff --git a/ethereum/.env.template b/ethereum/.env.template
@@ -17,3 +17,8 @@ INIT_GOV_CONTRACT=                # 0x000000000000000000000000000000000000000000
 WORMHOLE_BRIDGE_ADDRESS     # 0x68605AD7b15c732a30b1BbC62BE8F2A509D74b4D (only if wormhole exists)
 PYTH_TO_WORMHOLE_CHAIN_ID=  # 0x1
 PYTH_TO_WORMHOLE_EMITTER=   # 8fuAZUxHecYLMC76ZNjYzwRybUiDv9LhkRQsAccEykLr
+
+# The duration that a price feed stored in the contract is considered to be
+# valid, after this duration, the price feed is stale and will be invalid.
+# This value should derive from Pyth to wormhole latency, and target chain blocktime and latency.
+VALID_TIME_PERIOD_SECONDS= # 60
diff --git a/ethereum/.env.test b/ethereum/.env.test
@@ -22,3 +22,5 @@ BRIDGE_INIT_WETH=0xDDb64fE46a91D46ee29420539FC25FD07c5FEa3E
 #Pyth
 PYTH_TO_WORMHOLE_CHAIN_ID=0x1
 PYTH_TO_WORMHOLE_EMITTER=0x71f8dcb863d176e2c420ad6610cf687359612b6fb392e0642b0ca6b1f186aa3b
+
+VALID_TIME_PERIOD_SECONDS=60
diff --git a/ethereum/contracts/pyth/Pyth.sol b/ethereum/contracts/pyth/Pyth.sol
@@ -210,11 +210,6 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         }
     }
 
-    /// Maximum acceptable time period before price is considered to be stale.
-    ///
-    /// This includes attestation delay which currently might up to a minute.
-    uint private constant VALID_TIME_PERIOD_SECS = 180;
-
     function queryPriceFeed(bytes32 id) public view override returns (PythStructs.PriceFeed memory priceFeed){
 
         // Look up the latest price info for the given ID
@@ -224,7 +219,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         // Check that there is not a significant difference between this chain's time
         // and the price publish time.
         if (info.priceFeed.status == PythStructs.PriceStatus.TRADING && 
-            absDiff(block.timestamp, info.priceFeed.publishTime) > VALID_TIME_PERIOD_SECS) {
+            absDiff(block.timestamp, info.priceFeed.publishTime) > validTimePeriodSeconds()) {
             info.priceFeed.status = PythStructs.PriceStatus.UNKNOWN;
             // getLatestAvailablePrice* gets prevPrice when status is
             // unknown. So, now that status is being set to unknown,
diff --git a/ethereum/contracts/pyth/PythGetters.sol b/ethereum/contracts/pyth/PythGetters.sol
@@ -39,4 +39,8 @@ contract PythGetters is PythState {
     function singleUpdateFeeInWei() public view returns (uint) {
         return _state.singleUpdateFeeInWei;
     }
+
+    function validTimePeriodSeconds() public view returns (uint) {
+        return _state.validTimePeriodSeconds;
+    }
 }
diff --git a/ethereum/contracts/pyth/PythSetters.sol b/ethereum/contracts/pyth/PythSetters.sol
@@ -25,4 +25,8 @@ contract PythSetters is PythState {
     function setSingleUpdateFeeInWei(uint fee) internal {
         _state.singleUpdateFeeInWei = fee;
     }
+
+    function setValidTimePeriodSeconds(uint validTimePeriodSeconds) internal {
+        _state.validTimePeriodSeconds = validTimePeriodSeconds;
+    }
 }
diff --git a/ethereum/contracts/pyth/PythState.sol b/ethereum/contracts/pyth/PythState.sol
@@ -23,6 +23,11 @@ contract PythStorage {
         mapping(bytes32 => bool) isValidDataSource;
 
         uint singleUpdateFeeInWei;
+        
+        /// Maximum acceptable time period before price is considered to be stale.
+        /// This includes attestation delay, block time, and potential clock drift
+        /// between the source/target chains.
+        uint validTimePeriodSeconds;
     }
 }
 
diff --git a/ethereum/contracts/pyth/PythUpgradable.sol b/ethereum/contracts/pyth/PythUpgradable.sol
@@ -60,6 +60,11 @@ contract PythUpgradable is Initializable, OwnableUpgradeable, UUPSUpgradeable, P
         PythSetters.setSingleUpdateFeeInWei(newFee);
     }
 
+    /// Privileged function to update the valid time period for a price.
+    function updateValidTimePeriodSeconds(uint newValidTimePeriodSeconds) onlyOwner public {
+        PythSetters.setValidTimePeriodSeconds(newValidTimePeriodSeconds);
+    }
+
     /// Ensures the contract cannot be uninitialized and taken over.
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() initializer {}
diff --git a/ethereum/migrations/prod-receiver/7_pyth_make_valid_time_period_configurable.js b/ethereum/migrations/prod-receiver/7_pyth_make_valid_time_period_configurable.js
@@ -0,0 +1,25 @@
+require('dotenv').config({ path: "../.env" });
+
+const PythUpgradable = artifacts.require("PythUpgradable");
+const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
+
+const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
+
+/**
+ * This change:
+ * - Makes validTimePeriodSeconds configurable and sets its value.
+ *   The value depends on the network latency and block time. So
+ *   it is read from the network env file.
+ * 
+ * During this upgrade two transaction will be sent and in between validTimePeriodSeconds
+ * will be zero and `getCurrentPrice` will reject. At the time of doing this migration
+ * Pyth is not deployed on mainnet and current hard-coded value is large for some
+ * networks and it's better to reject rather than accept a price old in the past.
+ * 
+ */
+module.exports = async function (deployer) {
+    const proxy = await PythUpgradable.deployed();
+    await upgradeProxy(proxy.address, PythUpgradable, { deployer, unsafeSkipStorageCheck: true });
+
+    await proxy.updateValidTimePeriodSeconds(validTimePeriodSeconds);
+}
diff --git a/ethereum/migrations/prod/6_pyth_make_valid_time_period_configurable.js b/ethereum/migrations/prod/6_pyth_make_valid_time_period_configurable.js
@@ -0,0 +1,25 @@
+require('dotenv').config({ path: "../.env" });
+
+const PythUpgradable = artifacts.require("PythUpgradable");
+const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
+
+const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
+
+/**
+ * This change:
+ * - Makes validTimePeriodSeconds configurable and sets its value.
+ *   The value depends on the network latency and block time. So
+ *   it is read from the network env file.
+ * 
+ * During this upgrade two transaction will be sent and in between validTimePeriodSeconds
+ * will be zero and `getCurrentPrice` will reject. At the time of doing this migration
+ * Pyth is not deployed on mainnet and current hard-coded value is large for some
+ * networks and it's better to reject rather than accept a price old in the past.
+ * 
+ */
+module.exports = async function (deployer) {
+    const proxy = await PythUpgradable.deployed();
+    await upgradeProxy(proxy.address, PythUpgradable, { deployer, unsafeSkipStorageCheck: true });
+
+    await proxy.updateValidTimePeriodSeconds(validTimePeriodSeconds);
+}
diff --git a/ethereum/migrations/test/7_pyth_make_valid_time_period_configurable.js b/ethereum/migrations/test/7_pyth_make_valid_time_period_configurable.js
@@ -0,0 +1,25 @@
+require('dotenv').config({ path: "../.env" });
+
+const PythUpgradable = artifacts.require("PythUpgradable");
+const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
+
+const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
+
+/**
+ * This change:
+ * - Makes validTimePeriodSeconds configurable and sets its value.
+ *   The value depends on the network latency and block time. So
+ *   it is read from the network env file.
+ * 
+ * During this upgrade two transaction will be sent and in between validTimePeriodSeconds
+ * will be zero and `getCurrentPrice` will reject. At the time of doing this migration
+ * Pyth is not deployed on mainnet and current hard-coded value is large for some
+ * networks and it's better to reject rather than accept a price old in the past.
+ * 
+ */
+module.exports = async function (deployer) {
+    const proxy = await PythUpgradable.deployed();
+    await upgradeProxy(proxy.address, PythUpgradable, { deployer, unsafeSkipStorageCheck: true });
+
+    await proxy.updateValidTimePeriodSeconds(validTimePeriodSeconds);
+}
diff --git a/ethereum/test/pyth.js b/ethereum/test/pyth.js
@@ -5,7 +5,7 @@ const BigNumber = require("bignumber.js");
 const PythStructs = artifacts.require("PythStructs");
 
 const { deployProxy, upgradeProxy } = require("@openzeppelin/truffle-upgrades");
-const { expectRevert, expectEvent } = require("@openzeppelin/test-helpers");
+const { expectRevert, expectEvent, time } = require("@openzeppelin/test-helpers");
 const { assert } = require("chai");
 
 // Use "WormholeReceiver" if you are testing with Wormhole Receiver
@@ -33,6 +33,7 @@ contract("Pyth", function () {
     const insufficientFeeError = 
         "Insufficient paid fee amount";
 
+    // Place all atomic operations that are done within migrations here.
     beforeEach(async function () {
         this.pythProxy = await deployProxy(PythUpgradable, [
             (await Wormhole.deployed()).address,
@@ -44,6 +45,9 @@ contract("Pyth", function () {
             testPyth2WormholeChainId,
             testPyth2WormholeEmitter
         );
+
+        // Setting the validity time to 60 seconds
+        await this.pythProxy.updateValidTimePeriodSeconds(60);
     });
 
     it("should be initialized with the correct signers and values", async function () {
@@ -157,16 +161,48 @@ contract("Pyth", function () {
         const defaultAccount = accounts[0];
         assert.equal(await this.pythProxy.owner(), defaultAccount);
 
-        // Check initial fee is zero
+        // Check initial valid time period is zero
         assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
 
-        // Checks setting fee using another account reverts.
+        // Checks setting valid time period using another account reverts.
         await expectRevert(
             this.pythProxy.updateSingleUpdateFeeInWei(10, {from: accounts[1]}),
             notOwnerError,
         );
     });
 
+    it("should allow updating validTimePeriodSeconds by owner", async function () {
+        // Check that the owner is the default account Truffle
+        // has configured for the network.
+        const accounts = await web3.eth.getAccounts();
+        const defaultAccount = accounts[0];
+        assert.equal(await this.pythProxy.owner(), defaultAccount);
+
+        // Check valid time period is 60 (set in beforeEach)
+        assert.equal(await this.pythProxy.validTimePeriodSeconds(), 60);
+
+        // Set valid time period 
+        await this.pythProxy.updateValidTimePeriodSeconds(30);
+        assert.equal(await this.pythProxy.validTimePeriodSeconds(), 30);
+    });
+
+    it("should not allow updating validTimePeriodSeconds by another account", async function () {
+        // Check that the owner is the default account Truffle
+        // has configured for the network.
+        const accounts = await web3.eth.getAccounts();
+        const defaultAccount = accounts[0];
+        assert.equal(await this.pythProxy.owner(), defaultAccount);
+
+        // Check valid time period is 60 (set in beforeEach)
+        assert.equal(await this.pythProxy.validTimePeriodSeconds(), 60);
+
+        // Checks setting validity time using another account reverts.
+        await expectRevert(
+            this.pythProxy.updateValidTimePeriodSeconds(30, {from: accounts[1]}),
+            notOwnerError,
+        );
+    });
+
     // NOTE(2022-05-02): Raw hex payload obtained from format serialization unit tests in `p2w-sdk/rust`
     // Latest known addition: wire format v3
     //
@@ -568,6 +604,62 @@ contract("Pyth", function () {
         }
     });
 
+    it("changing validity time works", async function() {
+        const latestTime = await time.latest();
+        let rawBatch = generateRawBatchAttestation(
+            latestTime,
+            latestTime,
+            1337
+        );
+
+        await updatePriceFeeds(this.pythProxy, [rawBatch]);
+
+        // Setting the validity time to 30 seconds
+        await this.pythProxy.updateValidTimePeriodSeconds(30);
+
+        // Then prices should be available
+        for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
+            const price_id =
+                "0x" +
+                (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
+            let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
+            assert.equal(
+                priceFeedResult.status.toString(),
+                PythStructs.PriceStatus.TRADING.toString()
+            );
+        }
+
+        // One minute passes
+        await time.increase(time.duration.minutes(1));
+
+        // The prices should become unavailable now.
+        for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
+            const price_id =
+                "0x" +
+                (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
+            let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
+            assert.equal(
+                priceFeedResult.status.toString(),
+                PythStructs.PriceStatus.UNKNOWN.toString()
+            );
+        }
+
+        // Setting the validity time to 120 seconds
+        await this.pythProxy.updateValidTimePeriodSeconds(120);
+
+        // Then prices should be available because the valid period is now 120 seconds
+        for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
+            const price_id =
+                "0x" +
+                (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
+            let priceFeedResult = await this.pythProxy.queryPriceFeed(price_id);
+            assert.equal(
+                priceFeedResult.status.toString(),
+                PythStructs.PriceStatus.TRADING.toString()
+            );
+        }
+    });
+
     it("should accept a VM after adding its data source", async function () {
         let newChainId = "42424";
         let newEmitter = testPyth2WormholeEmitter.replace("a", "f");
