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

VIP: Signed Structs (EIP 712 Support) #1020

Open
fubuloubu opened this issue Sep 14, 2018 · 17 comments
Open

VIP: Signed Structs (EIP 712 Support) #1020

fubuloubu opened this issue Sep 14, 2018 · 17 comments
Labels
Easy Pickings Used to denote issues that should be easy to implement help wanted VIP: Approved VIP Approved

Comments

@fubuloubu
Copy link
Member

fubuloubu commented Sep 14, 2018

Simple Summary

Allow defining a struct that contains a signature field such that there is a standard algorithm for performing ecrecover on it.
Would be most useful with #1019

Abstract

In constructions like Plasma, it is often useful to define a transaction data structure to manage the complexity of a transaction type and how it is used to validate logic in the contract. Having a struct type that can define an extra signature field which can be processed in a standard algorithm to recover the signer of the transaction would be very helpful to streamline the design of these algorithms without too much complexity in the contract.

Motivation

We want to make working with different constructions like Plasma as simple and expressive as possible. Primatives like working with signed transactions are very commonplace in this constructions, and we should support primitives like this as first class citizens to make it easier and safer to write these kinds of contracts.

Specification

We might define a structure as follows that contains a VRS signature with which recovery of the signing address can be performed via exclusion of the signature, hashing of the rest of the struct as an RLP encoded object, and performing ecrecover(hash, r, s, v) of the result.
The field that would contain this signature in this special struct would be denoted by __sig__ .
get_signer(struct) would be the function that performed this function to return the signer's address.

struct Transaction:
    a: address
    b: uint256
    __sig__: VRS_SECP256K1  # Special type representing a VRS signature for a Secp256k1 signature
...

# Assert that the transaction's signer is the caller here.
assert get_signer(transaction) == msg.sender
...

There might potentially be defined a custom type to represent a VRS signature, but should be extensible with other types for other signature methods that may be included later.

Backwards Compatibility

This would be fully backwards compatible as it is a new featureset that would be added.

Copyright

Copyright and related rights waived via CC0

@jacqueswww jacqueswww added VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting VIP: Approved VIP Approved labels Sep 19, 2018
@jacqueswww
Copy link
Contributor

Approved, dependency on #1019 .

@fubuloubu
Copy link
Member Author

Note: dependency is only recommended to make it easier to work with. This proposal stands alone.

@charles-cooper
Copy link
Member

@jacqueswww
Copy link
Contributor

We will evaluate this one at implementation time, to probably use EIP712 :)

@charles-cooper
Copy link
Member

After absorbing EIP712 a little more, it seems that this VIP and EIP712 are fairly closely aligned and we can follow the spec in EIP712 for hashing/signing structs. I that __sig__ doesn't need to be part of the struct, but rather could be carried around as associated data. So for instance, following the analogy between eth_sign and eth_signTypedData, ecrecover for structs could look like ecrecover_packed(digest_typed_data(MyDomain, struct), sig) as in https://github.com/ethereum/EIPs/blob/f2d42c39e9318d4a706e1e06bacf09c9383a981e/assets/eip-712/Example.sol#L74.

The only thing in EIP712 which hasn't been considered yet for the scope of this VIP is the domain separator struct. I propose a new keyword to help define this struct, EIP712Domain, and if/when EIP712 is broadly implemented we can change the keyword simply to domain. This keyword can be implemented similarly to contract and struct, and have the following syntax:

EIP712Domain MyDomain:
    name: [string literal]
    chainId: [uint256 literal]
    verifyingContract: [address literal]
    salt: [bytes32 literal]

As in the EIP712 spec, the EIP712Domain definition must have one or more of those fields, but may omit unused fields. The EIP712Domain may have a name, which permits defining multiple domains in a single .vy file. This is allows the dapp developer to sign structs for multiple other contracts, or maintain multiple domains in a single contract. Additionally, I considered that a struct definition could include a domain so that digest_typed_data does not need to take a domain as a parameter, but to avoid complications around type-checking and usability, I think they should be defined and passed around completely separately.

@fubuloubu
Copy link
Member Author

Mostly agree with that assessment, with a few caveats.


"[The signature can] be carried around as associated data."

How would this work in practice?

My idea with having the signature defined as part of the struct would be to allow signed transactions to be submitted as struct data inputs to functions that may use that transaction to verify or make an assertion about some larger action, like the Plasma exit use case (reference my code).

I also wanted to make it easier to do core operations on these data structures, like recover the signing account, without having to directly handle the signature or anything. Something like assert msg.sender == recover_account(signed_transaction)

@fubuloubu fubuloubu changed the title VIP: Signed Structs VIP: Signed Structs (EIP 712 Support) Jan 24, 2019
@fubuloubu
Copy link
Member Author

Linking to ethereum/EIPs#712 for posterity

@charles-cooper
Copy link
Member

A couple conclusions from gitter:
EIP712Domain should also include version (of course!)
verifyingContract should allow self as an option
chainId should allow the chainId opcode as an option (dependent on EIP 1344)

Also, the contract should expose the domain separator values as public variables

@fubuloubu
Copy link
Member Author

Should we allow chainId before EIP 1344 is implemented?

@jacqueswww
Copy link
Contributor

Yes, rather safe than sorry.

@charles-cooper
Copy link
Member

charles-cooper commented Apr 4, 2019 via email

@fubuloubu
Copy link
Member Author

Note: Cannot implement this manually without #1406

@charles-cooper
Copy link
Member

Note: Cannot implement this manually without #1406

Really? I figure you only need abi.encode in order to compute the EIP712 hash.

@fubuloubu
Copy link
Member Author

Yeah, that was my point haha. Without abi.encode I can't implement EIP712 in Vyper manually.

This VIP would add an additional syntax to shortcut the manual process. It could also get away with not leveraging abi.encode internally, which is why this proposal does not depend on #1406

@fubuloubu
Copy link
Member Author

fubuloubu commented May 16, 2019

What do we think of the following as a specification for this type of message signing and signature recovery, using EIP712:

from vyper.messages import EIP712Domain, recover_message
from vyper.signatures import VRS_SECP256K1  # (v, r, s) tuple type, 65 bytes

# Can be any valid struct
# (gets signed offline using EIP712 API with above domain)
struct Transaction:
    a: address
    b: uint256

@public
def foo(txn: Transaction, sig: VRS_SECP256K1)
    # Assert that the transaction's signer is the caller here.
    myDomain: EIP712Domain = EIP712Domain({
        name: [string literal]                        # optional unless no others
        version: [string literal]                     # optional unless no others
        chainId: [uint256 literal or variable]        # optional unless no others
        verifyingContract: [address literal OR self]  # optional unless no others
        salt: [bytes32 literal]                       # optional unless no others
    })
    assert recover_message(txn, sig, type=myDomain) == msg.sender
    ...  # Stuff, now that we authenticated the signer

@fubuloubu fubuloubu removed the VIP: Discussion Used to denote VIPs and more complex issues that are waiting discussion in a meeting label Aug 8, 2019
@fubuloubu
Copy link
Member Author

With the upcoming Istanbul hardfork, we should include support for EIP-1344 in this feature.

@fubuloubu
Copy link
Member Author

Had another idea for syntax using decorators which I think would look much cleaner:

from vyper.message import EIP712Domain, EIP712Message, eip712_recover

EIP712Domain TransactionDomain:
    name: "My Protocol"      # must be a string
    version: "1.0"           # must be a string
    chainId: tx.chain_id     # this or uint256 literal
    verifyingContract: self  # this or address literal

# By decorating with this datatype, this informs
# the compiler to use the EIP712 Hashing strategy
@EIP712Message(TransactionDomain)
struct Transaction:
    a: address
    b: uint256

@public
def do_something(
    # Hash via EIP712 hashing strategy because of EIP712Message decorator
    _msg: Transaction,
    # bytes[65] under the hood, note: web3py uses RSV convention... but precompile uses VRS
    _sig: RSV_SECP256K1,
) -> address:
    # 1. Hash TransactionDomain and _msg together (3 hashes)
    # 2. Extract _sig to r, s, v components
    # 3. Run ecrecover precompile
    return eip712_recover(_msg, _sig)

@charles-cooper charles-cooper added help wanted Easy Pickings Used to denote issues that should be easy to implement labels Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy Pickings Used to denote issues that should be easy to implement help wanted VIP: Approved VIP Approved
Projects
None yet
Development

No branches or pull requests

3 participants