瀏覽代碼

[eth] Use PythErrors everywhere (#404)

* Remove unnecessary check
* Use PythErrors everywhere
Ali Behjati 3 年之前
父節點
當前提交
c3461e5e1c

+ 36 - 64
ethereum/contracts/pyth/Pyth.sol

@@ -7,6 +7,7 @@ import "../libraries/external/UnsafeBytesLib.sol";
 import "@pythnetwork/pyth-sdk-solidity/AbstractPyth.sol";
 import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
 
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 import "./PythGetters.sol";
 import "./PythSetters.sol";
 import "./PythInternalStructs.sol";
@@ -21,11 +22,10 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
     ) internal {
         setWormhole(wormhole);
 
-        require(
-            dataSourceEmitterChainIds.length ==
-                dataSourceEmitterAddresses.length,
-            "data source arguments should have the same length"
-        );
+        if (
+            dataSourceEmitterChainIds.length !=
+            dataSourceEmitterAddresses.length
+        ) revert PythErrors.InvalidArgument();
 
         for (uint i = 0; i < dataSourceEmitterChainIds.length; i++) {
             PythInternalStructs.DataSource memory ds = PythInternalStructs
@@ -34,10 +34,8 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
                     dataSourceEmitterAddresses[i]
                 );
 
-            require(
-                !PythGetters.isValidDataSource(ds.chainId, ds.emitterAddress),
-                "Data source already added"
-            );
+            if (PythGetters.isValidDataSource(ds.chainId, ds.emitterAddress))
+                revert PythErrors.InvalidArgument();
 
             _state.isValidDataSource[hashDataSource(ds)] = true;
             _state.validDataSources.push(ds);
@@ -57,7 +55,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         bytes[] calldata updateData
     ) public payable override {
         uint requiredFee = getUpdateFee(updateData);
-        require(msg.value >= requiredFee, "insufficient paid fee amount");
+        if (msg.value < requiredFee) revert PythErrors.InsufficientFee();
 
         for (uint i = 0; i < updateData.length; ) {
             updatePriceBatchFromVm(updateData[i]);
@@ -245,10 +243,8 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
                 }
             }
 
-            require(
-                attestationIndex <= attestationSize,
-                "INTERNAL: Consumed more than `attestationSize` bytes"
-            );
+            if (attestationIndex > attestationSize)
+                revert PythErrors.InvalidUpdateData();
         }
     }
 
@@ -259,10 +255,8 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         bytes32[] calldata priceIds,
         uint64[] calldata publishTimes
     ) external payable override {
-        require(
-            priceIds.length == publishTimes.length,
-            "priceIds and publishTimes arrays should have same length"
-        );
+        if (priceIds.length != publishTimes.length)
+            revert PythErrors.InvalidArgument();
 
         for (uint i = 0; i < priceIds.length; ) {
             // If the price does not exist, then the publish time is zero and
@@ -277,9 +271,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
             }
         }
 
-        revert(
-            "no prices in the submitted batch have fresh prices, so this update will have no effect"
-        );
+        revert PythErrors.NoFreshUpdate();
     }
 
     // This is an overwrite of the same method in AbstractPyth.sol
@@ -295,10 +287,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         price.price = info.price;
         price.conf = info.conf;
 
-        require(
-            price.publishTime != 0,
-            "price feed for the given id is not pushed or does not exist"
-        );
+        if (price.publishTime == 0) revert PythErrors.PriceFeedNotFound();
     }
 
     // This is an overwrite of the same method in AbstractPyth.sol
@@ -314,10 +303,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         price.price = info.emaPrice;
         price.conf = info.emaConf;
 
-        require(
-            price.publishTime != 0,
-            "price feed for the given id is not pushed or does not exist"
-        );
+        if (price.publishTime == 0) revert PythErrors.PriceFeedNotFound();
     }
 
     function parseBatchAttestationHeader(
@@ -334,18 +320,20 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
             {
                 uint32 magic = UnsafeBytesLib.toUint32(encoded, index);
                 index += 4;
-                require(magic == 0x50325748, "invalid magic value");
+                if (magic != 0x50325748) revert PythErrors.InvalidUpdateData();
 
                 uint16 versionMajor = UnsafeBytesLib.toUint16(encoded, index);
                 index += 2;
-                require(versionMajor == 3, "invalid version major, expected 3");
+                if (versionMajor != 3) revert PythErrors.InvalidUpdateData();
 
-                uint16 versionMinor = UnsafeBytesLib.toUint16(encoded, index);
+                // This value is only used as the check below which currently
+                // never reverts
+                // uint16 versionMinor = UnsafeBytesLib.toUint16(encoded, index);
                 index += 2;
-                require(
-                    versionMinor >= 0,
-                    "invalid version minor, expected 0 or more"
-                );
+
+                // This check is always false as versionMinor is 0, so it is commented.
+                // in the future that the minor version increases this will have effect.
+                // if(versionMinor < 0) revert InvalidUpdateData();
 
                 uint16 hdrSize = UnsafeBytesLib.toUint16(encoded, index);
                 index += 2;
@@ -370,10 +358,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
                 index += hdrSize;
 
                 // Payload ID of 2 required for batch headerBa
-                require(
-                    payloadId == 2,
-                    "invalid payload ID, expected 2 for BatchPriceAttestation"
-                );
+                if (payloadId != 2) revert PythErrors.InvalidUpdateData();
             }
 
             // Parse the number of attestations
@@ -386,10 +371,8 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
 
             // Given the message is valid the arithmetic below should not overflow, and
             // even if it overflows then the require would fail.
-            require(
-                encoded.length == (index + (attestationSize * nAttestations)),
-                "invalid BatchPriceAttestation size"
-            );
+            if (encoded.length != (index + (attestationSize * nAttestations)))
+                revert PythErrors.InvalidUpdateData();
         }
     }
 
@@ -398,12 +381,11 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
     ) internal view returns (IWormhole.VM memory vm) {
         {
             bool valid;
-            string memory reason;
-            (vm, valid, reason) = wormhole().parseAndVerifyVM(encodedVm);
-            require(valid, reason);
+            (vm, valid, ) = wormhole().parseAndVerifyVM(encodedVm);
+            if (!valid) revert PythErrors.InvalidWormholeVaa();
         }
 
-        require(verifyPythVM(vm), "invalid data source chain/emitter ID");
+        if (!verifyPythVM(vm)) revert PythErrors.InvalidUpdateDataSource();
     }
 
     function parsePriceFeedUpdates(
@@ -420,10 +402,8 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
         unchecked {
             {
                 uint requiredFee = getUpdateFee(updateData);
-                require(
-                    msg.value >= requiredFee,
-                    "insufficient paid fee amount"
-                );
+                if (msg.value < requiredFee)
+                    revert PythErrors.InsufficientFee();
             }
 
             priceFeeds = new PythStructs.PriceFeed[](priceIds.length);
@@ -482,10 +462,6 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
                             index,
                             attestationSize
                         );
-                    require(
-                        info.publishTime != 0,
-                        "price feed for the given id is not pushed or does not exist"
-                    );
 
                     priceFeeds[k].id = priceId;
                     priceFeeds[k].price.price = info.price;
@@ -513,10 +489,9 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
             }
 
             for (uint k = 0; k < priceIds.length; k++) {
-                require(
-                    priceFeeds[k].id != 0,
-                    "1 or more price feeds are not found in the updateData or they are out of the given time range"
-                );
+                if (priceFeeds[k].id == 0) {
+                    revert PythErrors.PriceFeedNotFoundWithinRange();
+                }
             }
         }
     }
@@ -526,10 +501,7 @@ abstract contract Pyth is PythGetters, PythSetters, AbstractPyth {
     ) public view override returns (PythStructs.PriceFeed memory priceFeed) {
         // Look up the latest price info for the given ID
         PythInternalStructs.PriceInfo memory info = latestPriceInfo(id);
-        require(
-            info.publishTime != 0,
-            "price feed for the given id is not pushed or does not exist"
-        );
+        if (info.publishTime == 0) revert PythErrors.PriceFeedNotFound();
 
         priceFeed.id = id;
         priceFeed.price.price = info.price;

+ 30 - 39
ethereum/contracts/pyth/PythGovernance.sol

@@ -7,6 +7,7 @@ import "./PythGovernanceInstructions.sol";
 import "./PythInternalStructs.sol";
 import "./PythGetters.sol";
 import "./PythSetters.sol";
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 
 import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Upgrade.sol";
 
@@ -37,19 +38,17 @@ abstract contract PythGovernance is
     function verifyGovernanceVM(
         bytes memory encodedVM
     ) internal returns (IWormhole.VM memory parsedVM) {
-        (IWormhole.VM memory vm, bool valid, string memory reason) = wormhole()
-            .parseAndVerifyVM(encodedVM);
-
-        require(valid, reason);
-        require(
-            isValidGovernanceDataSource(vm.emitterChainId, vm.emitterAddress),
-            "VAA is not coming from the governance data source"
+        (IWormhole.VM memory vm, bool valid, ) = wormhole().parseAndVerifyVM(
+            encodedVM
         );
 
-        require(
-            vm.sequence > lastExecutedGovernanceSequence(),
-            "VAA is older than the last executed governance VAA"
-        );
+        if (!valid) revert PythErrors.InvalidWormholeVaa();
+
+        if (!isValidGovernanceDataSource(vm.emitterChainId, vm.emitterAddress))
+            revert PythErrors.InvalidGovernanceDataSource();
+
+        if (vm.sequence <= lastExecutedGovernanceSequence())
+            revert PythErrors.OldGovernanceMessage();
 
         setLastExecutedGovernanceSequence(vm.sequence);
 
@@ -63,16 +62,12 @@ abstract contract PythGovernance is
             vm.payload
         );
 
-        require(
-            gi.targetChainId == chainId() || gi.targetChainId == 0,
-            "invalid target chain for this governance instruction"
-        );
+        if (gi.targetChainId != chainId() && gi.targetChainId != 0)
+            revert PythErrors.InvalidGovernanceTarget();
 
         if (gi.action == GovernanceAction.UpgradeContract) {
-            require(
-                gi.targetChainId != 0,
-                "upgrade with chain id 0 is not possible"
-            );
+            if (gi.targetChainId == 0)
+                revert PythErrors.InvalidGovernanceTarget();
             upgradeContract(parseUpgradeContractPayload(gi.payload));
         } else if (
             gi.action == GovernanceAction.AuthorizeGovernanceDataSourceTransfer
@@ -89,11 +84,10 @@ abstract contract PythGovernance is
         } else if (
             gi.action == GovernanceAction.RequestGovernanceDataSourceTransfer
         ) {
-            revert(
-                "RequestGovernanceDataSourceTransfer can be only part of AuthorizeGovernanceDataSourceTransfer message"
-            );
+            // RequestGovernanceDataSourceTransfer can be only part of AuthorizeGovernanceDataSourceTransfer message
+            revert PythErrors.InvalidGovernanceMessage();
         } else {
-            revert("invalid governance action");
+            revert PythErrors.InvalidGovernanceMessage();
         }
     }
 
@@ -119,21 +113,19 @@ abstract contract PythGovernance is
         // If it's valid then its emitter can take over the governance from the current emitter.
         // The VAA is checked here to ensure that the new governance data source is valid and can send message
         // through wormhole.
-        (IWormhole.VM memory vm, bool valid, string memory reason) = wormhole()
-            .parseAndVerifyVM(payload.claimVaa);
-        require(valid, reason);
+        (IWormhole.VM memory vm, bool valid, ) = wormhole().parseAndVerifyVM(
+            payload.claimVaa
+        );
+        if (!valid) revert PythErrors.InvalidWormholeVaa();
 
         GovernanceInstruction memory gi = parseGovernanceInstruction(
             vm.payload
         );
-        require(
-            gi.targetChainId == chainId() || gi.targetChainId == 0,
-            "invalid target chain for this governance instruction"
-        );
-        require(
-            gi.action == GovernanceAction.RequestGovernanceDataSourceTransfer,
-            "governance data source change inner vaa is not of claim action type"
-        );
+        if (gi.targetChainId != chainId() && gi.targetChainId != 0)
+            revert PythErrors.InvalidGovernanceTarget();
+
+        if (gi.action != GovernanceAction.RequestGovernanceDataSourceTransfer)
+            revert PythErrors.InvalidGovernanceMessage();
 
         RequestGovernanceDataSourceTransferPayload
             memory claimPayload = parseRequestGovernanceDataSourceTransferPayload(
@@ -141,11 +133,10 @@ abstract contract PythGovernance is
             );
 
         // Governance data source index is used to prevent replay attacks, so a claimVaa cannot be used twice.
-        require(
-            governanceDataSourceIndex() <
-                claimPayload.governanceDataSourceIndex,
-            "cannot upgrade to an older governance data source"
-        );
+        if (
+            governanceDataSourceIndex() >=
+            claimPayload.governanceDataSourceIndex
+        ) revert PythErrors.OldGovernanceMessage();
 
         setGovernanceDataSourceIndex(claimPayload.governanceDataSourceIndex);
 

+ 15 - 25
ethereum/contracts/pyth/PythGovernanceInstructions.sol

@@ -5,6 +5,7 @@ pragma solidity ^0.8.0;
 
 import "../libraries/external/BytesLib.sol";
 import "./PythInternalStructs.sol";
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 
 /**
  * @dev `PythGovernanceInstructions` defines a set of structs and parsing functions
@@ -75,17 +76,16 @@ contract PythGovernanceInstructions {
         uint index = 0;
 
         uint32 magic = encodedInstruction.toUint32(index);
-        require(magic == MAGIC, "invalid magic for GovernanceInstruction");
+
+        if (magic != MAGIC) revert PythErrors.InvalidGovernanceMessage();
+
         index += 4;
 
         uint8 modNumber = encodedInstruction.toUint8(index);
         gi.module = GovernanceModule(modNumber);
         index += 1;
 
-        require(
-            gi.module == MODULE,
-            "invalid module for GovernanceInstruction"
-        );
+        if (gi.module != MODULE) revert PythErrors.InvalidGovernanceTarget();
 
         uint8 actionNumber = encodedInstruction.toUint8(index);
         gi.action = GovernanceAction(actionNumber);
@@ -112,10 +112,8 @@ contract PythGovernanceInstructions {
         uc.newImplementation = address(encodedPayload.toAddress(index));
         index += 20;
 
-        require(
-            encodedPayload.length == index,
-            "invalid length for UpgradeContractPayload"
-        );
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
     }
 
     /// @dev Parse a AuthorizeGovernanceDataSourceTransferPayload (action 2) with minimal validation
@@ -142,10 +140,8 @@ contract PythGovernanceInstructions {
         sgdsClaim.governanceDataSourceIndex = encodedPayload.toUint32(index);
         index += 4;
 
-        require(
-            encodedPayload.length == index,
-            "invalid length for RequestGovernanceDataSourceTransferPayload"
-        );
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
     }
 
     /// @dev Parse a SetDataSourcesPayload (action 3) with minimal validation
@@ -169,10 +165,8 @@ contract PythGovernanceInstructions {
             index += 32;
         }
 
-        require(
-            encodedPayload.length == index,
-            "invalid length for SetDataSourcesPayload"
-        );
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
     }
 
     /// @dev Parse a SetFeePayload (action 4) with minimal validation
@@ -189,10 +183,8 @@ contract PythGovernanceInstructions {
 
         sf.newFee = uint256(val) * uint256(10) ** uint256(expo);
 
-        require(
-            encodedPayload.length == index,
-            "invalid length for SetFeePayload"
-        );
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
     }
 
     /// @dev Parse a SetValidPeriodPayload (action 5) with minimal validation
@@ -204,9 +196,7 @@ contract PythGovernanceInstructions {
         svp.newValidPeriod = uint256(encodedPayload.toUint64(index));
         index += 8;
 
-        require(
-            encodedPayload.length == index,
-            "invalid length for SetValidPeriodPayload"
-        );
+        if (encodedPayload.length != index)
+            revert PythErrors.InvalidGovernanceMessage();
     }
 }

+ 5 - 5
ethereum/contracts/pyth/PythUpgradable.sol

@@ -11,6 +11,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
 import "./PythGovernance.sol";
 import "./Pyth.sol";
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 
 contract PythUpgradable is
     Initializable,
@@ -126,11 +127,10 @@ contract PythUpgradable is
         _upgradeToAndCallUUPS(payload.newImplementation, new bytes(0), false);
 
         // Calling a method using `this.<method>` will cause a contract call that will use
-        // the new contract.
-        require(
-            this.pythUpgradableMagic() == 0x97a6f304,
-            "the new implementation is not a Pyth contract"
-        );
+        // the new contract. This call will fail if the method does not exists or the magic
+        // is different.
+        if (this.pythUpgradableMagic() != 0x97a6f304)
+            revert PythErrors.InvalidGovernanceMessage();
 
         emit ContractUpgraded(oldImplementation, _getImplementation());
     }

+ 3 - 10
ethereum/forge-test/GasBenchmark.t.sol

@@ -6,6 +6,7 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import "forge-std/Test.sol";
 
 import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
 import "./utils/WormholeTestUtils.t.sol";
 import "./utils/PythTestUtils.t.sol";
@@ -139,11 +140,7 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
     function testBenchmarkUpdatePriceFeedsIfNecessaryNotFresh() public {
         // Since the price is not advanced, the publishTimes are the same as the
         // ones in the contract.
-        vm.expectRevert(
-            bytes(
-                "no prices in the submitted batch have fresh prices, so this update will have no effect"
-            )
-        );
+        vm.expectRevert(PythErrors.NoFreshUpdate.selector);
 
         pyth.updatePriceFeedsIfNecessary{value: cachedPricesUpdateFee}(
             cachedPricesUpdateData,
@@ -183,11 +180,7 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
         bytes32[] memory ids = new bytes32[](1);
         ids[0] = priceIds[0];
 
-        vm.expectRevert(
-            bytes(
-                "1 or more price feeds are not found in the updateData or they are out of the given time range"
-            )
-        );
+        vm.expectRevert(PythErrors.PriceFeedNotFoundWithinRange.selector);
         pyth.parsePriceFeedUpdates{value: freshPricesUpdateFee}(
             freshPricesUpdateData,
             ids,

+ 6 - 13
ethereum/forge-test/Pyth.t.sol

@@ -6,6 +6,7 @@ import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
 import "forge-std/Test.sol";
 
 import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
+import "@pythnetwork/pyth-sdk-solidity/PythErrors.sol";
 import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
 import "./utils/WormholeTestUtils.t.sol";
 import "./utils/PythTestUtils.t.sol";
@@ -364,7 +365,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
         // Since attestations are not empty the fee should be at least 1
         assertGe(updateFee, 1);
 
-        vm.expectRevert(bytes("insufficient paid fee amount"));
+        vm.expectRevert(PythErrors.InsufficientFee.selector);
 
         pyth.parsePriceFeedUpdates{value: updateFee - 1}(
             updateData,
@@ -425,7 +426,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
 
         uint updateFee = pyth.getUpdateFee(updateData);
 
-        vm.expectRevert(bytes("invalid data source chain/emitter ID"));
+        vm.expectRevert(PythErrors.InvalidUpdateDataSource.selector);
         pyth.parsePriceFeedUpdates{value: updateFee}(
             updateData,
             priceIds,
@@ -455,7 +456,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
 
         uint updateFee = pyth.getUpdateFee(updateData);
 
-        vm.expectRevert(bytes("invalid data source chain/emitter ID"));
+        vm.expectRevert(PythErrors.InvalidUpdateDataSource.selector);
         pyth.parsePriceFeedUpdates{value: updateFee}(
             updateData,
             priceIds,
@@ -480,11 +481,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
         bytes32[] memory priceIds = new bytes32[](1);
         priceIds[0] = bytes32(uint(2));
 
-        vm.expectRevert(
-            bytes(
-                "1 or more price feeds are not found in the updateData or they are out of the given time range"
-            )
-        );
+        vm.expectRevert(PythErrors.PriceFeedNotFoundWithinRange.selector);
         pyth.parsePriceFeedUpdates{value: updateFee}(
             updateData,
             priceIds,
@@ -520,11 +517,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
         );
 
         // Request for parse after the time range should revert.
-        vm.expectRevert(
-            bytes(
-                "1 or more price feeds are not found in the updateData or they are out of the given time range"
-            )
-        );
+        vm.expectRevert(PythErrors.PriceFeedNotFoundWithinRange.selector);
         pyth.parsePriceFeedUpdates{value: updateFee}(
             updateData,
             priceIds,

+ 7 - 7
ethereum/package-lock.json

@@ -13,7 +13,7 @@
         "@certusone/wormhole-sdk-wasm": "^0.0.1",
         "@openzeppelin/contracts": "^4.5.0",
         "@openzeppelin/contracts-upgradeable": "^4.5.2",
-        "@pythnetwork/pyth-sdk-solidity": "^2.1.0",
+        "@pythnetwork/pyth-sdk-solidity": "^2.2.0",
         "@pythnetwork/xc-governance-sdk": "file:../third_party/pyth/xc-governance-sdk-js",
         "dotenv": "^10.0.0",
         "elliptic": "^6.5.2",
@@ -5642,9 +5642,9 @@
       "integrity": "sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA="
     },
     "node_modules/@pythnetwork/pyth-sdk-solidity": {
-      "version": "2.1.0",
-      "resolved": "https://registry.npmjs.org/@pythnetwork/pyth-sdk-solidity/-/pyth-sdk-solidity-2.1.0.tgz",
-      "integrity": "sha512-jHzqw+BHaCOAYwRNCgAUhcbNZrB5f3Arly3PaYN3/Tg7/5RQ95a9FD15XvJB1DB3yymUPIkmLYMur7Sh+e1G4A=="
+      "version": "2.2.0",
+      "resolved": "https://registry.npmjs.org/@pythnetwork/pyth-sdk-solidity/-/pyth-sdk-solidity-2.2.0.tgz",
+      "integrity": "sha512-LsRMmaf9MTflGSymqOJMepFk/3R7DyxMOJfLDB5RDSieyiq+RJ5IYIYnXAFsMrqkjibOtVxARcortHtE9VWwhw=="
     },
     "node_modules/@pythnetwork/xc-governance-sdk": {
       "resolved": "../third_party/pyth/xc-governance-sdk-js",
@@ -45038,9 +45038,9 @@
       "integrity": "sha1-p3c2C1s5oaLlEG+OhY8v0tBgxXA="
     },
     "@pythnetwork/pyth-sdk-solidity": {
-      "version": "2.1.0",
-      "resolved": "https://registry.npmjs.org/@pythnetwork/pyth-sdk-solidity/-/pyth-sdk-solidity-2.1.0.tgz",
-      "integrity": "sha512-jHzqw+BHaCOAYwRNCgAUhcbNZrB5f3Arly3PaYN3/Tg7/5RQ95a9FD15XvJB1DB3yymUPIkmLYMur7Sh+e1G4A=="
+      "version": "2.2.0",
+      "resolved": "https://registry.npmjs.org/@pythnetwork/pyth-sdk-solidity/-/pyth-sdk-solidity-2.2.0.tgz",
+      "integrity": "sha512-LsRMmaf9MTflGSymqOJMepFk/3R7DyxMOJfLDB5RDSieyiq+RJ5IYIYnXAFsMrqkjibOtVxARcortHtE9VWwhw=="
     },
     "@pythnetwork/xc-governance-sdk": {
       "version": "file:../third_party/pyth/xc-governance-sdk-js",

+ 1 - 1
ethereum/package.json

@@ -32,7 +32,7 @@
     "@certusone/wormhole-sdk-wasm": "^0.0.1",
     "@openzeppelin/contracts": "^4.5.0",
     "@openzeppelin/contracts-upgradeable": "^4.5.2",
-    "@pythnetwork/pyth-sdk-solidity": "^2.1.0",
+    "@pythnetwork/pyth-sdk-solidity": "^2.2.0",
     "@pythnetwork/xc-governance-sdk": "file:../third_party/pyth/xc-governance-sdk-js",
     "dotenv": "^10.0.0",
     "elliptic": "^6.5.2",

+ 61 - 50
ethereum/test/pyth.js

@@ -39,7 +39,6 @@ contract("Pyth", function () {
     "0x71f8dcb863d176e2c420ad6610cf687359612b6fb392e0642b0ca6b1f186aa3b";
   const notOwnerError =
     "Ownable: caller is not the owner -- Reason given: Ownable: caller is not the owner.";
-  const insufficientFeeError = "insufficient paid fee amount";
 
   // Place all atomic operations that are done within migrations here.
   beforeEach(async function () {
@@ -433,9 +432,9 @@ contract("Pyth", function () {
     assert.equal(feeInWei, 20);
 
     // When a smaller fee is payed it reverts
-    await expectRevert(
+    await expectRevertCustomError(
       updatePriceFeeds(this.pythProxy, [rawBatch1, rawBatch2], feeInWei - 1),
-      insufficientFeeError
+      "InsufficientFee"
     );
   });
 
@@ -571,11 +570,11 @@ contract("Pyth", function () {
   });
 
   it("should fail transaction if a price is not found", async function () {
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.queryPriceFeed(
         "0xdeadfeeddeadfeeddeadfeeddeadfeeddeadfeeddeadfeeddeadfeeddeadfeed"
       ),
-      "price feed for the given id is not pushed or does not exist"
+      "PriceFeedNotFound"
     );
   });
 
@@ -591,10 +590,7 @@ 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);
-      expectRevert(
-        this.pythProxy.getPrice(price_id),
-        "no price available which is recent enough"
-      );
+      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
     }
   });
 
@@ -610,10 +606,7 @@ 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);
-      expectRevert(
-        this.pythProxy.getPrice(price_id),
-        "no price available which is recent enough"
-      );
+      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
     }
   });
 
@@ -643,10 +636,7 @@ contract("Pyth", function () {
       const price_id =
         "0x" + (255 - (i % 256)).toString(16).padStart(2, "0").repeat(32);
 
-      expectRevert(
-        this.pythProxy.getPrice(price_id),
-        "no price available which is recent enough"
-      );
+      expectRevertCustomError(this.pythProxy.getPrice(price_id), "StalePrice");
     }
 
     // Setting the validity time to 120 seconds
@@ -753,9 +743,9 @@ contract("Pyth", function () {
       0
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.updatePriceFeeds(["0x" + vm]),
-      "invalid data source chain/emitter ID"
+      "InvalidUpdateDataSource"
     );
   });
 
@@ -779,9 +769,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaaWrongMagic),
-      "invalid magic for GovernanceInstruction"
+      "InvalidGovernanceMessage"
     );
 
     const wrongModule = Buffer.from(data);
@@ -794,9 +784,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaaWrongModule),
-      "invalid module for GovernanceInstruction"
+      "InvalidGovernanceTarget"
     );
 
     const outOfBoundModule = Buffer.from(data);
@@ -828,9 +818,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaaWrongEmitter),
-      "VAA is not coming from the governance data source"
+      "InvalidGovernanceDataSource"
     );
 
     const vaaWrongChain = await createVAAFromUint8Array(
@@ -840,9 +830,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaaWrongChain),
-      "VAA is not coming from the governance data source"
+      "InvalidGovernanceDataSource"
     );
   });
 
@@ -859,9 +849,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(wrongChainVaa),
-      "invalid target chain for this governance instruction"
+      "InvalidGovernanceTarget"
     );
 
     const dataForAllChains = new governance.SetValidPeriodInstruction(
@@ -908,9 +898,9 @@ contract("Pyth", function () {
 
     await this.pythProxy.executeGovernanceInstruction(vaaSeq1),
       // Replaying shouldn't work
-      await expectRevert(
+      await expectRevertCustomError(
         this.pythProxy.executeGovernanceInstruction(vaaSeq1),
-        "VAA is older than the last executed governance VAA"
+        "OldGovernanceMessage"
       );
 
     const vaaSeq2 = await createVAAFromUint8Array(
@@ -922,13 +912,13 @@ contract("Pyth", function () {
 
     await this.pythProxy.executeGovernanceInstruction(vaaSeq2),
       // Replaying shouldn't work
-      await expectRevert(
+      await expectRevertCustomError(
         this.pythProxy.executeGovernanceInstruction(vaaSeq1),
-        "VAA is older than the last executed governance VAA"
+        "OldGovernanceMessage"
       );
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaaSeq2),
-      "VAA is older than the last executed governance VAA"
+      "OldGovernanceMessage"
     );
   });
 
@@ -948,9 +938,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaa),
-      "upgrade with chain id 0 is not possible"
+      "InvalidGovernanceTarget"
     );
   });
 
@@ -1020,9 +1010,9 @@ contract("Pyth", function () {
       1
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(claimVaaHexString),
-      "VAA is not coming from the governance data source"
+      "InvalidGovernanceDataSource"
     );
 
     const claimVaa = Buffer.from(claimVaaHexString.substring(2), "hex");
@@ -1055,9 +1045,9 @@ contract("Pyth", function () {
     expect(newGovernanceDataSource.emitterAddress).equal(newEmitterAddress);
 
     // Verifies the data source has changed.
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(vaa),
-      "VAA is not coming from the governance data source"
+      "InvalidGovernanceDataSource"
     );
 
     // Make sure a claim vaa does not get executed
@@ -1075,9 +1065,9 @@ contract("Pyth", function () {
       2
     );
 
-    await expectRevert(
+    await expectRevertCustomError(
       this.pythProxy.executeGovernanceInstruction(claimLonelyVaa),
-      "RequestGovernanceDataSourceTransfer can be only part of AuthorizeGovernanceDataSourceTransfer message"
+      "InvalidGovernanceMessage"
     );
 
     // Transfer back the ownership to the old governance data source without increasing
@@ -1115,9 +1105,15 @@ contract("Pyth", function () {
       2
     );
 
-    await expectRevert(
-      this.pythProxy.executeGovernanceInstruction(transferBackVaaWrong),
-      "cannot upgrade to an older governance data source"
+    // This test fails without the hard coded gas limit.
+    // Without gas limit, it fails on a random place (in wormhole sig verification) which
+    // is probably because truffle cannot estimate the gas usage correctly. So the gas is
+    // hard-coded to a high value of 6.7m gas (close to ganache limit).
+    await expectRevertCustomError(
+      this.pythProxy.executeGovernanceInstruction(transferBackVaaWrong, {
+        gas: 6700000,
+      }),
+      "OldGovernanceMessage"
     );
   });
 
@@ -1163,9 +1159,9 @@ contract("Pyth", function () {
     );
 
     let rawBatch = generateRawBatchAttestation(100, 100, 1337);
-    await expectRevert(
+    await expectRevertCustomError(
       updatePriceFeeds(this.pythProxy, [rawBatch]),
-      "invalid data source chain/emitter ID"
+      "InvalidUpdateDataSource"
     );
 
     await updatePriceFeeds(
@@ -1202,9 +1198,9 @@ contract("Pyth", function () {
     assert.equal(await this.pythProxy.singleUpdateFeeInWei(), "5000");
 
     let rawBatch = generateRawBatchAttestation(100, 100, 1337);
-    await expectRevert(
+    await expectRevertCustomError(
       updatePriceFeeds(this.pythProxy, [rawBatch], 0),
-      insufficientFeeError
+      "InsufficientFee"
     );
 
     await updatePriceFeeds(this.pythProxy, [rawBatch], 5000);
@@ -1385,3 +1381,18 @@ function expectEventMultipleTimes(receipt, eventName, args, cnt) {
   const matches = getNumMatchingEvents(receipt, eventName, args);
   assert(matches === cnt, `Expected ${cnt} event matches, found ${matches}.`);
 }
+
+async function expectRevertCustomError(promise, reason) {
+  try {
+    await promise;
+    expect.fail("Expected promise to throw but it didn't");
+  } catch (revert) {
+    if (reason) {
+      const reasonId = web3.utils.keccak256(reason + "()").substr(0, 10);
+      expect(
+        JSON.stringify(revert),
+        `Expected custom error ${reason} (${reasonId})`
+      ).to.include(reasonId);
+    }
+  }
+}