[eth] - optimize ReceiverMessages parseAndVerifyVM (#901)

* feat(eth): optimize ReceiverMessages parseAndVerifyVM

* test(eth): update test setups to use wormholeReceiver

* chore(eth): remove console logging

* feat(eth): optimize & revert return type for parseAndVerifyVM

* fix(eth): add index boundary checks

* perf(eth): optimize verifySignature by passing in primitives instead of structs

* test(eth): add wormhole tests related to guardian set validity

* test(eth): add more parseAndVerify failure test cases

* test(eth): add more failure tests for parseAndVerify

* test(eth): add empty forge test, refactor/deduplicate

target_chains/ethereum/contracts/contracts/libraries/external/UnsafeCalldataBytesLib.sol

@@ -20,6 +20,13 @@ library UnsafeCalldataBytesLib {
         return _bytes[_start:_start + _length];
     }
 
+    function sliceFrom(
+        bytes calldata _bytes,
+        uint256 _start
+    ) internal pure returns (bytes calldata) {
+        return _bytes[_start:_bytes.length];
+    }
+
     function toAddress(
         bytes calldata _bytes,
         uint256 _start

target_chains/ethereum/contracts/contracts/pyth/Pyth.sol

@@ -75,7 +75,8 @@ abstract contract Pyth is
         for (uint i = 0; i < updateData.length; ) {
             if (
                 updateData[i].length > 4 &&
-                UnsafeBytesLib.toUint32(updateData[i], 0) == ACCUMULATOR_MAGIC
+                UnsafeCalldataBytesLib.toUint32(updateData[i], 0) ==
+                ACCUMULATOR_MAGIC
             ) {
                 totalNumUpdates += updatePriceInfosFromAccumulatorUpdate(
                     updateData[i]
@@ -143,7 +144,6 @@ abstract contract Pyth is
         // operations have proper require.
         unchecked {
             bytes memory encoded = vm.payload;
-
             (
                 uint index,
                 uint nAttestations,

target_chains/ethereum/contracts/contracts/pyth/PythAccumulator.sol

@@ -31,7 +31,7 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
     // This method is also used by batch attestation but moved here
     // as the batch attestation will deprecate soon.
     function parseAndVerifyPythVM(
-        bytes memory encodedVm
+        bytes calldata encodedVm
     ) internal view returns (IWormhole.VM memory vm) {
         {
             bool valid;
@@ -152,7 +152,6 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
 
                     // TODO: Do we need to emit an update for accumulator update? If so what should we emit?
                     // emit AccumulatorUpdate(vm.chainId, vm.sequence);
-
                     encodedPayload = vm.payload;
                 }
 
@@ -200,16 +199,19 @@ abstract contract PythAccumulator is PythGetters, PythSetters, AbstractPyth {
     }
 
     function parseWormholeMerkleHeaderNumUpdates(
-        bytes memory wormholeMerkleUpdate,
+        bytes calldata wormholeMerkleUpdate,
         uint offset
     ) internal pure returns (uint8 numUpdates) {
-        uint16 whProofSize = UnsafeBytesLib.toUint16(
+        uint16 whProofSize = UnsafeCalldataBytesLib.toUint16(
             wormholeMerkleUpdate,
             offset
         );
         offset += 2;
         offset += whProofSize;
-        numUpdates = UnsafeBytesLib.toUint8(wormholeMerkleUpdate, offset);
+        numUpdates = UnsafeCalldataBytesLib.toUint8(
+            wormholeMerkleUpdate,
+            offset
+        );
     }
 
     function extractPriceInfoFromMerkleProof(

target_chains/ethereum/contracts/contracts/wormhole-receiver/ReceiverMessages.sol

@@ -7,11 +7,17 @@ pragma experimental ABIEncoderV2;
 import "./ReceiverGetters.sol";
 import "./ReceiverStructs.sol";
 import "../libraries/external/BytesLib.sol";
+import "../libraries/external/UnsafeCalldataBytesLib.sol";
+
+error VmVersionIncompatible();
+error SignatureIndexesNotAscending();
 
 contract ReceiverMessages is ReceiverGetters {
     using BytesLib for bytes;
 
     /// @dev parseAndVerifyVM serves to parse an encodedVM and wholy validate it for consumption
+    /// WARNING: it intentionally sets vm.signatures to an empty array since it is not needed after it is validated in this function
+    /// since it not used anywhere. If you need to use vm.signatures, use parseVM and verifyVM separately.
     function parseAndVerifyVM(
         bytes calldata encodedVM
     )
@@ -19,8 +25,161 @@ contract ReceiverMessages is ReceiverGetters {
         view
         returns (ReceiverStructs.VM memory vm, bool valid, string memory reason)
     {
-        vm = parseVM(encodedVM);
+        uint index = 0;
-        (valid, reason) = verifyVM(vm);
+        unchecked {
+            {
+                vm.version = UnsafeCalldataBytesLib.toUint8(encodedVM, index);
+                index += 1;
+                if (vm.version != 1) {
+                    revert VmVersionIncompatible();
+                }
+            }
+
+            ReceiverStructs.GuardianSet memory guardianSet;
+            {
+                vm.guardianSetIndex = UnsafeCalldataBytesLib.toUint32(
+                    encodedVM,
+                    index
+                );
+                index += 4;
+                guardianSet = getGuardianSet(vm.guardianSetIndex);
+
+                /**
+                 * @dev Checks whether the guardianSet has zero keys
+                 * WARNING: This keys check is critical to ensure the guardianSet has keys present AND to ensure
+                 * that guardianSet key size doesn't fall to zero and negatively impact quorum assessment.  If guardianSet
+                 * key length is 0 and vm.signatures length is 0, this could compromise the integrity of both vm and
+                 * signature verification.
+                 */
+                if (guardianSet.keys.length == 0) {
+                    return (vm, false, "invalid guardian set");
+                }
+
+                /// @dev Checks if VM guardian set index matches the current index (unless the current set is expired).
+                if (
+                    vm.guardianSetIndex != getCurrentGuardianSetIndex() &&
+                    guardianSet.expirationTime < block.timestamp
+                ) {
+                    return (vm, false, "guardian set has expired");
+                }
+            }
+
+            // Parse Signatures
+            uint256 signersLen = UnsafeCalldataBytesLib.toUint8(
+                encodedVM,
+                index
+            );
+            index += 1;
+            {
+                // 66 is the length of each signature
+                // 1 (guardianIndex) + 32 (r) + 32 (s) + 1 (v)
+                uint hashIndex = index + (signersLen * 66);
+                if (hashIndex > encodedVM.length) {
+                    return (vm, false, "invalid signature length");
+                }
+                // Hash the body
+                vm.hash = keccak256(
+                    abi.encodePacked(
+                        keccak256(
+                            UnsafeCalldataBytesLib.sliceFrom(
+                                encodedVM,
+                                hashIndex
+                            )
+                        )
+                    )
+                );
+            }
+
+            {
+                uint8 lastIndex = 0;
+                for (uint i = 0; i < signersLen; i++) {
+                    ReceiverStructs.Signature memory sig;
+                    sig.guardianIndex = UnsafeCalldataBytesLib.toUint8(
+                        encodedVM,
+                        index
+                    );
+                    index += 1;
+
+                    sig.r = UnsafeCalldataBytesLib.toBytes32(encodedVM, index);
+                    index += 32;
+                    sig.s = UnsafeCalldataBytesLib.toBytes32(encodedVM, index);
+                    index += 32;
+                    sig.v =
+                        UnsafeCalldataBytesLib.toUint8(encodedVM, index) +
+                        27;
+                    index += 1;
+                    bool signatureValid;
+                    string memory invalidReason;
+                    (signatureValid, invalidReason) = verifySignature(
+                        i,
+                        lastIndex,
+                        vm.hash,
+                        sig.guardianIndex,
+                        sig.r,
+                        sig.s,
+                        sig.v,
+                        guardianSet.keys[sig.guardianIndex]
+                    );
+                    if (!signatureValid) {
+                        return (vm, false, invalidReason);
+                    }
+                    lastIndex = sig.guardianIndex;
+                }
+            }
+
+            /**
+             * @dev We're using a fixed point number transformation with 1 decimal to deal with rounding.
+             *   WARNING: This quorum check is critical to assessing whether we have enough Guardian signatures to validate a VM
+             *   if making any changes to this, obtain additional peer review. If guardianSet key length is 0 and
+             *   vm.signatures length is 0, this could compromise the integrity of both vm and signature verification.
+             */
+
+            if (
+                (((guardianSet.keys.length * 10) / 3) * 2) / 10 + 1 > signersLen
+            ) {
+                return (vm, false, "no quorum");
+            }
+
+            // purposely setting vm.signatures to empty array since we don't need it anymore
+            // and we've already verified it above
+            vm.signatures = new ReceiverStructs.Signature[](0);
+
+            // Parse the body
+            vm.timestamp = UnsafeCalldataBytesLib.toUint32(encodedVM, index);
+            index += 4;
+
+            vm.nonce = UnsafeCalldataBytesLib.toUint32(encodedVM, index);
+            index += 4;
+
+            vm.emitterChainId = UnsafeCalldataBytesLib.toUint16(
+                encodedVM,
+                index
+            );
+            index += 2;
+
+            vm.emitterAddress = UnsafeCalldataBytesLib.toBytes32(
+                encodedVM,
+                index
+            );
+            index += 32;
+
+            vm.sequence = UnsafeCalldataBytesLib.toUint64(encodedVM, index);
+            index += 8;
+
+            vm.consistencyLevel = UnsafeCalldataBytesLib.toUint8(
+                encodedVM,
+                index
+            );
+            index += 1;
+
+            if (index > encodedVM.length) {
+                return (vm, false, "invalid payload length");
+            }
+
+            vm.payload = UnsafeCalldataBytesLib.sliceFrom(encodedVM, index);
+
+            return (vm, true, "");
+        }
     }
 
     /**
@@ -84,6 +243,27 @@ contract ReceiverMessages is ReceiverGetters {
         return (true, "");
     }
 
+    function verifySignature(
+        uint i,
+        uint8 lastIndex,
+        bytes32 hash,
+        uint8 guardianIndex,
+        bytes32 r,
+        bytes32 s,
+        uint8 v,
+        address guardianSetKey
+    ) private pure returns (bool valid, string memory reason) {
+        /// Ensure that provided signature indices are ascending only
+        if (i != 0 && guardianIndex <= lastIndex) {
+            revert SignatureIndexesNotAscending();
+        }
+        /// Check to see if the signer of the signature does not match a specific Guardian key at the provided index
+        if (ecrecover(hash, v, r, s) != guardianSetKey) {
+            return (false, "VM signature invalid");
+        }
+        return (true, "");
+    }
+
     /**
      * @dev verifySignatures serves to validate arbitrary sigatures against an arbitrary guardianSet
      *  - it intentionally does not solve for expectations within guardianSet (you should use verifyVM if you need these protections)
@@ -98,21 +278,20 @@ contract ReceiverMessages is ReceiverGetters {
         uint8 lastIndex = 0;
         for (uint i = 0; i < signatures.length; i++) {
             ReceiverStructs.Signature memory sig = signatures[i];
-
+            (valid, reason) = verifySignature(
-            /// Ensure that provided signature indices are ascending only
+                i,
-            require(
+                lastIndex,
-                i == 0 || sig.guardianIndex > lastIndex,
+                hash,
-                "signature indices must be ascending"
+                sig.guardianIndex,
-            );
+                sig.r,
-            lastIndex = sig.guardianIndex;
+                sig.s,
-
+                sig.v,
-            /// Check to see if the signer of the signature does not match a specific Guardian key at the provided index
-            if (
-                ecrecover(hash, sig.v, sig.r, sig.s) !=
                 guardianSet.keys[sig.guardianIndex]
-            ) {
+            );
-                return (false, "VM signature invalid");
+            if (!valid) {
+                return (false, reason);
             }
+            lastIndex = sig.guardianIndex;
         }
 
         /// If we are here, we've validated that the provided signatures are valid for the provided guardianSet
@@ -130,7 +309,9 @@ contract ReceiverMessages is ReceiverGetters {
 
         vm.version = encodedVM.toUint8(index);
         index += 1;
-        require(vm.version == 1, "VM version incompatible");
+        if (vm.version != 1) {
+            revert VmVersionIncompatible();
+        }
 
         vm.guardianSetIndex = encodedVM.toUint32(index);
         index += 4;

target_chains/ethereum/contracts/forge-test/GasBenchmark.t.sol

@@ -25,6 +25,7 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
     // We will have less than 512 price for a foreseeable future.
     uint8 constant MERKLE_TREE_DEPTH = 9;
 
+    IWormhole public wormhole;
     IPyth public pyth;
 
     bytes32[] priceIds;
@@ -51,7 +52,9 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
     uint randSeed;
 
     function setUp() public {
-        pyth = IPyth(setUpPyth(setUpWormhole(NUM_GUARDIANS)));
+        address wormholeAddr = setUpWormholeReceiver(NUM_GUARDIANS);
+        wormhole = IWormhole(wormholeAddr);
+        pyth = IPyth(setUpPyth(wormholeAddr));
 
         priceIds = new bytes32[](NUM_PRICES);
         priceIds[0] = bytes32(
@@ -101,7 +104,6 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
             freshPricesWhMerkleUpdateData.push(updateData);
             freshPricesWhMerkleUpdateFee.push(updateFee);
         }
-
         // Populate the contract with the initial prices
         (
             cachedPricesWhBatchUpdateData,
@@ -417,4 +419,8 @@ contract GasBenchmark is Test, WormholeTestUtils, PythTestUtils {
     function testBenchmarkGetUpdateFeeWhMerkle5() public view {
         pyth.getUpdateFee(freshPricesWhMerkleUpdateData[4]);
     }
+
+    function testBenchmarkWormholeParseAndVerifyVMBatchAttestation() public {
+        wormhole.parseAndVerifyVM(freshPricesWhBatchUpdateData[0]);
+    }
 }

target_chains/ethereum/contracts/forge-test/Pyth.WormholeMerkleAccumulator.t.sol

@@ -26,7 +26,7 @@ contract PythWormholeMerkleAccumulatorTest is
     uint64 constant MAX_UINT64 = uint64(int64(-1));
 
     function setUp() public {
-        pyth = IPyth(setUpPyth(setUpWormhole(1)));
+        pyth = IPyth(setUpPyth(setUpWormholeReceiver(1)));
     }
 
     function assertPriceFeedMessageStored(
@@ -476,13 +476,6 @@ contract PythWormholeMerkleAccumulatorTest is
         );
     }
 
-    function isNotMatch(
-        bytes memory a,
-        bytes memory b
-    ) public pure returns (bool) {
-        return keccak256(a) != keccak256(b);
-    }
-
     /// @notice This method creates a forged invalid wormhole update data.
     /// The caller should pass the forgeItem as string and if it matches the
     /// expected value, that item will be forged to be invalid.

target_chains/ethereum/contracts/forge-test/Pyth.t.sol

@@ -19,7 +19,7 @@ contract PythTest is Test, WormholeTestUtils, PythTestUtils, RandTestUtils {
     uint64 constant MAX_UINT64 = uint64(int64(-1));
 
     function setUp() public {
-        pyth = IPyth(setUpPyth(setUpWormhole(1)));
+        pyth = IPyth(setUpPyth(setUpWormholeReceiver(1)));
     }
 
     function generateRandomPriceAttestations(

target_chains/ethereum/contracts/forge-test/VerificationExperiments.t.sol

@@ -67,7 +67,9 @@ contract VerificationExperiments is
     uint64 sequence;
 
     function setUp() public {
-        address payable wormhole = payable(setUpWormhole(NUM_GUARDIANS));
+        address payable wormhole = payable(
+            setUpWormholeReceiver(NUM_GUARDIANS)
+        );
 
         // Deploy experimental contract
         PythExperimental implementation = new PythExperimental();
@@ -82,7 +84,6 @@ contract VerificationExperiments is
 
         bytes32[] memory emitterAddresses = new bytes32[](1);
         emitterAddresses[0] = PythTestUtils.SOURCE_EMITTER_ADDRESS;
-
         pyth.initialize(
             wormhole,
             vm.addr(THRESHOLD_KEY),
@@ -134,6 +135,7 @@ contract VerificationExperiments is
             cachedPricesUpdateData,
             cachedPricesUpdateFee
         ) = generateWormholeUpdateDataAndFee(cachedPrices);
+
         pyth.updatePriceFeeds{value: cachedPricesUpdateFee}(
             cachedPricesUpdateData
         );

target_chains/ethereum/contracts/forge-test/utils/PythTestUtils.t.sol

@@ -368,7 +368,7 @@ contract PythTestUtilsTest is
     function testGenerateWhBatchUpdateWorks() public {
         IPyth pyth = IPyth(
             setUpPyth(
-                setUpWormhole(
+                setUpWormholeReceiver(
                     1 // Number of guardians
                 )
             )

target_chains/ethereum/contracts/forge-test/utils/WormholeTestUtils.t.sol

@@ -7,9 +7,21 @@ import "../../contracts/wormhole/Setup.sol";
 import "../../contracts/wormhole/Wormhole.sol";
 import "../../contracts/wormhole/interfaces/IWormhole.sol";
 
+import "../../contracts/wormhole-receiver/ReceiverImplementation.sol";
+import "../../contracts/wormhole-receiver/ReceiverSetup.sol";
+import "../../contracts/wormhole-receiver/WormholeReceiver.sol";
+import "../../contracts/wormhole-receiver/ReceiverGovernanceStructs.sol";
+
 import "forge-std/Test.sol";
 
 abstract contract WormholeTestUtils is Test {
+    uint256[] currentSigners;
+    address wormholeReceiverAddr;
+    uint16 constant CHAIN_ID = 2; // Ethereum
+    uint16 constant GOVERNANCE_CHAIN_ID = 1; // solana
+    bytes32 constant GOVERNANCE_CONTRACT =
+        0x0000000000000000000000000000000000000000000000000000000000000004;
+
     function setUpWormhole(uint8 numGuardians) public returns (address) {
         Implementation wormholeImpl = new Implementation();
         Setup wormholeSetup = new Setup();
@@ -17,9 +29,11 @@ abstract contract WormholeTestUtils is Test {
         Wormhole wormhole = new Wormhole(address(wormholeSetup), new bytes(0));
 
         address[] memory initSigners = new address[](numGuardians);
+        currentSigners = new uint256[](numGuardians);
 
         for (uint256 i = 0; i < numGuardians; ++i) {
-            initSigners[i] = vm.addr(i + 1); // i+1 is the private key for the i-th signer.
+            currentSigners[i] = i + 1;
+            initSigners[i] = vm.addr(currentSigners[i]); // i+1 is the private key for the i-th signer.
         }
 
         // These values are the default values used in our tilt test environment
@@ -27,14 +41,54 @@ abstract contract WormholeTestUtils is Test {
         Setup(address(wormhole)).setup(
             address(wormholeImpl),
             initSigners,
-            2, // Ethereum chain ID
+            CHAIN_ID, // Ethereum chain ID
-            1, // Governance source chain ID (1 = solana)
+            GOVERNANCE_CHAIN_ID, // Governance source chain ID (1 = solana)
-            0x0000000000000000000000000000000000000000000000000000000000000004 // Governance source address
+            GOVERNANCE_CONTRACT // Governance source address
         );
 
         return address(wormhole);
     }
 
+    function setUpWormholeReceiver(
+        uint8 numGuardians
+    ) public returns (address) {
+        ReceiverImplementation wormholeReceiverImpl = new ReceiverImplementation();
+        ReceiverSetup wormholeReceiverSetup = new ReceiverSetup();
+
+        WormholeReceiver wormholeReceiver = new WormholeReceiver(
+            address(wormholeReceiverSetup),
+            new bytes(0)
+        );
+
+        address[] memory initSigners = new address[](numGuardians);
+        currentSigners = new uint256[](numGuardians);
+
+        for (uint256 i = 0; i < numGuardians; ++i) {
+            currentSigners[i] = i + 1;
+            initSigners[i] = vm.addr(currentSigners[i]); // i+1 is the private key for the i-th signer.
+        }
+
+        // These values are the default values used in our tilt test environment
+        // and are not important.
+        ReceiverSetup(address(wormholeReceiver)).setup(
+            address(wormholeReceiverImpl),
+            initSigners,
+            CHAIN_ID, // Ethereum chain ID
+            GOVERNANCE_CHAIN_ID, // Governance source chain ID (1 = solana)
+            GOVERNANCE_CONTRACT // Governance source address
+        );
+        wormholeReceiverAddr = address(wormholeReceiver);
+
+        return wormholeReceiverAddr;
+    }
+
+    function isNotMatch(
+        bytes memory a,
+        bytes memory b
+    ) public pure returns (bool) {
+        return keccak256(a) != keccak256(b);
+    }
+
     function generateVaa(
         uint32 timestamp,
         uint16 emitterChainId,
@@ -58,7 +112,8 @@ abstract contract WormholeTestUtils is Test {
         bytes memory signatures = new bytes(0);
 
         for (uint256 i = 0; i < numSigners; ++i) {
-            (uint8 v, bytes32 r, bytes32 s) = vm.sign(i + 1, hash);
+            (uint8 v, bytes32 r, bytes32 s) = vm.sign(currentSigners[i], hash);
+
             // encodePacked uses padding for arrays and we don't want it, so we manually concat them.
             signatures = abi.encodePacked(
                 signatures,
@@ -71,37 +126,389 @@ abstract contract WormholeTestUtils is Test {
 
         vaa = abi.encodePacked(
             uint8(1), // Version
-            uint32(0), // Guardian set index. it is initialized by 0
+            IWormhole(wormholeReceiverAddr).getCurrentGuardianSetIndex(), // Guardian set index. it is initialized by 0
             numSigners,
             signatures,
             body
         );
     }
+
+    function forgeVaa(
+        uint32 timestamp,
+        uint16 emitterChainId,
+        bytes32 emitterAddress,
+        uint64 sequence,
+        bytes memory payload,
+        uint8 numSigners,
+        bytes memory forgeItem
+    ) public returns (bytes memory vaa) {
+        bytes memory body = abi.encodePacked(
+            timestamp,
+            uint32(0), // Nonce. It is zero for single VAAs.
+            emitterChainId,
+            emitterAddress,
+            sequence,
+            uint8(0), // Consistency level (sometimes no. confirmation block). Not important here.
+            payload
+        );
+
+        bytes32 hash = keccak256(abi.encodePacked(keccak256(body)));
+
+        bytes memory signatures = new bytes(0);
+
+        for (uint256 i = 0; i < numSigners; ++i) {
+            (uint8 v, bytes32 r, bytes32 s) = vm.sign(
+                isNotMatch(forgeItem, "vaaSignature")
+                    ? currentSigners[i]
+                    : currentSigners[i] + 1000,
+                hash
+            );
+            // encodePacked uses padding for arrays and we don't want it, so we manually concat them.
+            signatures = abi.encodePacked(
+                signatures,
+                isNotMatch(forgeItem, "vaaSignatureIndex")
+                    ? uint8(i)
+                    : uint8(0), // Guardian index of the signature
+                r,
+                s,
+                v - 27 // v is either 27 or 28. 27 is added to v in Eth (following BTC) but Wormhole doesn't use it.
+            );
+        }
+
+        vaa = abi.encodePacked(
+            isNotMatch(forgeItem, "vaaVersion") ? uint8(1) : uint8(2), // Version
+            isNotMatch(forgeItem, "vaaGuardianSetIndex")
+                ? uint32(0)
+                : uint32(1), // Guardian set index. it is initialized by 0
+            isNotMatch(forgeItem, "vaaNumSigners+")
+                ? isNotMatch(forgeItem, "vaaNumSigners-")
+                    ? numSigners
+                    : numSigners - 1
+                : numSigners + 1,
+            signatures,
+            body
+        );
+    }
+
+    function upgradeGuardianSet(uint256 numGuardians) public {
+        IWormhole wormhole = IWormhole(wormholeReceiverAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(wormholeReceiverAddr)
+        );
+        bytes memory newGuardians = new bytes(0);
+
+        for (uint256 i = 0; i < numGuardians; ++i) {
+            // encodePacked uses padding for arrays and we don't want it, so we manually concat them.
+            newGuardians = abi.encodePacked(newGuardians, vm.addr(i + 1 + 10));
+        }
+        uint32 newGuardianSetIndex = uint32(1);
+        bytes memory upgradeGuardianSetPayload = abi.encodePacked(
+            bytes32(
+                0x00000000000000000000000000000000000000000000000000000000436f7265
+            ), // "Core" ReceiverGovernance module
+            uint8(2), // action
+            uint16(0), // chain (unused)
+            wormhole.getCurrentGuardianSetIndex() + 1, // uint32 newGuardianSetIndex;
+            uint8(numGuardians), // uint8 numGuardians;
+            newGuardians // ReceiverStructs.GuardianSet newGuardianSet;
+        );
+        bytes memory setGuardianSetVaa = generateVaa(
+            112,
+            GOVERNANCE_CHAIN_ID, // emitter chainID (solana)
+            GOVERNANCE_CONTRACT, // gov emitter addr
+            10,
+            upgradeGuardianSetPayload,
+            4
+        );
+        whReceiverImpl.submitNewGuardianSet(setGuardianSetVaa);
+
+        currentSigners = new uint256[](numGuardians);
+        for (uint256 i = 0; i < numGuardians; ++i) {
+            currentSigners[i] = i + 1 + 10;
+        }
+    }
 }
 
 contract WormholeTestUtilsTest is Test, WormholeTestUtils {
+    uint32 constant TEST_VAA_TIMESTAMP = 112;
+    uint16 constant TEST_EMITTER_CHAIN_ID = 7;
+    bytes32 constant TEST_EMITTER_ADDR =
+        0x0000000000000000000000000000000000000000000000000000000000000bad;
+    uint64 constant TEST_SEQUENCE = 10;
+    bytes constant TEST_PAYLOAD = hex"deadbeaf";
+    uint8 constant TEST_NUM_SIGNERS = 4;
+
+    function assertVmMatchesTestValues(
+        Structs.VM memory vm,
+        bool valid,
+        string memory reason,
+        bytes memory vaa
+    ) private {
+        assertTrue(valid);
+        assertEq(reason, "");
+        assertEq(vm.timestamp, TEST_VAA_TIMESTAMP);
+        assertEq(vm.emitterChainId, TEST_EMITTER_CHAIN_ID);
+        assertEq(vm.emitterAddress, TEST_EMITTER_ADDR);
+        assertEq(vm.sequence, TEST_SEQUENCE);
+        assertEq(vm.payload, TEST_PAYLOAD);
+        // parseAndVerifyVM() returns an empty signatures array for gas savings since it's not used
+        // after its been verified. parseVM() returns the full signatures array.
+        vm = IWormhole(wormholeReceiverAddr).parseVM(vaa);
+        assertEq(vm.signatures.length, TEST_NUM_SIGNERS);
+    }
+
     function testGenerateVaaWorks() public {
-        IWormhole wormhole = IWormhole(setUpWormhole(5));
+        IWormhole wormhole = IWormhole(setUpWormholeReceiver(5));
 
         bytes memory vaa = generateVaa(
-            112,
+            TEST_VAA_TIMESTAMP,
-            7,
+            TEST_EMITTER_CHAIN_ID,
-            0x0000000000000000000000000000000000000000000000000000000000000bad,
+            TEST_EMITTER_ADDR,
-            10,
+            TEST_SEQUENCE,
-            hex"deadbeaf",
+            TEST_PAYLOAD,
-            4
+            TEST_NUM_SIGNERS
         );
 
-        (Structs.VM memory vm, bool valid, ) = wormhole.parseAndVerifyVM(vaa);
+        (Structs.VM memory vm, bool valid, string memory reason) = wormhole
-        assertTrue(valid);
+            .parseAndVerifyVM(vaa);
+        assertVmMatchesTestValues(vm, valid, reason, vaa);
+    }
 
-        assertEq(vm.timestamp, 112);
+    function testParseAndVerifyWorksWithoutForging() public {
-        assertEq(vm.emitterChainId, 7);
+        uint8 numGuardians = 5;
-        assertEq(
+        IWormhole wormhole = IWormhole(setUpWormholeReceiver(numGuardians));
-            vm.emitterAddress,
+        bytes memory vaa = forgeVaa(
-            0x0000000000000000000000000000000000000000000000000000000000000bad
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            ""
         );
-        assertEq(vm.payload, hex"deadbeaf");
+        (Structs.VM memory vm, bool valid, string memory reason) = wormhole
-        assertEq(vm.signatures.length, 4);
+            .parseAndVerifyVM(vaa);
+        assertVmMatchesTestValues(vm, valid, reason, vaa);
+    }
+
+    function testParseAndVerifyFailsIfVaaIsNotSignedByEnoughGuardians() public {
+        IWormhole wormhole = IWormhole(setUpWormholeReceiver(5));
+        bytes memory vaa = generateVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            1 //numSigners
+        );
+        (, bool valid, string memory reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "no quorum");
+    }
+
+    function testParseAndVerifyFailsIfVaaHasInvalidGuardianSetIndex() public {
+        uint8 numGuardians = 5;
+        IWormhole wormhole = IWormhole(setUpWormholeReceiver(numGuardians));
+        bytes memory vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaGuardianSetIndex"
+        );
+        (, bool valid, string memory reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "invalid guardian set");
+    }
+
+    function testParseAndVerifyFailsIfInvalidGuardianSignatureIndex() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the initial wormhole guardian set
+        bytes memory vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaSignatureIndex"
+        );
+        vm.expectRevert(
+            // workaround for this error not being in an external library
+            abi.encodeWithSignature("SignatureIndexesNotAscending()")
+        );
+        wormhole.parseAndVerifyVM(vaa);
+    }
+
+    function testParseAndVerifyFailsIfIncorrectVersion() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the initial wormhole guardian set
+        bytes memory vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaVersion"
+        );
+        vm.expectRevert(
+            // workaround for this error not being in an external library
+            abi.encodeWithSignature("VmVersionIncompatible()")
+        );
+        wormhole.parseAndVerifyVM(vaa);
+    }
+
+    function testUpgradeGuardianSetWorks() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        upgradeGuardianSet(5);
+        // generate the vaa and sign with the new wormhole guardian set
+        bytes memory vaa = generateVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS
+        );
+        uint32 guardianSetIdx = wormhole.getCurrentGuardianSetIndex();
+        vm.warp(block.timestamp + 5 days);
+
+        (Structs.VM memory vm, bool valid, string memory reason) = wormhole
+            .parseAndVerifyVM(vaa);
+        assertVmMatchesTestValues(vm, valid, reason, vaa);
+    }
+
+    function testParseAndVerifyWorksIfUsingPreviousVaaGuardianSetBeforeItExpires()
+        public
+    {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the initial wormhole guardian set
+        bytes memory vaa = generateVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS
+        );
+
+        upgradeGuardianSet(numGuardians);
+        uint32 guardianSetIdx = wormhole.getCurrentGuardianSetIndex();
+        uint previousGuardianSetExpiration = wormhole
+            .getGuardianSet(0)
+            .expirationTime;
+        // warp to 5 seconds before the previous guardian set expires
+        vm.warp(previousGuardianSetExpiration - 5);
+        (Structs.VM memory vm, bool valid, string memory reason) = wormhole
+            .parseAndVerifyVM(vaa);
+        assertVmMatchesTestValues(vm, valid, reason, vaa);
+    }
+
+    function testParseAndVerifyFailsIfVaaGuardianSetHasExpired() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the current wormhole guardian set
+        bytes memory vaa = generateVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS
+        );
+
+        upgradeGuardianSet(numGuardians);
+        uint32 guardianSetIdx = wormhole.getCurrentGuardianSetIndex();
+        vm.warp(block.timestamp + 5 days);
+        (, bool valid, string memory reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "guardian set has expired");
+    }
+
+    function testParseAndVerifyFailsIfInvalidGuardianSignature() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the current wormhole guardian set
+        bytes memory vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaSignature"
+        );
+
+        (, bool valid, string memory reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "VM signature invalid");
+    }
+
+    function testParseAndVerifyFailsIfInvalidNumSignatures() public {
+        uint8 numGuardians = 5;
+        address whAddr = setUpWormholeReceiver(numGuardians);
+        IWormhole wormhole = IWormhole(whAddr);
+        ReceiverImplementation whReceiverImpl = ReceiverImplementation(
+            payable(whAddr)
+        );
+        // generate the vaa and sign with the current wormhole guardian set
+        bytes memory vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaNumSigners+"
+        );
+
+        (, bool valid, string memory reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "invalid signature length");
+
+        vaa = forgeVaa(
+            TEST_VAA_TIMESTAMP,
+            TEST_EMITTER_CHAIN_ID,
+            TEST_EMITTER_ADDR,
+            TEST_SEQUENCE,
+            TEST_PAYLOAD,
+            TEST_NUM_SIGNERS,
+            "vaaNumSigners-"
+        );
+
+        (, valid, reason) = wormhole.parseAndVerifyVM(vaa);
+        assertEq(valid, false);
+        assertEq(reason, "VM signature invalid");
     }
 }