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

Support ERC-6492 (Discussion) #68

Open
mpeyfuss opened this issue Jul 12, 2024 · 2 comments
Open

Support ERC-6492 (Discussion) #68

mpeyfuss opened this issue Jul 12, 2024 · 2 comments

Comments

@mpeyfuss
Copy link

Wanted to open this issue as a place to discuss supporting ERC-6492. The ERC calls for the offchain verifier to essentially deploy the AA wallet and then verify the ERC-1271 signature after deploying.

This honestly feels like overkill to me, and surprises me that dapps should be responsible for deploying the contract for users.

Instead, I think we could work on recognizing the magic bytes32 value at the end of the signature to see the type of signature it is. Then, if the contract isn't deployed, could at least raise a better error message.

Let me know what you think! Happy to work on this in a PR!

@mpeyfuss
Copy link
Author

Looking at the spec more, eth_call is suggested which doesn't submit a tx to the blockchain. Was getting that confused with call in solidity! So there might be a standard path forward. Will think more on it.

@mpeyfuss
Copy link
Author

I started making a fork/PR for this, but ran into some questions about the desire to refactor parts of this repo as I could see inclusion of this, plus other future standards, making the actual siwe.py file much longer and complex. Instead, I made a quick update on our backend to account for ERC-6492 signatures and got it working. You can see the code sample below:

if signature.endswith(ERC6492_MAGIC_BYTES):
    # decode data from the signature
    factory_contract, factory_calldata, original_erc2171_signature = decode(
        ["address", "bytes", "bytes"], HexBytes(signature)
    )

    # create multicall contract
    w3 = web3(str(msg.chain_id))
    multicall_contract = w3.eth.contract(
        address=w3.to_checksum_address(MULTICALL3_ADDRESS), abi=MULTICALL3_ABI
    )

    # generate EIP-1271 calldata
    eip1271_contract = w3.eth.contract(
        address=msg.address, abi=EIP1271_CONTRACT_ABI
    )
    message = encode_defunct(text=msg.prepare_message())
    msg_hash = _hash_eip191_message(message)
    tx = eip1271_contract.functions.isValidSignature(
        msg_hash, original_erc2171_signature
    ).build_transaction()

    # first eth_call
    data = multicall_contract.functions.aggregate3(
        [
            (
                w3.to_checksum_address(factory_contract),  # target address
                True,  # allow failure
                factory_calldata,  # calldata
            ),
            (
                msg.address,  # target address
                True,  # allow failure
                HexBytes(tx["data"]),  # calldata
            ),
        ]
    ).call()

    sig_validation_successful = data[1][0]
    magic_value = decode(["bytes4"], data[1][1])[0].hex()

    # get signature repsonse
    return sig_validation_successful and magic_value == EIP1271_MAGICVALUE

Questions:

  1. How should this repo handle ABIs? Better to just put in a separate abi folder if we expect more to come in the future?
  2. How should the multicall3 address be handled? Someone may want to use their own multicall address if we choose one as the default, so having a way to override could be good. Could be simple to just add as an optional kwarg to the verify_message function. But for future proofing, could also see the use of a class being beneficial.

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

No branches or pull requests

1 participant