signer: address review concerns

cmd/signer/README.md

@@ -1,7 +1,7 @@
 **Signer API**
 ----
 The signer utility can be used to sign transactions and data and is meant as a replacement for geth's account management.
-This allows DApp's not to depend on geth's account management. When a DApp wants to sign data it can send the data to
+This allows DApps not to depend on geth's account management. When a DApp wants to sign data it can send the data to
 the signer, the signer will than provide the user with context and asks the user for permission to sign the data. If
 the users grants the signing request the signer will send the signature back to the DApp.
   

cmd/signer/core/api.go

@@ -98,7 +98,7 @@ type Metadata struct {
 
 // MetadataFromContext extracts Metadata from a given context.Context
 func MetadataFromContext(ctx context.Context) Metadata {
-	m := Metadata{"NA", "NA", "NA"}
+	m := Metadata{"NA", "NA", "NA"} // batman
 
 	if v := ctx.Value("remote"); v != nil {
 		m.Remote = v.(string)
@@ -444,7 +444,7 @@ func (api *SignerAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (c
 // SignHash is a helper function that calculates a hash for the given message that can be
 // safely used to calculate a signature from.
 //
-// The hash is calulcated as
+// The hash is calculated as
 //   keccak256("\x19Ethereum Signed Message:\n"${message length}${message}).
 //
 // This gives context to the signed message and prevents signing of transactions.

cmd/signer/core/api_test.go

@@ -229,7 +229,7 @@ func TestSignData(t *testing.T) {
 	control <- "wrongpassword"
 	h, err := api.Sign(context.Background(), a, []byte("EHLO world"))
 	if h != nil {
-		t.Errorf("Expected nil-data, got %h", h)
+		t.Errorf("Expected nil-data, got %x", h)
 	}
 	if err != keystore.ErrDecrypt {
 		t.Errorf("Expected ErrLocked! %v", err)
@@ -238,7 +238,7 @@ func TestSignData(t *testing.T) {
 	control <- "No way"
 	h, err = api.Sign(context.Background(), a, []byte("EHLO world"))
 	if h != nil {
-		t.Errorf("Expected nil-data, got %h", h)
+		t.Errorf("Expected nil-data, got %x", h)
 	}
 	if err != ErrRequestDenied {
 		t.Errorf("Expected ErrRequestDenied! %v", err)

cmd/signer/core/auditlog.go

@@ -1,4 +1,4 @@
-// Copyright 2017 The go-ethereum Authors
+// 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

cmd/signer/core/cliui.go

@@ -23,11 +23,11 @@ import (
 
 	"sync"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/internal/ethapi"
 	"github.com/ethereum/go-ethereum/log"
 	"golang.org/x/crypto/ssh/terminal"
-	"github.com/davecgh/go-spew/spew"
 )
 
 type CommandlineUI struct {
@@ -84,8 +84,8 @@ func (ui *CommandlineUI) readPasswordText(inputstring string) string {
 
 // confirm returns true if user enters 'Yes', otherwise false
 func (ui *CommandlineUI) confirm() bool {
-	fmt.Printf("Type 'Yes' to approve:\n")
+	fmt.Printf("Approve? [y/N]:\n")
-	if ui.readString() == "Yes" {
+	if ui.readString() == "y" {
 		return true
 	}
 	fmt.Println("-----------------------")

cmd/signer/core/stdioui.go

@@ -27,15 +27,12 @@ import (
 )
 
 type StdIOUI struct {
-	//client *jsonrpc2.Client
 	client rpc.Client
-	//	codec  rpc.ClientCodec
+	mu     sync.Mutex
-	mu sync.Mutex
 }
 
 func NewStdIOUI() *StdIOUI {
 	log.Info("NewStdIOUI")
-	//	in, out := bufio.NewReader(os.Stdin), os.Stdout
 	client, err := rpc.DialContext(context.Background(), "stdio://")
 	if err != nil {
 		log.Crit("Could not create stdio client", "err", err)

cmd/signer/core/validation.go

@@ -108,7 +108,7 @@ func (v *Validator) validate(msgs *ValidationMessages, txargs *SendTxArgs, metho
 	// 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: moth "data" and "input" are set and are not identical`)
+		return errors.New(`Ambiguous request: both "data" and "input" are set and are not identical`)
 	}
 	var (
 		data []byte

cmd/signer/main.go

@@ -402,7 +402,7 @@ func signer(c *cli.Context) error {
 		ipcApiUrl = "n/a"
 	)
 	rpcApi := []rpc.API{
-		rpc.API{
+		{
 			Namespace: "account",
 			Public:    true,
 			Service:   api,

cmd/signer/rules/rules_test.go

@@ -128,7 +128,7 @@ func initRuleEngine(js string) (*rulesetUi, error) {
 func TestListRequest(t *testing.T) {
 	accs := make([]core.Account, 5)
 
-	for i, _ := range accs {
+	for i := range accs {
 		addr := fmt.Sprintf("000000000000000000000000000000000000000%x", i)
 		acc := core.Account{
 			Address: common.BytesToAddress(common.Hex2Bytes(addr)),
@@ -626,7 +626,7 @@ function ApproveSignData(r){
 		Rawdata: raw,
 	})
 	if err != nil {
-		t.Fatalf("Unexpected error", err)
+		t.Fatalf("Unexpected error %v", err)
 	}
 	if !resp.Approved {
 		t.Fatalf("Expected approved")

cmd/signer/storage/aes_gcm_storage_test.go

@@ -52,11 +52,11 @@ func TestEncryption(t *testing.T) {
 func TestFileStorage(t *testing.T) {
 
 	a := map[string]storedCredential{
-		"secret": storedCredential{
+		"secret": {
 			Iv:         common.Hex2Bytes("cdb30036279601aeee60f16b"),
 			CipherText: common.Hex2Bytes("f311ac49859d7260c2c464c28ffac122daf6be801d3cfd3edcbde7e00c9ff74f"),
 		},
-		"secret2": storedCredential{
+		"secret2": {
 			Iv:         common.Hex2Bytes("afb8a7579bf971db9f8ceeed"),
 			CipherText: common.Hex2Bytes("2df87baf86b5073ef1f03e3cc738de75b511400f5465bb0ddeacf47ae4dc267d"),
 		},