-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
ERC-1271 : Standard Signature Validation Method for Contracts #1271
Comments
Realizing that this is more than just validating signatures; we're actually asking the question "given this action, does the caller have the ability to it, given this proof"? so perhaps the better metaphor is
where the default behavior might be to treat Although this implies that the proof handling and the access control are part of the same function/contract where they could potentially be different, although I'm not sure how that separation would generalize at all (i.e. in the case of ECDSA signatures, the validity is just the recovery, which produces an Thoughts, @PhABC? |
That's a good point! A signature is usually used to guarantee that a given address allows something, but other schemes not relying on signature methods could also achieve the same goal. Pinging @alexvandesande & @abandeali1 for comments. |
I personally prefer using a 32 byte hash over an I could see it making sense to use the word |
The reason why we opted for a dynamic byte array is because the For instance, imagine you use a smart account that has some key management properties, where different keys have different roles (RBAC style). Now imagine that the private key on your phone can sell some game assets, but can't any other types of tokens. Your ledger private key can sell any type of asset. If the caller contract (e.g. 0x contract) only passes the hash of the message, the smart account has no way of knowing what the asset being traded is. The smart account would see two hashes that were indeed signed by two private key you own, but it would have no way of verifying if the action signed is valid based on it's own internal conditions. In this case, your smart account would either need to allow all your private keys to exchange assets via 0x (let them sign hashes) or prevent them all. In general, what i'm trying to say is that I think there could always be additional rules not encompassed by the caller contract that the called contract requires. |
Likewise, my worry is that a hash loses information that may be necessary for validation. That said, I'm also worried about the complexity of passing arbitrary data via a bytes call; we've just accidentally re-created the need for abi encoding/decoding and whoops, we should probably just use solidity for that, so now we're back where we started, creating context-specific methods. |
You can always encode any extra data about the
You can imagine adding any amount of extra data to |
@abandeali1 I'm not convinced that overloading the signature ("proof") argument is a ton better than |
@PhABC any feedback on restricting this to hash+signature based validation? |
@shrugs my point is that it can be done with just a hash+signature, but I wouldn't expect that to be frequently used. |
Thanks for writing this up @PhABC, it's quite good. The discussion around @abandeali1's suggestion is very interesting, but we need to show some examples of usage in order to best decide on fixed or arbitrary size I think a good example to analyze is that of an m-of-n multisig smart account. A valid signature for this account could be the concatenation of m (signature, signer) pairs, where the m signers are in the set of n signers, and each signature is valid for the message and respective signer. I think this is enough motivation for the signature to be of arbitrary size. Now suppose that a family of messages can only be considered signed when the full set of n signers approve. It's common to increase the required number of signatures for large value transfers, so I think it would make sense for the signature scheme to have something analogous to that. The validity of the signature then depends on some parameter in the message being signed. Per @abandeali1's suggestion, the parameter should be included as part of the signature, but then the contract would have to additionally verify that the parameter in the signature is indeed the one contained in the message. If the message is hashed, then the entire message has to also be included in the signature in order to verify the hash. That would be a lot of redundant bytes being passed around. IMO this justifies having the message be of arbitrary size, and not necessarily a hash. Do you all agree the scenario described makes sense? A mostly unrelated question about the EIP. Would it be valid for an implementation of |
I forgot to comment on the suggestion to use the names "action" and "proof". Although I kind of like "proof", I would prefer to keep the current names as they're consistent with those of cryptographic signing, and agnostic to any particular use case. |
@frangio overloading the With With So in the worst case scenario of using Now let's look that the case where we we use |
To be honest, the cost differences here are probably minimal. I think the main downside for the |
The complexity of converting a I'm happy with, in order of preference:
For naming, I'd prefer how about some fun voting, multiple choice ok:
(hmm, github should have twitter-style polls that only followers of an issue at time-of-poll can vote on) |
It seems like we'll need to include more information than just action/signature. I'm having trouble visualizing how to abstract signature validation like this so that contracts can recursively "sign". I think we need to add the "delegate" concept here so we can do "recursive" validity. i.e. I'm trying to sign something with my contract, but I want to proxy that validity to another contract. I ended up writing something like this; does it make sense? // added this function to ECRecovery, so it's used as _hash.isSignedBy(signer, sig)
function isSignedBy(bytes32 _hash, address _signer, bytes _sig)
internal
view
returns (bool)
{
// if the signer address supports SignatureDelegation, delegate to it
if (_signer.supportsInterface(InterfaceId_SignatureDelegate)) {
return ISignatureDelegate(_signer).isValidAction(_signer, _hash, _sig);
}
// otherwise make sure the hash was personally signed by the EOA account
return _signer == recover(toEthSignedMessageHash(_hash), _sig);
}
/**
* @dev An action is valid iff the _sig of the _action is from an key with the ACTION role
*/
function isValidAction(address _signer, bytes32 _action, bytes _sig)
view
public
returns (bool)
{
// permission
bool hasPermission = _signer == address(this) || hasRole(_signer, ROLE_ACTION);
// validity
bool isValid = _action.isSignedBy(_signer, _sig);
return hasPermission && isValid;
} I haven't written any tests for this, but this allows, for example, a multisig to own an identity that owns another identity that can sign 0x orders and operate with off-chain tooling that expects signatures. |
So, the information that you added was the account for which you need to verify validity, right? That's an interesting thing to point out. With |
Regarding @shrugs I do like both term pairs ; "proof" is probably more future proof, while "signature" will be more easily understood for a while given the current practices in the space. Action might not be general enough however, where it's not clear how the term "action" fits with the hash of an 0x order for example. Regardless, we can always change the terms in the future since argument naming changes should be backward compatible. Regarding Both allow for passing extra data to the receiving contract, so it's really a question of efficiency. It does seem like both have tradeoffs ; In terms of converting contract C {
function toBytes() public pure returns (bytes) {
bytes32 a;
return abi.encodePacked(a);
}
} which costs about 210 gas. Not sure about Edit : Christian from the Solidity team told me the Regarding passing signer as argument That's a good point @shrugs. As a side note, I think isValid = IValidator(validatorAddress).isValidSignature(
hash,
signerAddress,
signature
); Code from here. The above also works for contracts being signers. Also, why couldn't |
Although I'm realizing there still needs to be a way to recover the contract signer for a signature. Maybe it could be the highly compacted chain of contract addresses like |
@phabcd if the account |
With respect to what? I'm not sure I get what this is specifically referring to. |
sorry, was on mobile; basically, the logic to evaluate
|
Ah, I understand. So the idea behind having an EOA account using a signature validator contract is for the scenario where the user wants to hold its funds in EOA but does not want to use ECDSA signature scheme, hence need to use a validator contract. It's a pretty niche case, but you could imagine things like BLS signatures or other aggregation signature methods to more efficiently verify many signatures in batch or even quantum secure signature schemes. |
I think we're misunderstood, again; I hadn't considered that case, actually. Removing any preconditions, here is some proposed code. // interface
function isValidSignature(bytes _action, bytes _sig) function isSignedBy(bytes _action, address _signer, bytes _sig)
internal
view
returns (bool)
{
// if the signer address supports signature validation, ask for its permissions/validity
// which means _sig can be anything
if (_signer.supportsInterface(InterfaceId_SignatureDelegate)) {
return ISignatureValidator(_signer).isValidSignature(_action, _sig);
}
// otherwise make sure the hash was personally signed by the EOA account
// which means _sig should be highly compacted vrs
bytes32 signedHash = toEthSignedMessageHash(BytesConverter.toBytes32(_action));
return _signer == recover(signedHash, _sig);
} // "isValidSignature" implementation for identity contracts
/**
* @dev An action is valid iff the _sig of the _action is from an key with the ACTION purpose
* @param _action
* @param _sig [[address] [address] [...]] <address> <v> <r> <s>
*/
function isValidSignature(bytes _action, bytes _sig)
view
public
returns (bool)
{
// the fact that this method has been called means the caller knows/expects that
// this contract has permission to do a thing.
(nextSigner, sig) = splitNextSignerAndSig(_sig);
// permission
bytes32 keyId = KeyUtils.idForAddress(nextSigner);
bool hasPermission = keyHasPurpose(keyId, PURPOSE_ACTION);
// validity
bool isValid = _action.isSignedBy(nextSigner, _sig);
return hasPermission && isValid;
} // now the 0x contracts can do something like
// https://github.com/0xProject/0x-monorepo/blob/development/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol#L188
BytesConverter.toBytes(hash).isSignedBy(signerAddress, signature);
// ... and everything should cascade downwards. So identity contracts can own identity contracts (i.e., multisigs can own multisigs) and we can encoded arbitrary validation logic as well (your identity contract would support BLS verification). The recursive nature of validation is what I think we were missing. without it, we can only do single-level signature validation. |
Is there anything in the current EIP text that you think doesn't enable this? I was going to suggest that |
presumably we're acting on the contract's behalf, so if it lied and said something was authorized when it wasn't, that'd be its own issue... but I'd have to take some time to think of attack scenarios. |
@frangio if a contract doesn't support a method, it might just return garbage data; that's why 165 does two calls to make doubly sure that it actually supports the But the current interface definition in the spec is fine, since it claims |
The previous attempt to get this EIP out of draft was not successful. Could someone summarize the status of this EIP, and what are the blockers? @PhABC? |
While it would import backward compatibility, the registry adds extra complexity and risks. While I understand these risks can be mitigated by deploying a single trusted registry per blockchain, which the same address on all networks, it would still increase the complexity of apps wanting to check signatures. An analogy would be 165 vs 1820. The only usecase I know where 1820 is used, is for 777 recipient ... and this is a major pain is the ***. I'm also worried about the storage cost of the registry, and what would happen if a "renting model" is applied to storage. |
Honestly I'm still surprised that this ERC is taking so long to finalize... In my opinion it had a temporary shift in direction when it required the data as a parameter yet the complexity was dramatically higher with little benefit and the standard was reverted to use the hash instead again. I think there is value to creating a new ERC where you can call a contract to hash and validate data to be signed but it definitely should not block this ERC from finalizing ERC-1271 in my opinion should be minimal and focused on doing exactly what |
@albertocuestacanada @pedrouid Updated the EIP based on @MicahZoltu's feeback from last year when attempted to push to final. New attempt at Last Call ; #3628 |
Really like this EIP. Just a small question, shouldn't this be an interface that makes use of ERC165? pragma solidity ^0.5.0;
contract ERC1271 {
// bytes4(keccak256("isValidSignature(bytes32,bytes)")
bytes4 constant internal MAGICVALUE = 0x1626ba7e;
/**
* @dev Should return whether the signature provided is valid for the provided hash
* @param _hash Hash of the data to be signed
* @param _signature Signature byte array associated with _hash
*
* MUST return the bytes4 magic value 0x1626ba7e when function passes.
* MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
* MUST allow external calls
*/
function isValidSignature(
bytes32 _hash,
bytes memory _signature)
public
view
returns (bytes4 magicValue);
} so... interface IERC1271 is IERC165 {
// bytes4(keccak256("isValidSignature(bytes32,bytes)")
bytes4 private constant _INTERFACE_ID_ERC1271 = 0x1626ba7e;
/**
* @dev Should return whether the signature provided is valid for the provided hash
* @param _hash Hash of the data to be signed
* @param _signature Signature byte array associated with _hash
*
* MUST return the bytes4 magic value 0x1626ba7e when function passes.
* MUST NOT modify state (using STATICCALL for solc < 0.5, view modifier for solc > 0.5)
* MUST allow external calls
*/
function isValidSignature(
bytes32 _hash,
bytes memory _signature)
public
view
returns (bytes4 magicValue);
function supportsInterface(bytes4 interfaceID) external view returns (bool);
} Could be completely misunderstanding this, but wouldn't you want to have ERC165 functionality in here? e.g. (bool success) = IERC165(_contract).supportsInterface(_INTERFACE_ID_ERC1271); |
@MicahZoltu @PhABC @lightclient how is this still on Last Call since 25 Jun 2021??? Shouldn't this be Final already? |
@pedrouid unless an author submits a PR, we don't usually do it ourselves :) |
Yeah I didn't expect Editors to do it... I actually thought Final Calls were enforced by CI to merge after 2 weeks |
That would be a really neat feature :) |
Closing this for housekeeping purposes. Feel free to continue using this issue for discussion about EIP-1271. |
Nice work with this EIP.
In my opinion, this is a proper implementation.
To approve a digest someone has to provide a valid signature anyway, so this is not any less secure. Another exemplary implementation could take |
@lukasz-glen excellent question and you are 100% correct. The signature can be anything, the EIP does not force any particular format or structure for the signature as it's signer contract specific. |
Is there any reference implementation for the ERC20 based tokens with permit function? |
What's wrong with this one?
What for actually? I was thinking about the same some time ago. I concluded that smart contract wallets (for instance) do not need permit, they should use regular approve. Even if gasless, it should be in ERC4337 style. |
OZ permit does not have ERC1271 validation. Apologies, my question should contain that 1271 context.
I am also in doubt, but would like to read opinions on this matter. In fact the question is whether the new ERC20 implementations should include ERC1271? |
ERC-2612 (permit) can't be made fully ERC-1271 compatible because it receives an |
I would change the perspective. What do we lose if 2612 and 3009 use 3009 can be used to overcome 2612 can be used in combination with 2771 to provide gasless tx. 3009 can be used to provide gasless tx also if I understand it correctly. But smartcontract wallets can provide gasless tx in a different way. To be clear. I think that it would be better if 2612 and 3009 would have |
Hi @frangio and authors, EIP-1271 is a great piece of standard. I have a clarification question that I like to ask: What's the rationale in choosing a
Thank you! |
explicit bytes was chosen instead of a Boolean to decrease the risks of errors when validating, akin to type safety. It’s a common practice in case there were more than 2 types of messages, but also to better catch mock implementations. I believe it also makes it easier to distinguish and error from an invalid signature, but I forgot the nuances. Bytes32 is too big/ not worth it since we aren’t concerned with collisions here. |
Hey @PhABC, it is still not entirely clear for me what the difference between returning It's also more expensive to validate and deal with a I appreciate the work put in here; remarkable EIP. |
A contract that doesnot implemente ERC-1271 may have a fallback function, and that fallback function may return data. The whole poinr of |
Thanks a lot for the explanation @Amxx , make sense! |
I actually liked the use case presented in the very first message of this thread about decentralized exchanges. If decentralized exchange implements EIP1271 which is basically for the reason that smart contracts can also hold funds, then DEX smart contract after "isValidSignature" succeeds, also needs to call: token.transferFrom(maker, taker, value). Note that maker is smart contract now. How can that transfer succeed ? The only choice I can think of is asset contract(i.e maker) must be able to first approve the tokens to be spendable by exchange smart contract. This can only be done if maker smart contract includes the code of ".call" that can call any function externally(including "approve") on token. I can't think of any other use case - if the maker contract doesn't have the possibility of "calling" external functions on other smart contracts beforehand, then use case the first message in this thread talks about is doomed. Am I wrong or I'm not seeing an better use case or I am right and that's it ? |
@novaknole Every single implementation of smart contract wallet (or account abstraction) uses ERC-1271(Gnosis safe, Sequence, Ambire, Pimlico, Immutable Passport, etc.). AA wallets don't have EOA signers like regular wallets, hence can only sign messages via ERC-1271 (and 6492) The example outlined in ERC-1271 with exchanges was written well before AA became popular. If it were re-written today it would be centered around AA wallets. |
No answer here, yet USDC updated its contracts to reflect ERC1271. |
Simple Description
Many blockchain based applications allow users to sign off-chain messages instead of directly requesting users to do an on-chain transaction. This is the case for decentralized exchanges with off-chain orderbooks like 0x and etherdelta. These applications usually assume that the message will be signed by the same address that owns the assets. However, one can hold assets directly in their regular account (controlled by a private key) or in a smart contract that acts as a wallet (e.g. a multisig contract). The current design of many smart contracts prevent contract based accounts from interacting with them, since contracts do not possess private keys and therefore can not directly sign messages. The proposal here outlines a standard way for contracts to verify if a provided signature is valid when the account is a contract.
See EIP draft.
The text was updated successfully, but these errors were encountered: