-
Notifications
You must be signed in to change notification settings - Fork 312
Test demonstrating ECRECOVER opcode #4183
base: master
Are you sure you want to change the base?
Conversation
public static class EcRecover | ||
{ | ||
// TODO: Not sure what this is yet. | ||
private const int RecId = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so my understanding is, normally an uncompressed secp256k1 pubkey has a lot of redundant information. For a given x
coordinate there are only 2 possible y
values. So instead of storing the complete x and y coordinates (64 bytes), you can almost halve the space requirement by only storing x with a single byte (32 + 1 = 33 bytes) to indicate which of the solutions it is. So recId is that indicator in this case.
I would need to look into it a little to see how you are supposed to infer it when recovering from a signature. Dark magic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, it's coming back to me a little. There was a softfork mandating that all signature S-values past a certain block height be the 'low' value (at most N/2, where N is the curve order). So I think you'd have to try both the potential pubkeys and see which one gives that outcome.
May be mistaken, still researching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so after looking at the NBitcoin implementation in more detail:
- I don't think
recId
should be hardcoded like this, as it can have several different values depending on whether the pubkey is compressed, and other characteristics of the curve point that comprises the pubkey.1
may be working in your test by coincidence more than anything else. - recId gets stored in the first byte of an encoded signature if you are using NBitcoin's message signing functionality anyway. An
ECDSASignature
instance does not have this data as it is only the raw R & S values. - I would therefore suggest using
PubKey.RecoverCompact()
instead. SeeKey.SignCompact()
for the operation that will return the signature in the encoded form with recId prepended. ForRecoverCompact
we do obviously need the hash of the message available, but that is a minor tweak of the methods you've already implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structurally LGTM. No comment on the RecId/crypto side, will await @zeptin's continued feedback
Need to test that it is seamless to enable this in contracts