From b0e872cd21ad87f06ecf04b57772102689e8273b Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 15 Jan 2019 13:32:44 +0100 Subject: [PATCH] clef, signer: address some review concerns --- cmd/clef/README.md | 5 +---- cmd/clef/extapi_changelog.md | 1 - signer/core/api.go | 1 - signer/core/signed_data.go | 16 ++++++---------- signer/core/signed_data_test.go | 23 +++++------------------ 5 files changed, 12 insertions(+), 34 deletions(-) diff --git a/cmd/clef/README.md b/cmd/clef/README.md index 5e6f8661c7..98e85d5a11 100644 --- a/cmd/clef/README.md +++ b/cmd/clef/README.md @@ -489,11 +489,8 @@ Response ### account_ecRecover #### Sign data -<<<<<<< HEAD -======= ->>>>>>> c72099670... Added example RPC calls for account_signData and account_signTypedData - Derive the address from the account that was used to sign data with content type `text/plain` and the signature. +Derive the address from the account that was used to sign data with content type `text/plain` and the signature. #### Arguments - data [data]: data that was signed diff --git a/cmd/clef/extapi_changelog.md b/cmd/clef/extapi_changelog.md index 7adc339605..25f819bddb 100644 --- a/cmd/clef/extapi_changelog.md +++ b/cmd/clef/extapi_changelog.md @@ -15,7 +15,6 @@ The addition of `contentType` makes it possible to use the method for different * The external `account_Ecrecover`-method was removed. * The external `account_Import`-method was removed. - #### 3.0.0 * The external `account_List`-method was changed to not expose `url`, which contained info about the local filesystem. It now returns only a list of addresses. diff --git a/signer/core/api.go b/signer/core/api.go index 16968f7ad8..33ec28bb8a 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -176,7 +176,6 @@ type ( ContentType string `json:"content_type"` Address common.MixedcaseAddress `json:"address"` Rawdata interface{} `json:"raw_data"` - //Message string `json:"message"` Message []*NameValueType `json:"message"` Hash hexutil.Bytes `json:"hash"` Meta Metadata `json:"meta"` diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 3f8f7da5ac..bddb2b1001 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -29,17 +29,15 @@ import ( "strings" "unicode" - "golang.org/x/crypto/sha3" - - "github.com/ethereum/go-ethereum/common/math" - "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/rlp" + "golang.org/x/crypto/sha3" ) type SigFormat struct { @@ -118,7 +116,7 @@ type TypedDataDomain struct { Salt string `json:"salt"` } -var typedDataRegexp = regexp.MustCompile(`^((address|bool|bytes|string)|((bytes)([1-9]|[1-2][0-9]|3[0-2]))|((int|uint)(8|16|32|64|128|256)))(\[])?$`) +var typedDataRegexp = regexp.MustCompile(`^((address|bool|bytes|string)|((bytes)([1-9]|[1-2][0-9]|3[0-2]))|((int|uint)(8|16|32|64|128|256)))(\[\])?$`) var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Z](\w*)(\[])?$`) // Sign receives a request and produces a signature @@ -222,7 +220,7 @@ func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.M }, } req = &SignDataRequest{ContentType: mediaType, Rawdata: cliqueData, Message: message, Hash: sighash} - case TextPlain.Mime: + default: // also case TextPlain.Mime: // Calculates an Ethereum ECDSA signature for: // hash = keccak256("\x19${byteVersion}Ethereum Signed Message:\n${message length}${message}") plainData, err := hexutil.Decode(data.(string)) @@ -239,8 +237,6 @@ func (api *SignerAPI) determineSignatureFormat(contentType string, addr common.M } req = &SignDataRequest{ContentType: mediaType, Rawdata: plainData, Message: message, Hash: sighash} - default: - return nil, fmt.Errorf("content type '%s' not implemented for signing", contentType) } return req, nil @@ -315,7 +311,6 @@ func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAd return nil, err } sighash := crypto.Keccak256([]byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash)))) - //output := typedData.PrettyPrint() message := typedData.Format() req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: typedData.Map(), Message: message, Hash: sighash} signature, err := api.Sign(ctx, addr, req) @@ -391,6 +386,7 @@ func (typedData *TypedData) EncodeType(primaryType string) hexutil.Bytes { return buffer.Bytes() } +// TypeHash creates the keccak256 hash of the data func (typedData *TypedData) TypeHash(primaryType string) hexutil.Bytes { return crypto.Keccak256(typedData.EncodeType(primaryType)) } @@ -628,7 +624,7 @@ func (typedData *TypedData) PrettyPrint() string { return output.String() } -// Format returns a representation of d, which can be easily displayed by a user-interface +// Format returns a representation of typedData, which can be easily displayed by a user-interface // without in-depth knowledge about 712 rules func (typedData *TypedData) Format() []*NameValueType { var nvts []*NameValueType diff --git a/signer/core/signed_data_test.go b/signer/core/signed_data_test.go index b424f05f0b..63fb0402d0 100644 --- a/signer/core/signed_data_test.go +++ b/signer/core/signed_data_test.go @@ -275,7 +275,7 @@ func TestEncodeData(t *testing.T) { } } -func TestMalformedData1(t *testing.T) { +func TestMalformedDomainkeys(t *testing.T) { // Verifies that malformed domain keys are properly caught: //{ // "name": "Ether Mail", @@ -365,7 +365,7 @@ func TestMalformedData1(t *testing.T) { } } -func TestMalformedData2(t *testing.T) { +func TestTypeMismatch(t *testing.T) { // Verifies that: // 1. Mismatches between the given type and data, i.e. `Person` and // the data item is a string, are properly caught: @@ -472,7 +472,7 @@ func TestMalformedData2(t *testing.T) { } } -func TestMalformedData3(t *testing.T) { +func TestMalformedTypesAndExtradata(t *testing.T) { // Verifies several quirks // 1. Using dynamic types and only validating the prefix: //{ @@ -567,7 +567,7 @@ func TestMalformedData3(t *testing.T) { } } -func TestMalformedData4(t *testing.T) { +func TestTypeMismatch(t *testing.T) { // Verifies data that doesn't fit into it: //{ // "test": 65536 <-- test defined as uint8 @@ -606,19 +606,6 @@ func TestFormatter(t *testing.T) { if err != nil { t.Fatalf("unmarshalling failed %v", err) } - - //x := PrettyPrintPrimitiveValue("address", "wallet", "0x123123123", 1) - //fmt.Printf(x) - //y := FormatPrimitiveValue("address", "wallet", "0x123123123") - //fmt.Printf("%v\n", y) - - //fmt.Printf(d.PrettyPrintData(d.PrimaryType,d.Message, 1)) - - //formatted := d.FormatData(d.PrimaryType,d.Message) - //for _,item := range formatted{ - // fmt.Printf("%v\n", item.Pprint(0)) - //} - formatted := d.Format() for _, item := range formatted { fmt.Printf("%v\n", item.Pprint(0)) @@ -629,7 +616,7 @@ func TestFormatter(t *testing.T) { } -func TestMalformedData5(t *testing.T) { +func TestCustomTypeAsArray(t *testing.T) { var jsonTypedData = ` { "types": {