Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial commit for draft review #1

Merged
merged 2 commits into from
Feb 27, 2025
Merged

Conversation

christianangelopoulos
Copy link
Contributor

throwing initial thoughts in this PR

@christianangelopoulos
Copy link
Contributor Author

christianangelopoulos commented Feb 24, 2025

We need the witness fields to be added.

I'm thinking we add witness stuff in the AllowanceOrTransfer as just raw bytes and deconstruct it using functions in permit. if it's empty then the permit can just skip the witness — otherwise, just pack the bytes32 witness and string witnessTypeString into a raw bytes object in the struct. You can just manually decode by taking the first 32 bytes and taking the rest as string

    struct AllowanceOrTransfer {
        uint48 transferOrExpiration;
        address token;
        address spender;
        uint160 amountDelta;
        bytes witness; // tightly packed bytes32 + appended string
    }

    struct ChainPermits {
        uint64 chainId;
        uint48 nonce;
        AllowanceOrTransfer[] permits;
    }

    struct Permit3Proof {
        bytes32 preHash;
        ChainPermits permits;
        bytes32[] followingHashes;
    }

@christianangelopoulos
Copy link
Contributor Author

christianangelopoulos commented Feb 24, 2025

Talked to @re1ro and he convinced me that this is not the right approach. Instead we need a new function that can parse the permits with witness data, and a reference implementation of how contracts might consume witness bytes from a single signature on multiple contracts

Here is what I am thinking:

bytes32[] witnesses;
bytes32 witnessHash = keccak256(witnessArray);

For example for a permit with one action on OP, Base and Arbitrum might construct the witness like this:

struct OptimismContractData {
    uint256 chainId;
    bytes32 otherData1;
    bytes32 otherData2;
    // ... other fields
}

witnesses = [ OptimismDataHash, BaseDataHash, ArbitrumDataHash]
witnessHash = keccak256(witnessArray);

A contract on optimism might consume the witness like this:

    function consumePermit(
        address owner,
        uint256 deadline,
        uint48 timestamp,
        Permit3Proof memory proof,
        bytes calldata signature,
        bytes32[] memory witnesses,
        uint32 expectedPosition,
        OptimismContractData calldata contractData
    ) external {
    
        bytes32 optimismContractHash = keccak256(abi.encode(contractData));

        require(witnesses[expectedPosition] == optimismContractHash, "Invalid witness position");

        require(contractData.chainId == 10, "Invalid chain ID");

        bytes32 witnessHash = keccak256(abi.encodePacked(witnesses));
        
        // Verify and consume the permit
        IPermit3(permit3Address).permitWithWitness(
            owner,
            deadline,
            timestamp,
            proof,
            signature,
            witnessHash
        );

        // Do something with permit and witness data
        // e.g. transfer tokens based on contractData
    }

A possible way to avoid the need for the expected position variable is to use the Permit3 style calculation of the witness hash.

bytes32 witnessHash = keccak256(keccak256(keccak256(ArbitrumDataHash), BaseDataHash), OptimismDataHash)

This would allow the contract to consume the permit with the correct witness data without the need for the expectedPosition variable as it could just use the final hash as the chain specific witness hash.

@christianangelopoulos christianangelopoulos marked this pull request as ready for review February 27, 2025 01:13
@christianangelopoulos christianangelopoulos merged commit ead1b86 into main Feb 27, 2025
2 checks passed
@christianangelopoulos christianangelopoulos deleted the draft-review branch February 27, 2025 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant