signer/core/apitypes: support more input types for eip-712 encoding #26074
Conversation
ghost
requested review from
holiman and
ligi
and removed request for
holiman and
ligi
EIP 712 spec
The EIP basically outlines how a human-readable text-based structure is encoded to a canonical form, and hashed/signed. The inputs are typically json inputs, and address would therefore be a hex-encoded string. What is the usecase where some different type would be passed into |
|
Also, please add a testcase for the change to parsing integers. e.g --- a/signer/core/apitypes/signed_data_internal_test.go
+++ b/signer/core/apitypes/signed_data_internal_test.go
@@ -128,6 +128,7 @@ func TestParseInteger(t *testing.T) {
{"int32", "-123", big.NewInt(-123)},
{"uint32", "0xff", big.NewInt(0xff)},
{"int8", "0xffff", nil},
+ {"int32", big.NewInt(-124), big.NewInt(-124)},
} {
res, err := parseInteger(tt.t, tt.v)
if tt.exp == nil && res == nil { |
Our use case is making contract transactions that require the same data structure to be used as an input to the typed data hasher and encoded using the ABI at multiple times. Despite the types being the same, it doesn't make sense that some APIs expect different types for the same underlying thing. These data structures are internally generated and not provided from a human. EIP712 may have been intended for human-readable inputs, but that doesn't imply one shouldn't use programmatically. |
signer/core/apitypes/types.go
Outdated
| stringValue, isString := encValue.(string) | ||
| if isString { | ||
| if !common.IsHexAddress(stringValue) { | ||
| return nil, dataMismatchError(encType, encValue) | ||
| } | ||
| retval := make([]byte, 32) | ||
| copy(retval[12:], common.HexToAddress(stringValue).Bytes()) | ||
| return retval, nil | ||
| } | ||
| byteSlice, isByteSlice := encValue.([]byte) | ||
| if isByteSlice { | ||
| if len(byteSlice) == 20 { | ||
| retval := make([]byte, 32) | ||
| copy(retval[12:], byteSlice) | ||
| return retval, nil | ||
| } else { | ||
| return nil, errors.New("if a byte slice is passed in, in place of an address, it must be either 20 or 32 bytes long") | ||
| } | ||
| } | ||
| twentyByteArr, isTwentyByteArr := encValue.([20]byte) | ||
| if isTwentyByteArr { | ||
| retval := make([]byte, 32) | ||
| copy(retval[12:], twentyByteArr[:]) | ||
| return retval, nil |
There was a problem hiding this comment.
| stringValue, isString := encValue.(string) | |
| if isString { | |
| if !common.IsHexAddress(stringValue) { | |
| return nil, dataMismatchError(encType, encValue) | |
| } | |
| retval := make([]byte, 32) | |
| copy(retval[12:], common.HexToAddress(stringValue).Bytes()) | |
| return retval, nil | |
| } | |
| byteSlice, isByteSlice := encValue.([]byte) | |
| if isByteSlice { | |
| if len(byteSlice) == 20 { | |
| retval := make([]byte, 32) | |
| copy(retval[12:], byteSlice) | |
| return retval, nil | |
| } else { | |
| return nil, errors.New("if a byte slice is passed in, in place of an address, it must be either 20 or 32 bytes long") | |
| } | |
| } | |
| twentyByteArr, isTwentyByteArr := encValue.([20]byte) | |
| if isTwentyByteArr { | |
| retval := make([]byte, 32) | |
| copy(retval[12:], twentyByteArr[:]) | |
| return retval, nil | |
| retval := make([]byte, 32) | |
| switch val := encValue.(type) { | |
| case string: | |
| if common.IsHexAddress(val) { | |
| copy(retval[12:], common.HexToAddress(val).Bytes()) | |
| return retval, nil | |
| } | |
| case []byte: | |
| if len(val) == 20 { | |
| copy(retval[12:], val) | |
| return retval, nil | |
| } | |
| case [20]byte: | |
| copy(retval[12:], val[:]) | |
| return retval, nil | |
| } |
IMO a bit clearer, and definitely shorter
There was a problem hiding this comment.
Also, we need tests for these type-conversions, (and preferrably the fail-cases too)
There was a problem hiding this comment.
Added tests for this, and adopted your change.
holiman
changed the title
…thereum#26074) * apitypes: synchronize handling of types * signer/core/apitypes: improve array check * apitypes: add a test for big.Int -> int32 * signer/core/apitypes: Add a test for parsing addresses from [20]byte, []byte and string * signer/core/apitypes: add some testcases Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Martin Holst Swende <martin@swende.se>
[32]byte) even for arguments wherebytes32is specified. This makes it terribly inconvenient to work with fixed size byte arrays.big.Ints in certain places, despite it being internally converted to abig.Intinternally.This PR solves both problems.