signer: cliui formatting, remove local path disclosure, update extapi docs

cmd/clef/extapi_changelog.md

@@ -1,6 +1,11 @@
 ### Changelog for external API
 
 
+### 3.0.0
+
+* The external `accounts_List`-method was changed to not expose `url`, which contained information about the local machine. It now returns a set of addresses: `[]common.Address`.
+* The method was also renamed into `accounts_listAccounts`. 
+
 
 #### 2.0.0
 

cmd/clef/intapi_changelog.md

@@ -1,5 +1,9 @@
 ### Changelog for internal API (ui-api)
 
+### 3.0.0
+
+* The external `accounts_List`-method was changed to not expose `url`, which contained information about the local machine. Thus, the internal method `approveListing` was changed correspondingly, to contain only a set of addresses: `[]common.Address`
+
 ### 2.0.0
 
 * Modify how `call_info` on a transaction is conveyed. New format:

cmd/clef/main.go

@@ -591,7 +591,7 @@ func testExternalUI(api *core.SignerAPI) {
 	checkErr("SignTransaction", err)
 	_, err = api.Sign(ctx, common.MixedcaseAddress{}, common.Hex2Bytes("01020304"))
 	checkErr("Sign", err)
-	_, err = api.List(ctx)
+	_, err = api.ListAccounts(ctx)
 	checkErr("List", err)
 	_, err = api.New(ctx)
 	checkErr("New", err)

signer/core/api.go

@@ -38,8 +38,8 @@ import (
 
 // ExternalAPI defines the external API through which signing requests are made.
 type ExternalAPI interface {
-	// List available accounts
+	// ListAccounts lists available accounts (addresses)
-	List(ctx context.Context) (Accounts, error)
+	ListAccounts(ctx context.Context) ([]common.Address, error)
 	// New request to create a new account
 	New(ctx context.Context) (accounts.Account, error)
 	// SignTransaction request to sign the specified transaction
@@ -66,7 +66,7 @@ type SignerUI interface {
 	ApproveImport(request *ImportRequest) (ImportResponse, error)
 	// ApproveListing prompt the user for confirmation to list accounts
 	// the list of accounts to list can be modified by the UI
-	ApproveListing(request *ListRequest) (ListResponse, error)
+	ApproveListing(request *ListAccountsRequest) (ListAccountsResponse, error)
 	// ApproveNewAccount prompt the user for confirmation to create new Account, and reveal to caller
 	ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error)
 	// ShowError displays error message to user
@@ -172,12 +172,12 @@ type (
 		Approved bool   `json:"approved"`
 		Password string `json:"password"`
 	}
-	ListRequest struct {
+	ListAccountsRequest struct {
-		Accounts []Account `json:"accounts"`
+		Accounts []common.Address `json:"accounts"`
-		Meta     Metadata  `json:"meta"`
+		Meta     Metadata         `json:"meta"`
 	}
-	ListResponse struct {
+	ListAccountsResponse struct {
-		Accounts []Account `json:"accounts"`
+		Accounts []common.Address `json:"accounts"`
 	}
 	Message struct {
 		Text string `json:"text"`
@@ -225,17 +225,16 @@ func NewSignerAPI(chainID int64, ksLocation string, noUSB bool, ui SignerUI, abi
 	return &SignerAPI{big.NewInt(chainID), accounts.NewManager(backends...), ui, NewValidator(abidb)}
 }
 
-// List returns the set of wallet this signer manages. Each wallet can contain
+// ListAccounts returns the set of wallet this signer manages. Each wallet can contain
 // multiple accounts.
-func (api *SignerAPI) List(ctx context.Context) (Accounts, error) {
+func (api *SignerAPI) ListAccounts(ctx context.Context) ([]common.Address, error) {
-	var accs []Account
+	addresses := make([]common.Address, 0) // return [] instead of nil if empty
 	for _, wallet := range api.am.Wallets() {
-		for _, acc := range wallet.Accounts() {
+		for _, account := range wallet.Accounts() {
-			acc := Account{Typ: "Account", URL: wallet.URL(), Address: acc.Address}
+			addresses = append(addresses, account.Address)
-			accs = append(accs, acc)
 		}
 	}
-	result, err := api.UI.ApproveListing(&ListRequest{Accounts: accs, Meta: MetadataFromContext(ctx)})
+	result, err := api.UI.ApproveListing(&ListAccountsRequest{Accounts: addresses, Meta: MetadataFromContext(ctx)})
 	if err != nil {
 		return nil, err
 	}

signer/core/api_test.go

@@ -80,17 +80,17 @@ func (ui *HeadlessUI) ApproveImport(request *ImportRequest) (ImportResponse, err
 	}
 	return ImportResponse{false, "", ""}, nil
 }
-func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error) {
+func (ui *HeadlessUI) ApproveListing(request *ListAccountsRequest) (ListAccountsResponse, error) {
 
 	switch <-ui.controller {
 	case "A":
-		return ListResponse{request.Accounts}, nil
+		return ListAccountsResponse{request.Accounts}, nil
 	case "1":
-		l := make([]Account, 1)
+		l := make([]common.Address, 1)
 		l[0] = request.Accounts[1]
-		return ListResponse{l}, nil
+		return ListAccountsResponse{l}, nil
 	default:
-		return ListResponse{nil}, nil
+		return ListAccountsResponse{nil}, nil
 	}
 }
 func (ui *HeadlessUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
@@ -162,9 +162,9 @@ func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
 		t.Fatal("Empty address should be returned")
 	}
 }
-func list(control chan string, api *SignerAPI, t *testing.T) []Account {
+func list(control chan string, api *SignerAPI, t *testing.T) []common.Address {
 	control <- "A"
-	list, err := api.List(context.Background())
+	list, err := api.ListAccounts(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -193,7 +193,7 @@ func TestNewAcc(t *testing.T) {
 	// Testing listing:
 	// Listing one Account
 	control <- "1"
-	list, err := api.List(context.Background())
+	list, err := api.ListAccounts(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -202,7 +202,7 @@ func TestNewAcc(t *testing.T) {
 	}
 	// Listing denied
 	control <- "Nope"
-	list, err = api.List(context.Background())
+	list, err = api.ListAccounts(context.Background())
 	if len(list) != 0 {
 		t.Fatalf("List should be empty")
 	}
@@ -218,11 +218,11 @@ func TestSignData(t *testing.T) {
 	createAccount(control, api, t)
 	createAccount(control, api, t)
 	control <- "1"
-	list, err := api.List(context.Background())
+	list, err := api.ListAccounts(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
-	a := common.NewMixedcaseAddress(list[0].Address)
+	a := common.NewMixedcaseAddress(list[0])
 
 	control <- "Y"
 	control <- "wrongpassword"
@@ -275,7 +275,7 @@ func mkTestTx(from common.MixedcaseAddress) SendTxArgs {
 func TestSignTx(t *testing.T) {
 
 	var (
-		list      Accounts
+		list      []common.Address
 		res, res2 *ethapi.SignTransactionResult
 		err       error
 	)
@@ -283,11 +283,11 @@ func TestSignTx(t *testing.T) {
 	api, control := setup(t)
 	createAccount(control, api, t)
 	control <- "A"
-	list, err = api.List(context.Background())
+	list, err = api.ListAccounts(context.Background())
 	if err != nil {
 		t.Fatal(err)
 	}
-	a := common.NewMixedcaseAddress(list[0].Address)
+	a := common.NewMixedcaseAddress(list[0])
 
 	methodSig := "test(uint)"
 	tx := mkTestTx(a)

signer/core/auditlog.go

@@ -33,12 +33,10 @@ type AuditLogger struct {
 	api ExternalAPI
 }
 
-func (l *AuditLogger) List(ctx context.Context) (Accounts, error) {
+func (l *AuditLogger) ListAccounts(ctx context.Context) ([]common.Address, error) {
-	l.log.Info("List", "type", "request", "metadata", MetadataFromContext(ctx).String())
+	l.log.Info("ListAccounts", "type", "request", "metadata", MetadataFromContext(ctx).String())
-	res, e := l.api.List(ctx)
+	res, e := l.api.ListAccounts(ctx)
-
+	l.log.Info("ListAccounts", "type", "response", "data", res)
-	l.log.Info("List", "type", "response", "data", res.String())
-
 	return res, e
 }
 

signer/core/cliui.go

@@ -109,20 +109,23 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro
 			fmt.Printf("\nWARNING: Invalid checksum on to-address!\n\n")
 		}
 	} else {
-		fmt.Printf("to:    <contact creation>\n")
+		fmt.Printf("To:    <contact creation>\n")
 	}
-	fmt.Printf("from:  %v\n", request.Transaction.From.String())
+	fmt.Printf("Trom:  %v\n", request.Transaction.From.String())
-	fmt.Printf("value: %v wei\n", weival)
+	fmt.Printf("Value: %v wei\n", weival)
+	fmt.Printf("Gas: %v\n", request.Transaction.Gas)
+	fmt.Printf("Gasprice: %v wei\n", request.Transaction.GasPrice.ToInt())
+	fmt.Printf("Nonce: %v\n", request.Transaction.Nonce)
 	if request.Transaction.Data != nil {
 		d := *request.Transaction.Data
 		if len(d) > 0 {
-			fmt.Printf("data:  %v\n", common.Bytes2Hex(d))
+			fmt.Printf("Data:  %v\n", common.Bytes2Hex(d))
 		}
 	}
 	if request.Callinfo != nil {
 		fmt.Printf("\nTransaction validation:\n")
 		for _, m := range request.Callinfo {
-			fmt.Printf("  * %s : %s", m.Typ, m.Message)
+			fmt.Printf("  * %s : %s\n", m.Typ, m.Message)
 		}
 		fmt.Println()
 
@@ -187,7 +190,7 @@ func (ui *CommandlineUI) ApproveImport(request *ImportRequest) (ImportResponse,
 
 // 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) (ListResponse, error) {
+func (ui *CommandlineUI) ApproveListing(request *ListAccountsRequest) (ListAccountsResponse, error) {
 
 	ui.mu.Lock()
 	defer ui.mu.Unlock()
@@ -196,14 +199,14 @@ func (ui *CommandlineUI) ApproveListing(request *ListRequest) (ListResponse, err
 	fmt.Printf("A request has been made to list all accounts. \n")
 	fmt.Printf("You can select which accounts the caller can see\n")
 	for _, account := range request.Accounts {
-		fmt.Printf("\t[x] %v\n", account.Address.Hex())
+		fmt.Printf("\t[x] %v\n", account.Hex())
 	}
 	fmt.Printf("-------------------------------------------\n")
 	showMetadata(request.Meta)
 	if !ui.confirm() {
-		return ListResponse{nil}, nil
+		return ListAccountsResponse{nil}, nil
 	}
-	return ListResponse{request.Accounts}, nil
+	return ListAccountsResponse{request.Accounts}, nil
 }
 
 // ApproveNewAccount prompt the user for confirmation to create new Account, and reveal to caller

signer/core/stdioui.go

@@ -73,8 +73,8 @@ func (ui *StdIOUI) ApproveImport(request *ImportRequest) (ImportResponse, error)
 	return result, err
 }
 
-func (ui *StdIOUI) ApproveListing(request *ListRequest) (ListResponse, error) {
+func (ui *StdIOUI) ApproveListing(request *ListAccountsRequest) (ListAccountsResponse, error) {
-	var result ListResponse
+	var result ListAccountsResponse
 	err := ui.dispatch("ApproveListing", request, &result)
 	return result, err
 }

signer/rules/rules.go

@@ -194,7 +194,7 @@ func (r *rulesetUI) ApproveImport(request *core.ImportRequest) (core.ImportRespo
 	return r.next.ApproveImport(request)
 }
 
-func (r *rulesetUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
+func (r *rulesetUI) ApproveListing(request *core.ListAccountsRequest) (core.ListAccountsResponse, error) {
 	jsonreq, err := json.Marshal(request)
 	approved, err := r.checkApproval("ApproveListing", jsonreq, err)
 	if err != nil {
@@ -202,9 +202,9 @@ func (r *rulesetUI) ApproveListing(request *core.ListRequest) (core.ListResponse
 		return r.next.ApproveListing(request)
 	}
 	if approved {
-		return core.ListResponse{Accounts: request.Accounts}, nil
+		return core.ListAccountsResponse{Accounts: request.Accounts}, nil
 	}
-	return core.ListResponse{}, err
+	return core.ListAccountsResponse{}, err
 }
 
 func (r *rulesetUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) {

signer/rules/rules_test.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/core/types"
@@ -93,8 +92,8 @@ func (alwaysDenyUI) ApproveImport(request *core.ImportRequest) (core.ImportRespo
 	return core.ImportResponse{Approved: false, OldPassword: "", NewPassword: ""}, nil
 }
 
-func (alwaysDenyUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
+func (alwaysDenyUI) ApproveListing(request *core.ListAccountsRequest) (core.ListAccountsResponse, error) {
-	return core.ListResponse{Accounts: nil}, nil
+	return core.ListAccountsResponse{Accounts: nil}, nil
 }
 
 func (alwaysDenyUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) {
@@ -125,15 +124,11 @@ func initRuleEngine(js string) (*rulesetUI, error) {
 }
 
 func TestListRequest(t *testing.T) {
-	accs := make([]core.Account, 5)
+	accs := make([]common.Address, 5)
 
 	for i := range accs {
 		addr := fmt.Sprintf("000000000000000000000000000000000000000%x", i)
-		acc := core.Account{
+		accs[i] = common.BytesToAddress(common.Hex2Bytes(addr))
-			Address: common.BytesToAddress(common.Hex2Bytes(addr)),
-			URL:     accounts.URL{Scheme: "test", Path: fmt.Sprintf("acc-%d", i)},
-		}
-		accs[i] = acc
 	}
 
 	js := `function ApproveListing(){ return "Approve" }`
@@ -143,7 +138,7 @@ func TestListRequest(t *testing.T) {
 		t.Errorf("Couldn't create evaluator %v", err)
 		return
 	}
-	resp, err := r.ApproveListing(&core.ListRequest{
+	resp, err := r.ApproveListing(&core.ListAccountsRequest{
 		Accounts: accs,
 		Meta:     core.Metadata{Remote: "remoteip", Local: "localip", Scheme: "inproc"},
 	})
@@ -220,9 +215,9 @@ func (d *dummyUI) ApproveImport(request *core.ImportRequest) (core.ImportRespons
 	return core.ImportResponse{}, core.ErrRequestDenied
 }
 
-func (d *dummyUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
+func (d *dummyUI) ApproveListing(request *core.ListAccountsRequest) (core.ListAccountsResponse, error) {
 	d.calls = append(d.calls, "ApproveListing")
-	return core.ListResponse{}, core.ErrRequestDenied
+	return core.ListAccountsResponse{}, core.ErrRequestDenied
 }
 
 func (d *dummyUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) {
@@ -532,9 +527,9 @@ func (d *dontCallMe) ApproveImport(request *core.ImportRequest) (core.ImportResp
 	return core.ImportResponse{}, core.ErrRequestDenied
 }
 
-func (d *dontCallMe) ApproveListing(request *core.ListRequest) (core.ListResponse, error) {
+func (d *dontCallMe) ApproveListing(request *core.ListAccountsRequest) (core.ListAccountsResponse, error) {
 	d.t.Fatalf("Did not expect next-handler to be called")
-	return core.ListResponse{}, core.ErrRequestDenied
+	return core.ListAccountsResponse{}, core.ErrRequestDenied
 }
 
 func (d *dontCallMe) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) {