evm: cleanup: remove module from governance struct (#150)

Author: nik-suri

evm/src/wormhole/Governance.sol

@@ -46,7 +46,7 @@ contract Governance {
     * @dev General purpose governance message to call arbitrary methods on a governed smart contract.
     *      This message adheres to the Wormhole governance packet standard: https://github.com/wormhole-foundation/wormhole/blob/main/whitepapers/0002_governance_messaging.md
     *      The wire format for this message is:
-    *      - module - 32 bytes
+    *      - MODULE - 32 bytes
     *      - action - 1 byte
     *      - chain - 2 bytes
     *      - governanceContract - 20 bytes
@@ -55,7 +55,6 @@ contract Governance {
     *      - callData - `callDataLength` bytes
     */
     struct GeneralPurposeGovernanceMessage {
-        bytes32 module;
         uint8 action;
         uint16 chain;
         address governanceContract;
@@ -72,10 +71,6 @@ contract Governance {
         GeneralPurposeGovernanceMessage memory message =
             parseGeneralPurposeGovernanceMessage(verified.payload);
 
-        if (message.module != MODULE) {
-            revert InvalidModule(message.module);
-        }
-
         if (message.action != uint8(GovernanceAction.EVM_CALL)) {
             revert InvalidAction(message.action);
         }
@@ -138,7 +133,7 @@ contract Governance {
         }
         uint16 callDataLength = uint16(m.callData.length);
         return abi.encodePacked(
-            m.module,
+            MODULE,
             m.action,
             m.chain,
             m.governanceContract,
@@ -154,7 +149,13 @@ contract Governance {
         returns (GeneralPurposeGovernanceMessage memory message)
     {
         uint256 offset = 0;
-        (message.module, offset) = encoded.asBytes32Unchecked(offset);
+
+        bytes32 module;
+        (module, offset) = encoded.asBytes32Unchecked(offset);
+        if (module != MODULE) {
+            revert InvalidModule(module);
+        }
+
         (message.action, offset) = encoded.asUint8Unchecked(offset);
         (message.chain, offset) = encoded.asUint16Unchecked(offset);
         (message.governanceContract, offset) = encoded.asAddressUnchecked(offset);

evm/test/wormhole/Governance.t.sol

@@ -2,6 +2,7 @@
 pragma solidity >=0.8.8 <0.9.0;
 
 import "../../src/wormhole/Governance.sol";
+import "forge-std/console.sol";
 import "forge-std/Test.sol";
 import "openzeppelin-contracts/contracts/access/Ownable.sol";
 import "wormhole-solidity-sdk/testing/helpers/WormholeSimulator.sol";
@@ -41,7 +42,6 @@ contract GovernanceTest is Test {
     }
 
     function buildGovernanceVaa(
-        bytes32 module,
         uint8 action,
         uint16 chainId,
         address governanceContract,
@@ -50,55 +50,51 @@ contract GovernanceTest is Test {
     ) public view returns (bytes memory) {
         Governance.GeneralPurposeGovernanceMessage memory message = Governance
             .GeneralPurposeGovernanceMessage({
-            module: module,
             action: action,
             chain: chainId,
             governanceContract: governanceContract,
             governedContract: governedContract,
             callData: callData
         });
 
-        IWormhole.VM memory vaa = IWormhole.VM({
+        IWormhole.VM memory vaa =
+            buildVaa(governance.encodeGeneralPurposeGovernanceMessage(message));
+
+        return guardian.encodeAndSignMessage(vaa);
+    }
+
+    function buildVaa(bytes memory payload) public view returns (IWormhole.VM memory) {
+        return IWormhole.VM({
             version: 1,
             timestamp: uint32(block.timestamp),
             nonce: 0,
             emitterChainId: wormhole.governanceChainId(),
             emitterAddress: wormhole.governanceContract(),
             sequence: 0,
             consistencyLevel: 200,
-            payload: governance.encodeGeneralPurposeGovernanceMessage(message),
+            payload: payload,
             guardianSetIndex: 0,
             signatures: new IWormhole.Signature[](0),
             hash: bytes32("")
         });
-
-        return guardian.encodeAndSignMessage(vaa);
     }
 
     function test_invalidModule() public {
-        uint16 thisChain = wormhole.chainId();
         bytes32 coreBridgeModule =
             0x00000000000000000000000000000000000000000000000000000000436f7265;
-
-        bytes memory signed = buildGovernanceVaa(
-            coreBridgeModule,
-            uint8(Governance.GovernanceAction.EVM_CALL),
-            thisChain,
-            address(governance),
-            address(myContract),
-            abi.encodeWithSignature("governanceStuff()")
-        );
+        bytes memory restOfPayload =
+            "0x0100022e234dae75c793f67a35089c9d99245e1c58470bf62849f9a0b5bf2913b396098f7c7019b51a820a000471cd25f9";
+        bytes memory badModulePayload = abi.encodePacked(coreBridgeModule, restOfPayload);
 
         vm.expectRevert(abi.encodeWithSignature("InvalidModule(bytes32)", coreBridgeModule));
-        governance.performGovernance(signed);
+        governance.parseGeneralPurposeGovernanceMessage(badModulePayload);
     }
 
     // TODO: this should ideally test all actions that != 1
     function test_invalidAction() public {
         uint16 thisChain = wormhole.chainId();
 
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.UNDEFINED),
             thisChain,
             address(governance),
@@ -113,7 +109,6 @@ contract GovernanceTest is Test {
     // TODO: this should ideally test all chainIds that != wormhole.chainId()
     function test_invalidChain() public {
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.EVM_CALL),
             0,
             address(governance),
@@ -131,7 +126,6 @@ contract GovernanceTest is Test {
         address random = address(0x1234);
 
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.EVM_CALL),
             thisChain,
             random,
@@ -147,7 +141,6 @@ contract GovernanceTest is Test {
         uint16 thisChain = wormhole.chainId();
 
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.EVM_CALL),
             thisChain,
             address(governance),
@@ -163,7 +156,6 @@ contract GovernanceTest is Test {
         uint16 thisChain = wormhole.chainId();
 
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.EVM_CALL),
             thisChain,
             address(governance),
@@ -181,7 +173,6 @@ contract GovernanceTest is Test {
         uint16 thisChain = wormhole.chainId();
 
         bytes memory signed = buildGovernanceVaa(
-            governance.MODULE(),
             uint8(Governance.GovernanceAction.EVM_CALL),
             thisChain,
             address(governance),