EIPs
EIPs copied to clipboard
Update EIP-4494: Include EIP-1271 and EIP-2098 as required
EIP-1271:
- EIP-4494 currently makes no mention of EIP-1271 but I strongly assume that there is interest to support signature validation through a smart contract
- There are strong reasons to believe so as @dievardump's reference implementation uses OZ's
SignatureChecker
that in turn implements EIP-1271 - Unclear to me: Whether or not the authors of the EIP document want to further mention 1271. Anyways, at least I think it should be mentioned in the header as required.
EIP-2098:
- document mentions: "sig is a valid secp256k1 or EIP-2098 signature from owner of the tokenId:"
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):
(fail) eip-4494.md
classification |
---|
updateEIP |
- eip-4494.md requires approval from one of (@dievardump, @wschwab)
@MicahZoltu @lightclient could one of you manually merge this?
We need to get approval from one of the authors before merging. The author may prefer to remove the reference to 2098 to resolve that one, and they may not want to require 1271 for some reason. We should not let 4494 go to review without resolving the 2098 issue though.
@dievardump @wschwab ☝🏼
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.
I don't see why 1271 or 2098 should be required. We have designed the EIP to be compatible with them, but at no point in the implementation are they required. Feel free to change my mind.
Feel free to change my mind.
It should be explicitly stated that they may be supported, then, because EIP-1271 is assumed to not be supported.
I would prefer not to add those as required, because I think the implementers should be able to decide which ones they want to support. Including possible future signature standards (which is one of the reasons why we used bytes instead of a decomposed form)
So we should probably remove the eip-2098
mentions and make that more generic if this is a problem and is what leads to
assume that "EIP-1271 to not be supported."
As a reference, "EIP-2098" is mentioned 3 times
/// @param sig a traditional or EIP-2098 signature
sig is a valid secp256k1 or EIP-2098 signature from owner of the tokenId:
in order to support EIP-2098 signatures (64 bytes)
I don't see why 1271
fair.
2098 should be required
You mention EIP-2098 as also @dievardump points out, and it is how you format the signature as bytes
, and in that case, IMO, it should be added to the requirements.
we point it out, but we never say that a 4494-compliant implementation MUST support either, imho a requirement should be something required, not something that was merely accommodated
we point it out, but we never say that a 4494-compliant implementation MUST support either, imho a requirement should be something required, not something that was merely accommodated
fine. defining it like this can be a pro and a con.
Pro: if a better compact signature scheme arrives it may be usable
con: being loose can lead to undefined behavior.
I tend to prefer being very explicit and specific but it's ur guy's call, feel free to close
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.
This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.