From 33f5b8a5bfd6967c7f8b86d7d98d0a21698378f8 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Wed, 30 Nov 2022 12:13:46 +0100 Subject: [PATCH] [eth] Remove ownership entirely in favor of governance (#405) --- ethereum/.env.template | 1 + ethereum/contracts/pyth/Pyth.sol | 12 + ethereum/contracts/pyth/PythUpgradable.sol | 77 +--- ethereum/forge-test/utils/PythTestUtils.t.sol | 9 +- .../10_pyth_enable_governance.js | 30 -- .../migrations/prod-receiver/3_deploy_pyth.js | 26 +- .../prod/11_pyth_renounce_ownership.js | 15 - ethereum/migrations/prod/2_deploy_pyth.js | 13 + .../prod/9_pyth_enable_governance.js | 25 -- .../test/10_pyth_renounce_ownership.js | 22 -- ethereum/migrations/test/3_deploy_pyth.js | 22 +- .../test/9_pyth_enable_governance.js | 27 -- ethereum/test/pyth.js | 347 +++++------------- .../pyth/xc-governance-sdk-js/src/index.ts | 1 + .../xc-governance-sdk-js/src/instructions.ts | 2 +- 15 files changed, 165 insertions(+), 464 deletions(-) delete mode 100644 ethereum/migrations/prod-receiver/10_pyth_enable_governance.js delete mode 100644 ethereum/migrations/prod/11_pyth_renounce_ownership.js delete mode 100644 ethereum/migrations/prod/9_pyth_enable_governance.js delete mode 100644 ethereum/migrations/test/10_pyth_renounce_ownership.js delete mode 100644 ethereum/migrations/test/9_pyth_enable_governance.js diff --git a/ethereum/.env.template b/ethereum/.env.template index 8e8875a1..b0067e02 100644 --- a/ethereum/.env.template +++ b/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 diff --git a/ethereum/contracts/pyth/Pyth.sol b/ethereum/contracts/pyth/Pyth.sol index 552594d9..95a39504 100644 --- a/ethereum/contracts/pyth/Pyth.sol +++ b/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); } diff --git a/ethereum/contracts/pyth/PythUpgradable.sol b/ethereum/contracts/pyth/PythUpgradable.sol index 6c84733e..7f9a2a22 100644 --- a/ethereum/contracts/pyth/PythUpgradable.sol +++ b/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) { diff --git a/ethereum/forge-test/utils/PythTestUtils.t.sol b/ethereum/forge-test/utils/PythTestUtils.t.sol index 67cd89dd..d6436b88 100644 --- a/ethereum/forge-test/utils/PythTestUtils.t.sol +++ b/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); diff --git a/ethereum/migrations/prod-receiver/10_pyth_enable_governance.js b/ethereum/migrations/prod-receiver/10_pyth_enable_governance.js deleted file mode 100644 index c8b24f8e..00000000 --- a/ethereum/migrations/prod-receiver/10_pyth_enable_governance.js +++ /dev/null @@ -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 - ); -}; diff --git a/ethereum/migrations/prod-receiver/3_deploy_pyth.js b/ethereum/migrations/prod-receiver/3_deploy_pyth.js index e06c180b..2abb09a3 100644 --- a/ethereum/migrations/prod-receiver/3_deploy_pyth.js +++ b/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, ], diff --git a/ethereum/migrations/prod/11_pyth_renounce_ownership.js b/ethereum/migrations/prod/11_pyth_renounce_ownership.js deleted file mode 100644 index c5dfa24a..00000000 --- a/ethereum/migrations/prod/11_pyth_renounce_ownership.js +++ /dev/null @@ -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(); -}; diff --git a/ethereum/migrations/prod/2_deploy_pyth.js b/ethereum/migrations/prod/2_deploy_pyth.js index e06c180b..e221a43a 100644 --- a/ethereum/migrations/prod/2_deploy_pyth.js +++ b/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, ], diff --git a/ethereum/migrations/prod/9_pyth_enable_governance.js b/ethereum/migrations/prod/9_pyth_enable_governance.js deleted file mode 100644 index 6a91b484..00000000 --- a/ethereum/migrations/prod/9_pyth_enable_governance.js +++ /dev/null @@ -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 - ); -}; diff --git a/ethereum/migrations/test/10_pyth_renounce_ownership.js b/ethereum/migrations/test/10_pyth_renounce_ownership.js deleted file mode 100644 index 76c23554..00000000 --- a/ethereum/migrations/test/10_pyth_renounce_ownership.js +++ /dev/null @@ -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(); -}; diff --git a/ethereum/migrations/test/3_deploy_pyth.js b/ethereum/migrations/test/3_deploy_pyth.js index 5a363b04..7de88adb 100644 --- a/ethereum/migrations/test/3_deploy_pyth.js +++ b/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, ], diff --git a/ethereum/migrations/test/9_pyth_enable_governance.js b/ethereum/migrations/test/9_pyth_enable_governance.js deleted file mode 100644 index c89b95f8..00000000 --- a/ethereum/migrations/test/9_pyth_enable_governance.js +++ /dev/null @@ -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 - ); -}; diff --git a/ethereum/test/pyth.js b/ethereum/test/pyth.js index d317167b..91b81847 100644 --- a/ethereum/test/pyth.js +++ b/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); + assert.equal(owner, "0x0000000000000000000000000000000000000000"); }); - 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); - }); - - 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 () { diff --git a/third_party/pyth/xc-governance-sdk-js/src/index.ts b/third_party/pyth/xc-governance-sdk-js/src/index.ts index 35612d76..bf215a56 100644 --- a/third_party/pyth/xc-governance-sdk-js/src/index.ts +++ b/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"; diff --git a/third_party/pyth/xc-governance-sdk-js/src/instructions.ts b/third_party/pyth/xc-governance-sdk-js/src/instructions.ts index 1de67e77..53714a04 100644 --- a/third_party/pyth/xc-governance-sdk-js/src/instructions.ts +++ b/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,