Эх сурвалжийг харах

[eth] Remove ownership entirely in favor of governance (#405)

Ali Behjati 3 жил өмнө
parent
commit
33f5b8a5bf

+ 1 - 0
ethereum/.env.template

@@ -19,4 +19,5 @@ PYTHNET_CHAIN_ID= # 0x1a
 PYTHNET_EMITTER= # 0xa27839d641b07743c0cb5f68c51f8cd31d2c0762bec00dc6fcd25433ef1ab5b6
 GOVERNANCE_CHAIN_ID= # 0x1
 GOVERNANCE_EMITTER= # 0x63278d271099bfd491951b3e648f08b1c71631e4a53674ad43e8f9f98068c385
+GOVERNANCE_INITIAL_SEQUENCE=0 # This is optional and default is 0
 SINGLE_UPDATE_FEE_IN_WEI=0

+ 12 - 0
ethereum/contracts/pyth/Pyth.sol

@@ -17,6 +17,9 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         address wormhole,
         uint16[] calldata dataSourceEmitterChainIds,
         bytes32[] calldata dataSourceEmitterAddresses,
+        uint16 governanceEmitterChainId,
+        bytes32 governanceEmitterAddress,
+        uint64 governanceInitialSequence,
         uint validTimePeriodSeconds,
         uint singleUpdateFeeInWei
     ) internal {
@@ -41,6 +44,15 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
             _state.validDataSources.push(ds);
         }
 
+        {
+            PythInternalStructs.DataSource memory ds = PythInternalStructs
+                .DataSource(governanceEmitterChainId, governanceEmitterAddress);
+            PythSetters.setGovernanceDataSource(ds);
+            PythSetters.setLastExecutedGovernanceSequence(
+                governanceInitialSequence
+            );
+        }
+
         PythSetters.setValidTimePeriodSeconds(validTimePeriodSeconds);
         PythSetters.setSingleUpdateFeeInWei(singleUpdateFeeInWei);
     }

+ 9 - 68
ethereum/contracts/pyth/PythUpgradable.sol

@@ -24,6 +24,9 @@ contract PythUpgradable is
         address wormhole,
         uint16[] calldata dataSourceEmitterChainIds,
         bytes32[] calldata dataSourceEmitterAddresses,
+        uint16 governanceEmitterChainId,
+        bytes32 governanceEmitterAddress,
+        uint64 governanceInitialSequence,
         uint validTimePeriodSeconds,
         uint singleUpdateFeeInWei
     ) public initializer {
@@ -34,78 +37,14 @@ contract PythUpgradable is
             wormhole,
             dataSourceEmitterChainIds,
             dataSourceEmitterAddresses,
+            governanceEmitterChainId,
+            governanceEmitterAddress,
+            governanceInitialSequence,
             validTimePeriodSeconds,
             singleUpdateFeeInWei
         );
-    }
-
-    /// Privileged function to specify additional data sources in the contract
-    function addDataSource(uint16 chainId, bytes32 emitter) public onlyOwner {
-        PythInternalStructs.DataSource memory ds = PythInternalStructs
-            .DataSource(chainId, emitter);
-        require(
-            !PythGetters.isValidDataSource(ds.chainId, ds.emitterAddress),
-            "Data source already added"
-        );
-
-        _state.isValidDataSource[hashDataSource(ds)] = true;
-        _state.validDataSources.push(ds);
-    }
-
-    /// Privileged fucntion to remove the specified data source. Assumes _state.validDataSources has no duplicates.
-    function removeDataSource(
-        uint16 chainId,
-        bytes32 emitter
-    ) public onlyOwner {
-        PythInternalStructs.DataSource memory ds = PythInternalStructs
-            .DataSource(chainId, emitter);
-        require(
-            PythGetters.isValidDataSource(ds.chainId, ds.emitterAddress),
-            "Data source not found, not removing"
-        );
-
-        _state.isValidDataSource[hashDataSource(ds)] = false;
-
-        for (uint i = 0; i < _state.validDataSources.length; ++i) {
-            // Find the source to remove
-            if (
-                _state.validDataSources[i].chainId == ds.chainId ||
-                _state.validDataSources[i].emitterAddress == ds.emitterAddress
-            ) {
-                // Copy last element to overwrite the target data source
-                _state.validDataSources[i] = _state.validDataSources[
-                    _state.validDataSources.length - 1
-                ];
-                // Remove the last element we just preserved
-                _state.validDataSources.pop();
-
-                break;
-            }
-        }
-    }
-
-    /// Privileged function to update the price update fee
-    function updateSingleUpdateFeeInWei(uint newFee) public onlyOwner {
-        PythSetters.setSingleUpdateFeeInWei(newFee);
-    }
-
-    /// Privileged function to update the valid time period for a price.
-    function updateValidTimePeriodSeconds(
-        uint newValidTimePeriodSeconds
-    ) public onlyOwner {
-        PythSetters.setValidTimePeriodSeconds(newValidTimePeriodSeconds);
-    }
 
-    // Privileged function to update the governance emitter
-    function updateGovernanceDataSource(
-        uint16 chainId,
-        bytes32 emitter,
-        uint64 sequence
-    ) public onlyOwner {
-        PythInternalStructs.DataSource memory ds = PythInternalStructs
-            .DataSource(chainId, emitter);
-        PythSetters.setGovernanceDataSource(ds);
-        PythSetters.setLastExecutedGovernanceSequence(sequence);
+        renounceOwnership();
     }
 
     /// Ensures the contract cannot be uninitialized and taken over.
@@ -113,6 +52,8 @@ contract PythUpgradable is
     constructor() initializer {}
 
     // Only allow the owner to upgrade the proxy to a new implementation.
+    // The contract has no owner so this function will always revert
+    // but UUPSUpgradeable expects this method to be implemented.
     function _authorizeUpgrade(address) internal override onlyOwner {}
 
     function pythUpgradableMagic() public pure returns (uint32) {

+ 3 - 6
ethereum/forge-test/utils/PythTestUtils.t.sol

@@ -39,14 +39,11 @@ abstract contract PythTestUtils is Test, WormholeTestUtils {
             wormhole,
             emitterChainIds,
             emitterAddresses,
-            60, // Valid time period in seconds
-            1 // single update fee in wei
-        );
-
-        pyth.updateGovernanceDataSource(
             GOVERNANCE_EMITTER_CHAIN_ID,
             GOVERNANCE_EMITTER_ADDRESS,
-            0
+            0, // Initial governance sequence
+            60, // Valid time period in seconds
+            1 // single update fee in wei
         );
 
         return address(pyth);

+ 0 - 30
ethereum/migrations/prod-receiver/10_pyth_enable_governance.js

@@ -1,30 +0,0 @@
-const loadEnv = require("../../scripts/loadEnv");
-loadEnv("../../");
-
-const PythUpgradable = artifacts.require("PythUpgradable");
-const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
-const governanceEmitter = process.env.GOVERNANCE_EMITTER;
-
-console.log("governanceEmitter: " + governanceEmitter);
-console.log("governanceChainId: " + governanceChainId);
-
-const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
-
-/**
- * Version 1.0.0
- *
- * This change:
- * - Add Governance coming from the Wormhole to manage the contract.
- */
-module.exports = async function (deployer) {
-  const proxy = await PythUpgradable.deployed();
-  await upgradeProxy(proxy.address, PythUpgradable, {
-    deployer,
-    unsafeSkipStorageCheck: true,
-  });
-  await proxy.updateGovernanceDataSource(
-    governanceChainId,
-    governanceEmitter,
-    0
-  );
-};

+ 14 - 12
ethereum/migrations/prod-receiver/3_deploy_pyth.js

@@ -15,34 +15,36 @@ const emitterAddresses = [
   process.env.SOLANA_EMITTER,
   process.env.PYTHNET_EMITTER,
 ];
+const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
+const governanceEmitter = process.env.GOVERNANCE_EMITTER;
+// Default value for this field is 0
+const governanceInitialSequence = Number(
+  process.env.GOVERNANCE_INITIAL_SEQUENCE ?? "0"
+);
+
 const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
 const singleUpdateFeeInWei = Number(process.env.SINGLE_UPDATE_FEE_IN_WEI);
 
 console.log("emitterChainIds: " + emitterChainIds);
 console.log("emitterAddresses: " + emitterAddresses);
+console.log("governanceEmitter: " + governanceEmitter);
+console.log("governanceChainId: " + governanceChainId);
+console.log("governanceInitialSequence: " + governanceInitialSequence);
 console.log("validTimePeriodSeconds: " + validTimePeriodSeconds);
 console.log("singleUpdateFeeInWei: " + singleUpdateFeeInWei);
 
 module.exports = async function (deployer, network) {
-  const cluster = process.env.CLUSTER;
-  const chainName = process.env.WORMHOLE_CHAIN_NAME;
-
-  assert(cluster !== undefined && chainName !== undefined);
-
-  const wormholeBridgeAddress =
-    CONTRACTS[cluster.toUpperCase()][chainName].core;
-  assert(wormholeBridgeAddress !== undefined);
-
-  console.log("Wormhole bridge address: " + wormholeBridgeAddress);
-
   // Deploy the proxy. This will return an instance of PythUpgradable,
   // with the address field corresponding to the fronting ERC1967Proxy.
   let proxyInstance = await deployProxy(
     PythUpgradable,
     [
-      wormholeBridgeAddress,
+      (await WormholeReceiver.deployed()).address,
       emitterChainIds,
       emitterAddresses,
+      governanceChainId,
+      governanceEmitter,
+      governanceInitialSequence,
       validTimePeriodSeconds,
       singleUpdateFeeInWei,
     ],

+ 0 - 15
ethereum/migrations/prod/11_pyth_renounce_ownership.js

@@ -1,15 +0,0 @@
-const loadEnv = require("../../scripts/loadEnv");
-loadEnv("../../");
-
-const PythUpgradable = artifacts.require("PythUpgradable");
-
-/**
- * Version 1.0.0 - 2nd step
- *
- * This change:
- * - Renounce single ownership, the contract will be managed by only the governance
- */
-module.exports = async function (_deployer) {
-  const proxy = await PythUpgradable.deployed();
-  await proxy.renounceOwnership();
-};

+ 13 - 0
ethereum/migrations/prod/2_deploy_pyth.js

@@ -15,11 +15,21 @@ const emitterAddresses = [
   process.env.SOLANA_EMITTER,
   process.env.PYTHNET_EMITTER,
 ];
+const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
+const governanceEmitter = process.env.GOVERNANCE_EMITTER;
+// Default value for this field is 0
+const governanceInitialSequence = Number(
+  process.env.GOVERNANCE_INITIAL_SEQUENCE ?? "0"
+);
+
 const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
 const singleUpdateFeeInWei = Number(process.env.SINGLE_UPDATE_FEE_IN_WEI);
 
 console.log("emitterChainIds: " + emitterChainIds);
 console.log("emitterAddresses: " + emitterAddresses);
+console.log("governanceEmitter: " + governanceEmitter);
+console.log("governanceChainId: " + governanceChainId);
+console.log("governanceInitialSequence: " + governanceInitialSequence);
 console.log("validTimePeriodSeconds: " + validTimePeriodSeconds);
 console.log("singleUpdateFeeInWei: " + singleUpdateFeeInWei);
 
@@ -43,6 +53,9 @@ module.exports = async function (deployer, network) {
       wormholeBridgeAddress,
       emitterChainIds,
       emitterAddresses,
+      governanceChainId,
+      governanceEmitter,
+      governanceInitialSequence,
       validTimePeriodSeconds,
       singleUpdateFeeInWei,
     ],

+ 0 - 25
ethereum/migrations/prod/9_pyth_enable_governance.js

@@ -1,25 +0,0 @@
-const loadEnv = require("../../scripts/loadEnv");
-loadEnv("../../");
-
-const PythUpgradable = artifacts.require("PythUpgradable");
-const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
-const governanceEmitter = process.env.GOVERNANCE_EMITTER;
-
-console.log("governanceEmitter: " + governanceEmitter);
-console.log("governanceChainId: " + governanceChainId);
-
-/**
- * Version 1.0.0 - 1st step
- *
- * This change:
- * - Moves away single ownership to Governance coming from the Wormhole to
- *   manage the contract.
- */
-module.exports = async function (deployer) {
-  const proxy = await PythUpgradable.deployed();
-  await proxy.updateGovernanceDataSource(
-    governanceChainId,
-    governanceEmitter,
-    0
-  );
-};

+ 0 - 22
ethereum/migrations/test/10_pyth_renounce_ownership.js

@@ -1,22 +0,0 @@
-const loadEnv = require("../../scripts/loadEnv");
-loadEnv("../../");
-
-const PythUpgradable = artifacts.require("PythUpgradable");
-const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
-const governanceEmitter = process.env.GOVERNANCE_EMITTER;
-
-console.log("governanceEmitter: " + governanceEmitter);
-console.log("governanceChainId: " + governanceChainId);
-
-const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
-
-/**
- * Version 1.0.0 - 2nd step
- *
- * This change:
- * - Renounce single ownership, the contract will be managed by only the governance
- */
-module.exports = async function (_deployer) {
-  const proxy = await PythUpgradable.deployed();
-  await proxy.renounceOwnership();
-};

+ 18 - 4
ethereum/migrations/test/3_deploy_pyth.js

@@ -8,15 +8,26 @@ const Wormhole = artifacts.require("Wormhole");
 
 const pyth2WormholeChainId = process.env.SOLANA_CHAIN_ID;
 const pyth2WormholeEmitter = process.env.SOLANA_EMITTER;
+
+const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
+const governanceEmitter = process.env.GOVERNANCE_EMITTER;
+// Default value for this field is 0
+const governanceInitialSequence = Number(
+  process.env.GOVERNANCE_INITIAL_SEQUENCE ?? "0"
+);
+
 const validTimePeriodSeconds = Number(process.env.VALID_TIME_PERIOD_SECONDS);
 const singleUpdateFeeInWei = Number(process.env.SINGLE_UPDATE_FEE_IN_WEI);
 
 const { deployProxy } = require("@openzeppelin/truffle-upgrades");
 
-console.log(
-  "Deploying Pyth with emitter",
-  pyth2WormholeEmitter.toString("hex")
-);
+console.log("pyth2WormholeChainId: " + pyth2WormholeChainId);
+console.log("pyth2WormholeEmitter: " + pyth2WormholeEmitter);
+console.log("governanceEmitter: " + governanceEmitter);
+console.log("governanceChainId: " + governanceChainId);
+console.log("governanceInitialSequence: " + governanceInitialSequence);
+console.log("validTimePeriodSeconds: " + validTimePeriodSeconds);
+console.log("singleUpdateFeeInWei: " + singleUpdateFeeInWei);
 
 module.exports = async function (deployer) {
   // Deploy the proxy script
@@ -26,6 +37,9 @@ module.exports = async function (deployer) {
       (await Wormhole.deployed()).address,
       [pyth2WormholeChainId],
       [pyth2WormholeEmitter],
+      governanceChainId,
+      governanceEmitter,
+      governanceInitialSequence,
       validTimePeriodSeconds,
       singleUpdateFeeInWei,
     ],

+ 0 - 27
ethereum/migrations/test/9_pyth_enable_governance.js

@@ -1,27 +0,0 @@
-const loadEnv = require("../../scripts/loadEnv");
-loadEnv("../../");
-
-const PythUpgradable = artifacts.require("PythUpgradable");
-const governanceChainId = process.env.GOVERNANCE_CHAIN_ID;
-const governanceEmitter = process.env.GOVERNANCE_EMITTER;
-
-console.log("governanceEmitter: " + governanceEmitter);
-console.log("governanceChainId: " + governanceChainId);
-
-const { upgradeProxy } = require("@openzeppelin/truffle-upgrades");
-
-/**
- * Version 1.0.0 - 1st step
- *
- * This change:
- * - Moves away single ownership to Governance coming from the Wormhole to
- *   manage the contract.
- */
-module.exports = async function (deployer) {
-  const proxy = await PythUpgradable.deployed();
-  await proxy.updateGovernanceDataSource(
-    governanceChainId,
-    governanceEmitter,
-    0
-  );
-};

+ 93 - 254
ethereum/test/pyth.js

@@ -37,8 +37,6 @@ contract("Pyth", function () {
   const testPyth2WormholeChainId = "1";
   const testPyth2WormholeEmitter =
     "0x71f8dcb863d176e2c420ad6610cf687359612b6fb392e0642b0ca6b1f186aa3b";
-  const notOwnerError =
-    "Ownable: caller is not the owner -- Reason given: Ownable: caller is not the owner.";
 
   // Place all atomic operations that are done within migrations here.
   beforeEach(async function () {
@@ -46,16 +44,12 @@ contract("Pyth", function () {
       (await Wormhole.deployed()).address,
       [testPyth2WormholeChainId],
       [testPyth2WormholeEmitter],
+      testGovernanceChainId,
+      testGovernanceEmitter,
+      0, // Initial governance sequence
       60, // Validity time in seconds
       0, // single update fee in wei
     ]);
-
-    // Setting the governance data source to 0x1 (solana) and some random emitter address
-    await this.pythProxy.updateGovernanceDataSource(
-      testGovernanceChainId,
-      testGovernanceEmitter,
-      0
-    );
   });
 
   it("should be initialized with the correct signers and values", async function () {
@@ -65,152 +59,17 @@ contract("Pyth", function () {
     );
   });
 
-  it("should allow upgrades from the owner", async function () {
-    // Check that the owner is the default account Truffle
-    // has configured for the network. upgradeProxy will send
-    // transactions from the default account.
-    const accounts = await web3.eth.getAccounts();
-    const defaultAccount = accounts[0];
+  it("there should be no owner", async function () {
+    // Check that the ownership is renounced.
     const owner = await this.pythProxy.owner();
-    assert.equal(owner, defaultAccount);
-
-    // Try and upgrade the proxy
-    const newImplementation = await upgradeProxy(
-      this.pythProxy.address,
-      MockPythUpgrade
-    );
-
-    // Check that the new upgrade is successful
-    assert.equal(await newImplementation.isUpgradeActive(), true);
-    assert.equal(this.pythProxy.address, newImplementation.address);
-  });
-
-  it("should allow ownership transfer", 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 that another account can't transfer the ownership
-    await expectRevert(
-      this.pythProxy.transferOwnership(accounts[1], {
-        from: accounts[1],
-      }),
-      notOwnerError
-    );
-
-    // Transfer the ownership to another account
-    await this.pythProxy.transferOwnership(accounts[2], {
-      from: defaultAccount,
-    });
-    assert.equal(await this.pythProxy.owner(), accounts[2]);
-
-    // Check that the original account can't transfer the ownership back to itself
-    await expectRevert(
-      this.pythProxy.transferOwnership(defaultAccount, {
-        from: defaultAccount,
-      }),
-      notOwnerError
-    );
-
-    // Check that the new owner can transfer the ownership back to the original account
-    await this.pythProxy.transferOwnership(defaultAccount, {
-      from: accounts[2],
-    });
-    assert.equal(await this.pythProxy.owner(), defaultAccount);
+    assert.equal(owner, "0x0000000000000000000000000000000000000000");
   });
 
-  it("should not allow upgrades from the another account", async function () {
-    // This test is slightly convoluted as, due to a limitation of Truffle,
-    // we cannot specify which account upgradeProxy send transactions from:
-    // it will always use the default account.
-    //
-    // Therefore, we transfer the ownership to another account first,
-    // and then attempt an upgrade using the default account.
-
-    // 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);
-
-    // Transfer the ownership to another account
-    const newOwnerAccount = accounts[1];
-    await this.pythProxy.transferOwnership(newOwnerAccount, {
-      from: defaultAccount,
-    });
-    assert.equal(await this.pythProxy.owner(), newOwnerAccount);
-
-    // Try and upgrade using the default account, which will fail
-    // because we are no longer the owner.
+  it("deployer cannot upgrade the contract", async function () {
+    // upgrade proxy should fail
     await expectRevert(
       upgradeProxy(this.pythProxy.address, MockPythUpgrade),
-      notOwnerError
-    );
-  });
-
-  it("should allow updating singleUpdateFeeInWei 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 initial fee is zero
-    assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
-
-    // Set fee
-    await this.pythProxy.updateSingleUpdateFeeInWei(10);
-    assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 10);
-  });
-
-  it("should not allow updating singleUpdateFeeInWei 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 initial valid time period is zero
-    assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
-
-    // 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
+      "Ownable: caller is not the owner."
     );
   });
 
@@ -360,6 +219,28 @@ contract("Pyth", function () {
     return await contract.updatePriceFeeds(updateData, { value: valueInWei });
   }
 
+  /**
+   * Create a governance instruction VAA from the Instruction object. Then
+   * Submit and execute it on the contract.
+   * @param contract Pyth contract
+   * @param {governance.Instruction} governanceInstruction
+   * @param {number} sequence
+   */
+  async function createAndThenSubmitGovernanceInstructionVaa(
+    contract,
+    governanceInstruction,
+    sequence
+  ) {
+    await contract.executeGovernanceInstruction(
+      await createVAAFromUint8Array(
+        governanceInstruction.serialize(),
+        testGovernanceChainId,
+        testGovernanceEmitter,
+        sequence
+      )
+    );
+  }
+
   it("should attest price updates over wormhole", async function () {
     let ts = 1647273460;
     let rawBatch = generateRawBatchAttestation(ts - 5, ts, 1337);
@@ -406,18 +287,30 @@ contract("Pyth", function () {
     });
   });
 
-  it("should not attest price updates with when required fee is not given", 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);
+  /**
+   * Set fee to `newFee` by creating and submitting a governance instruction for it.
+   * @param contarct Pyth contract
+   * @param {number} newFee
+   * @param {number=} governanceSequence Sequence number of the governance instruction. Defaults to 1.
+   */
+  async function setFeeTo(contract, newFee, governanceSequence) {
+    await createAndThenSubmitGovernanceInstructionVaa(
+      contract,
+      new governance.SetFeeInstruction(
+        governance.CHAINS.ethereum,
+        BigInt(newFee),
+        BigInt(0)
+      ),
+      governanceSequence ?? 1
+    );
+  }
 
+  it("should not attest price updates with when required fee is not given", async function () {
     // Check initial fee is zero
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
 
-    // Set fee
-    await this.pythProxy.updateSingleUpdateFeeInWei(10);
+    // Set fee to 10
+    await setFeeTo(this.pythProxy, 10);
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 10);
 
     let ts = 1647273460;
@@ -439,17 +332,11 @@ contract("Pyth", function () {
   });
 
   it("should attest price updates with when required fee is given", 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 initial fee is zero
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
 
-    // Set fee
-    await this.pythProxy.updateSingleUpdateFeeInWei(10);
+    // Set fee to 10
+    await setFeeTo(this.pythProxy, 10);
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 10);
 
     let ts = 1647273460;
@@ -469,17 +356,11 @@ contract("Pyth", function () {
   });
 
   it("should attest price updates with required fee even if more fee is given", 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 initial fee is zero
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 0);
 
-    // Set fee
-    await this.pythProxy.updateSingleUpdateFeeInWei(10);
+    // Set fee to 10
+    await setFeeTo(this.pythProxy, 10);
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), 10);
 
     let ts = 1647273460;
@@ -590,7 +471,10 @@ contract("Pyth", function () {
     for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
       const price_id =
         "0x" + (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
-      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
+      await expectRevertCustomError(
+        this.pythProxy.getPrice(price_id),
+        "StalePrice"
+      );
     }
   });
 
@@ -606,10 +490,35 @@ contract("Pyth", function () {
     for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
       const price_id =
         "0x" + (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
-      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
+      await expectRevertCustomError(
+        this.pythProxy.getPrice(price_id),
+        "StalePrice"
+      );
     }
   });
 
+  /**
+   * Set valid time period to `newValidPeriod` by creating and submitting a
+   * governance instruction for it.
+   * @param contract Pyth contract
+   * @param {number} newValidPeriod
+   * @param {number=} governanceSequence Sequence number of the governance instruction. Defaults to 1.
+   */
+  async function setValidPeriodTo(
+    contract,
+    newValidPeriod,
+    governanceSequence
+  ) {
+    await createAndThenSubmitGovernanceInstructionVaa(
+      contract,
+      new governance.SetValidPeriodInstruction(
+        governance.CHAINS.ethereum,
+        BigInt(newValidPeriod)
+      ),
+      governanceSequence ?? 1
+    );
+  }
+
   it("changing validity time works", async function () {
     const latestTime = await time.latest();
     let rawBatch = generateRawBatchAttestation(latestTime, latestTime, 1337);
@@ -617,7 +526,8 @@ contract("Pyth", function () {
     await updatePriceFeeds(this.pythProxy, [rawBatch]);
 
     // Setting the validity time to 30 seconds
-    await this.pythProxy.updateValidTimePeriodSeconds(30);
+    await setValidPeriodTo(this.pythProxy, 30, 1);
+    assert.equal(await this.pythProxy.validTimePeriodSeconds(), 30);
 
     // Then prices should be available
     for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
@@ -636,11 +546,15 @@ contract("Pyth", function () {
       const price_id =
         "0x" + (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
 
-      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
+      await expectRevertCustomError(
+        this.pythProxy.getPrice(price_id),
+        "StalePrice"
+      );
     }
 
     // Setting the validity time to 120 seconds
-    await this.pythProxy.updateValidTimePeriodSeconds(120);
+    await setValidPeriodTo(this.pythProxy, 120, 2);
+    assert.equal(await this.pythProxy.validTimePeriodSeconds(), 120);
 
     // Then prices should be available because the valid period is now 120 seconds
     for (var i = 1; i <= RAW_BATCH_ATTESTATION_COUNT; i++) {
@@ -684,71 +598,6 @@ contract("Pyth", function () {
     }
   });
 
-  it("should accept a VM after adding its data source", async function () {
-    let newChainId = "42424";
-    let newEmitter = testPyth2WormholeEmitter.replace("a", "f");
-
-    await this.pythProxy.addDataSource(newChainId, newEmitter);
-
-    let currentTimestamp = (await web3.eth.getBlock("latest")).timestamp;
-    let rawBatch = generateRawBatchAttestation(
-      currentTimestamp - 5,
-      currentTimestamp,
-      1337
-    );
-    let vm = await signAndEncodeVM(
-      1,
-      1,
-      newChainId,
-      newEmitter,
-      0,
-      rawBatch,
-      [testSigner1PK],
-      0,
-      0
-    );
-
-    await this.pythProxy.updatePriceFeeds(["0x" + vm]);
-  });
-
-  it("should reject a VM after removing its data source", async function () {
-    // Add 2 new data sources to produce a non-trivial data source state.
-    let newChainId = "42424";
-    let newEmitter = testPyth2WormholeEmitter.replace("a", "f");
-    await this.pythProxy.addDataSource(newChainId, newEmitter);
-
-    let newChainId2 = "42425";
-    let newEmitter2 = testPyth2WormholeEmitter.replace("a", "e");
-    await this.pythProxy.addDataSource(newChainId2, newEmitter2);
-
-    // Remove the first one added
-    await this.pythProxy.removeDataSource(newChainId, newEmitter);
-
-    // Sign a batch with the removed data source
-    let currentTimestamp = (await web3.eth.getBlock("latest")).timestamp;
-    let rawBatch = generateRawBatchAttestation(
-      currentTimestamp - 5,
-      currentTimestamp,
-      1337
-    );
-    let vm = await signAndEncodeVM(
-      1,
-      1,
-      newChainId,
-      newEmitter,
-      0,
-      rawBatch,
-      [testSigner1PK],
-      0,
-      0
-    );
-
-    await expectRevertCustomError(
-      this.pythProxy.updatePriceFeeds(["0x" + vm]),
-      "InvalidUpdateDataSource"
-    );
-  });
-
   // Governance
 
   // Logics that apply to all governance messages
@@ -1233,16 +1082,6 @@ contract("Pyth", function () {
     // and adding it here will cause more complexity (and is not so short).
   });
 
-  // Renounce ownership works
-  it("Renouncing ownership should work", async function () {
-    await this.pythProxy.updateValidTimePeriodSeconds(100);
-    await this.pythProxy.renounceOwnership();
-    await expectRevert(
-      this.pythProxy.updateValidTimePeriodSeconds(60),
-      "Ownable: caller is not the owner"
-    );
-  });
-
   // Version
 
   it("Make sure version is the npm package version", async function () {

+ 1 - 0
third_party/pyth/xc-governance-sdk-js/src/index.ts

@@ -9,6 +9,7 @@ export {
   SetValidPeriodInstruction,
   RequestGovernanceDataSourceTransferInstruction,
   AuthorizeGovernanceDataSourceTransferInstruction,
+  Instruction,
 } from "./instructions";
 
 export { CHAINS, ChainId } from "@certusone/wormhole-sdk";

+ 1 - 1
third_party/pyth/xc-governance-sdk-js/src/instructions.ts

@@ -68,7 +68,7 @@ export class DataSource implements Serializable {
 // Magic is `PTGM` encoded as a 4 byte data: Pyth Governance Message
 const MAGIC: number = 0x5054474d;
 
-abstract class Instruction implements Serializable {
+export abstract class Instruction implements Serializable {
   constructor(
     private module: Module,
     private action: number,