From e39277dd9c9d3c97e25a048807addf8c1c60b086 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Mon, 26 Jun 2023 11:31:19 +0200 Subject: [PATCH] [eth] Add setWormholeAddress governance message (#917) * [eth] Add setWormholeAddress governance message * Address review feedbacks * Add signatures to errors This will help with debugging * Update abi --- governance/xc_governance_sdk_js/src/index.ts | 1 + .../xc_governance_sdk_js/src/instructions.ts | 11 ++ .../contracts/pyth/PythGovernance.sol | 53 +++++++ .../pyth/PythGovernanceInstructions.sol | 20 ++- .../ethereum/contracts/hardhat.config.ts | 2 +- target_chains/ethereum/contracts/test/pyth.js | 140 ++++++++++++++++++ .../ethereum/contracts/truffle-config.js | 2 +- .../ethereum/sdk/solidity/PythErrors.sol | 16 ++ .../sdk/solidity/abis/PythErrors.json | 5 + 9 files changed, 247 insertions(+), 3 deletions(-) diff --git a/governance/xc_governance_sdk_js/src/index.ts b/governance/xc_governance_sdk_js/src/index.ts index 41d60d3e..57a01901 100644 --- a/governance/xc_governance_sdk_js/src/index.ts +++ b/governance/xc_governance_sdk_js/src/index.ts @@ -2,6 +2,7 @@ export { DataSource, AptosAuthorizeUpgradeContractInstruction, EthereumUpgradeContractInstruction, + EthereumSetWormholeAddress, HexString20Bytes, HexString32Bytes, SetDataSourcesInstruction, diff --git a/governance/xc_governance_sdk_js/src/instructions.ts b/governance/xc_governance_sdk_js/src/instructions.ts index 476e13aa..fbac46ec 100644 --- a/governance/xc_governance_sdk_js/src/instructions.ts +++ b/governance/xc_governance_sdk_js/src/instructions.ts @@ -14,6 +14,7 @@ enum TargetAction { SetFee, SetValidPeriod, RequestGovernanceDataSourceTransfer, + SetWormholeAddress, } abstract class HexString implements Serializable { @@ -194,3 +195,13 @@ export class RequestGovernanceDataSourceTransferInstruction extends TargetInstru .build(); } } + +export class EthereumSetWormholeAddress extends TargetInstruction { + constructor(targetChainId: ChainId, private address: HexString20Bytes) { + super(TargetAction.SetWormholeAddress, targetChainId); + } + + protected serializePayload(): Buffer { + return this.address.serialize(); + } +} diff --git a/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol b/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol index 32030292..31c4a52a 100644 --- a/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol +++ b/target_chains/ethereum/contracts/contracts/pyth/PythGovernance.sol @@ -34,6 +34,10 @@ abstract contract PythGovernance is ); event FeeSet(uint oldFee, uint newFee); event ValidPeriodSet(uint oldValidPeriod, uint newValidPeriod); + event WormholeAddressSet( + address oldWormholeAddress, + address newWormholeAddress + ); function verifyGovernanceVM( bytes memory encodedVM @@ -86,6 +90,13 @@ abstract contract PythGovernance is ) { // RequestGovernanceDataSourceTransfer can be only part of AuthorizeGovernanceDataSourceTransfer message revert PythErrors.InvalidGovernanceMessage(); + } else if (gi.action == GovernanceAction.SetWormholeAddress) { + if (gi.targetChainId == 0) + revert PythErrors.InvalidGovernanceTarget(); + setWormholeAddress( + parseSetWormholeAddressPayload(gi.payload), + encodedVM + ); } else { revert PythErrors.InvalidGovernanceMessage(); } @@ -190,4 +201,46 @@ abstract contract PythGovernance is emit ValidPeriodSet(oldValidPeriod, validTimePeriodSeconds()); } + + function setWormholeAddress( + SetWormholeAddressPayload memory payload, + bytes memory encodedVM + ) internal { + address oldWormholeAddress = address(wormhole()); + setWormhole(payload.newWormholeAddress); + + // We want to verify that the new wormhole address is valid, so we make sure that it can + // parse and verify the same governance VAA that is used to set it. + (IWormhole.VM memory vm, bool valid, ) = wormhole().parseAndVerifyVM( + encodedVM + ); + + if (!valid) revert PythErrors.InvalidGovernanceMessage(); + + if (!isValidGovernanceDataSource(vm.emitterChainId, vm.emitterAddress)) + revert PythErrors.InvalidGovernanceMessage(); + + if (vm.sequence != lastExecutedGovernanceSequence()) + revert PythErrors.InvalidWormholeAddressToSet(); + + GovernanceInstruction memory gi = parseGovernanceInstruction( + vm.payload + ); + + if (gi.action != GovernanceAction.SetWormholeAddress) + revert PythErrors.InvalidWormholeAddressToSet(); + + // Purposefully, we don't check whether the chainId is the same as the current chainId because + // we might want to change the chain id of the wormhole contract. + + // The following check is not necessary for security, but is a sanity check that the new wormhole + // contract parses the payload correctly. + SetWormholeAddressPayload + memory newPayload = parseSetWormholeAddressPayload(gi.payload); + + if (newPayload.newWormholeAddress != payload.newWormholeAddress) + revert PythErrors.InvalidWormholeAddressToSet(); + + emit WormholeAddressSet(oldWormholeAddress, address(wormhole())); + } } diff --git a/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol b/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol index f70218d6..48fd5cba 100644 --- a/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol +++ b/target_chains/ethereum/contracts/contracts/pyth/PythGovernanceInstructions.sol @@ -30,7 +30,8 @@ contract PythGovernanceInstructions { SetDataSources, // 2 SetFee, // 3 SetValidPeriod, // 4 - RequestGovernanceDataSourceTransfer // 5 + RequestGovernanceDataSourceTransfer, // 5 + SetWormholeAddress // 6 } struct GovernanceInstruction { @@ -69,6 +70,10 @@ contract PythGovernanceInstructions { uint newValidPeriod; } + struct SetWormholeAddressPayload { + address newWormholeAddress; + } + /// @dev Parse a GovernanceInstruction function parseGovernanceInstruction( bytes memory encodedInstruction @@ -199,4 +204,17 @@ contract PythGovernanceInstructions { if (encodedPayload.length != index) revert PythErrors.InvalidGovernanceMessage(); } + + /// @dev Parse a UpdateWormholeAddressPayload (action 6) with minimal validation + function parseSetWormholeAddressPayload( + bytes memory encodedPayload + ) public pure returns (SetWormholeAddressPayload memory sw) { + uint index = 0; + + sw.newWormholeAddress = address(encodedPayload.toAddress(index)); + index += 20; + + if (encodedPayload.length != index) + revert PythErrors.InvalidGovernanceMessage(); + } } diff --git a/target_chains/ethereum/contracts/hardhat.config.ts b/target_chains/ethereum/contracts/hardhat.config.ts index 929339a7..0a7d3fea 100644 --- a/target_chains/ethereum/contracts/hardhat.config.ts +++ b/target_chains/ethereum/contracts/hardhat.config.ts @@ -80,7 +80,7 @@ module.exports = { settings: { optimizer: { enabled: true, - runs: 5000, + runs: 2000, }, }, }, diff --git a/target_chains/ethereum/contracts/test/pyth.js b/target_chains/ethereum/contracts/test/pyth.js index 70ba56ef..d17ee16a 100644 --- a/target_chains/ethereum/contracts/test/pyth.js +++ b/target_chains/ethereum/contracts/test/pyth.js @@ -10,8 +10,18 @@ const { const { assert, expect } = require("chai"); // Use "WormholeReceiver" if you are testing with Wormhole Receiver +const Setup = artifacts.require("Setup"); +const Implementation = artifacts.require("Implementation"); const Wormhole = artifacts.require("Wormhole"); +const ReceiverSetup = artifacts.require("ReceiverSetup"); +const ReceiverImplementation = artifacts.require("ReceiverImplementation"); +const WormholeReceiver = artifacts.require("WormholeReceiver"); + +const wormholeGovernanceChainId = governance.CHAINS.solana; +const wormholeGovernanceContract = + "0x0000000000000000000000000000000000000000000000000000000000000004"; + const PythUpgradable = artifacts.require("PythUpgradable"); const MockPythUpgrade = artifacts.require("MockPythUpgrade"); const MockUpgradeableProxy = artifacts.require("MockUpgradeableProxy"); @@ -1069,6 +1079,136 @@ contract("Pyth", function () { // and adding it here will cause more complexity (and is not so short). }); + it("Setting wormhole address should work", async function () { + // Deploy a new wormhole contract + const newSetup = await Setup.new(); + const newImpl = await Implementation.new(); + + // encode initialisation data + const initData = newSetup.contract.methods + .setup( + newImpl.address, + [testSigner1.address], + governance.CHAINS.polygon, // changing the chain id to polygon + wormholeGovernanceChainId, + wormholeGovernanceContract + ) + .encodeABI(); + + const newWormhole = await Wormhole.new(newSetup.address, initData); + + // Creating the vaa to set the new wormhole address + const data = new governance.EthereumSetWormholeAddress( + governance.CHAINS.ethereum, + new governance.HexString20Bytes(newWormhole.address) + ).serialize(); + + const vaa = await createVAAFromUint8Array( + data, + testGovernanceChainId, + testGovernanceEmitter, + 1 + ); + + assert.equal(await this.pythProxy.chainId(), governance.CHAINS.ethereum); + + const oldWormholeAddress = await this.pythProxy.wormhole(); + + const receipt = await this.pythProxy.executeGovernanceInstruction(vaa); + expectEvent(receipt, "WormholeAddressSet", { + oldWormholeAddress: oldWormholeAddress, + newWormholeAddress: newWormhole.address, + }); + + assert.equal(await this.pythProxy.wormhole(), newWormhole.address); + assert.equal(await this.pythProxy.chainId(), governance.CHAINS.polygon); + }); + + it("Setting wormhole address to WormholeReceiver should work", async function () { + // Deploy a new wormhole receiver contract + const newReceiverSetup = await ReceiverSetup.new(); + const newReceiverImpl = await ReceiverImplementation.new(); + + // encode initialisation data + const initData = newReceiverSetup.contract.methods + .setup( + newReceiverImpl.address, + [testSigner1.address], + governance.CHAINS.polygon, // changing the chain id to polygon + wormholeGovernanceChainId, + wormholeGovernanceContract + ) + .encodeABI(); + + const newWormholeReceiver = await WormholeReceiver.new( + newReceiverSetup.address, + initData + ); + + // Creating the vaa to set the new wormhole address + const data = new governance.EthereumSetWormholeAddress( + governance.CHAINS.ethereum, + new governance.HexString20Bytes(newWormholeReceiver.address) + ).serialize(); + + const vaa = await createVAAFromUint8Array( + data, + testGovernanceChainId, + testGovernanceEmitter, + 1 + ); + + assert.equal(await this.pythProxy.chainId(), governance.CHAINS.ethereum); + + const oldWormholeAddress = await this.pythProxy.wormhole(); + + const receipt = await this.pythProxy.executeGovernanceInstruction(vaa); + expectEvent(receipt, "WormholeAddressSet", { + oldWormholeAddress: oldWormholeAddress, + newWormholeAddress: newWormholeReceiver.address, + }); + + assert.equal(await this.pythProxy.wormhole(), newWormholeReceiver.address); + assert.equal(await this.pythProxy.chainId(), governance.CHAINS.polygon); + }); + + it("Setting wormhole address to a wrong contract should reject", async function () { + // Deploy a new wormhole contract + const newSetup = await Setup.new(); + const newImpl = await Implementation.new(); + + // encode initialisation data + const initData = newSetup.contract.methods + .setup( + newImpl.address, + [testSigner2.address], // A wrong signer + governance.CHAINS.ethereum, + wormholeGovernanceChainId, + wormholeGovernanceContract + ) + .encodeABI(); + + const newWormhole = await Wormhole.new(newSetup.address, initData); + + // Creating the vaa to set the new wormhole address + const data = new governance.EthereumSetWormholeAddress( + governance.CHAINS.ethereum, + new governance.HexString20Bytes(newWormhole.address) + ).serialize(); + + const wrongVaa = await createVAAFromUint8Array( + data, + testGovernanceChainId, + testGovernanceEmitter, + 1 + ); + + await expectRevertCustomError( + this.pythProxy.executeGovernanceInstruction(wrongVaa), + "InvalidGovernanceMessage" + ); + }); + // Version it("Make sure version is the npm package version", async function () { diff --git a/target_chains/ethereum/contracts/truffle-config.js b/target_chains/ethereum/contracts/truffle-config.js index 9254ca86..dfd86a25 100644 --- a/target_chains/ethereum/contracts/truffle-config.js +++ b/target_chains/ethereum/contracts/truffle-config.js @@ -288,7 +288,7 @@ module.exports = { settings: { optimizer: { enabled: true, - runs: 5000, + runs: 2000, }, }, }, diff --git a/target_chains/ethereum/sdk/solidity/PythErrors.sol b/target_chains/ethereum/sdk/solidity/PythErrors.sol index b134f9d8..2b545774 100644 --- a/target_chains/ethereum/sdk/solidity/PythErrors.sol +++ b/target_chains/ethereum/sdk/solidity/PythErrors.sol @@ -4,29 +4,45 @@ pragma solidity ^0.8.0; library PythErrors { // Function arguments are invalid (e.g., the arguments lengths mismatch) + // Signature: 0xa9cb9e0d error InvalidArgument(); // Update data is coming from an invalid data source. + // Signature: 0xe60dce71 error InvalidUpdateDataSource(); // Update data is invalid (e.g., deserialization error) + // Signature: 0xe69ffece error InvalidUpdateData(); // Insufficient fee is paid to the method. + // Signature: 0x025dbdd4 error InsufficientFee(); // There is no fresh update, whereas expected fresh updates. + // Signature: 0xde2c57fa error NoFreshUpdate(); // There is no price feed found within the given range or it does not exists. + // Signature: 0x45805f5d error PriceFeedNotFoundWithinRange(); // Price feed not found or it is not pushed on-chain yet. + // Signature: 0x14aebe68 error PriceFeedNotFound(); // Requested price is stale. + // Signature: 0x19abf40e error StalePrice(); // Given message is not a valid Wormhole VAA. + // Signature: 0x2acbe915 error InvalidWormholeVaa(); // Governance message is invalid (e.g., deserialization error). + // Signature: 0x97363b35 error InvalidGovernanceMessage(); // Governance message is not for this contract. + // Signature: 0x63daeb77 error InvalidGovernanceTarget(); // Governance message is coming from an invalid data source. + // Signature: 0x360f2d87 error InvalidGovernanceDataSource(); // Governance message is old. + // Signature: 0x88d1b847 error OldGovernanceMessage(); + // The wormhole address to set in SetWormholeAddress governance is invalid. + // Signature: 0x13d3ed82 + error InvalidWormholeAddressToSet(); } diff --git a/target_chains/ethereum/sdk/solidity/abis/PythErrors.json b/target_chains/ethereum/sdk/solidity/abis/PythErrors.json index d59027e9..f8fdc192 100644 --- a/target_chains/ethereum/sdk/solidity/abis/PythErrors.json +++ b/target_chains/ethereum/sdk/solidity/abis/PythErrors.json @@ -34,6 +34,11 @@ "name": "InvalidUpdateDataSource", "type": "error" }, + { + "inputs": [], + "name": "InvalidWormholeAddressToSet", + "type": "error" + }, { "inputs": [], "name": "InvalidWormholeVaa",