signer: change the stdio jsonrpc to use legacy namespace conventions (#19047)
This PR will will break existing UIs, since it changes all calls like ApproveSignTransaction to be on the form ui_approveSignTransaction. This is to make it possible for the UI to reuse the json-rpc library from go-ethereum, which uses this convention. Also, this PR removes some unused structs, after import/export were removed from the external api (so no longer needs internal methods for approval) One more breaking change is introduced, removing passwords from the ApproveSignTxResponse and the likes. This makes the manual interface more like the rulebased interface, and integrates nicely with the credential storage. Thus, the way it worked before, it would be tempting for the UI to implement 'remember password' functionality. The way it is now, it will be easy instead to tell clef to store passwords and use them. If a pw is not found in the credential store, the user is prompted to provide the password.
This commit is contained in:
Martin Holst Swende
committed by
Péter Szilágyi
Péter Szilágyi
parent
eb199f1fc2
commit
5f94f8c7e7
@ -19,7 +19,6 @@ package core
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"math/big"
|
||||
@ -35,62 +34,49 @@ 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/storage"
|
||||
)
|
||||
|
||||
//Used for testing
|
||||
type HeadlessUI struct {
|
||||
controller chan string
|
||||
type headlessUi struct {
|
||||
approveCh chan string // to send approve/deny
|
||||
inputCh chan string // to send password
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
|
||||
return UserInputResponse{}, errors.New("not implemented")
|
||||
func (ui *headlessUi) OnInputRequired(info UserInputRequest) (UserInputResponse, error) {
|
||||
input := <-ui.inputCh
|
||||
return UserInputResponse{Text: input}, nil
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) OnSignerStartup(info StartupInfo) {
|
||||
}
|
||||
func (ui *HeadlessUI) RegisterUIServer(api *UIServerAPI) {
|
||||
}
|
||||
func (ui *headlessUi) OnSignerStartup(info StartupInfo) {}
|
||||
func (ui *headlessUi) RegisterUIServer(api *UIServerAPI) {}
|
||||
func (ui *headlessUi) OnApprovedTx(tx ethapi.SignTransactionResult) {}
|
||||
|
||||
func (ui *HeadlessUI) OnApprovedTx(tx ethapi.SignTransactionResult) {
|
||||
fmt.Printf("OnApproved()\n")
|
||||
}
|
||||
func (ui *headlessUi) ApproveTx(request *SignTxRequest) (SignTxResponse, error) {
|
||||
|
||||
func (ui *HeadlessUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error) {
|
||||
|
||||
switch <-ui.controller {
|
||||
switch <-ui.approveCh {
|
||||
case "Y":
|
||||
return SignTxResponse{request.Transaction, true, <-ui.controller}, nil
|
||||
case "M": //Modify
|
||||
return 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, <-ui.controller}, nil
|
||||
return SignTxResponse{request.Transaction, true}, nil
|
||||
default:
|
||||
return SignTxResponse{request.Transaction, false, ""}, nil
|
||||
return SignTxResponse{request.Transaction, false}, nil
|
||||
}
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) {
|
||||
if "Y" == <-ui.controller {
|
||||
return SignDataResponse{true, <-ui.controller}, nil
|
||||
}
|
||||
return SignDataResponse{false, ""}, nil
|
||||
func (ui *headlessUi) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) {
|
||||
approved := "Y" == <-ui.approveCh
|
||||
return SignDataResponse{approved}, nil
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ApproveExport(request *ExportRequest) (ExportResponse, error) {
|
||||
return ExportResponse{<-ui.controller == "Y"}, nil
|
||||
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ApproveImport(request *ImportRequest) (ImportResponse, error) {
|
||||
if "Y" == <-ui.controller {
|
||||
return ImportResponse{true, <-ui.controller, <-ui.controller}, nil
|
||||
}
|
||||
return ImportResponse{false, "", ""}, nil
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error) {
|
||||
switch <-ui.controller {
|
||||
func (ui *headlessUi) ApproveListing(request *ListRequest) (ListResponse, error) {
|
||||
approval := <-ui.approveCh
|
||||
//fmt.Printf("approval %s\n", approval)
|
||||
switch approval {
|
||||
case "A":
|
||||
return ListResponse{request.Accounts}, nil
|
||||
case "1":
|
||||
@ -102,19 +88,19 @@ func (ui *HeadlessUI) ApproveListing(request *ListRequest) (ListResponse, error)
|
||||
}
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
|
||||
if "Y" == <-ui.controller {
|
||||
return NewAccountResponse{true, <-ui.controller}, nil
|
||||
func (ui *headlessUi) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) {
|
||||
if "Y" == <-ui.approveCh {
|
||||
return NewAccountResponse{true}, nil
|
||||
}
|
||||
return NewAccountResponse{false, ""}, nil
|
||||
return NewAccountResponse{false}, nil
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ShowError(message string) {
|
||||
func (ui *headlessUi) ShowError(message string) {
|
||||
//stdout is used by communication
|
||||
fmt.Fprintln(os.Stderr, message)
|
||||
}
|
||||
|
||||
func (ui *HeadlessUI) ShowInfo(message string) {
|
||||
func (ui *headlessUi) ShowInfo(message string) {
|
||||
//stdout is used by communication
|
||||
fmt.Fprintln(os.Stderr, message)
|
||||
}
|
||||
@ -131,25 +117,20 @@ func tmpDirName(t *testing.T) string {
|
||||
return d
|
||||
}
|
||||
|
||||
func setup(t *testing.T) (*SignerAPI, chan string) {
|
||||
|
||||
controller := make(chan string, 20)
|
||||
|
||||
func setup(t *testing.T) (*SignerAPI, *headlessUi) {
|
||||
db, err := NewAbiDBFromFile("../../cmd/clef/4byte.json")
|
||||
if err != nil {
|
||||
t.Fatal(err.Error())
|
||||
}
|
||||
var (
|
||||
ui = &HeadlessUI{controller}
|
||||
am = StartClefAccountManager(tmpDirName(t), true, true)
|
||||
api = NewSignerAPI(am, 1337, true, ui, db, true)
|
||||
)
|
||||
return api, controller
|
||||
}
|
||||
func createAccount(control chan string, api *SignerAPI, t *testing.T) {
|
||||
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{})
|
||||
return api, ui
|
||||
|
||||
control <- "Y"
|
||||
control <- "a_long_password"
|
||||
}
|
||||
func createAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
|
||||
ui.approveCh <- "Y"
|
||||
ui.inputCh <- "a_long_password"
|
||||
_, err := api.New(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
@ -158,14 +139,13 @@ func createAccount(control chan string, api *SignerAPI, t *testing.T) {
|
||||
time.Sleep(250 * time.Millisecond)
|
||||
}
|
||||
|
||||
func failCreateAccountWithPassword(control chan string, api *SignerAPI, password string, t *testing.T) {
|
||||
func failCreateAccountWithPassword(ui *headlessUi, api *SignerAPI, password string, t *testing.T) {
|
||||
|
||||
control <- "Y"
|
||||
control <- password
|
||||
control <- "Y"
|
||||
control <- password
|
||||
control <- "Y"
|
||||
control <- password
|
||||
ui.approveCh <- "Y"
|
||||
// We will be asked three times to provide a suitable password
|
||||
ui.inputCh <- password
|
||||
ui.inputCh <- password
|
||||
ui.inputCh <- password
|
||||
|
||||
addr, err := api.New(context.Background())
|
||||
if err == nil {
|
||||
@ -176,8 +156,8 @@ func failCreateAccountWithPassword(control chan string, api *SignerAPI, password
|
||||
}
|
||||
}
|
||||
|
||||
func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
|
||||
control <- "N"
|
||||
func failCreateAccount(ui *headlessUi, api *SignerAPI, t *testing.T) {
|
||||
ui.approveCh <- "N"
|
||||
addr, err := api.New(context.Background())
|
||||
if err != ErrRequestDenied {
|
||||
t.Fatal(err)
|
||||
@ -187,19 +167,20 @@ func failCreateAccount(control chan string, api *SignerAPI, t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func list(control chan string, api *SignerAPI, t *testing.T) []common.Address {
|
||||
control <- "A"
|
||||
list, err := api.List(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return list
|
||||
func list(ui *headlessUi, api *SignerAPI, t *testing.T) ([]common.Address, error) {
|
||||
ui.approveCh <- "A"
|
||||
return api.List(context.Background())
|
||||
|
||||
}
|
||||
|
||||
func TestNewAcc(t *testing.T) {
|
||||
api, control := setup(t)
|
||||
verifyNum := func(num int) {
|
||||
if list := list(control, api, t); len(list) != num {
|
||||
list, err := list(control, api, t)
|
||||
if err != nil {
|
||||
t.Errorf("Unexpected error %v", err)
|
||||
}
|
||||
if len(list) != num {
|
||||
t.Errorf("Expected %d accounts, got %d", num, len(list))
|
||||
}
|
||||
}
|
||||
@ -212,18 +193,16 @@ func TestNewAcc(t *testing.T) {
|
||||
failCreateAccount(control, api, t)
|
||||
createAccount(control, api, t)
|
||||
failCreateAccount(control, api, t)
|
||||
|
||||
verifyNum(4)
|
||||
|
||||
// Fail to create this, due to bad password
|
||||
failCreateAccountWithPassword(control, api, "short", t)
|
||||
failCreateAccountWithPassword(control, api, "longerbutbad\rfoo", t)
|
||||
|
||||
verifyNum(4)
|
||||
|
||||
// Testing listing:
|
||||
// Listing one Account
|
||||
control <- "1"
|
||||
control.approveCh <- "1"
|
||||
list, err := api.List(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
@ -232,7 +211,7 @@ func TestNewAcc(t *testing.T) {
|
||||
t.Fatalf("List should only show one Account")
|
||||
}
|
||||
// Listing denied
|
||||
control <- "Nope"
|
||||
control.approveCh <- "Nope"
|
||||
list, err = api.List(context.Background())
|
||||
if len(list) != 0 {
|
||||
t.Fatalf("List should be empty")
|
||||
@ -269,7 +248,7 @@ func TestSignTx(t *testing.T) {
|
||||
|
||||
api, control := setup(t)
|
||||
createAccount(control, api, t)
|
||||
control <- "A"
|
||||
control.approveCh <- "A"
|
||||
list, err = api.List(context.Background())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
@ -279,8 +258,8 @@ func TestSignTx(t *testing.T) {
|
||||
methodSig := "test(uint)"
|
||||
tx := mkTestTx(a)
|
||||
|
||||
control <- "Y"
|
||||
control <- "wrongpassword"
|
||||
control.approveCh <- "Y"
|
||||
control.inputCh <- "wrongpassword"
|
||||
res, err = api.SignTransaction(context.Background(), tx, &methodSig)
|
||||
if res != nil {
|
||||
t.Errorf("Expected nil-response, got %v", res)
|
||||
@ -288,7 +267,7 @@ func TestSignTx(t *testing.T) {
|
||||
if err != keystore.ErrDecrypt {
|
||||
t.Errorf("Expected ErrLocked! %v", err)
|
||||
}
|
||||
control <- "No way"
|
||||
control.approveCh <- "No way"
|
||||
res, err = api.SignTransaction(context.Background(), tx, &methodSig)
|
||||
if res != nil {
|
||||
t.Errorf("Expected nil-response, got %v", res)
|
||||
@ -296,8 +275,9 @@ func TestSignTx(t *testing.T) {
|
||||
if err != ErrRequestDenied {
|
||||
t.Errorf("Expected ErrRequestDenied! %v", err)
|
||||
}
|
||||
control <- "Y"
|
||||
control <- "a_long_password"
|
||||
// Sign with correct password
|
||||
control.approveCh <- "Y"
|
||||
control.inputCh <- "a_long_password"
|
||||
res, err = api.SignTransaction(context.Background(), tx, &methodSig)
|
||||
|
||||
if err != nil {
|
||||
@ -310,8 +290,8 @@ func TestSignTx(t *testing.T) {
|
||||
if parsedTx.Value().Cmp(tx.Value.ToInt()) != 0 {
|
||||
t.Errorf("Expected value to be unchanged, expected %v got %v", tx.Value, parsedTx.Value())
|
||||
}
|
||||
control <- "Y"
|
||||
control <- "a_long_password"
|
||||
control.approveCh <- "Y"
|
||||
control.inputCh <- "a_long_password"
|
||||
|
||||
res2, err = api.SignTransaction(context.Background(), tx, &methodSig)
|
||||
if err != nil {
|
||||
@ -322,8 +302,8 @@ func TestSignTx(t *testing.T) {
|
||||
}
|
||||
|
||||
//The tx is modified by the UI
|
||||
control <- "M"
|
||||
control <- "a_long_password"
|
||||
control.approveCh <- "M"
|
||||
control.inputCh <- "a_long_password"
|
||||
|
||||
res2, err = api.SignTransaction(context.Background(), tx, &methodSig)
|
||||
if err != nil {
|
||||
@ -341,31 +321,3 @@ func TestSignTx(t *testing.T) {
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/*
|
||||
func TestAsyncronousResponses(t *testing.T){
|
||||
|
||||
//Set up one account
|
||||
api, control := setup(t)
|
||||
createAccount(control, api, t)
|
||||
|
||||
// Two transactions, the second one with larger value than the first
|
||||
tx1 := mkTestTx()
|
||||
newVal := big.NewInt(0).Add((*big.Int) (tx1.Value), big.NewInt(1))
|
||||
tx2 := mkTestTx()
|
||||
tx2.Value = (*hexutil.Big)(newVal)
|
||||
|
||||
control <- "W" //wait
|
||||
control <- "Y" //
|
||||
control <- "a_long_password"
|
||||
control <- "Y" //
|
||||
control <- "a_long_password"
|
||||
|
||||
var err error
|
||||
|
||||
h1, err := api.SignTransaction(context.Background(), common.HexToAddress("1111"), tx1, nil)
|
||||
h2, err := api.SignTransaction(context.Background(), common.HexToAddress("2222"), tx2, nil)
|
||||
|
||||
|
||||
}
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user