SM-781: extracted common Ecrecover func #73
Conversation
…vices-api and vsd
| // Ecrecover mimics the ecrecover opcode, returning the address that signed | ||
| // hash with signature. sig must have length 65 and the last byte, the recovery | ||
| // byte usually denoted v, must be 27 or 28. | ||
| func Ecrecover(hash, sig []byte) (common.Address, error) { |
There was a problem hiding this comment.
This is was an exact copy of https://github.com/DIMO-Network/devices-api/blob/a26b001b1680292ad42ce59e52af7e63970d8cf2/internal/controllers/helpers/handlers.go#L161, but we definitely can improve it
crypto/crypto.go
Outdated
| func VerifySignature(payload []byte, signature string, ethAddr string) (bool, error) { | ||
| addr := common.HexToAddress(ethAddr) | ||
| sig := common.FromHex(signature) | ||
| hash := crypto.Keccak256Hash(payload) |
There was a problem hiding this comment.
If you just want bytes, you could use crypto.Keccak256.
crypto/crypto.go
Outdated
| // ethAddr string The Ethereum address of the signer. | ||
| // @return bool Indicates whether the signature is valid or not. | ||
| // @return error If there was an issue during the verification process. | ||
| func VerifySignature(payload []byte, signature string, ethAddr string) (bool, error) { |
There was a problem hiding this comment.
signature could be []byte, ethAddr could be common.Address. I don't have a huge opinion on this.
There was a problem hiding this comment.
I do like using common.Address across the board.
There was a problem hiding this comment.
updated signature, make sense to me
crypto/crypto.go
Outdated
| addr := common.HexToAddress(ethAddr) | ||
| sig := common.FromHex(signature) |
There was a problem hiding this comment.
If we are going to take in strings, and we do have an error return, maybe we could check that these are valid?
There was a problem hiding this comment.
I think we will delegate validation to the client code
|
@elffjs , once you have time, please take another look |
Proposed Changes
We want to extract the Ecrecover function to shared, as it is already used in several places like benthos-plugin, devices-api and vehicle-signal-decoding.