-
Notifications
You must be signed in to change notification settings - Fork 14
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
Eip1271 Support #34
base: main
Are you sure you want to change the base?
Eip1271 Support #34
Conversation
Fixes #29 Adds Support for [EIP-1271](https://eips.ethereum.org/EIPS/eip-1271) for allowing contract accounts to give and receive delegations. Required one additional bit to the `SignedDelegation` and `SignedInvocation` types: `signerIsContract`. If this bit is `true`, then the first 20 bytes of the `signature` field is treated as the address of a contract to treat as the intended signer, and that contract is sent the remaining `signature: bytes` along with the delegation type hash to determine for itself whether this proof should be treated as valid authorization. I formerly was somewhat against using EIP-1271 in this way, because I had seen some contract accounts merely allow assigning a single signer as their EIP1271 recovery strategy, which completely undermines the entire point of having a contract account. My position on this has evolved a bit, to believe that correct usage is possible, and so we shouldn't let the possibility of flawed contract accounts prevent good ones from participating in this. For example, good usage might look like a multisig might have a custom datastructure for representing which accounts' signatures are included, and including all the signatures combined in the `signature` payload. Larger controller sets like a token-weighted DAO might involve too many signatures to submit as a delegation/invocation as `calldata`, but I'll leave that research as to do.
Some things I might consider before merging:
|
Oh this PR also fixes the timestamp-enforcer, which was mixed up. I've since learned it was being deprecated. Oh well. That's what happens when you require all tests to pass before a push :D |
minor change on off chain js libs; sign invocations by calling .signInvocations() - not .signInvocation() |
Ah I am probably going to make another change to this. I might make that change in eip712-codegen first so I can minimize the rewriting in the future. https://twitter.com/frangio_/status/1629994287080374272?s=46&t= |
Ok the codegen rewrite is working, so I'm in a good position to re-implement these changes with a few others I've been wanting to make, all with auto-generated signature verification code now too: |
Fixes #29
Adds Support for EIP-1271 for allowing contract accounts to give and receive delegations.
Required one additional bit to the
SignedDelegation
andSignedInvocation
types:signerIsContract
. If this bit istrue
, then the first 20 bytes of thesignature
field is treated as the address of a contract to treat as the intended signer, and that contract is sent the remainingsignature: bytes
along with the delegation type hash to determine for itself whether this proof should be treated as valid authorization.I formerly was somewhat against using EIP-1271 in this way, because I had seen some contract accounts merely allow assigning a single signer as their EIP1271 recovery strategy, which completely undermines the entire point of having a contract account. My position on this has evolved a bit, to believe that correct usage is possible, and so we shouldn't let the possibility of flawed contract accounts prevent good ones from participating in this.
For example, good usage might look like a multisig might have a custom datastructure for representing which accounts' signatures are included, and including all the signatures combined in the
signature
payload.Larger controller sets like a token-weighted DAO might involve too many signatures to submit as a delegation/invocation as
calldata
, but I'll leave exploration for how to best support that to those contracts' owners.