signer: refactored request/response, made use of urfave.cli

cmd/signer/api.go

@@ -49,74 +49,113 @@ type Metadata struct {
 	scheme string
 }
 
-// The SignTxRequest contains info about a transaction tos sign
+type credPreference int
+
+const (
+	forgetPw credPreference = iota
+	rememberPwNow
+)
+
+type Credentials struct {
+	password string
+}
+
+// SignTxRequest contains info about a transaction to sign
 type SignTxRequest struct {
 	transaction *types.Transaction
 	from        accounts.Account
 }
+type SignTxResponse struct {
+	hash     common.Hash
+	approved bool
+	pw       string
+}
 
 type ExportRequest struct {
 	account accounts.Account
 	file    string
 }
+type ExportResponse struct {
+	approved bool
+}
+
 type ImportRequest struct {
 	account accounts.Account
 }
+
+type ImportResponse struct {
+	approved    bool
+	oldPassword string
+	newPassword string
+}
+
 type SignDataRequest struct {
 	account accounts.Account
 	rawdata hexutil.Bytes
 	message string
 	hash    hexutil.Bytes
 }
-type ApprovalStatus struct {
+type SignDataResponse struct {
-	hash     common.Hash
 	approved bool
 	pw       string
 }
 
 type NewAccountRequest struct{}
+type NewAccountResponse struct {
+	approved bool
+	pw       string
+}
 
 type ListRequest struct {
 	accounts []Account
 }
-type ListApproval struct {
+type ListResponse struct {
 	accounts []Account
 }
 
 // SignerUI specifies what method a UI needs to implement to be able to be used as a UI
 // for the signer
 type SignerUI interface {
-	ApproveTx(request *SignTxRequest, metadata Metadata, ch chan ApprovalStatus)
+
-	ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan ApprovalStatus)
+	// ApproveTx prompt the user for confirmation to request to sign transaction
-	ApproveExport(request *ExportRequest, metadata Metadata, ch chan ApprovalStatus)
+	ApproveTx(request *SignTxRequest, metadata Metadata, ch chan SignTxResponse)
-	ApproveImport(request *ImportRequest, metadata Metadata, ch chan ApprovalStatus)
+	// ApproveSignData prompt the user for confirmation to request to sign data
-	ApproveListing(request *ListRequest, metadata Metadata, ch chan ListApproval)
+	ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan SignDataResponse)
-	ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan bool)
+	// ApproveExport prompt the user for confirmation to export encrypted account json
-	//	In case signing fails, bad password etc
+	ApproveExport(request *ExportRequest, metadata Metadata, ch chan ExportResponse)
+	// ApproveImport prompt the user for confirmation to import account json
+	ApproveImport(request *ImportRequest, metadata Metadata, ch chan ImportResponse)
+	// ApproveListing prompt the user for confirmation to list accounts
+	// the list of accounts to list can be modified by the ui
+	ApproveListing(request *ListRequest, metadata Metadata, ch chan ListResponse)
+	// ApproveNewAccount prompt the user for confirmation to create new account, and reveal to caller
+	ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan NewAccountResponse)
+	// ShowError displays error message to user
 	ShowError(message string)
+	// ShowInfo displays info message to user
 	ShowInfo(message string)
 }
 
 type HeadlessUI struct {
 }
 
-func (ui *HeadlessUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch chan ApprovalStatus) {
+func (ui *HeadlessUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch chan SignTxResponse) {
-	ch <- ApprovalStatus{request.transaction.Hash(), true, ""}
+	ch <- SignTxResponse{request.transaction.Hash(), true, ""}
 }
-func (ui *HeadlessUI) ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan ApprovalStatus) {
+func (ui *HeadlessUI) ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan SignDataResponse) {
-	ch <- ApprovalStatus{common.Hash{}, true, ""}
+	ch <- SignDataResponse{true, ""}
 }
-func (ui *HeadlessUI) ApproveExport(request *ExportRequest, metadata Metadata, ch chan ApprovalStatus) {
+func (ui *HeadlessUI) ApproveExport(request *ExportRequest, metadata Metadata, ch chan ExportResponse) {
-	ch <- ApprovalStatus{common.Hash{}, true, ""}
+	ch <- ExportResponse{true}
 }
-func (ui *HeadlessUI) ApproveImport(request *ImportRequest, metadata Metadata, ch chan ApprovalStatus) {
+func (ui *HeadlessUI) ApproveImport(request *ImportRequest, metadata Metadata, ch chan ImportResponse) {
-	ch <- ApprovalStatus{common.Hash{}, true, ""}
+	ch <- ImportResponse{true, "", ""}
 }
-func (ui *HeadlessUI) ApproveListing(request *ListRequest, metadata Metadata, ch chan ListApproval) {
+func (ui *HeadlessUI) ApproveListing(request *ListRequest, metadata Metadata, ch chan ListResponse) {
-	ch <- ListApproval{request.accounts}
+	ch <- ListResponse{request.accounts}
 }
-func (ui *HeadlessUI) ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan bool) {
+func (ui *HeadlessUI) ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan ImportResponse) {
-	ch <- true
+	ch <- ImportResponse{true, "", ""}
 }
 func (ui *HeadlessUI) ShowError(message string) {
 	//stdout is used by communication
@@ -163,13 +202,13 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI) *Si
 func metaData(ctx context.Context) Metadata {
 	m := Metadata{"NA", "NA", "NA"}
 
-	if v := ctx.Value("remote"); v != nil{
+	if v := ctx.Value("remote"); v != nil {
 		m.remote = v.(string)
 	}
-	if v := ctx.Value("scheme"); v != nil{
+	if v := ctx.Value("scheme"); v != nil {
 		m.scheme = v.(string)
 	}
-	if v := ctx.Value("local"); v != nil{
+	if v := ctx.Value("local"); v != nil {
 		m.local = v.(string)
 	}
 	return m
@@ -179,7 +218,7 @@ func metaData(ctx context.Context) Metadata {
 // multiple accounts.
 func (api *SignerAPI) List(ctx context.Context) ([]Account, error) {
 
-	ch := make(chan ListApproval, 1)
+	ch := make(chan ListResponse, 1)
 
 	var accounts []Account
 	for _, wallet := range api.am.Wallets() {
@@ -204,10 +243,10 @@ func (api *SignerAPI) New(ctx context.Context, passphrase string) (accounts.Acco
 	if len(be) == 0 {
 		return accounts.Account{}, errors.New("password based accounts not supported")
 	}
-	ch := make(chan bool, 1)
+	ch := make(chan NewAccountResponse, 1)
 	api.ui.ApproveNewAccount(&NewAccountRequest{}, metaData(ctx), ch)
 
-	if <-ch {
+	if resp := <-ch; resp.approved {
 		return be[0].(*keystore.KeyStore).NewAccount(passphrase)
 	}
 	return accounts.Account{}, fmt.Errorf("Request denied")
@@ -230,7 +269,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address,
 		tx = types.NewTransaction(uint64(*args.Nonce), *args.To, (*big.Int)(args.Value), (*big.Int)(args.Gas), (*big.Int)(args.GasPrice), args.Data)
 	}
 
-	ch := make(chan ApprovalStatus, 1)
+	ch := make(chan SignTxResponse, 1)
 	api.ui.ApproveTx(&SignTxRequest{transaction: tx, from: acc}, metaData(ctx), ch)
 
 	if result := <-ch; result.approved {
@@ -266,10 +305,9 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.Address, passwd stri
 	if err != nil {
 		return nil, err
 	}
-	ch := make(chan ApprovalStatus, 1)
+	ch := make(chan SignDataResponse, 1)
 
-	msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data)
+	sighash, msg := signHash(data)
-	sighash := crypto.Keccak256([]byte(msg))
 
 	api.ui.ApproveSignData(&SignDataRequest{account: account, rawdata: data, message: msg, hash: sighash}, metaData(ctx), ch)
 
@@ -306,7 +344,8 @@ func (api *SignerAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (c
 	}
 	sig[64] -= 27 // Transform yellow paper V from 27/28 to 0/1
 
-	rpk, err := crypto.Ecrecover(signHash(data), sig)
+	hash, _ := signHash(data)
+	rpk, err := crypto.Ecrecover(hash, sig)
 	if err != nil {
 		return common.Address{}, err
 	}
@@ -322,9 +361,9 @@ func (api *SignerAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (c
 //   keccak256("\x19Ethereum Signed Message:\n"${message length}${message}).
 //
 // This gives context to the signed message and prevents signing of transactions.
-func signHash(data []byte) []byte {
+func signHash(data []byte) ([]byte, string) {
 	msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data)
-	return crypto.Keccak256([]byte(msg))
+	return crypto.Keccak256([]byte(msg)), msg
 }
 
 // Export returns encrypted private key associated with the given address in web3 keystore format.
@@ -341,7 +380,7 @@ func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.Raw
 	if url.Scheme != keystore.KeyStoreScheme {
 		return nil, fmt.Errorf("account is not a password protected account")
 	}
-	ch := make(chan ApprovalStatus, 1)
+	ch := make(chan ExportResponse, 1)
 
 	api.ui.ApproveExport(&ExportRequest{account: account, file: url.Path}, metaData(ctx), ch)
 
@@ -361,7 +400,7 @@ func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage, passp
 		return Account{}, errors.New("password based accounts not supported")
 	}
 
-	ch := make(chan ApprovalStatus, 1)
+	ch := make(chan ImportResponse, 1)
 
 	api.ui.ApproveImport(&ImportRequest{}, metaData(ctx), ch)
 	if resp := <-ch; resp.approved {

cmd/signer/cliui.go

@@ -18,31 +18,64 @@ package main
 import (
 	"bufio"
 	"fmt"
-	"github.com/ethereum/go-ethereum/common"
 	"os"
+	"strings"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/log"
+	"golang.org/x/crypto/ssh/terminal"
 )
 
 type CommandlineUI struct {
+	in *bufio.Reader
 }
 
 func NewCommandlineUI() *CommandlineUI {
-	return &CommandlineUI{}
+	return &CommandlineUI{bufio.NewReader(os.Stdin)}
 }
-func confirm() bool {
+
-	fmt.Printf("Type 'Yes' to approve\n$>")
+// readString reads a single line from stdin, trimming if from spaces, enforcing
-	scanner := bufio.NewScanner(os.Stdin)
+// non-emptyness.
-	scanner.Scan()
+func (ui *CommandlineUI) readString() string {
-	answer := scanner.Text()
+	for {
-	if answer == "Yes" {
+		fmt.Printf("> ")
+		text, err := ui.in.ReadString('\n')
+		if err != nil {
+			log.Crit("Failed to read user input", "err", err)
+		}
+		if text = strings.TrimSpace(text); text != "" {
+			return text
+		}
+	}
+}
+
+// readPassword reads a single line from stdin, trimming it from the trailing new
+// line and returns it. The input will not be echoed.
+func (ui *CommandlineUI) readPassword() string {
+	fmt.Printf("> ")
+	text, err := terminal.ReadPassword(int(os.Stdin.Fd()))
+	if err != nil {
+		log.Crit("Failed to read password", "err", err)
+	}
+	fmt.Println()
+	return string(text)
+}
+
+// confirm returns true if user enters 'Yes', otherwise false
+func (ui *CommandlineUI) confirm() bool {
+	fmt.Printf("Type 'Yes' to approve\n")
+	if ui.readString() == "Yes" {
 		return true
 	}
 	return false
 }
+
 func showMetadata(metadata Metadata) {
 	fmt.Printf("Request info: %v -> %v -> %v\n", metadata.remote, metadata.scheme, metadata.local)
 }
 
-func (ui *CommandlineUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch chan ApprovalStatus) {
+// ApproveTx prompt the user for confirmation to request to sign transaction
+func (ui *CommandlineUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch chan SignTxResponse) {
 
 	fmt.Printf("--------- Transaction request-------------\n")
 	fmt.Printf("to:    %v\n", request.transaction.To())
@@ -51,9 +84,11 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest, metadata Metadata, ch
 	fmt.Printf("data:  %v\n", common.Bytes2Hex(request.transaction.Data()))
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(metadata)
-	ch <- ApprovalStatus{common.Hash{}, confirm(), ""}
+	ch <- SignTxResponse{request.transaction.Hash(), ui.confirm(), ""}
 }
-func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan ApprovalStatus) {
+
+// ApproveSignData prompt the user for confirmation to request to sign data
+func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest, metadata Metadata, ch chan SignDataResponse) {
 
 	fmt.Printf("-------- Sign data request--------------\n")
 	fmt.Printf("account:  %x\n", request.account.Address)
@@ -62,9 +97,11 @@ func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest, metadata Meta
 	fmt.Printf("message hash:  %v\n", request.hash)
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(metadata)
-	ch <- ApprovalStatus{common.Hash{}, confirm(), ""}
+	ch <- SignDataResponse{ui.confirm(), ""}
 }
-func (ui *CommandlineUI) ApproveExport(request *ExportRequest, metadata Metadata, ch chan ApprovalStatus) {
+
+// ApproveExport prompt the user for confirmation to export encrypted account json
+func (ui *CommandlineUI) ApproveExport(request *ExportRequest, metadata Metadata, ch chan ExportResponse) {
 	fmt.Printf("-------- Export account request--------------\n")
 	fmt.Printf("A request has been made to export the (encrypted) keyfile\n")
 	fmt.Printf("Approving this operation means that the caller obtains the (encrypted) contents\n")
@@ -73,16 +110,21 @@ func (ui *CommandlineUI) ApproveExport(request *ExportRequest, metadata Metadata
 	fmt.Printf("keyfile:  \n%v\n", request.file)
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(metadata)
-	ch <- ApprovalStatus{common.Hash{}, confirm(), ""}
+	ch <- ExportResponse{ui.confirm()}
 }
-func (ui *CommandlineUI) ApproveImport(request *ImportRequest, metadata Metadata, ch chan ApprovalStatus) {
+
+// ApproveImport prompt the user for confirmation to import account json
+func (ui *CommandlineUI) ApproveImport(request *ImportRequest, metadata Metadata, ch chan ImportResponse) {
 	fmt.Printf("-------- Export account request--------------\n")
 	fmt.Printf("A request has been made to import an encrypted keyfile\n")
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(metadata)
-	ch <- ApprovalStatus{common.Hash{}, confirm(), ""}
+	ch <- ImportResponse{ui.confirm(), "", ""}
 }
-func (ui *CommandlineUI) ApproveListing(request *ListRequest, metadata Metadata, ch chan ListApproval) {
+
+// ApproveListing prompt the user for confirmation to list accounts
+// the list of accounts to list can be modified by the ui
+func (ui *CommandlineUI) ApproveListing(request *ListRequest, metadata Metadata, ch chan ListResponse) {
 
 	fmt.Printf("-------- List account request--------------\n")
 	fmt.Printf("A request has been made to list all accounts. \n")
@@ -92,26 +134,31 @@ func (ui *CommandlineUI) ApproveListing(request *ListRequest, metadata Metadata,
 	}
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(metadata)
-	if confirm() {
+	if ui.confirm() {
-		ch <- ListApproval{request.accounts}
+		ch <- ListResponse{request.accounts}
 	} else {
-		ch <- ListApproval{nil}
+		ch <- ListResponse{nil}
 	}
 }
-func (ui *CommandlineUI) ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan bool) {
+
+// ApproveNewAccount prompt the user for confirmation to create new account, and reveal to caller
+func (ui *CommandlineUI) ApproveNewAccount(requst *NewAccountRequest, metadata Metadata, ch chan NewAccountResponse) {
 	fmt.Printf("-------- New account request--------------\n")
 	fmt.Printf("A request has been made to create a new. \n")
 	fmt.Printf("Approving this operation means that a new account is created,\n")
 	fmt.Printf("and the address show to the caller\n")
 	showMetadata(metadata)
-	ch <- confirm()
+	ch <- NewAccountResponse{ui.confirm(), ""}
 }
 
+// ShowError displays error message to user
 func (ui *CommandlineUI) ShowError(message string) {
-	//stdout is used by communication
+
 	fmt.Printf("ERROR: %v", message)
 }
+
+// ShowInfo displays info message to user
 func (ui *CommandlineUI) ShowInfo(message string) {
-	//stdout is used by communication
+
 	fmt.Printf("Info: %v", message)
 }

cmd/signer/main.go

@@ -19,54 +19,79 @@
 package main
 
 import (
-	"flag"
 	"io"
 	"os"
 	"path/filepath"
 
+	"fmt"
+	"net"
+
 	"github.com/ethereum/go-ethereum/cmd/utils"
+	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/node"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rpc"
-	"github.com/ethereum/go-ethereum/log"
+	"gopkg.in/urfave/cli.v1"
-	"net"
-	"fmt"
-)
-
-var (
-	ksLocation = flag.String("keystore", filepath.Join(node.DefaultDataDir(), "keystore"), "Directory for the keystore")
-	chainID    = flag.Int64("chainid", params.MainnetChainConfig.ChainId.Int64(), "chain identifier")
 )
 
 func main() {
-	flag.Parse()
 
-	var (
+	app := cli.NewApp()
-		server = rpc.NewServer()
+	app.Name = "signer"
-		api    = NewSignerAPI(*chainID, *ksLocation, true, NewCommandlineUI())
+	app.Usage = "Manage ethereum account operations"
-		listener net.Listener
+	app.Flags = []cli.Flag{
-		err error
+		cli.Int64Flag{
-	)
+			Name:  "chainid",
-
+			Value: params.MainnetChainConfig.ChainId.Int64(),
-	// register signer API with server
+			Usage: "chain identifier",
-	if err = server.RegisterName("account", api); err != nil {
+		},
-		utils.Fatalf("Could not register signer API: %v", err)
+		cli.IntFlag{
+			Name:  "loglevel",
+			Value: 4,
+			Usage: "log level to emit to the screen",
+		},
+		cli.StringFlag{
+			Name:  "keystore",
+			Value: filepath.Join(node.DefaultDataDir(), "keystore"),
+			Usage: "Directory for the keystore",
+		},
 	}
+	app.Action = func(c *cli.Context) error {
+		// Set up the logger to print everything and the random generator
+		log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(c.Int("loglevel")), log.StreamHandler(os.Stdout, log.TerminalFormat(true))))
 
-	endpoint := "localhost:8550"
+		var (
+			server   = rpc.NewServer()
+			api      = NewSignerAPI(c.Int64("chainid"), c.String("keystore"), true, NewCommandlineUI())
+			listener net.Listener
+			err      error
+		)
 
-	if listener, err = net.Listen("tcp", endpoint); err != nil {
+		// register signer API with server
-		utils.Fatalf("Could not start http listener: %v", err)
+		if err = server.RegisterName("account", api); err != nil {
+			utils.Fatalf("Could not register signer API: %v", err)
+		}
+
+		endpoint := "localhost:8550"
+
+		if listener, err = net.Listen("tcp", endpoint); err != nil {
+			utils.Fatalf("Could not start http listener: %v", err)
+		}
+		log.Info(fmt.Sprintf("HTTP endpoint opened: http://%s", endpoint))
+		cors := []string{"*"}
+
+		rpc.NewHTTPServer(cors, server).Serve(listener)
+		return nil
 	}
-	log.Info(fmt.Sprintf("HTTP endpoint opened: http://%s", endpoint))
+	app.Run(os.Args)
-	fmt.Printf("HTTP endpoint opened: http://%s\n", endpoint)
-	cors := []string{"*"}
 
-	rpc.NewHTTPServer(cors, server).Serve(listener)
 }
-// Create account
-// #curl -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_new","params":["test"],"id":67}' localhost:8550
 
+// Create account
+// curl -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_new","params":["test"],"id":67}' localhost:8550
+
+// List accounts
+// curl -i -H "Content-Type: application/json" -X POST --data '{"jsonrpc":"2.0","method":"account_list","params":[""],"id":67}' http://localhost:8550/
 type rwc struct {
 	io.Reader
 	io.Writer