fix(erc7683): EIP712 encoding

[paco0x]

This PR fixes the inconsistencies with EIP712 in the ERC7683Across contract.

1. encodeType

If the struct type references other struct types (and these in turn reference even more struct types), then the set of referenced struct types is collected, sorted by name and appended to the encoding.

When encoding the PermitWitnessTransferFrom struct from permit2, other referenced struct types should be sorted by name. This point is also mentioned in Uniswap's document: https://docs.uniswap.org/contracts/permit2/reference/signature-transfer

So the full normalized type definition should be like(struct fields are omitted for simplicity):

PermitWitnessTransferFrom(...)AcrossOrderData(...)CrossChainOrder(...)TokenPermissions(...)

This patch adds a new constant PERMIT2_CROSS_CHAIN_ORDER_TYPE for this normalized type.

2. encodeData

Definition of encodeData
The encoding of a struct instance is enc(value₁) ‖ enc(value₂) ‖ … ‖ enc(valueₙ), i.e. the concatenation of the encoded member values in the order that they appear in the type. Each encoded member value is exactly 32-byte long.

This patch changes abi.encodePacked to abi.encode when encoding CrossChainOrder and AcrossOrderData since every member should be in 32 bytes.

The dynamic values bytes and string are encoded as a keccak256 hash of their contents.

orderData.message should be hashed before encoding.

The typed-data hash after fixing has already been cross-verified with viem and ethers. I can add tests if it's required.

[nicholaspai]

I agree with the second point which i've implemented in #519 but on the first point I think we are already compliant.

If the struct type references other struct types (and these in turn reference even more struct types), then the set of referenced struct types is collected, sorted by name and appended to the encoding.

An example encoding is Transaction(Person from,Person to,Asset tx)Asset(address token,uint256 amount)Person(address wallet,string name).

EIP712

So I think in our case, the Permit2 witnessTypeString is CrossChainOrder which references AcrossOrderData and also needs to append TokenPermissionsType, but think the only important thing is that AcrossOrderData follows CrossChainOrder but precedes TokenPermissionsType

[paco0x]

I agree with the second point which i've implemented in #519 but on the first point I think we are already compliant.

If the struct type references other struct types (and these in turn reference even more struct types), then the set of referenced struct types is collected, sorted by name and appended to the encoding.

An example encoding is Transaction(Person from,Person to,Asset tx)Asset(address token,uint256 amount)Person(address wallet,string name).

EIP712

So I think in our case, the Permit2 witnessTypeString is CrossChainOrder which references AcrossOrderData and also needs to append TokenPermissionsType, but think the only important thing is that AcrossOrderData follows CrossChainOrder but precedes TokenPermissionsType

the set of referenced struct types is collected means it's a flattened list of subtypes, including all the referenced types and nested referenced types.

So [CrossChainOrder, AcrossOrderData, TopkenPermissionsType] are subtypes of PermitWitnessTransferFrom and should be sorted alphabetically when computing the type hash of type PermitWitnessTransferFrom.

In your example, [Person, Asset] are subtypes of Transaction and are sorted alphabetically. But there isn't nested subtypes in this case.

You can also check the implementation details in ethers.js here: https://github.com/ethers-io/ethers.js/blob/92761872198cf6c9334570da3d110bca2bafa641/src.ts/hash/typed-data.ts#L327-L342

It can be verified with the code (ethers v6):

import assert from "assert";
import { TypedDataEncoder } from "ethers";

const types = {
  PermitWitnessTransferFrom: [
    { name: "permitted", type: "TokenPermissions" },
    { name: "spender", type: "address" },
    { name: "nonce", type: "uint256" },
    { name: "deadline", type: "uint256" },
    { name: "witness", type: "CrossChainOrder" },
  ],
  CrossChainOrder: [
    { name: "settlerContract", type: "address" },
    { name: "swapper", type: "address" },
    { name: "nonce", type: "uint256" },
    { name: "originChainId", type: "uint32" },
    { name: "initiateDeadline", type: "uint32" },
    { name: "fillDeadline", type: "uint32" },
    { name: "orderData", type: "AcrossOrderData" },
  ],
  AcrossOrderData: [
    { name: "inputToken", type: "address" },
    { name: "inputAmount", type: "uint256" },
    { name: "outputToken", type: "address" },
    { name: "outputAmount", type: "uint256" },
    { name: "bridgeToken", type: "address" },
    { name: "bridgeTokenAmountOutMinimum", type: "uint256" },
    { name: "destinationChainId", type: "uint32" },
    { name: "recipient", type: "address" },
    { name: "exclusivityDeadline", type: "uint32" },
    { name: "message", type: "bytes" },
  ],
  TokenPermissions: [
    { name: "token", type: "address" },
    { name: "amount", type: "uint256" },
  ],
};

const encoder = new TypedDataEncoder(types);
const result = encoder.encodeType("PermitWitnessTransferFrom");

const expectedResult =
  "PermitWitnessTransferFrom(TokenPermissions permitted,address spender,uint256 nonce,uint256 deadline,CrossChainOrder witness)" +
  "AcrossOrderData(address inputToken,uint256 inputAmount,address outputToken,uint256 outputAmount,address bridgeToken,uint256 bridgeTokenAmountOutMinimum,uint32 destinationChainId,address recipient,uint32 exclusivityDeadline,bytes message)" +
  "CrossChainOrder(address settlerContract,address swapper,uint256 nonce,uint32 originChainId,uint32 initiateDeadline,uint32 fillDeadline,AcrossOrderData orderData)" +
  "TokenPermissions(address token,uint256 amount)";

assert.equal(expectedResult, result);

The result is:

PermitWitnessTransferFrom(TokenPermissions permitted,address spender,uint256 nonce,uint256 deadline,CrossChainOrder witness)AcrossOrderData(address inputToken,uint256 inputAmount,address outputToken,uint256 outputAmount,address bridgeToken,uint256 bridgeTokenAmountOutMinimum,uint32 destinationChainId,address recipient,uint32 exclusivityDeadline,bytes message)CrossChainOrder(address settlerContract,address swapper,uint256 nonce,uint32 originChainId,uint32 initiateDeadline,uint32 fillDeadline,AcrossOrderData orderData)TokenPermissions(address token,uint256 amount)

Also in viem, the implementation here: https://github.com/wevm/viem/blob/d6053239c4106c160f66863a8afff43961569f5e/src/utils/signature/hashTypedData.ts#L173-L218

the function findTypeDependencies is a recursive function to collect all the subtypes(including nested) of a primary type.

They don't export the encodeType function, but we can extract it to test it out

import assert from "assert";

// from viem/src/utils/signature/hashTypedData.ts
type MessageTypeProperty = {
  name: string;
  type: string;
};

function encodeType({
  primaryType,
  types,
}: {
  primaryType: string;
  types: Record<string, MessageTypeProperty[]>;
}) {
  let result = "";
  const unsortedDeps = findTypeDependencies({ primaryType, types });
  unsortedDeps.delete(primaryType);

  const deps = [primaryType, ...Array.from(unsortedDeps).sort()];
  for (const type of deps) {
    result += `${type}(${types[type]
      .map(({ name, type: t }) => `${t} ${name}`)
      .join(",")})`;
  }

  return result;
}

function findTypeDependencies(
  {
    primaryType: primaryType_,
    types,
  }: {
    primaryType: string;
    types: Record<string, MessageTypeProperty[]>;
  },
  results: Set<string> = new Set()
): Set<string> {
  const match = primaryType_.match(/^\w*/u);
  const primaryType = match?.[0]!;
  if (results.has(primaryType) || types[primaryType] === undefined) {
    return results;
  }

  results.add(primaryType);

  for (const field of types[primaryType]) {
    findTypeDependencies({ primaryType: field.type, types }, results);
  }
  return results;
}

const types = {
  PermitWitnessTransferFrom: [
    { name: "permitted", type: "TokenPermissions" },
    { name: "spender", type: "address" },
    { name: "nonce", type: "uint256" },
    { name: "deadline", type: "uint256" },
    { name: "witness", type: "CrossChainOrder" },
  ],
  CrossChainOrder: [
    { name: "settlerContract", type: "address" },
    { name: "swapper", type: "address" },
    { name: "nonce", type: "uint256" },
    { name: "originChainId", type: "uint32" },
    { name: "initiateDeadline", type: "uint32" },
    { name: "fillDeadline", type: "uint32" },
    { name: "orderData", type: "AcrossOrderData" },
  ],
  AcrossOrderData: [
    { name: "inputToken", type: "address" },
    { name: "inputAmount", type: "uint256" },
    { name: "outputToken", type: "address" },
    { name: "outputAmount", type: "uint256" },
    { name: "bridgeToken", type: "address" },
    { name: "bridgeTokenAmountOutMinimum", type: "uint256" },
    { name: "destinationChainId", type: "uint32" },
    { name: "recipient", type: "address" },
    { name: "exclusivityDeadline", type: "uint32" },
    { name: "message", type: "bytes" },
  ],
  TokenPermissions: [
    { name: "token", type: "address" },
    { name: "amount", type: "uint256" },
  ],
};

const result = encodeType({
  primaryType: "PermitWitnessTransferFrom",
  types,
});

const expectedResult =
  "PermitWitnessTransferFrom(TokenPermissions permitted,address spender,uint256 nonce,uint256 deadline,CrossChainOrder witness)" +
  "AcrossOrderData(address inputToken,uint256 inputAmount,address outputToken,uint256 outputAmount,address bridgeToken,uint256 bridgeTokenAmountOutMinimum,uint32 destinationChainId,address recipient,uint32 exclusivityDeadline,bytes message)" +
  "CrossChainOrder(address settlerContract,address swapper,uint256 nonce,uint32 originChainId,uint32 initiateDeadline,uint32 fillDeadline,AcrossOrderData orderData)" +
  "TokenPermissions(address token,uint256 amount)";

assert.equal(expectedResult, result);

The result is also consistent with the result of ethers.js

[mrice32]

@paco0x need to merge in a recent change to get ci running.

We've discussed this internally and we agree with your interpretation! Thanks for correcting this -- huge help!

LGTM!

[mrice32]

@paco0x I don't think I have permissions to merge master into your branch.

Could you pull the most recent changes from master and run git merge master and then push the result? I want to make sure you get credit for the contribution :)

[paco0x]

@paco0x I don't think I have permissions to merge master into your branch.

Could you pull the most recent changes from master and run git merge master and then push the result? I want to make sure you get credit for the contribution :)

Thx! I just merged master and updated it a little bit to reduce some duplicated code.