cmd/clef, signer: make fourbyte its own package, break dep cycle (#19450)

* cmd/clef, signer: make fourbytes its own package, break dep cycle

* signer/fourbyte: pull in a sanitized 4byte database

cmd/clef/main.go

@@ -14,13 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
 
-// signer is a utility that can be used so sign transactions and
-// arbitrary data.
 package main
 
-//go:generate go-bindata -o bindata.go resources/4byte.json
-//go:generate gofmt -s -w bindata.go
-
 import (
 	"bufio"
 	"context"
@@ -54,6 +49,7 @@ import (
 	"github.com/ethereum/go-ethereum/rlp"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/ethereum/go-ethereum/signer/core"
+	"github.com/ethereum/go-ethereum/signer/fourbyte"
 	"github.com/ethereum/go-ethereum/signer/rules"
 	"github.com/ethereum/go-ethereum/signer/storage"
 	"gopkg.in/urfave/cli.v1"
@@ -364,15 +360,12 @@ func signer(c *cli.Context) error {
 	}
 	// 4bytedb data
 	fourByteLocal := c.GlobalString(customDBFlag.Name)
-	data, err := Asset("resources/4byte.json")
-	if err != nil {
-		utils.Fatalf(err.Error())
-	}
-	db, err := core.NewAbiDBFromFiles(data, fourByteLocal)
+	db, err := fourbyte.NewWithFile(fourByteLocal)
 	if err != nil {
 		utils.Fatalf(err.Error())
 	}
-	log.Info("Loaded 4byte db", "signatures", db.Size(), "local", fourByteLocal)
+	embeds, locals := db.Size()
+	log.Info("Loaded 4byte database", "embeds", embeds, "locals", locals, "local", fourByteLocal)
 
 	var (
 		api       core.ExternalAPI

signer/core/abihelper.go

@@ -1,257 +0,0 @@
-// Copyright 2018 The go-ethereum Authors
-// This file is part of go-ethereum.
-//
-// go-ethereum is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// go-ethereum is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
-//
-// You should have received a copy of the GNU General Public License
-// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
-
-package core
-
-import (
-	"bytes"
-	"encoding/hex"
-	"encoding/json"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"regexp"
-	"strings"
-
-	"github.com/ethereum/go-ethereum/accounts/abi"
-	"github.com/ethereum/go-ethereum/common"
-)
-
-type decodedArgument struct {
-	soltype abi.Argument
-	value   interface{}
-}
-type decodedCallData struct {
-	signature string
-	name      string
-	inputs    []decodedArgument
-}
-
-// String implements stringer interface, tries to use the underlying value-type
-func (arg decodedArgument) String() string {
-	var value string
-	switch val := arg.value.(type) {
-	case fmt.Stringer:
-		value = val.String()
-	default:
-		value = fmt.Sprintf("%v", val)
-	}
-	return fmt.Sprintf("%v: %v", arg.soltype.Type.String(), value)
-}
-
-// String implements stringer interface for decodedCallData
-func (cd decodedCallData) String() string {
-	args := make([]string, len(cd.inputs))
-	for i, arg := range cd.inputs {
-		args[i] = arg.String()
-	}
-	return fmt.Sprintf("%s(%s)", cd.name, strings.Join(args, ","))
-}
-
-// parseCallData matches the provided call data against the abi definition,
-// and returns a struct containing the actual go-typed values
-func parseCallData(calldata []byte, abidata string) (*decodedCallData, error) {
-
-	if len(calldata) < 4 {
-		return nil, fmt.Errorf("Invalid ABI-data, incomplete method signature of (%d bytes)", len(calldata))
-	}
-
-	sigdata, argdata := calldata[:4], calldata[4:]
-	if len(argdata)%32 != 0 {
-		return nil, fmt.Errorf("Not ABI-encoded data; length should be a multiple of 32 (was %d)", len(argdata))
-	}
-
-	abispec, err := abi.JSON(strings.NewReader(abidata))
-	if err != nil {
-		return nil, fmt.Errorf("Failed parsing JSON ABI: %v, abidata: %v", err, abidata)
-	}
-
-	method, err := abispec.MethodById(sigdata)
-	if err != nil {
-		return nil, err
-	}
-
-	v, err := method.Inputs.UnpackValues(argdata)
-	if err != nil {
-		return nil, err
-	}
-
-	decoded := decodedCallData{signature: method.Sig(), name: method.Name}
-
-	for n, argument := range method.Inputs {
-		if err != nil {
-			return nil, fmt.Errorf("Failed to decode argument %d (signature %v): %v", n, method.Sig(), err)
-		}
-		decodedArg := decodedArgument{
-			soltype: argument,
-			value:   v[n],
-		}
-		decoded.inputs = append(decoded.inputs, decodedArg)
-	}
-
-	// We're finished decoding the data. At this point, we encode the decoded data to see if it matches with the
-	// original data. If we didn't do that, it would e.g. be possible to stuff extra data into the arguments, which
-	// is not detected by merely decoding the data.
-
-	var (
-		encoded []byte
-	)
-	encoded, err = method.Inputs.PackValues(v)
-
-	if err != nil {
-		return nil, err
-	}
-
-	if !bytes.Equal(encoded, argdata) {
-		was := common.Bytes2Hex(encoded)
-		exp := common.Bytes2Hex(argdata)
-		return nil, fmt.Errorf("WARNING: Supplied data is stuffed with extra data. \nWant %s\nHave %s\nfor method %v", exp, was, method.Sig())
-	}
-	return &decoded, nil
-}
-
-// MethodSelectorToAbi converts a method selector into an ABI struct. The returned data is a valid json string
-// which can be consumed by the standard abi package.
-func MethodSelectorToAbi(selector string) ([]byte, error) {
-
-	re := regexp.MustCompile(`^([^\)]+)\(([a-z0-9,\[\]]*)\)`)
-
-	type fakeArg struct {
-		Type string `json:"type"`
-	}
-	type fakeABI struct {
-		Name   string    `json:"name"`
-		Type   string    `json:"type"`
-		Inputs []fakeArg `json:"inputs"`
-	}
-	groups := re.FindStringSubmatch(selector)
-	if len(groups) != 3 {
-		return nil, fmt.Errorf("Did not match: %v (%v matches)", selector, len(groups))
-	}
-	name := groups[1]
-	args := groups[2]
-	arguments := make([]fakeArg, 0)
-	if len(args) > 0 {
-		for _, arg := range strings.Split(args, ",") {
-			arguments = append(arguments, fakeArg{arg})
-		}
-	}
-	abicheat := fakeABI{
-		name, "function", arguments,
-	}
-	return json.Marshal([]fakeABI{abicheat})
-
-}
-
-type AbiDb struct {
-	db           map[string]string
-	customdb     map[string]string
-	customdbPath string
-}
-
-// NewEmptyAbiDB exists for test purposes
-func NewEmptyAbiDB() (*AbiDb, error) {
-	return &AbiDb{make(map[string]string), make(map[string]string), ""}, nil
-}
-
-// NewAbiDBFromFile loads signature database from file, and
-// errors if the file is not valid json. Does no other validation of contents
-func NewAbiDBFromFile(path string) (*AbiDb, error) {
-	raw, err := ioutil.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	db, err := NewEmptyAbiDB()
-	if err != nil {
-		return nil, err
-	}
-	if err := json.Unmarshal(raw, &db.db); err != nil {
-		return nil, err
-	}
-	return db, nil
-}
-
-// NewAbiDBFromFiles loads both the standard signature database (resource file)and a custom database.
-// The latter will be used to write new values into if they are submitted via the API
-func NewAbiDBFromFiles(raw []byte, custom string) (*AbiDb, error) {
-
-	db := &AbiDb{make(map[string]string), make(map[string]string), custom}
-	db.customdbPath = custom
-
-	if err := json.Unmarshal(raw, &db.db); err != nil {
-		return nil, err
-	}
-	// Custom file may not exist. Will be created during save, if needed
-	if _, err := os.Stat(custom); err == nil {
-		raw, err = ioutil.ReadFile(custom)
-		if err != nil {
-			return nil, err
-		}
-		if err := json.Unmarshal(raw, &db.customdb); err != nil {
-			return nil, err
-		}
-	}
-	return db, nil
-}
-
-// LookupMethodSelector checks the given 4byte-sequence against the known ABI methods.
-// OBS: This method does not validate the match, it's assumed the caller will do so
-func (db *AbiDb) LookupMethodSelector(id []byte) (string, error) {
-	if len(id) < 4 {
-		return "", fmt.Errorf("Expected 4-byte id, got %d", len(id))
-	}
-	sig := hex.EncodeToString(id[:4])
-	if key, exists := db.db[sig]; exists {
-		return key, nil
-	}
-	if key, exists := db.customdb[sig]; exists {
-		return key, nil
-	}
-	return "", fmt.Errorf("Signature %v not found", sig)
-}
-
-func (db *AbiDb) Size() int {
-	return len(db.db)
-}
-
-// saveCustomAbi saves a signature ephemerally. If custom file is used, also saves to disk
-func (db *AbiDb) saveCustomAbi(selector, signature string) error {
-	db.customdb[signature] = selector
-	if db.customdbPath == "" {
-		return nil //Not an error per se, just not used
-	}
-	d, err := json.Marshal(db.customdb)
-	if err != nil {
-		return err
-	}
-	err = ioutil.WriteFile(db.customdbPath, d, 0600)
-	return err
-}
-
-// AddSignature to the database, if custom database saving is enabled.
-// OBS: This method does _not_ validate the correctness of the data,
-// it is assumed that the caller has already done so
-func (db *AbiDb) AddSignature(selector string, data []byte) error {
-	if len(data) < 4 {
-		return nil
-	}
-	_, err := db.LookupMethodSelector(data[:4])
-	if err == nil {
-		return nil
-	}
-	sig := hex.EncodeToString(data[:4])
-	return db.saveCustomAbi(selector, sig)
-}

signer/core/api.go

@@ -92,12 +92,25 @@ type UIClientAPI interface {
 	RegisterUIServer(api *UIServerAPI)
 }
 
+// Validator defines the methods required to validate a transaction against some
+// sanity defaults as well as any underlying 4byte method database.
+//
+// Use fourbyte.Database as an implementation. It is separated out of this package
+// to allow pieces of the signer package to be used without having to load the
+// 7MB embedded 4byte dump.
+type Validator interface {
+	// ValidateTransaction does a number of checks on the supplied transaction, and
+	// returns either a list of warnings, or an error (indicating that the transaction
+	// should be immediately rejected).
+	ValidateTransaction(selector *string, tx *SendTxArgs) (*ValidationMessages, error)
+}
+
 // SignerAPI defines the actual implementation of ExternalAPI
 type SignerAPI struct {
 	chainID     *big.Int
 	am          *accounts.Manager
 	UI          UIClientAPI
-	validator   *Validator
+	validator   Validator
 	rejectMode  bool
 	credentials storage.Storage
 }
@@ -235,11 +248,11 @@ var ErrRequestDenied = errors.New("Request denied")
 // key that is generated when a new Account is created.
 // noUSB disables USB support that is required to support hardware devices such as
 // ledger and trezor.
-func NewSignerAPI(am *accounts.Manager, chainID int64, noUSB bool, ui UIClientAPI, abidb *AbiDb, advancedMode bool, credentials storage.Storage) *SignerAPI {
+func NewSignerAPI(am *accounts.Manager, chainID int64, noUSB bool, ui UIClientAPI, validator Validator, advancedMode bool, credentials storage.Storage) *SignerAPI {
 	if advancedMode {
 		log.Info("Clef is in advanced mode: will warn instead of reject")
 	}
-	signer := &SignerAPI{big.NewInt(chainID), am, ui, NewValidator(abidb), !advancedMode, credentials}
+	signer := &SignerAPI{big.NewInt(chainID), am, ui, validator, !advancedMode, credentials}
 	if !noUSB {
 		signer.startUSBListener()
 	}
@@ -458,7 +471,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth
 		err    error
 		result SignTxResponse
 	)
-	msgs, err := api.validator.ValidateTransaction(&args, methodSelector)
+	msgs, err := api.validator.ValidateTransaction(methodSelector, &args)
 	if err != nil {
 		return nil, err
 	}

signer/core/api_test.go

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
 //
-package core
+package core_test
 
 import (
 	"bytes"
@@ -34,6 +34,8 @@ import (
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/internal/ethapi"
 	"github.com/ethereum/go-ethereum/rlp"
+	"github.com/ethereum/go-ethereum/signer/core"
+	"github.com/ethereum/go-ethereum/signer/fourbyte"
 	"github.com/ethereum/go-ethereum/signer/storage"
 )
 
@@ -43,56 +45,56 @@ type headlessUi struct {
 	inputCh   chan string // to send password
 }
 
-func (ui *headlessUi) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
+func (ui *headlessUi) OnInputRequired(info core.UserInputRequest) (core.UserInputResponse, error) {
 	input := <-ui.inputCh
-	return UserInputResponse{Text: input}, nil
+	return core.UserInputResponse{Text: input}, nil
 }
 
-func (ui *headlessUi) OnSignerStartup(info StartupInfo)             {}
-func (ui *headlessUi) RegisterUIServer(api *UIServerAPI)            {}
+func (ui *headlessUi) OnSignerStartup(info core.StartupInfo)        {}
+func (ui *headlessUi) RegisterUIServer(api *core.UIServerAPI)       {}
 func (ui *headlessUi) OnApprovedTx(tx ethapi.SignTransactionResult) {}
 
-func (ui *headlessUi) ApproveTx(request *SignTxRequest) (SignTxResponse, error) {
+func (ui *headlessUi) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) {
 
 	switch <-ui.approveCh {
 	case "Y":
-		return SignTxResponse{request.Transaction, true}, nil
+		return core.SignTxResponse{request.Transaction, true}, nil
 	case "M": // modify
 		// The headless UI always modifies the transaction
 		old := big.Int(request.Transaction.Value)
 		newVal := big.NewInt(0).Add(&old, big.NewInt(1))
 		request.Transaction.Value = hexutil.Big(*newVal)
-		return SignTxResponse{request.Transaction, true}, nil
+		return core.SignTxResponse{request.Transaction, true}, nil
 	default:
-		return SignTxResponse{request.Transaction, false}, nil
+		return core.SignTxResponse{request.Transaction, false}, nil
 	}
 }
 
-func (ui *headlessUi) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) {
+func (ui *headlessUi) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) {
 	approved := "Y" == <-ui.approveCh
-	return SignDataResponse{approved}, nil
+	return core.SignDataResponse{approved}, nil
 }
 
-func (ui *headlessUi) ApproveListing(request *ListRequest) (ListResponse, error) {
+func (ui *headlessUi) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
 	approval := <-ui.approveCh
 	//fmt.Printf("approval %s\n", approval)
 	switch approval {
 	case "A":
-		return ListResponse{request.Accounts}, nil
+		return core.ListResponse{request.Accounts}, nil
 	case "1":
 		l := make([]accounts.Account, 1)
 		l[0] = request.Accounts[1]
-		return ListResponse{l}, nil
+		return core.ListResponse{l}, nil
 	default:
-		return ListResponse{nil}, nil
+		return core.ListResponse{nil}, nil
 	}
 }
 
-func (ui *headlessUi) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
+func (ui *headlessUi) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) {
 	if "Y" == <-ui.approveCh {
-		return NewAccountResponse{true}, nil
+		return core.NewAccountResponse{true}, nil
 	}
-	return NewAccountResponse{false}, nil
+	return core.NewAccountResponse{false}, nil
 }
 
 func (ui *headlessUi) ShowError(message string) {
@@ -117,18 +119,18 @@ func tmpDirName(t *testing.T) string {
 	return d
 }
 
-func setup(t *testing.T) (*SignerAPI, *headlessUi) {
-	db, err := NewAbiDBFromFile("../../cmd/clef/4byte.json")
+func setup(t *testing.T) (*core.SignerAPI, *headlessUi) {
+	db, err := fourbyte.New()
 	if err != nil {
 		t.Fatal(err.Error())
 	}
 	ui := &headlessUi{make(chan string, 20), make(chan string, 20)}
-	am := StartClefAccountManager(tmpDirName(t), true, true)
-	api := NewSignerAPI(am, 1337, true, ui, db, true, &storage.NoStorage{})
+	am := core.StartClefAccountManager(tmpDirName(t), true, true)
+	api := core.NewSignerAPI(am, 1337, true, ui, db, true, &storage.NoStorage{})
 	return api, ui
 
 }
-func createAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
+func createAccount(ui *headlessUi, api *core.SignerAPI, t *testing.T) {
 	ui.approveCh <- "Y"
 	ui.inputCh <- "a_long_password"
 	_, err := api.New(context.Background())
@@ -139,7 +141,7 @@ func createAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
 	time.Sleep(250 * time.Millisecond)
 }
 
-func failCreateAccountWithPassword(ui *headlessUi, api *SignerAPI, password string, t *testing.T) {
+func failCreateAccountWithPassword(ui *headlessUi, api *core.SignerAPI, password string, t *testing.T) {
 
 	ui.approveCh <- "Y"
 	// We will be asked three times to provide a suitable password
@@ -156,10 +158,10 @@ func failCreateAccountWithPassword(ui *headlessUi, api *SignerAPI, password stri
 	}
 }
 
-func failCreateAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
+func failCreateAccount(ui *headlessUi, api *core.SignerAPI, t *testing.T) {
 	ui.approveCh <- "N"
 	addr, err := api.New(context.Background())
-	if err != ErrRequestDenied {
+	if err != core.ErrRequestDenied {
 		t.Fatal(err)
 	}
 	if addr != (common.Address{}) {
@@ -167,7 +169,7 @@ func failCreateAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
 	}
 }
 
-func list(ui *headlessUi, api *SignerAPI, t *testing.T) ([]common.Address, error) {
+func list(ui *headlessUi, api *core.SignerAPI, t *testing.T) ([]common.Address, error) {
 	ui.approveCh <- "A"
 	return api.List(context.Background())
 
@@ -216,19 +218,19 @@ func TestNewAcc(t *testing.T) {
 	if len(list) != 0 {
 		t.Fatalf("List should be empty")
 	}
-	if err != ErrRequestDenied {
+	if err != core.ErrRequestDenied {
 		t.Fatal("Expected deny")
 	}
 }
 
-func mkTestTx(from common.MixedcaseAddress) SendTxArgs {
+func mkTestTx(from common.MixedcaseAddress) core.SendTxArgs {
 	to := common.NewMixedcaseAddress(common.HexToAddress("0x1337"))
 	gas := hexutil.Uint64(21000)
 	gasPrice := (hexutil.Big)(*big.NewInt(2000000000))
 	value := (hexutil.Big)(*big.NewInt(1e18))
 	nonce := (hexutil.Uint64)(0)
 	data := hexutil.Bytes(common.Hex2Bytes("01020304050607080a"))
-	tx := SendTxArgs{
+	tx := core.SendTxArgs{
 		From:     from,
 		To:       &to,
 		Gas:      gas,
@@ -272,7 +274,7 @@ func TestSignTx(t *testing.T) {
 	if res != nil {
 		t.Errorf("Expected nil-response, got %v", res)
 	}
-	if err != ErrRequestDenied {
+	if err != core.ErrRequestDenied {
 		t.Errorf("Expected ErrRequestDenied! %v", err)
 	}
 	// Sign with correct password

signer/core/signed_data_test.go

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
 //
-package core
+package core_test
 
 import (
 	"context"
@@ -26,9 +26,10 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/signer/core"
 )
 
-var typesStandard = Types{
+var typesStandard = core.Types{
 	"EIP712Domain": {
 		{
 			Name: "name",
@@ -147,7 +148,7 @@ var jsonTypedData = `
 
 const primaryType = "Mail"
 
-var domainStandard = TypedDataDomain{
+var domainStandard = core.TypedDataDomain{
 	"Ether Mail",
 	"1",
 	big.NewInt(1),
@@ -167,7 +168,7 @@ var messageStandard = map[string]interface{}{
 	"contents": "Hello, Bob!",
 }
 
-var typedData = TypedData{
+var typedData = core.TypedData{
 	Types:       typesStandard,
 	PrimaryType: primaryType,
 	Domain:      domainStandard,
@@ -188,7 +189,7 @@ func TestSignData(t *testing.T) {
 
 	control.approveCh <- "Y"
 	control.inputCh <- "wrongpassword"
-	signature, err := api.SignData(context.Background(), TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
+	signature, err := api.SignData(context.Background(), core.TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
 	if signature != nil {
 		t.Errorf("Expected nil-data, got %x", signature)
 	}
@@ -196,17 +197,17 @@ func TestSignData(t *testing.T) {
 		t.Errorf("Expected ErrLocked! '%v'", err)
 	}
 	control.approveCh <- "No way"
-	signature, err = api.SignData(context.Background(), TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
+	signature, err = api.SignData(context.Background(), core.TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
 	if signature != nil {
 		t.Errorf("Expected nil-data, got %x", signature)
 	}
-	if err != ErrRequestDenied {
+	if err != core.ErrRequestDenied {
 		t.Errorf("Expected ErrRequestDenied! '%v'", err)
 	}
 	// text/plain
 	control.approveCh <- "Y"
 	control.inputCh <- "a_long_password"
-	signature, err = api.SignData(context.Background(), TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
+	signature, err = api.SignData(context.Background(), core.TextPlain.Mime, a, hexutil.Encode([]byte("EHLO world")))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -226,13 +227,13 @@ func TestSignData(t *testing.T) {
 }
 
 func TestDomainChainId(t *testing.T) {
-	withoutChainID := TypedData{
-		Types: Types{
-			"EIP712Domain": []Type{
+	withoutChainID := core.TypedData{
+		Types: core.Types{
+			"EIP712Domain": []core.Type{
 				{Name: "name", Type: "string"},
 			},
 		},
-		Domain: TypedDataDomain{
+		Domain: core.TypedDataDomain{
 			Name: "test",
 		},
 	}
@@ -241,14 +242,14 @@ func TestDomainChainId(t *testing.T) {
 		t.Errorf("Expected the chainId key to not be present in the domain map")
 	}
 
-	withChainID := TypedData{
-		Types: Types{
-			"EIP712Domain": []Type{
+	withChainID := core.TypedData{
+		Types: core.Types{
+			"EIP712Domain": []core.Type{
 				{Name: "name", Type: "string"},
 				{Name: "chainId", Type: "uint256"},
 			},
 		},
-		Domain: TypedDataDomain{
+		Domain: core.TypedDataDomain{
 			Name:    "test",
 			ChainId: big.NewInt(1),
 		},
@@ -383,7 +384,7 @@ func TestMalformedDomainkeys(t *testing.T) {
       }
     }
 `
-	var malformedDomainTypedData TypedData
+	var malformedDomainTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &malformedDomainTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -471,7 +472,7 @@ func TestMalformedTypesAndExtradata(t *testing.T) {
       }
     }
 `
-	var malformedTypedData TypedData
+	var malformedTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -567,7 +568,7 @@ func TestTypeMismatch(t *testing.T) {
       }
     }
 `
-	var mismatchTypedData TypedData
+	var mismatchTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &mismatchTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -589,7 +590,7 @@ func TestTypeOverflow(t *testing.T) {
 	//{
 	//	"test": 65536 <-- test defined as uint8
 	//}
-	var overflowTypedData TypedData
+	var overflowTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &overflowTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -667,7 +668,7 @@ func TestArray(t *testing.T) {
 	      }
 	    }
 	`
-	var arrayTypedData TypedData
+	var arrayTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &arrayTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -780,7 +781,7 @@ func TestCustomTypeAsArray(t *testing.T) {
     }
 
 `
-	var malformedTypedData TypedData
+	var malformedTypedData core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)
@@ -792,8 +793,7 @@ func TestCustomTypeAsArray(t *testing.T) {
 }
 
 func TestFormatter(t *testing.T) {
-
-	var d TypedData
+	var d core.TypedData
 	err := json.Unmarshal([]byte(jsonTypedData), &d)
 	if err != nil {
 		t.Fatalf("unmarshalling failed '%v'", err)

signer/core/types.go

@@ -41,13 +41,13 @@ const (
 	INFO = "Info"
 )
 
-func (vs *ValidationMessages) crit(msg string) {
+func (vs *ValidationMessages) Crit(msg string) {
 	vs.Messages = append(vs.Messages, ValidationInfo{CRIT, msg})
 }
-func (vs *ValidationMessages) warn(msg string) {
+func (vs *ValidationMessages) Warn(msg string) {
 	vs.Messages = append(vs.Messages, ValidationInfo{WARN, msg})
 }
-func (vs *ValidationMessages) info(msg string) {
+func (vs *ValidationMessages) Info(msg string) {
 	vs.Messages = append(vs.Messages, ValidationInfo{INFO, msg})
 }
 

signer/core/validation.go

@@ -17,147 +17,11 @@
 package core
 
 import (
-	"bytes"
 	"errors"
-	"fmt"
-	"math/big"
 	"regexp"
-
-	"github.com/ethereum/go-ethereum/common"
 )
 
-// The validation package contains validation checks for transactions
-// - ABI-data validation
-// - Transaction semantics validation
-// The package provides warnings for typical pitfalls
-
-type Validator struct {
-	db *AbiDb
-}
-
-func NewValidator(db *AbiDb) *Validator {
-	return &Validator{db}
-}
-
-func testSelector(selector string, data []byte) (*decodedCallData, error) {
-	if selector == "" {
-		return nil, fmt.Errorf("selector not found")
-	}
-	abiData, err := MethodSelectorToAbi(selector)
-	if err != nil {
-		return nil, err
-	}
-	info, err := parseCallData(data, string(abiData))
-	if err != nil {
-		return nil, err
-	}
-	return info, nil
-
-}
-
-// validateCallData checks if the ABI-data + methodselector (if given) can be parsed and seems to match
-func (v *Validator) validateCallData(msgs *ValidationMessages, data []byte, methodSelector *string) {
-	if len(data) == 0 {
-		return
-	}
-	if len(data) < 4 {
-		msgs.warn("Tx contains data which is not valid ABI")
-		return
-	}
-	if arglen := len(data) - 4; arglen%32 != 0 {
-		msgs.warn(fmt.Sprintf("Not ABI-encoded data; length should be a multiple of 32 (was %d)", arglen))
-	}
-	var (
-		info *decodedCallData
-		err  error
-	)
-	// Check the provided one
-	if methodSelector != nil {
-		info, err = testSelector(*methodSelector, data)
-		if err != nil {
-			msgs.warn(fmt.Sprintf("Tx contains data, but provided ABI signature could not be matched: %v", err))
-		} else {
-			msgs.info(info.String())
-			//Successfull match. add to db if not there already (ignore errors there)
-			v.db.AddSignature(*methodSelector, data[:4])
-		}
-		return
-	}
-	// Check the db
-	selector, err := v.db.LookupMethodSelector(data[:4])
-	if err != nil {
-		msgs.warn(fmt.Sprintf("Tx contains data, but the ABI signature could not be found: %v", err))
-		return
-	}
-	info, err = testSelector(selector, data)
-	if err != nil {
-		msgs.warn(fmt.Sprintf("Tx contains data, but provided ABI signature could not be matched: %v", err))
-	} else {
-		msgs.info(info.String())
-	}
-}
-
-// validateSemantics checks if the transactions 'makes sense', and generate warnings for a couple of typical scenarios
-func (v *Validator) validate(msgs *ValidationMessages, txargs *SendTxArgs, methodSelector *string) error {
-	// Prevent accidental erroneous usage of both 'input' and 'data'
-	if txargs.Data != nil && txargs.Input != nil && !bytes.Equal(*txargs.Data, *txargs.Input) {
-		// This is a showstopper
-		return errors.New(`Ambiguous request: both "data" and "input" are set and are not identical`)
-	}
-	var (
-		data []byte
-	)
-	// Place data on 'data', and nil 'input'
-	if txargs.Input != nil {
-		txargs.Data = txargs.Input
-		txargs.Input = nil
-	}
-	if txargs.Data != nil {
-		data = *txargs.Data
-	}
-
-	if txargs.To == nil {
-		//Contract creation should contain sufficient data to deploy a contract
-		// A typical error is omitting sender due to some quirk in the javascript call
-		// e.g. https://github.com/ethereum/go-ethereum/issues/16106
-		if len(data) == 0 {
-			if txargs.Value.ToInt().Cmp(big.NewInt(0)) > 0 {
-				// Sending ether into black hole
-				return errors.New("Tx will create contract with value but empty code!")
-			}
-			// No value submitted at least
-			msgs.crit("Tx will create contract with empty code!")
-		} else if len(data) < 40 { //Arbitrary limit
-			msgs.warn(fmt.Sprintf("Tx will will create contract, but payload is suspiciously small (%d b)", len(data)))
-		}
-		// methodSelector should be nil for contract creation
-		if methodSelector != nil {
-			msgs.warn("Tx will create contract, but method selector supplied; indicating intent to call a method.")
-		}
-
-	} else {
-		if !txargs.To.ValidChecksum() {
-			msgs.warn("Invalid checksum on to-address")
-		}
-		// Normal transaction
-		if bytes.Equal(txargs.To.Address().Bytes(), common.Address{}.Bytes()) {
-			// Sending to 0
-			msgs.crit("Tx destination is the zero address!")
-		}
-		// Validate calldata
-		v.validateCallData(msgs, data, methodSelector)
-	}
-	return nil
-}
-
-// ValidateTransaction does a number of checks on the supplied transaction, and returns either a list of warnings,
-// or an error, indicating that the transaction should be immediately rejected
-func (v *Validator) ValidateTransaction(txArgs *SendTxArgs, methodSelector *string) (*ValidationMessages, error) {
-	msgs := &ValidationMessages{}
-	return msgs, v.validate(msgs, txArgs, methodSelector)
-}
-
-var Printable7BitAscii = regexp.MustCompile("^[A-Za-z0-9!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~ ]+$")
+var printable7BitAscii = regexp.MustCompile("^[A-Za-z0-9!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~ ]+$")
 
 // ValidatePasswordFormat returns an error if the password is too short, or consists of characters
 // outside the range of the printable 7bit ascii set
@@ -165,7 +29,7 @@ func ValidatePasswordFormat(password string) error {
 	if len(password) < 10 {
 		return errors.New("password too short (<10 characters)")
 	}
-	if !Printable7BitAscii.MatchString(password) {
+	if !printable7BitAscii.MatchString(password) {
 		return errors.New("password contains invalid characters - only 7bit printable ascii allowed")
 	}
 	return nil

signer/core/validation_test.go

@@ -16,126 +16,7 @@
 
 package core
 
-import (
-	"fmt"
-	"math/big"
-	"testing"
-
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/common/hexutil"
-)
-
-func mixAddr(a string) (*common.MixedcaseAddress, error) {
-	return common.NewMixedcaseAddressFromString(a)
-}
-func toHexBig(h string) hexutil.Big {
-	b := big.NewInt(0).SetBytes(common.FromHex(h))
-	return hexutil.Big(*b)
-}
-func toHexUint(h string) hexutil.Uint64 {
-	b := big.NewInt(0).SetBytes(common.FromHex(h))
-	return hexutil.Uint64(b.Uint64())
-}
-func dummyTxArgs(t txtestcase) *SendTxArgs {
-	to, _ := mixAddr(t.to)
-	from, _ := mixAddr(t.from)
-	n := toHexUint(t.n)
-	gas := toHexUint(t.g)
-	gasPrice := toHexBig(t.gp)
-	value := toHexBig(t.value)
-	var (
-		data, input *hexutil.Bytes
-	)
-	if t.d != "" {
-		a := hexutil.Bytes(common.FromHex(t.d))
-		data = &a
-	}
-	if t.i != "" {
-		a := hexutil.Bytes(common.FromHex(t.i))
-		input = &a
-
-	}
-	return &SendTxArgs{
-		From:     *from,
-		To:       to,
-		Value:    value,
-		Nonce:    n,
-		GasPrice: gasPrice,
-		Gas:      gas,
-		Data:     data,
-		Input:    input,
-	}
-}
-
-type txtestcase struct {
-	from, to, n, g, gp, value, d, i string
-	expectErr                       bool
-	numMessages                     int
-}
-
-func TestValidator(t *testing.T) {
-	var (
-		// use empty db, there are other tests for the abi-specific stuff
-		db, _ = NewEmptyAbiDB()
-		v     = NewValidator(db)
-	)
-	testcases := []txtestcase{
-		// Invalid to checksum
-		{from: "000000000000000000000000000000000000dead", to: "000000000000000000000000000000000000dead",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 1},
-		// valid 0x000000000000000000000000000000000000dEaD
-		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 0},
-		// conflicting input and data
-		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x01", i: "0x02", expectErr: true},
-		// Data can't be parsed
-		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x0102", numMessages: 1},
-		// Data (on Input) can't be parsed
-		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", i: "0x0102", numMessages: 1},
-		// Send to 0
-		{from: "000000000000000000000000000000000000dead", to: "0x0000000000000000000000000000000000000000",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 1},
-		// Create empty contract (no value)
-		{from: "000000000000000000000000000000000000dead", to: "",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x00", numMessages: 1},
-		// Create empty contract (with value)
-		{from: "000000000000000000000000000000000000dead", to: "",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", expectErr: true},
-		// Small payload for create
-		{from: "000000000000000000000000000000000000dead", to: "",
-			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x01", numMessages: 1},
-	}
-	for i, test := range testcases {
-		msgs, err := v.ValidateTransaction(dummyTxArgs(test), nil)
-		if err == nil && test.expectErr {
-			t.Errorf("Test %d, expected error", i)
-			for _, msg := range msgs.Messages {
-				fmt.Printf("* %s: %s\n", msg.Typ, msg.Message)
-			}
-		}
-		if err != nil && !test.expectErr {
-			t.Errorf("Test %d, unexpected error: %v", i, err)
-		}
-		if err == nil {
-			got := len(msgs.Messages)
-			if got != test.numMessages {
-				for _, msg := range msgs.Messages {
-					fmt.Printf("* %s: %s\n", msg.Typ, msg.Message)
-				}
-				t.Errorf("Test %d, expected %d messages, got %d", i, test.numMessages, got)
-			} else {
-				//Debug printout, remove later
-				for _, msg := range msgs.Messages {
-					fmt.Printf("* [%d] %s: %s\n", i, msg.Typ, msg.Message)
-				}
-				fmt.Println()
-			}
-		}
-	}
-}
+import "testing"
 
 func TestPasswordValidation(t *testing.T) {
 	testcases := []struct {

signer/fourbyte/abi.go

@@ -0,0 +1,164 @@
+// Copyright 2018 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package fourbyte
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/ethereum/go-ethereum/accounts/abi"
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// decodedCallData is an internal type to represent a method call parsed according
+// to an ABI method signature.
+type decodedCallData struct {
+	signature string
+	name      string
+	inputs    []decodedArgument
+}
+
+// decodedArgument is an internal type to represent an argument parsed according
+// to an ABI method signature.
+type decodedArgument struct {
+	soltype abi.Argument
+	value   interface{}
+}
+
+// String implements stringer interface, tries to use the underlying value-type
+func (arg decodedArgument) String() string {
+	var value string
+	switch val := arg.value.(type) {
+	case fmt.Stringer:
+		value = val.String()
+	default:
+		value = fmt.Sprintf("%v", val)
+	}
+	return fmt.Sprintf("%v: %v", arg.soltype.Type.String(), value)
+}
+
+// String implements stringer interface for decodedCallData
+func (cd decodedCallData) String() string {
+	args := make([]string, len(cd.inputs))
+	for i, arg := range cd.inputs {
+		args[i] = arg.String()
+	}
+	return fmt.Sprintf("%s(%s)", cd.name, strings.Join(args, ","))
+}
+
+// verifySelector checks whether the ABI encoded data blob matches the requested
+// function signature.
+func verifySelector(selector string, calldata []byte) (*decodedCallData, error) {
+	// Parse the selector into an ABI JSON spec
+	abidata, err := parseSelector(selector)
+	if err != nil {
+		return nil, err
+	}
+	// Parse the call data according to the requested selector
+	return parseCallData(calldata, string(abidata))
+}
+
+// selectorRegexp is used to validate that a 4byte database selector corresponds
+// to a valid ABI function declaration.
+//
+// Note, although uppercase letters are not part of the ABI spec, this regexp
+// still accepts it as the general format is valid. It will be rejected later
+// by the type checker.
+var selectorRegexp = regexp.MustCompile(`^([^\)]+)\(([A-Za-z0-9,\[\]]*)\)`)
+
+// parseSelector converts a method selector into an ABI JSON spec. The returned
+// data is a valid JSON string which can be consumed by the standard abi package.
+func parseSelector(selector string) ([]byte, error) {
+	// Define a tiny fake ABI struct for JSON marshalling
+	type fakeArg struct {
+		Type string `json:"type"`
+	}
+	type fakeABI struct {
+		Name   string    `json:"name"`
+		Type   string    `json:"type"`
+		Inputs []fakeArg `json:"inputs"`
+	}
+	// Validate the selector and extract it's components
+	groups := selectorRegexp.FindStringSubmatch(selector)
+	if len(groups) != 3 {
+		return nil, fmt.Errorf("invalid selector %s (%v matches)", selector, len(groups))
+	}
+	name := groups[1]
+	args := groups[2]
+
+	// Reassemble the fake ABI and constuct the JSON
+	arguments := make([]fakeArg, 0)
+	if len(args) > 0 {
+		for _, arg := range strings.Split(args, ",") {
+			arguments = append(arguments, fakeArg{arg})
+		}
+	}
+	return json.Marshal([]fakeABI{{name, "function", arguments}})
+}
+
+// parseCallData matches the provided call data against the ABI definition and
+// returns a struct containing the actual go-typed values.
+func parseCallData(calldata []byte, abidata string) (*decodedCallData, error) {
+	// Validate the call data that it has the 4byte prefix and the rest divisible by 32 bytes
+	if len(calldata) < 4 {
+		return nil, fmt.Errorf("invalid call data, incomplete method signature (%d bytes < 4)", len(calldata))
+	}
+	sigdata := calldata[:4]
+
+	argdata := calldata[4:]
+	if len(argdata)%32 != 0 {
+		return nil, fmt.Errorf("invalid call data; length should be a multiple of 32 bytes (was %d)", len(argdata))
+	}
+	// Validate the called method and upack the call data accordingly
+	abispec, err := abi.JSON(strings.NewReader(abidata))
+	if err != nil {
+		return nil, fmt.Errorf("invalid method signature (%s): %v", abidata, err)
+	}
+	method, err := abispec.MethodById(sigdata)
+	if err != nil {
+		return nil, err
+	}
+	values, err := method.Inputs.UnpackValues(argdata)
+	if err != nil {
+		return nil, err
+	}
+	// Everything valid, assemble the call infos for the signer
+	decoded := decodedCallData{signature: method.Sig(), name: method.Name}
+	for i := 0; i < len(method.Inputs); i++ {
+		decoded.inputs = append(decoded.inputs, decodedArgument{
+			soltype: method.Inputs[i],
+			value:   values[i],
+		})
+	}
+	// We're finished decoding the data. At this point, we encode the decoded data
+	// to see if it matches with the original data. If we didn't do that, it would
+	// be possible to stuff extra data into the arguments, which is not detected
+	// by merely decoding the data.
+	encoded, err := method.Inputs.PackValues(values)
+	if err != nil {
+		return nil, err
+	}
+	if !bytes.Equal(encoded, argdata) {
+		was := common.Bytes2Hex(encoded)
+		exp := common.Bytes2Hex(argdata)
+		return nil, fmt.Errorf("WARNING: Supplied data is stuffed with extra data. \nWant %s\nHave %s\nfor method %v", exp, was, method.Sig())
+	}
+	return &decoded, nil
+}

signer/core/abihelper_test.go → signer/fourbyte/abi_test.go

@@ -1,24 +1,22 @@
 // Copyright 2018 The go-ethereum Authors
-// This file is part of go-ethereum.
+// This file is part of the go-ethereum library.
 //
-// go-ethereum is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
 // the Free Software Foundation, either version 3 of the License, or
 // (at your option) any later version.
 //
-// go-ethereum is distributed in the hope that it will be useful,
+// The go-ethereum library is distributed in the hope that it will be useful,
 // but WITHOUT ANY WARRANTY; without even the implied warranty of
 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
+// GNU Lesser General Public License for more details.
 //
-// You should have received a copy of the GNU General Public License
-// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
 
-package core
+package fourbyte
 
 import (
-	"fmt"
-	"io/ioutil"
 	"math/big"
 	"reflect"
 	"strings"
@@ -29,7 +27,6 @@ import (
 )
 
 func verify(t *testing.T, jsondata, calldata string, exp []interface{}) {
-
 	abispec, err := abi.JSON(strings.NewReader(jsondata))
 	if err != nil {
 		t.Fatal(err)
@@ -53,6 +50,7 @@ func verify(t *testing.T, jsondata, calldata string, exp []interface{}) {
 		}
 	}
 }
+
 func TestNewUnpacker(t *testing.T) {
 	type unpackTest struct {
 		jsondata string
@@ -96,11 +94,9 @@ func TestNewUnpacker(t *testing.T) {
 	for _, c := range testcases {
 		verify(t, c.jsondata, c.calldata, c.exp)
 	}
-
 }
 
 func TestCalldataDecoding(t *testing.T) {
-
 	// send(uint256)                              : a52c101e
 	// compareAndApprove(address,uint256,uint256) : 751e1079
 	// issue(address[],uint256)                   : 42958b54
@@ -111,7 +107,7 @@ func TestCalldataDecoding(t *testing.T) {
 	{"type":"function","name":"issue","inputs":[{"name":"a","type":"address[]"},{"name":"a","type":"uint256"}]},
 	{"type":"function","name":"sam","inputs":[{"name":"a","type":"bytes"},{"name":"a","type":"bool"},{"name":"a","type":"uint256[]"}]}
 ]`
-	//Expected failures
+	// Expected failures
 	for i, hexdata := range []string{
 		"a52c101e00000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000042",
 		"a52c101e000000000000000000000000000000000000000000000000000000000000001200",
@@ -122,9 +118,9 @@ func TestCalldataDecoding(t *testing.T) {
 		// Too short
 		"751e10790000000000000000000000000000000000000000000000000000000000000012",
 		"751e1079FFffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
-		//Not valid multiple of 32
+		// Not valid multiple of 32
 		"deadbeef00000000000000000000000000000000000000000000000000000000000000",
-		//Too short 'issue'
+		// Too short 'issue'
 		"42958b5400000000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000042",
 		// Too short compareAndApprove
 		"a52c101e00ff0000000000000000000000000000000000000000000000000000000000120000000000000000000000000000000000000000000000000000000000000042",
@@ -137,7 +133,7 @@ func TestCalldataDecoding(t *testing.T) {
 			t.Errorf("test %d: expected decoding to fail: %s", i, hexdata)
 		}
 	}
-	//Expected success
+	// Expected success
 	for i, hexdata := range []string{
 		// From https://github.com/ethereum/wiki/wiki/Ethereum-Contract-ABI
 		"a5643bf20000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000000000000464617665000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003",
@@ -147,7 +143,7 @@ func TestCalldataDecoding(t *testing.T) {
 		"42958b54" +
 			// start of dynamic type
 			"0000000000000000000000000000000000000000000000000000000000000040" +
-			//uint256
+			// uint256
 			"0000000000000000000000000000000000000000000000000000000000000001" +
 			// length of  array
 			"0000000000000000000000000000000000000000000000000000000000000002" +
@@ -162,79 +158,7 @@ func TestCalldataDecoding(t *testing.T) {
 	}
 }
 
-func TestSelectorUnmarshalling(t *testing.T) {
-	var (
-		db        *AbiDb
-		err       error
-		abistring []byte
-		abistruct abi.ABI
-	)
-
-	db, err = NewAbiDBFromFile("../../cmd/clef/4byte.json")
-	if err != nil {
-		t.Fatal(err)
-	}
-	fmt.Printf("DB size %v\n", db.Size())
-	for id, selector := range db.db {
-
-		abistring, err = MethodSelectorToAbi(selector)
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		abistruct, err = abi.JSON(strings.NewReader(string(abistring)))
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		m, err := abistruct.MethodById(common.Hex2Bytes(id[2:]))
-		if err != nil {
-			t.Error(err)
-			return
-		}
-		if m.Sig() != selector {
-			t.Errorf("Expected equality: %v != %v", m.Sig(), selector)
-		}
-	}
-
-}
-
-func TestCustomABI(t *testing.T) {
-	d, err := ioutil.TempDir("", "signer-4byte-test")
-	if err != nil {
-		t.Fatal(err)
-	}
-	filename := fmt.Sprintf("%s/4byte_custom.json", d)
-	abidb, err := NewAbiDBFromFiles([]byte(""), filename)
-	if err != nil {
-		t.Fatal(err)
-	}
-	// Now we'll remove all existing signatures
-	abidb.db = make(map[string]string)
-	calldata := common.Hex2Bytes("a52c101edeadbeef")
-	_, err = abidb.LookupMethodSelector(calldata)
-	if err == nil {
-		t.Fatalf("Should not find a match on empty db")
-	}
-	if err = abidb.AddSignature("send(uint256)", calldata); err != nil {
-		t.Fatalf("Failed to save file: %v", err)
-	}
-	_, err = abidb.LookupMethodSelector(calldata)
-	if err != nil {
-		t.Fatalf("Should find a match for abi signature, got: %v", err)
-	}
-	//Check that it wrote to file
-	abidb2, err := NewAbiDBFromFile(filename)
-	if err != nil {
-		t.Fatalf("Failed to create new abidb: %v", err)
-	}
-	_, err = abidb2.LookupMethodSelector(calldata)
-	if err != nil {
-		t.Fatalf("Save failed: should find a match for abi signature after loading from disk")
-	}
-}
-
-func TestMaliciousAbiStrings(t *testing.T) {
+func TestMaliciousABIStrings(t *testing.T) {
 	tests := []string{
 		"func(uint256,uint256,[]uint256)",
 		"func(uint256,uint256,uint256,)",
@@ -242,7 +166,7 @@ func TestMaliciousAbiStrings(t *testing.T) {
 	}
 	data := common.Hex2Bytes("4401a6e40000000000000000000000000000000000000000000000000000000000000012")
 	for i, tt := range tests {
-		_, err := testSelector(tt, data)
+		_, err := verifySelector(tt, data)
 		if err == nil {
 			t.Errorf("test %d: expected error for selector '%v'", i, tt)
 		}

signer/fourbyte/fourbyte.go

@@ -0,0 +1,143 @@
+// Copyright 2019 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+//go:generate go-bindata -nometadata -o 4byte.go -pkg fourbyte 4byte.json
+//go:generate gofmt -s -w 4byte.go
+
+// Package fourbyte contains the 4byte database.
+package fourbyte
+
+import (
+	"encoding/hex"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"os"
+)
+
+// Database is a 4byte database with the possibility of maintaining an immutable
+// set (embedded) into the process and a mutable set (loaded and written to file).
+type Database struct {
+	embedded   map[string]string
+	custom     map[string]string
+	customPath string
+}
+
+// newEmpty exists for testing purposes.
+func newEmpty() *Database {
+	return &Database{
+		embedded: make(map[string]string),
+		custom:   make(map[string]string),
+	}
+}
+
+// New loads the standard signature database embedded in the package.
+func New() (*Database, error) {
+	return NewWithFile("")
+}
+
+// NewFromFile loads signature database from file, and errors if the file is not
+// valid JSON. The constructor does no other validation of contents. This method
+// does not load the embedded 4byte database.
+//
+// The provided path will be used to write new values into if they are submitted
+// via the API.
+func NewFromFile(path string) (*Database, error) {
+	raw, err := os.Open(path)
+	if err != nil {
+		return nil, err
+	}
+	defer raw.Close()
+
+	db := newEmpty()
+	if err := json.NewDecoder(raw).Decode(&db.embedded); err != nil {
+		return nil, err
+	}
+	return db, nil
+}
+
+// NewWithFile loads both the standard signature database (embedded resource
+// file) as well as a custom database. The latter will be used to write new
+// values into if they are submitted via the API.
+func NewWithFile(path string) (*Database, error) {
+	db := &Database{make(map[string]string), make(map[string]string), path}
+	db.customPath = path
+
+	blob, err := Asset("4byte.json")
+	if err != nil {
+		return nil, err
+	}
+	if err := json.Unmarshal(blob, &db.embedded); err != nil {
+		return nil, err
+	}
+	// Custom file may not exist. Will be created during save, if needed.
+	if _, err := os.Stat(path); err == nil {
+		if blob, err = ioutil.ReadFile(path); err != nil {
+			return nil, err
+		}
+		if err := json.Unmarshal(blob, &db.custom); err != nil {
+			return nil, err
+		}
+	}
+	return db, nil
+}
+
+// Size returns the number of 4byte entries in the embedded and custom datasets.
+func (db *Database) Size() (int, int) {
+	return len(db.embedded), len(db.custom)
+}
+
+// Selector checks the given 4byte ID against the known ABI methods.
+//
+// This method does not validate the match, it's assumed the caller will do.
+func (db *Database) Selector(id []byte) (string, error) {
+	if len(id) < 4 {
+		return "", fmt.Errorf("expected 4-byte id, got %d", len(id))
+	}
+	sig := hex.EncodeToString(id[:4])
+	if selector, exists := db.embedded[sig]; exists {
+		return selector, nil
+	}
+	if selector, exists := db.custom[sig]; exists {
+		return selector, nil
+	}
+	return "", fmt.Errorf("signature %v not found", sig)
+}
+
+// AddSelector inserts a new 4byte entry into the database. If custom database
+// saving is enabled, the new dataset is also persisted to disk.
+//
+// Node, this method does _not_ validate the correctness of the data. It assumes
+// the caller has already done so.
+func (db *Database) AddSelector(selector string, data []byte) error {
+	// If the selector is already known, skip duplicating it
+	if len(data) < 4 {
+		return nil
+	}
+	if _, err := db.Selector(data[:4]); err == nil {
+		return nil
+	}
+	// Inject the custom selector into the database and persist if needed
+	db.custom[hex.EncodeToString(data[:4])] = selector
+	if db.customPath == "" {
+		return nil
+	}
+	blob, err := json.Marshal(db.custom)
+	if err != nil {
+		return err
+	}
+	return ioutil.WriteFile(db.customPath, blob, 0600)
+}

signer/fourbyte/fourbyte_test.go

@@ -0,0 +1,91 @@
+// Copyright 2018 The go-ethereum Authors
+// This file is part of go-ethereum.
+//
+// go-ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// go-ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
+
+package fourbyte
+
+import (
+	"fmt"
+	"io/ioutil"
+	"strings"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/accounts/abi"
+	"github.com/ethereum/go-ethereum/common"
+)
+
+// Tests that all the selectors contained in the 4byte database are valid.
+func TestEmbeddedDatabase(t *testing.T) {
+	db, err := New()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for id, selector := range db.embedded {
+		abistring, err := parseSelector(selector)
+		if err != nil {
+			t.Errorf("Failed to convert selector to ABI: %v", err)
+			continue
+		}
+		abistruct, err := abi.JSON(strings.NewReader(string(abistring)))
+		if err != nil {
+			t.Errorf("Failed to parse ABI: %v", err)
+			continue
+		}
+		m, err := abistruct.MethodById(common.Hex2Bytes(id))
+		if err != nil {
+			t.Errorf("Failed to get method by id (%s): %v", id, err)
+			continue
+		}
+		if m.Sig() != selector {
+			t.Errorf("Selector mismatch: have %v, want %v", m.Sig(), selector)
+		}
+	}
+}
+
+// Tests that custom 4byte datasets can be handled too.
+func TestCustomDatabase(t *testing.T) {
+	// Create a new custom 4byte database with no embedded component
+	tmpdir, err := ioutil.TempDir("", "signer-4byte-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	filename := fmt.Sprintf("%s/4byte_custom.json", tmpdir)
+
+	db, err := NewWithFile(filename)
+	if err != nil {
+		t.Fatal(err)
+	}
+	db.embedded = make(map[string]string)
+
+	// Ensure the database is empty, insert and verify
+	calldata := common.Hex2Bytes("a52c101edeadbeef")
+	if _, err = db.Selector(calldata); err == nil {
+		t.Fatalf("Should not find a match on empty database")
+	}
+	if err = db.AddSelector("send(uint256)", calldata); err != nil {
+		t.Fatalf("Failed to save file: %v", err)
+	}
+	if _, err = db.Selector(calldata); err != nil {
+		t.Fatalf("Failed to find a match for abi signature: %v", err)
+	}
+	// Check that the file as persisted to disk by creating a new instance
+	db2, err := NewFromFile(filename)
+	if err != nil {
+		t.Fatalf("Failed to create new abidb: %v", err)
+	}
+	if _, err = db2.Selector(calldata); err != nil {
+		t.Fatalf("Failed to find a match for persisted abi signature: %v", err)
+	}
+}

signer/fourbyte/validation.go

@@ -0,0 +1,117 @@
+// Copyright 2018 The go-ethereum Authors
+// This file is part of go-ethereum.
+//
+// go-ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// go-ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
+
+package fourbyte
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/signer/core"
+)
+
+// ValidateTransaction does a number of checks on the supplied transaction, and
+// returns either a list of warnings, or an error (indicating that the transaction
+// should be immediately rejected).
+func (db *Database) ValidateTransaction(selector *string, tx *core.SendTxArgs) (*core.ValidationMessages, error) {
+	messages := new(core.ValidationMessages)
+
+	// Prevent accidental erroneous usage of both 'input' and 'data' (show stopper)
+	if tx.Data != nil && tx.Input != nil && !bytes.Equal(*tx.Data, *tx.Input) {
+		return nil, errors.New(`ambiguous request: both "data" and "input" are set and are not identical`)
+	}
+	// Place data on 'data', and nil 'input'
+	var data []byte
+	if tx.Input != nil {
+		tx.Data = tx.Input
+		tx.Input = nil
+	}
+	if tx.Data != nil {
+		data = *tx.Data
+	}
+	// Contract creation doesn't validate call data, handle first
+	if tx.To == nil {
+		// Contract creation should contain sufficient data to deploy a contract. A
+		// typical error is omitting sender due to some quirk in the javascript call
+		// e.g. https://github.com/ethereum/go-ethereum/issues/16106.
+		if len(data) == 0 {
+			// Prevent sending ether into black hole (show stopper)
+			if tx.Value.ToInt().Cmp(big.NewInt(0)) > 0 {
+				return nil, errors.New("tx will create contract with value but empty code")
+			}
+			// No value submitted at least, critically Warn, but don't blow up
+			messages.Crit("Transaction will create contract with empty code")
+		} else if len(data) < 40 { // arbitrary heuristic limit
+			messages.Warn(fmt.Sprintf("Transaction will will create contract, but payload is suspiciously small (%d bytes)", len(data)))
+		}
+		// Method selector should be nil for contract creation
+		if selector != nil {
+			messages.Warn("Transaction will create contract, but method selector supplied, indicating intent to call a method")
+		}
+		return messages, nil
+	}
+	// Not a contract creation, validate as a plain transaction
+	if !tx.To.ValidChecksum() {
+		messages.Warn("Invalid checksum on recipient address")
+	}
+	if bytes.Equal(tx.To.Address().Bytes(), common.Address{}.Bytes()) {
+		messages.Crit("Transaction recipient is the zero address")
+	}
+	// Semantic fields validated, try to make heads or tails of the call data
+	db.validateCallData(selector, data, messages)
+	return messages, nil
+}
+
+// validateCallData checks if the ABI call-data + method selector (if given) can
+// be parsed and seems to match.
+func (db *Database) validateCallData(selector *string, data []byte, messages *core.ValidationMessages) {
+	// If the data is empty, we have a plain value transfer, nothing more to do
+	if len(data) == 0 {
+		return
+	}
+	// Validate the call data that it has the 4byte prefix and the rest divisible by 32 bytes
+	if len(data) < 4 {
+		messages.Warn("Transaction data is not valid ABI (missing the 4 byte call prefix)")
+		return
+	}
+	if n := len(data) - 4; n%32 != 0 {
+		messages.Warn(fmt.Sprintf("Transaction data is not valid ABI (length should be a multiple of 32 (was %d))", n))
+	}
+	// If a custom method selector was provided, validate with that
+	if selector != nil {
+		if info, err := verifySelector(*selector, data); err != nil {
+			messages.Warn(fmt.Sprintf("Transaction contains data, but provided ABI signature could not be matched: %v", err))
+		} else {
+			messages.Info(info.String())
+			db.AddSelector(*selector, data[:4])
+		}
+		return
+	}
+	// No method selector was provided, check the database for embedded ones
+	embedded, err := db.Selector(data[:4])
+	if err != nil {
+		messages.Warn(fmt.Sprintf("Transaction contains data, but the ABI signature could not be found: %v", err))
+		return
+	}
+	if info, err := verifySelector(embedded, data); err != nil {
+		messages.Warn(fmt.Sprintf("Transaction contains data, but provided ABI signature could not be varified: %v", err))
+	} else {
+		messages.Info(info.String())
+	}
+}

signer/fourbyte/validation_test.go

@@ -0,0 +1,137 @@
+// Copyright 2018 The go-ethereum Authors
+// This file is part of go-ethereum.
+//
+// go-ethereum is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// go-ethereum is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
+
+package fourbyte
+
+import (
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/ethereum/go-ethereum/signer/core"
+)
+
+func mixAddr(a string) (*common.MixedcaseAddress, error) {
+	return common.NewMixedcaseAddressFromString(a)
+}
+func toHexBig(h string) hexutil.Big {
+	b := big.NewInt(0).SetBytes(common.FromHex(h))
+	return hexutil.Big(*b)
+}
+func toHexUint(h string) hexutil.Uint64 {
+	b := big.NewInt(0).SetBytes(common.FromHex(h))
+	return hexutil.Uint64(b.Uint64())
+}
+func dummyTxArgs(t txtestcase) *core.SendTxArgs {
+	to, _ := mixAddr(t.to)
+	from, _ := mixAddr(t.from)
+	n := toHexUint(t.n)
+	gas := toHexUint(t.g)
+	gasPrice := toHexBig(t.gp)
+	value := toHexBig(t.value)
+	var (
+		data, input *hexutil.Bytes
+	)
+	if t.d != "" {
+		a := hexutil.Bytes(common.FromHex(t.d))
+		data = &a
+	}
+	if t.i != "" {
+		a := hexutil.Bytes(common.FromHex(t.i))
+		input = &a
+
+	}
+	return &core.SendTxArgs{
+		From:     *from,
+		To:       to,
+		Value:    value,
+		Nonce:    n,
+		GasPrice: gasPrice,
+		Gas:      gas,
+		Data:     data,
+		Input:    input,
+	}
+}
+
+type txtestcase struct {
+	from, to, n, g, gp, value, d, i string
+	expectErr                       bool
+	numMessages                     int
+}
+
+func TestTransactionValidation(t *testing.T) {
+	var (
+		// use empty db, there are other tests for the abi-specific stuff
+		db = newEmpty()
+	)
+	testcases := []txtestcase{
+		// Invalid to checksum
+		{from: "000000000000000000000000000000000000dead", to: "000000000000000000000000000000000000dead",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 1},
+		// valid 0x000000000000000000000000000000000000dEaD
+		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 0},
+		// conflicting input and data
+		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x01", i: "0x02", expectErr: true},
+		// Data can't be parsed
+		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x0102", numMessages: 1},
+		// Data (on Input) can't be parsed
+		{from: "000000000000000000000000000000000000dead", to: "0x000000000000000000000000000000000000dEaD",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", i: "0x0102", numMessages: 1},
+		// Send to 0
+		{from: "000000000000000000000000000000000000dead", to: "0x0000000000000000000000000000000000000000",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", numMessages: 1},
+		// Create empty contract (no value)
+		{from: "000000000000000000000000000000000000dead", to: "",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x00", numMessages: 1},
+		// Create empty contract (with value)
+		{from: "000000000000000000000000000000000000dead", to: "",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", expectErr: true},
+		// Small payload for create
+		{from: "000000000000000000000000000000000000dead", to: "",
+			n: "0x01", g: "0x20", gp: "0x40", value: "0x01", d: "0x01", numMessages: 1},
+	}
+	for i, test := range testcases {
+		msgs, err := db.ValidateTransaction(nil, dummyTxArgs(test))
+		if err == nil && test.expectErr {
+			t.Errorf("Test %d, expected error", i)
+			for _, msg := range msgs.Messages {
+				t.Logf("* %s: %s", msg.Typ, msg.Message)
+			}
+		}
+		if err != nil && !test.expectErr {
+			t.Errorf("Test %d, unexpected error: %v", i, err)
+		}
+		if err == nil {
+			got := len(msgs.Messages)
+			if got != test.numMessages {
+				for _, msg := range msgs.Messages {
+					t.Logf("* %s: %s", msg.Typ, msg.Message)
+				}
+				t.Errorf("Test %d, expected %d messages, got %d", i, test.numMessages, got)
+			} else {
+				//Debug printout, remove later
+				for _, msg := range msgs.Messages {
+					t.Logf("* [%d] %s: %s", i, msg.Typ, msg.Message)
+				}
+				t.Log()
+			}
+		}
+	}
+}