diff --git a/cmd/signer/README.md b/cmd/signer/README.md index f1d68dc7a7..104d163425 100644 --- a/cmd/signer/README.md +++ b/cmd/signer/README.md @@ -386,6 +386,8 @@ A UI should conform to the following rules. * A UI MUST NOT load any external resources that were not embedded/part of the UI package. * For example, not load icons, stylesheets from the internet * Not load files from the filesystem, unless they reside in the same local directory (e.g. config files) +* A Graphical UI MUST show the blocky-identicon for ethereum addresses. +* A UI MUST warn display approproate warning if the destination-account is formatted with invalid checksum. * A UI MUST NOT open any ports or services * The signer opens the public port * A UI SHOULD verify the permissions on the signer binary, and refuse to execute or warn if permissions allow non-user write. @@ -400,15 +402,8 @@ along with the UI. Some snags and todos -* Currently, the API does not make it possible for the signer to forward data about the -checksum, since the addresses are common.Address, and not String. This should be changed upstream, -so that they are some more complex form with both common.Address and the original string (?) - - * The audit-log perhaps leave some things to be desired. I have not found a perfect way to save an audit log of events. -* Some more fields should be added to calldata, e.g http-header `Origin`. - * The signer should take a startup param "--no-change", for UI:s that do not contain the capability to perform changes to things, only approve/deny. Such a UI should be able to start the signer in a more secure mode by telling it that it only wants approve/deny capabilities. @@ -434,6 +429,11 @@ put together is a bit of a hack into the http server. This could probably be gre * Geth relay - Geth should be started in `geth --external_signer localhost:8550`. +* Geth checksum + - Currently, the Geth API:s use `common.Address` in the arguments to transaction submission (e.g `to` field). This + type is 20 `bytes`, and is incapable of carrying checksum information. The signer uses `common.MixedcaseAddress`, which + retains the original input. + - The Geth api should switch to use the same type, and relay `to`-account verbatim to the external api. * Wallets / accounts. Add API methods for wallets. @@ -441,4 +441,4 @@ put together is a bit of a hack into the http server. This could probably be gre * There needs to be a very good structure around rules. Either a full language (lua/js) or a limited but flexible syntax based on e.g. json/yaml. Kind of like a firewall ruleset. * This implies that the signer would remember passwords, which is very problematic. However, a good UI implementation will want these things, and it would be better to implement it once in the signer, than having UI:s develop their own remember-password logic. - + diff --git a/cmd/signer/abihelper.go b/cmd/signer/abihelper.go index 720a482147..eca12eba2c 100644 --- a/cmd/signer/abihelper.go +++ b/cmd/signer/abihelper.go @@ -20,11 +20,12 @@ import ( "bytes" "encoding/json" "fmt" - "github.com/ethereum/go-ethereum/accounts/abi" - "github.com/ethereum/go-ethereum/common" "io/ioutil" "strings" + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/common" + "regexp" ) diff --git a/cmd/signer/abihelper_test.go b/cmd/signer/abihelper_test.go index cc6158f7df..04fe553663 100644 --- a/cmd/signer/abihelper_test.go +++ b/cmd/signer/abihelper_test.go @@ -18,10 +18,11 @@ package main import ( "fmt" - "github.com/ethereum/go-ethereum/accounts/abi" - "github.com/ethereum/go-ethereum/common" "strings" "testing" + + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/common" ) func TestCalldataDecoding(t *testing.T) { diff --git a/cmd/signer/api.go b/cmd/signer/api.go index 9882032889..5f653d80e4 100644 --- a/cmd/signer/api.go +++ b/cmd/signer/api.go @@ -25,6 +25,7 @@ import ( "math/big" "bytes" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/accounts/usbwallet" @@ -54,18 +55,18 @@ type Metadata struct { type ( // SignTxRequest contains info about a Transaction to sign SignTxRequest struct { - Transaction TransactionArg `json:"transaction"` - From common.Address `json:"fromaccount"` - Callinfo string `json:"call_info"` - Meta Metadata `json:"meta"` + Transaction TransactionArg `json:"transaction"` + From common.MixedcaseAddress `json:"fromaccount"` + Callinfo string `json:"call_info"` + Meta Metadata `json:"meta"` } // SignTxResponse result from SignTxRequest SignTxResponse struct { //The UI may make changes to the TX - Transaction TransactionArg `json:"transaction"` - From common.Address `json:"fromaccount"` - Approved bool `json:"approved"` - Password string `json:"password"` + Transaction TransactionArg `json:"transaction"` + From common.MixedcaseAddress `json:"fromaccount"` + Approved bool `json:"approved"` + Password string `json:"password"` } // ExportRequest info about query to export accounts ExportRequest struct { @@ -86,11 +87,11 @@ type ( NewPassword string `json:"new_password"` } SignDataRequest struct { - Address common.Address `json:"address"` - Rawdata hexutil.Bytes `json:"raw_data"` - Message string `json:"message"` - Hash hexutil.Bytes `json:"hash"` - Meta Metadata `json:"meta"` + Address common.MixedcaseAddress `json:"address"` + Rawdata hexutil.Bytes `json:"raw_data"` + Message string `json:"message"` + Hash hexutil.Bytes `json:"hash"` + Meta Metadata `json:"meta"` } SignDataResponse struct { Approved bool `json:"approved"` @@ -111,7 +112,7 @@ type ( Accounts []Account `json:"accounts"` } Message struct { - Message string `json:"message"` + Text string `json:"text"` } ) @@ -246,7 +247,7 @@ func toTransaction(args *TransactionArg) *types.Transaction { if args.To == nil { return types.NewContractCreation(uint64(*args.Nonce), (*big.Int)(args.Value), (*big.Int)(args.Gas), (*big.Int)(args.GasPrice), args.Data) } else { - return types.NewTransaction(uint64(*args.Nonce), *args.To, (*big.Int)(args.Value), (*big.Int)(args.Gas), (*big.Int)(args.GasPrice), args.Data) + return types.NewTransaction(uint64(*args.Nonce), args.To.Address(), (*big.Int)(args.Value), (*big.Int)(args.Gas), (*big.Int)(args.GasPrice), args.Data) } } @@ -260,10 +261,8 @@ func logDiff(original *SignTxRequest, new *SignTxResponse) bool { log.Info("Sender-account changed by UI", "was", f0, "is", f1) } if t0, t1 := original.Transaction.To, new.Transaction.To; t0 != t1 { - if t0 == nil || t1 == nil || !bytes.Equal(t0.Bytes(), t1.Bytes()) { - log.Info("Recipient-account changed by UI", "was", t0, "is", t1) - modified = true - } + log.Info("Recipient-account changed by UI", "was", t0, "is", t1) + modified = true } if g0, g1 := (*big.Int)(original.Transaction.Gas), (*big.Int)(new.Transaction.Gas); g0 != g1 { if g0 == nil || g1 == nil || g0.Cmp(g1) != 0 { @@ -299,7 +298,7 @@ func logDiff(original *SignTxRequest, new *SignTxResponse) bool { // SignTransaction signs the given Transaction and returns it in an RLP encoded form // that can be posted to `eth_sendRawTransaction`. -func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address, args TransactionArg, methodSelector *string) (hexutil.Bytes, error) { +func (api *SignerAPI) SignTransaction(ctx context.Context, from common.MixedcaseAddress, args TransactionArg, methodSelector *string) (hexutil.Bytes, error) { var ( err error @@ -350,7 +349,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address, acc accounts.Account wallet accounts.Wallet ) - acc = accounts.Account{Address: result.From} + acc = accounts.Account{Address: result.From.Address()} wallet, err = api.am.Find(acc) if err != nil { return nil, err @@ -376,7 +375,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, from common.Address, // The key used to calculate the signature is decrypted with the given password. // // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_sign -func (api *SignerAPI) Sign(ctx context.Context, addr common.Address, data hexutil.Bytes) (hexutil.Bytes, error) { +func (api *SignerAPI) Sign(ctx context.Context, addr common.MixedcaseAddress, data hexutil.Bytes) (hexutil.Bytes, error) { sighash, msg := signHash(data) @@ -393,7 +392,7 @@ func (api *SignerAPI) Sign(ctx context.Context, addr common.Address, data hexuti } // Look up the wallet containing the requested signer - account := accounts.Account{Address: addr} + account := accounts.Account{Address: addr.Address()} wallet, err := api.am.Find(account) if err != nil { return nil, err diff --git a/cmd/signer/api_test.go b/cmd/signer/api_test.go index d448ebce38..9a6fc30897 100644 --- a/cmd/signer/api_test.go +++ b/cmd/signer/api_test.go @@ -4,18 +4,19 @@ import ( "bytes" "context" "fmt" - "github.com/ethereum/go-ethereum/accounts/keystore" - "github.com/ethereum/go-ethereum/cmd/utils" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/rlp" "io/ioutil" "math/big" "os" "path/filepath" "testing" "time" + + "github.com/ethereum/go-ethereum/accounts/keystore" + "github.com/ethereum/go-ethereum/cmd/utils" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" ) //Used for testing @@ -198,7 +199,7 @@ func TestSignData(t *testing.T) { if err != nil { t.Fatal(err) } - a := list[0].Address + a := common.NewMixedcaseAddress(list[0].Address) control <- "Y" control <- "wrongpassword" @@ -231,7 +232,7 @@ func TestSignData(t *testing.T) { } } func mkTestTx() TransactionArg { - to := common.HexToAddress("0x1337") + to := common.NewMixedcaseAddress(common.HexToAddress("0x1337")) gas := (*hexutil.Big)(big.NewInt(21000)) gasPrice := (*hexutil.Big)(big.NewInt(2000000000)) value := (*hexutil.Big)(big.NewInt(1e18)) @@ -262,7 +263,7 @@ func TestSignTx(t *testing.T) { if err != nil { t.Fatal(err) } - a := list[0].Address + a := common.NewMixedcaseAddress(list[0].Address) methodSig := "test(uint)" tx := mkTestTx() diff --git a/cmd/signer/auditlog.go b/cmd/signer/auditlog.go index 42122cfaa3..b1e9419f2e 100644 --- a/cmd/signer/auditlog.go +++ b/cmd/signer/auditlog.go @@ -3,9 +3,10 @@ package main import ( "bufio" "fmt" - "github.com/ethereum/go-ethereum/rpc" "io" "time" + + "github.com/ethereum/go-ethereum/rpc" ) type AuditLogger struct { diff --git a/cmd/signer/cliui.go b/cmd/signer/cliui.go index 28b9dd083c..b729b13de3 100644 --- a/cmd/signer/cliui.go +++ b/cmd/signer/cliui.go @@ -21,10 +21,11 @@ import ( "os" "strings" + "sync" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" "golang.org/x/crypto/ssh/terminal" - "sync" ) type CommandlineUI struct { @@ -98,13 +99,16 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro ui.mu.Lock() defer ui.mu.Unlock() weival := request.Transaction.Value.ToInt() - toval := "" - if request.Transaction.To != nil { - toval = request.Transaction.To.Hex() - } fmt.Printf("--------- Transaction request-------------\n") - fmt.Printf("to: %v\n", toval) - fmt.Printf("from: %v\n", request.From.Hex()) + if to := request.Transaction.To; to != nil { + fmt.Printf("to: %v\n", to.Original()) + if !to.ValidChecksum() { + fmt.Printf("\nWARNING: Invalid checksum on to-address!\n\n") + } + } else { + fmt.Printf("to: \n") + } + fmt.Printf("from: %v\n", request.From.String()) fmt.Printf("value: %v wei\n", weival) if len(request.Transaction.Data) > 0 { fmt.Printf("data: %v\n", common.Bytes2Hex(request.Transaction.Data)) diff --git a/cmd/signer/main.go b/cmd/signer/main.go index 7b0d11cbef..d0b5c955b0 100644 --- a/cmd/signer/main.go +++ b/cmd/signer/main.go @@ -26,6 +26,7 @@ import ( "context" "encoding/json" + "github.com/ethereum/go-ethereum/cmd/utils" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" @@ -33,7 +34,6 @@ import ( "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" "gopkg.in/urfave/cli.v1" - "io" ) func main() { @@ -96,19 +96,20 @@ func main() { app.Action = func(c *cli.Context) error { var ( - ui SignerUI - logOutput io.Writer + ui SignerUI ) - + // Set up the logger to print everything + logOutput := os.Stdout if c.Bool("stdio-ui") { logOutput = os.Stderr + } + log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(c.Int("loglevel")), log.StreamHandler(logOutput, log.TerminalFormat(true)))) + + if c.Bool("stdio-ui") { ui = NewStdIOUI() } else { ui = NewCommandlineUI() - logOutput = os.Stdout } - // Set up the logger to print everything - log.Root().SetHandler(log.LvlFilterHandler(log.Lvl(c.Int("loglevel")), log.StreamHandler(logOutput, log.TerminalFormat(true)))) if c.Bool("stdio-ui") { log.Info("Using stdin/stdout as UI-channel") } @@ -190,9 +191,9 @@ func testExternalUI(api *SignerAPI) { } var err error - _, err = api.SignTransaction(ctx, common.Address{}, TransactionArg{}, nil) + _, err = api.SignTransaction(ctx, common.MixedcaseAddress{}, TransactionArg{}, nil) checkErr("SignTransaction", err) - _, err = api.Sign(ctx, common.Address{}, common.Hex2Bytes("01020304")) + _, err = api.Sign(ctx, common.MixedcaseAddress{}, common.Hex2Bytes("01020304")) checkErr("Sign", err) _, err = api.List(ctx) checkErr("List", err) diff --git a/cmd/signer/pythonsigner.py b/cmd/signer/pythonsigner.py index 309b801460..636fbd7acf 100644 --- a/cmd/signer/pythonsigner.py +++ b/cmd/signer/pythonsigner.py @@ -1,7 +1,7 @@ import os,sys, subprocess from tinyrpc.transports import ServerTransport from tinyrpc.protocols.jsonrpc import JSONRPCProtocol -from tinyrpc.dispatch import RPCDispatcher +from tinyrpc.dispatch import public,RPCDispatcher from tinyrpc.server import RPCServer """ This is a POC example of how to write a custom UI for the signer. The UI starts the @@ -34,108 +34,112 @@ class PipeTransport(ServerTransport): def receive_message(self): data = self.input.readline() - #print(">> {}".format( data)) + print(">> {}".format( data)) return None, urlparse.unquote(data) def send_reply(self, context, reply): - #print("<< {}".format( reply)) + print("<< {}".format( reply)) self.output.write(reply) self.output.write("\n") -dispatcher = RPCDispatcher() +class StdIOHandler(): -@dispatcher.public -def ApproveTx(transaction = None, fromaccount = None, call_info = None, meta = None): - """ - Example request: - - {"jsonrpc":"2.0","method":"ApproveTx","params":{"transaction":{"to":null,"gas":null,"gasPrice":null,"value":null,"data":"0x","nonce":null},"fromaccount":"0x0000000000000000000000000000000000000000","call_info":null,"meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":2} + def __init__(self): + pass - :param transaction: transaction info - :param call_info: info abou the call, e.g. if ABI info could not be - :param meta: metadata about the request, e.g. where the call comes from - :return: - """ - return { - "approved" : False, - "transaction" : None, -# "fromaccount" : fromaccount, - "password" : None, - } + @public + def ApproveTx(self,transaction = None, fromaccount = None, call_info = None, meta = None): + """ + Example request: + + {"jsonrpc":"2.0","method":"ApproveTx","params":{"transaction":{"to":null,"gas":null,"gasPrice":null,"value":null,"data":"0x","nonce":null},"from":"0x0000000000000000000000000000000000000000","call_info":null,"meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":2} -@dispatcher.public -def ApproveSignData(address=None, raw_data = None, message = None, hash = None, meta = None): - """ Example request + :param transaction: transaction info + :param call_info: info abou the call, e.g. if ABI info could not be + :param meta: metadata about the request, e.g. where the call comes from + :return: + """ + return { + "approved" : False, + "transaction" : None, + #"fromaccount" : fromaccount, + "password" : None, + } - {"jsonrpc":"2.0","method":"ApproveSignData","params":{"address":"0x0000000000000000000000000000000000000000","raw_data":"0x01020304","message":"\u0019Ethereum Signed Message:\n4\u0001\u0002\u0003\u0004","hash":"0x7e3a4e7a9d1744bc5c675c25e1234ca8ed9162bd17f78b9085e48047c15ac310","meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":3} + @public + def ApproveSignData(self,address=None, raw_data = None, message = None, hash = None, meta = None): + """ Example request + + {"jsonrpc":"2.0","method":"ApproveSignData","params":{"address":"0x0000000000000000000000000000000000000000","raw_data":"0x01020304","message":"\u0019Ethereum Signed Message:\n4\u0001\u0002\u0003\u0004","hash":"0x7e3a4e7a9d1744bc5c675c25e1234ca8ed9162bd17f78b9085e48047c15ac310","meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":3} - """ - return {"approved": False, - "password" : None} + """ + return {"approved": False, + "password" : None} -@dispatcher.public -def ApproveExport(address = None, meta = None): - """ Example request + @public + def ApproveExport(self,address = None, meta = None): + """ Example request - {"jsonrpc":"2.0","method":"ApproveExport","params":{"address":"0x0000000000000000000000000000000000000000","meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":5} + {"jsonrpc":"2.0","method":"ApproveExport","params":{"address":"0x0000000000000000000000000000000000000000","meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":5} - """ - return {"approved" : False} + """ + return {"approved" : False} -@dispatcher.public -def ApproveImport(meta = None): - """ Example request + @public + def ApproveImport(self,meta = None): + """ Example request - {"jsonrpc":"2.0","method":"ApproveImport","params":{"Meta":{}},"id":4} + {"jsonrpc":"2.0","method":"ApproveImport","params":{"Meta":{}},"id":4} - """ - return {"approved" : False, "old_password": "", "new_password": ""} + """ + return {"approved" : False, "old_password": "", "new_password": ""} -@dispatcher.public -def ApproveListing(accounts=None, meta = None): - """ Example request + @public + def ApproveListing(self,accounts=None, meta = None): + """ Example request - {"jsonrpc":"2.0","method":"ApproveListing","params":{"accounts":[{"type":"Account","url":"keystore:///home/user/ethereum/keystore/file","address":"0x010101010101010010101010101abcdef0001337"}],"Meta":{}},"id":2} - """ - return {'accounts': []} + {"jsonrpc":"2.0","method":"ApproveListing","params":{"accounts":[{"type":"Account","url":"keystore:///home/user/ethereum/keystore/file","address":"0x010101010101010010101010101abcdef0001337"}],"Meta":{}},"id":2} + """ + return {'accounts': []} -@dispatcher.public -def ApproveNewAccount(meta = None): - """ - Example request + @public + def ApproveNewAccount(self,meta = None): + """ + Example request - {"jsonrpc":"2.0","method":"ApproveNewAccount","params":{"meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":5} + {"jsonrpc":"2.0","method":"ApproveNewAccount","params":{"meta":{"remote":"signer binary","local":"main","scheme":"in-proc"}},"id":5} - :return: - """ - return {"approved": False, "password": ""} + :return: + """ + return {"approved": False, "password": ""} -@dispatcher.public -def ShowError(message = ""): - """ - Example request: + @public + def ShowError(self,message = {}): + """ + Example request: - {"jsonrpc":"2.0","method":"ShowInfo","params":{"message":"Testing 'ShowError'"},"id":1} + {"jsonrpc":"2.0","method":"ShowInfo","params":{"message":"Testing 'ShowError'"},"id":1} - :param text: to show - :return: nothing - """ - sys.stderr.write("Error: {}\n".format( message)) - return + :param message: to show + :return: nothing + """ + if 'text' in message.keys(): + sys.stderr.write("Error: {}\n".format( message['text'])) + return -@dispatcher.public -def ShowInfo(message = ""): - """ - Example request - {"jsonrpc":"2.0","method":"ShowInfo","params":{"message":"Testing 'ShowInfo'"},"id":0} - - :param text: to display - :return:nothing - """ - sys.stdout.write("Info: {}\n".format( message)) - return + @public + def ShowInfo(self,message = {}): + """ + Example request + {"jsonrpc":"2.0","method":"ShowInfo","params":{"message":"Testing 'ShowInfo'"},"id":0} + :param message: to display + :return:nothing + """ + if 'text' in message.keys(): + sys.stdout.write("Error: {}\n".format( message['text'])) + return def main(args): @@ -143,11 +147,13 @@ def main(args): if len(args) > 0 and args[0] == "test": cmd.extend(["--stdio-ui-test"]) print("cmd: {}".format(" ".join(cmd))) + dispatcher = RPCDispatcher() + dispatcher.register_instance(StdIOHandler(), '') # line buffered p = subprocess.Popen(cmd, bufsize=1, universal_newlines=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE) - transport = PipeTransport(p.stdout, p.stdin) + rpc_server = RPCServer( - transport, + PipeTransport(p.stdout, p.stdin), JSONRPCProtocol(), dispatcher ) diff --git a/cmd/signer/stdioui.go b/cmd/signer/stdioui.go index a14202d3ad..9d677c0b9f 100644 --- a/cmd/signer/stdioui.go +++ b/cmd/signer/stdioui.go @@ -20,11 +20,10 @@ package main import ( "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/rpc" - "io" - "os" "sync" + "github.com/ethereum/go-ethereum/rpc" + "context" ) @@ -36,47 +35,33 @@ type StdIOUI struct { } 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) } return &StdIOUI{client: *client} - //return &StdIOUI{client: jsonrpc2.NewClient(&rwc{in, out})} - } -func (ui StdIOUI) dispatch(serviceMethod string, args interface{}, reply interface{}) error { - - // ui.mu.Lock() - // defer ui.mu.Unlock() - //This is not synchronized, which should not be necssary. Ideally, the UI should be able - // to get requests and send responses out-of-order -- thus the rpc has an ID. - - // in, out := bufio.NewReader(os.Stdin), os.Stdout - // codec := jsonrpc.NewClientCodec(&rwc{in, out}) - - // c := rpc.NewClientWithCodec(codec) - // return c.Call(serviceMethod, args, &reply) - log.Info("Writing to client") +// dispatch sends a request over the stdio +func (ui *StdIOUI) dispatch(serviceMethod string, args interface{}, reply interface{}) error { err := ui.client.Call(&reply, serviceMethod, args) - log.Info("Writing to client done") - // err := ui.client.Call(serviceMethod, args, &reply) if err != nil { log.Info("Error", "exc", err.Error()) } return err } -func (ui StdIOUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error) { - result := SignTxResponse{} +func (ui *StdIOUI) ApproveTx(request *SignTxRequest) (SignTxResponse, error) { + var result SignTxResponse if err := ui.dispatch("ApproveTx", request, &result); err != nil { return result, err } return result, nil } -func (ui StdIOUI) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) { +func (ui *StdIOUI) ApproveSignData(request *SignDataRequest) (SignDataResponse, error) { var result SignDataResponse if err := ui.dispatch("ApproveSignData", request, &result); err != nil { return result, err @@ -84,7 +69,7 @@ func (ui StdIOUI) ApproveSignData(request *SignDataRequest) (SignDataResponse, e return result, nil } -func (ui StdIOUI) ApproveExport(request *ExportRequest) (ExportResponse, error) { +func (ui *StdIOUI) ApproveExport(request *ExportRequest) (ExportResponse, error) { var result ExportResponse if err := ui.dispatch("ApproveExport", request, &result); err != nil { return result, err @@ -92,7 +77,7 @@ func (ui StdIOUI) ApproveExport(request *ExportRequest) (ExportResponse, error) return result, nil } -func (ui StdIOUI) ApproveImport(request *ImportRequest) (ImportResponse, error) { +func (ui *StdIOUI) ApproveImport(request *ImportRequest) (ImportResponse, error) { var result ImportResponse if err := ui.dispatch("ApproveImport", request, &result); err != nil { return result, err @@ -100,7 +85,7 @@ func (ui StdIOUI) ApproveImport(request *ImportRequest) (ImportResponse, error) return result, nil } -func (ui StdIOUI) ApproveListing(request *ListRequest) (ListResponse, error) { +func (ui *StdIOUI) ApproveListing(request *ListRequest) (ListResponse, error) { var result ListResponse if err := ui.dispatch("ApproveListing", request, &result); err != nil { return result, err @@ -108,7 +93,7 @@ func (ui StdIOUI) ApproveListing(request *ListRequest) (ListResponse, error) { return result, nil } -func (ui StdIOUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) { +func (ui *StdIOUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountResponse, error) { var result NewAccountResponse if err := ui.dispatch("ApproveNewAccount", request, &result); err != nil { return result, err @@ -116,29 +101,16 @@ func (ui StdIOUI) ApproveNewAccount(request *NewAccountRequest) (NewAccountRespo return result, nil } -func (ui StdIOUI) ShowError(message string) { +func (ui *StdIOUI) ShowError(message string) { err := ui.dispatch("ShowError", &Message{message}, nil) if err != nil { log.Info("Error calling 'ShowError'", "exc", err.Error(), "msg", message) } } -func (ui StdIOUI) ShowInfo(message string) { +func (ui *StdIOUI) ShowInfo(message string) { err := ui.dispatch("ShowInfo", Message{message}, nil) if err != nil { log.Info("Error calling 'ShowInfo'", "exc", err.Error(), "msg", message) } } - -type rwc struct { - io.Reader - io.Writer -} - -func (r *rwc) Close() error { - if err := os.Stdin.Close(); err != nil { - return err - } - return os.Stdout.Close() - //return nil -} diff --git a/cmd/signer/types.go b/cmd/signer/types.go index dce8c7179b..1195142495 100644 --- a/cmd/signer/types.go +++ b/cmd/signer/types.go @@ -18,10 +18,11 @@ package main import ( "encoding/json" + "strings" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "strings" ) type Accounts []Account @@ -50,12 +51,12 @@ func (a Account) String() string { // TransactionArg represents a Transaction for the signer. type TransactionArg struct { - To *common.Address `json:"to"` - Gas *hexutil.Big `json:"gas"` - GasPrice *hexutil.Big `json:"gasPrice"` - Value *hexutil.Big `json:"value"` - Data hexutil.Bytes `json:"data"` - Nonce *hexutil.Uint64 `json:"nonce"` + To *common.MixedcaseAddress `json:"to"` + Gas *hexutil.Big `json:"gas"` + GasPrice *hexutil.Big `json:"gasPrice"` + Value *hexutil.Big `json:"value"` + Data hexutil.Bytes `json:"data"` + Nonce *hexutil.Uint64 `json:"nonce"` } func (t TransactionArg) String() string { diff --git a/common/types.go b/common/types.go index fdc67480c2..918f78bab5 100644 --- a/common/types.go +++ b/common/types.go @@ -23,6 +23,7 @@ import ( "math/rand" "reflect" + "encoding/json" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/crypto/sha3" ) @@ -240,3 +241,56 @@ func (a *UnprefixedAddress) UnmarshalText(input []byte) error { func (a UnprefixedAddress) MarshalText() ([]byte, error) { return []byte(hex.EncodeToString(a[:])), nil } + +// MixedcaseAddress retains the original string, which may or may not be +// correctly checksummed +// +// TODO! Should we really keep both addr and original, or _only_ original, and +// always calculate addr on the fly? That would reduce the possibilities for errors +// if some caller modifies the original at some point. NB: If we do so, we should still +// parse the addr in UnmarshalJSON to ensure that the format is correct, e.g. correct size and +// hex-encoded and such +type MixedcaseAddress struct { + addr Address + original string +} +// NewMixedcaseAddress constructor (mainly for testing) +func NewMixedcaseAddress(addr Address) MixedcaseAddress { + return MixedcaseAddress{addr: addr, original: addr.Hex()} +} + +// UnmarshalJSON parses MixedcaseAddress +func (ma *MixedcaseAddress) UnmarshalJSON(input []byte) error { + if err := hexutil.UnmarshalFixedJSON(addressT, input, ma.addr[:]); err != nil { + return err + } + return json.Unmarshal(input, &ma.original) +} + +// MarshalJSON marshals the original value +func (ma *MixedcaseAddress) MarshalJSON() ([]byte, error) { + return json.Marshal(ma.original) +} + +// Address returns the address +func (ma *MixedcaseAddress) Address() Address { + return ma.addr +} + +// String implements fmt.Stringer +func (ma *MixedcaseAddress) String() string { + if ma.ValidChecksum() { + return fmt.Sprintf("%s [chksum ok]", ma.original) + } + return fmt.Sprintf("%s [chksum INVALID]", ma.original) +} + +// ValidChecksum returns true if the address has valid checksum +func (ma *MixedcaseAddress) ValidChecksum() bool { + return ma.original == ma.addr.Hex() +} + +// Original returns the mixed-case input string +func (ma *MixedcaseAddress) Original() string { + return ma.original +} diff --git a/common/types_test.go b/common/types_test.go index db636812ce..9e0c5be3ad 100644 --- a/common/types_test.go +++ b/common/types_test.go @@ -18,6 +18,7 @@ package common import ( "encoding/json" + "math/big" "strings" "testing" @@ -149,3 +150,46 @@ func BenchmarkAddressHex(b *testing.B) { testAddr.Hex() } } + +func TestMixedcaseAccount_Address(t *testing.T) { + + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-55.md + // Note: 0X{checksum_addr} is not valid according to spec above + + var res []struct { + A MixedcaseAddress + Valid bool + } + if err := json.Unmarshal([]byte(`[ + {"A" : "0xae967917c465db8578ca9024c205720b1a3651A9", "Valid": false}, + {"A" : "0xAe967917c465db8578ca9024c205720b1a3651A9", "Valid": true}, + {"A" : "0XAe967917c465db8578ca9024c205720b1a3651A9", "Valid": false}, + {"A" : "0x1111111111111111111112222222222223333323", "Valid": true} + ]`), &res); err != nil { + t.Fatal(err) + } + + for _, r := range res { + if got := r.A.ValidChecksum(); got != r.Valid { + t.Errorf("Expected checksum %v, got checksum %v, input %v", r.Valid, got, r.A.String()) + } + } + + //These should throw exceptions: + var r2 []MixedcaseAddress + for _, r := range []string{ + `["0x11111111111111111111122222222222233333"]`, // Too short + `["0x111111111111111111111222222222222333332"]`, // Too short + `["0x11111111111111111111122222222222233333234"]`, // Too long + `["0x111111111111111111111222222222222333332344"]`, // Too long + `["1111111111111111111112222222222223333323"]`, // Missing 0x + `["x1111111111111111111112222222222223333323"]`, // Missing 0 + `["0xG111111111111111111112222222222223333323"]`, //Non-hex + } { + if err := json.Unmarshal([]byte(r), &r2); err == nil { + t.Errorf("Expected failure, input %v", r) + } + + } + +} diff --git a/rpc/client.go b/rpc/client.go index 77e803e2cd..68745c6cbe 100644 --- a/rpc/client.go +++ b/rpc/client.go @@ -215,7 +215,6 @@ func (io StdIOConn) SetWriteDeadline(t time.Time) error { return &net.OpError{Op: "set", Net: "stdio", Source: nil, Addr: nil, Err: errors.New("deadline not supported")} } func DialStdIO(ctx context.Context) (*Client, error) { - return newClient(ctx, func(_ context.Context) (net.Conn, error) { return StdIOConn{}, nil }) @@ -227,7 +226,6 @@ func newClient(initctx context.Context, connectFunc func(context.Context) (net.C return nil, err } _, isHTTP := conn.(*httpConn) - c := &Client{ writeConn: conn, isHTTP: isHTTP, @@ -567,13 +565,13 @@ func (c *Client) dispatch(conn net.Conn) { } case err := <-c.readErr: - log.Debug(fmt.Sprintf("<-readErr: %v", err)) + log.Debug("<-readErr", "err", err) c.closeRequestOps(err) conn.Close() reading = false case newconn := <-c.reconnected: - log.Debug(fmt.Sprintf("<-reconnected: (reading=%t) %v", reading, conn.RemoteAddr())) + log.Debug("<-reconnected", "reading", reading, "remote", conn.RemoteAddr()) if reading { // Wait for the previous read loop to exit. This is a rare case. conn.Close() @@ -630,7 +628,7 @@ func (c *Client) closeRequestOps(err error) { func (c *Client) handleNotification(msg *jsonrpcMessage) { if !strings.HasSuffix(msg.Method, notificationMethodSuffix) { - log.Debug(fmt.Sprint("dropping non-subscription message: ", msg)) + log.Debug("dropping non-subscription message", "msg", msg) return } var subResult struct { @@ -638,7 +636,7 @@ func (c *Client) handleNotification(msg *jsonrpcMessage) { Result json.RawMessage `json:"result"` } if err := json.Unmarshal(msg.Params, &subResult); err != nil { - log.Debug(fmt.Sprint("dropping invalid subscription message: ", msg)) + log.Debug("dropping invalid subscription message", "msg", msg) return } if c.subs[subResult.ID] != nil { @@ -649,7 +647,7 @@ func (c *Client) handleNotification(msg *jsonrpcMessage) { func (c *Client) handleResponse(msg *jsonrpcMessage) { op := c.respWait[string(msg.ID)] if op == nil { - log.Debug(fmt.Sprintf("unsolicited response %v", msg)) + log.Debug("unsolicited response", "msg", msg) return } delete(c.respWait, string(msg.ID))