more testing

d10r · d10r · commit 53f953f09a5b · 2026-01-30T20:35:57.000+01:00
diff --git a/packages/ethereum-contracts/contracts/interfaces/utils/IUserDefinedMacro.sol b/packages/ethereum-contracts/contracts/interfaces/utils/IUserDefinedMacro.sol
@@ -45,10 +45,19 @@ interface IUserDefinedMacro {
      */
 }
 
+// Interface for a macro used with the Only712MacroForwarder.
+// Metaphor: a macro is like an api, an action is like an endpoint.
+// Each action can have its own type definition (list of arguments).
+// TODO: for multi-action macros, the getters probably all need to get the encoded message as an argument
 interface IUserDefined712Macro is IUserDefinedMacro {
-    // TODO: this probably needs to be a function of the message, for the dispatching pattern
-    // the metaphor being: a macro is like an api, an action is like an endpoint (defining the set of arguments)
-    function getMessageTypeHash() external view returns (bytes32);
-    // TODO: should it take the known and required fields already decoded instead?
+    // Primary type name (required by the EIP712 type definition), usually rendered prominently by wallets.
+    // From a users perspective, it should concisely name the action/intent to be signed.
+    function getPrimaryTypeName() external view returns (string memory);
+
+    // The EIP-712 type definition of the action, required by Only712MacroForwarder.
+    function getMessageTypeDefinition() external view returns (string memory);
+
+    // The struct hash of the action, required by Only712MacroForwarder.
+    // This hash must be constructed based on the type definition and the data, according to the EIP-712 standard.
     function getMessageStructHash(bytes memory message) external view returns (bytes32);
 }
diff --git a/packages/ethereum-contracts/contracts/utils/Only712MacroForwarder.sol b/packages/ethereum-contracts/contracts/utils/Only712MacroForwarder.sol
@@ -15,6 +15,7 @@ import { ForwarderBase } from "./ForwarderBase.sol";
 contract Only712MacroForwarder is ForwarderBase, EIP712 {
 
     // top-level data structure
+    // TODO: is "payload" a good name? Does EIP-712 give a good hint for naming this? Something "primary"?
     struct Payload {
         PayloadMeta meta;
         PayloadMessage message;
@@ -26,7 +27,8 @@ contract Only712MacroForwarder is ForwarderBase, EIP712 {
         //string language;
         //string disclaimer;
     }
-    bytes32 internal constant _TYPEHASH_META = keccak256("Meta(string domain,string version)");
+    bytes internal constant _TYPEDEF_META = "Meta(string domain,string version)";
+    bytes32 internal constant _TYPEHASH_META = keccak256(_TYPEDEF_META);
     struct PayloadMessage {
         string title;
         //string description;
@@ -39,14 +41,16 @@ contract Only712MacroForwarder is ForwarderBase, EIP712 {
         //uint256 validBefore;
         uint256 nonce;
     }
-    bytes32 internal constant _TYPEHASH_SECURITY = keccak256("Security(string provider,uint256 nonce)");
+    bytes internal constant _TYPEDEF_SECURITY = "Security(string provider,uint256 nonce)";
+    bytes32 internal constant _TYPEHASH_SECURITY = keccak256(_TYPEDEF_SECURITY);
 
     error InvalidPayload(string message);
     error InvalidProvider(string provider);
     error InvalidSignature();
 
-    // TODO: should this be something like "Clear Sign" instead?
-    constructor(ISuperfluid host, address /*registry*/) ForwarderBase(host) EIP712("Only712MacroForwarder", "1") {}
+    // Here EIP712 domain name and version are set.
+    // TODO: should the name include "Superfluid"?
+    constructor(ISuperfluid host, address /*registry*/) ForwarderBase(host) EIP712("ClearSigning", "1") {}
 
     /**
      * @dev Run the macro with encoded payload (generic + macro specific fragments).
@@ -83,11 +87,41 @@ contract Only712MacroForwarder is ForwarderBase, EIP712 {
         return retVal;
     }
 
+    // TODO: should this exist?
+    function getTypeDefinition(IUserDefined712Macro m) external view returns (string memory) {
+        return _getTypeDefinition(m);
+    }
+
+    // TODO: should this exist?
+    function getTypeHash(IUserDefined712Macro m) public view returns (bytes32) {
+        return keccak256(abi.encodePacked(_getTypeDefinition(m)));
+    }
+
+    // TODO: should this exist?
+    function getStructHash(IUserDefined712Macro m, bytes calldata params) external view returns (bytes32) {
+        return _getStructHash(m, abi.decode(params, (Payload)));
+    }
+
     function getDigest(IUserDefined712Macro m, bytes calldata params) external view returns (bytes32) {
         return _getDigest(m, abi.decode(params, (Payload)));
     }
 
-    function _getDigest(IUserDefined712Macro m, Payload memory payload) internal view returns (bytes32) {
+    // ==============================
+    // Internal functions
+    // ==============================
+
+    function _getTypeDefinition(IUserDefined712Macro m) internal view returns (string memory) {
+        return string(abi.encodePacked(
+            m.getPrimaryTypeName(),
+            "(Meta meta,Message message,Security security)",
+            // nested components need to be in alphabetical order
+            m.getMessageTypeDefinition(),
+            _TYPEDEF_META,
+            _TYPEDEF_SECURITY
+        ));
+    }
+
+    function _getStructHash(IUserDefined712Macro m, Payload memory payload) internal view returns (bytes32) {
         bytes32 metaStructHash = _getMetaStructHash(payload.meta);
 
         // the message fragment is handled by the user macro.
@@ -98,29 +132,23 @@ contract Only712MacroForwarder is ForwarderBase, EIP712 {
         bytes32 securityStructHash = _getSecurityStructHash(payload.security);
 
         // get the typehash
-        bytes32 primaryTypeHash = keccak256(
-            abi.encodePacked(
-                // TODO: shall we name it "ClearSign"?
-                "Payload(Meta meta,Message message,Security security)",
-                // nested components need to be in alphabetical order
-                m.getMessageTypeHash(),
-                _TYPEHASH_META,
-                _TYPEHASH_SECURITY
+        bytes32 primaryTypeHash = getTypeHash(m);
+
+        // calculate the struct hash
+        bytes32 structHash = keccak256(
+            abi.encode(
+                primaryTypeHash,
+                metaStructHash,
+                messageStructHash,
+                securityStructHash
             )
         );
+        return structHash;
+    }
 
-        // calculate the digest of the entire payload
-        bytes32 digest = _hashTypedDataV4(
-            keccak256(
-                abi.encode(
-                    primaryTypeHash,
-                    metaStructHash,
-                    messageStructHash,
-                    securityStructHash
-                )
-            )      
-        );
-        return digest;
+    function _getDigest(IUserDefined712Macro m, Payload memory payload) internal view returns (bytes32) {
+        bytes32 structHash = _getStructHash(m, payload);
+        return _hashTypedDataV4(structHash);
     }
 
     function _getMetaStructHash(PayloadMeta memory meta) internal pure returns (bytes32) {
diff --git a/packages/ethereum-contracts/test/foundry/utils/Only712MacroForwarder.t.sol b/packages/ethereum-contracts/test/foundry/utils/Only712MacroForwarder.t.sol
@@ -2,17 +2,34 @@
 pragma solidity ^0.8.23;
 
 import { VmSafe } from "forge-std/Vm.sol";
+import { console } from "forge-std/console.sol";
 import { ISuperfluid, ISuperfluidToken } from "../../../contracts/interfaces/superfluid/ISuperfluid.sol";
 import { IUserDefined712Macro } from "../../../contracts/interfaces/utils/IUserDefinedMacro.sol";
 import { Only712MacroForwarder } from "../../../contracts/utils/Only712MacroForwarder.sol";
 import { FoundrySuperfluidTester } from "../FoundrySuperfluidTester.t.sol";
 
-// ============== Minimal mock macro for Only712MacroForwarder ==============
+string constant MESSAGE_TITLE = "Hello 712";
+string constant PRIMARY_TYPE_NAME = "MinimalExample";
+string constant META_DOMAIN = "minimalmacro.xyz";
+string constant META_VERSION = "1";
+string constant SECURITY_PROVIDER = "macros.superfluid.eth";
+
+// returns the encoded payload for the example macro
+function getTestPayload() pure returns (bytes memory) {
+    Only712MacroForwarder.Payload memory payload = Only712MacroForwarder.Payload({
+        meta: Only712MacroForwarder.PayloadMeta({ domain: META_DOMAIN, version: META_VERSION }),
+        message: Only712MacroForwarder.PayloadMessage({ title: MESSAGE_TITLE, customPayload: new bytes(0) }),
+        security: Only712MacroForwarder.PayloadSecurity({ provider: SECURITY_PROVIDER, nonce: 1 })
+    });
+    return abi.encode(payload);
+}
+
+// ============== Minimal macro for Only712MacroForwarder ==============
 // Implements IUserDefined712Macro and uses *no* postCheck logic.
 // Message has only the required `title`; `customPayload` is expected to be empty.
 contract Minimal712Macro is IUserDefined712Macro {
-    bytes32 internal constant MESSAGE_TYPEHASH = keccak256("Message(string title)");
-    bytes32 internal constant EXPECTED_TITLE_HASH = keccak256(bytes("Hello 712"));
+
+    string public constant MESSAGE_TYPE_DEFINITION = "Message(string title)";
 
     function buildBatchOperations(ISuperfluid, bytes memory, address)
         external
@@ -27,15 +44,20 @@ contract Minimal712Macro is IUserDefined712Macro {
         // intentionally empty
     }
 
-    function getMessageTypeHash() external pure override returns (bytes32) {
-        return MESSAGE_TYPEHASH;
+    function getMessageTypeDefinition() external pure override returns (string memory) {
+        return MESSAGE_TYPE_DEFINITION;
+    }
+
+    function getPrimaryTypeName() external pure override returns (string memory) {
+        return PRIMARY_TYPE_NAME;
     }
 
     function getMessageStructHash(bytes memory message) external pure override returns (bytes32) {
         (string memory title, bytes memory customPayload) = abi.decode(message, (string, bytes));
-        require(keccak256(bytes(title)) == EXPECTED_TITLE_HASH, "wrong title");
+        require(keccak256(bytes(title)) == keccak256(bytes(MESSAGE_TITLE)), "wrong title");
         require(customPayload.length == 0, "customPayload not empty");
-        return keccak256(abi.encode(MESSAGE_TYPEHASH, keccak256(bytes(title))));
+        bytes32 messageTypeHash = keccak256(abi.encodePacked(MESSAGE_TYPE_DEFINITION));
+        return keccak256(abi.encode(messageTypeHash, keccak256(bytes(title))));
     }
 }
 
@@ -60,9 +82,9 @@ contract Only712MacroForwarderTest is FoundrySuperfluidTester {
      * @dev Smoke test: build payload, get digest via getDigest(), sign with vm.createWallet + vm.sign,
      *      call runMacro(m, params, signer, signature), assert success.
      */
-    function test_Minimal712Macro_runMacro_smoke() external {
+    function testRunMacro() external {
         VmSafe.Wallet memory signer = vm.createWallet("signer");
-        bytes memory params = _buildPayload();
+        bytes memory params = getTestPayload();
         bytes32 digest = forwarder.getDigest(IUserDefined712Macro(address(minimal712Macro)), params);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(signer, digest);
         bytes memory signatureVRS = abi.encodePacked(r, s, v);
@@ -72,12 +94,131 @@ contract Only712MacroForwarderTest is FoundrySuperfluidTester {
         assertTrue(ok);
     }
 
-    function _buildPayload() internal pure returns (bytes memory) {
-        Only712MacroForwarder.Payload memory payload = Only712MacroForwarder.Payload({
-            meta: Only712MacroForwarder.PayloadMeta({ domain: "test.xyz", version: "1" }),
-            message: Only712MacroForwarder.PayloadMessage({ title: "Hello 712", customPayload: new bytes(0) }),
-            security: Only712MacroForwarder.PayloadSecurity({ provider: "macros.superfluid.eth", nonce: 1 })
-        });
-        return abi.encode(payload);
+    function testDigestCalculation() external view {
+        // check the type definition
+        string memory typeDefinition = forwarder.getTypeDefinition(minimal712Macro);
+        string memory expectedTypeDefinition = "MinimalExample(Meta meta,Message message,Security security)Message(string title)Meta(string domain,string version)Security(string provider,uint256 nonce)";
+        assertEq(typeDefinition, expectedTypeDefinition, "typeDefinition mismatch");
+
+        // check the type hash
+        bytes32 typeHash = forwarder.getTypeHash(minimal712Macro);
+        bytes32 expectedTypeHash = vm.eip712HashType(expectedTypeDefinition);
+        assertEq(typeHash, expectedTypeHash, "typeHash mismatch");
+
+        // check the struct hash (includes type hash and the struct data)
+        bytes memory payload = getTestPayload();
+        bytes32 structHash = forwarder.getStructHash(minimal712Macro, payload);
+        bytes32 expectedStructHash = vm.eip712HashStruct(typeDefinition, payload);
+        assertEq(structHash, expectedStructHash, "structHash mismatch");
+
+        // check the digest
+        bytes32 digest = forwarder.getDigest(minimal712Macro, payload);
+        string memory dataToBeSignedJson = getDataToBeSignedJson();
+        console.log(dataToBeSignedJson);
+        bytes32 expectedDigest = vm.eip712HashTypedData(dataToBeSignedJson);
+        assertEq(digest, expectedDigest, "digest mismatch");
+    }
+
+    // example: https://github.com/vaquita-fi/vaquita-lisk/blob/c4964af9157c9cca9cfb167ac1a4450e36edb29e/contracts/test/VaquitaPool.t.sol#L142
+    // The splitting up into many functions avoids stack too deep error.
+    function getDataToBeSignedJson() internal view returns (string memory) {
+        return string(abi.encodePacked(
+            '{',
+            '"types": {', _getTypesJson(), '},',
+            '"primaryType": "MinimalExample",', // leaving this as literal in order to fit onto the stack
+            '"domain": {', _getDomainJson(), '},',
+            '"message": {',
+            '"meta": {', _getMetaJson(), '},',
+            '"message": {', _getMessageJson(), '},',
+            '"security": {', _getSecurityJson(), '}',
+            '}',
+            '}'
+        ));
+    }
+
+    function _getTypesJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            _getEIP712DomainTypeJson(),
+            _getMinimalExampleTypeJson(),
+            _getMessageTypeJson(),
+            _getMetaTypeJson(),
+            _getSecurityTypeJson()
+        ));
+    }
+
+    function _getEIP712DomainTypeJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"EIP712Domain": [',
+            '{"name": "name", "type": "string"},',
+            '{"name": "version", "type": "string"},',
+            '{"name": "chainId", "type": "uint256"},',
+            '{"name": "verifyingContract", "type": "address"}',
+            '],'
+        ));
+    }
+
+    function _getMinimalExampleTypeJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"MinimalExample": [',
+            '{"name": "meta", "type": "Meta"},',
+            '{"name": "message", "type": "Message"},',
+            '{"name": "security", "type": "Security"}',
+            '],'
+        ));
+    }
+
+    function _getMessageTypeJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"Message": [',
+            '{"name": "title", "type": "string"}',
+            '],'
+        ));
+    }
+
+    function _getMetaTypeJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"Meta": [',
+            '{"name": "domain", "type": "string"},',
+            '{"name": "version", "type": "string"}',
+            '],'
+        ));
+    }
+
+    function _getSecurityTypeJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"Security": [',
+            '{"name": "provider", "type": "string"},',
+            '{"name": "nonce", "type": "uint256"}',
+            ']'
+        ));
+    }
+
+    function _getDomainJson() internal view returns (string memory) {
+        return string(abi.encodePacked(
+            '"name": "ClearSigning",',
+            '"version": "1",',
+            '"chainId": ', vm.toString(block.chainid), ',',
+            '"verifyingContract": "', vm.toString(address(forwarder)), '"'
+        ));
+    }
+
+    function _getMetaJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"domain": "', META_DOMAIN, '",',
+            '"version": "', META_VERSION, '"'
+        ));
+    }
+
+    function _getMessageJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"title": "', MESSAGE_TITLE, '"'
+        ));
+    }
+
+    function _getSecurityJson() internal pure returns (string memory) {
+        return string(abi.encodePacked(
+            '"provider": "', SECURITY_PROVIDER, '",',
+            '"nonce": ', '1'
+        ));
     }
 }
