From 16f3c31773c6daf57afe718719821a897fa1a2c7 Mon Sep 17 00:00:00 2001 From: Eli Date: Fri, 4 May 2018 01:04:17 -0700 Subject: [PATCH] signer: fix golint errors (#16653) * signer/*: golint fixes Specifically naming and comment formatting for documentation * signer/*: fixed naming error crashing build * signer/*: corrected error * signer/core: fix tiny error whitespace * signer/rules: fix test refactor --- signer/core/abihelper.go | 13 ++--- signer/core/api.go | 2 +- signer/core/cliui.go | 1 + signer/core/types.go | 4 +- signer/core/validation.go | 2 +- signer/rules/rules.go | 36 ++++++------ signer/rules/rules_test.go | 94 +++++++++++++++---------------- signer/storage/aes_gcm_storage.go | 5 +- 8 files changed, 78 insertions(+), 79 deletions(-) diff --git a/signer/core/abihelper.go b/signer/core/abihelper.go index 2674c7346a5b..1d4fbc7dc22d 100644 --- a/signer/core/abihelper.go +++ b/signer/core/abihelper.go @@ -94,13 +94,12 @@ func parseCallData(calldata []byte, abidata string) (*decodedCallData, error) { for n, argument := range method.Inputs { if err != nil { return nil, fmt.Errorf("Failed to decode argument %d (signature %v): %v", n, method.Sig(), err) - } else { - decodedArg := decodedArgument{ - soltype: argument, - value: v[n], - } - decoded.inputs = append(decoded.inputs, decodedArg) } + decodedArg := decodedArgument{ + soltype: argument, + value: v[n], + } + decoded.inputs = append(decoded.inputs, decodedArg) } // We're finished decoding the data. At this point, we encode the decoded data to see if it matches with the @@ -240,7 +239,7 @@ func (db *AbiDb) saveCustomAbi(selector, signature string) error { return err } -// Adds a signature to the database, if custom database saving is enabled. +// AddSignature to the database, if custom database saving is enabled. // OBS: This method does _not_ validate the correctness of the data, // it is assumed that the caller has already done so func (db *AbiDb) AddSignature(selector string, data []byte) error { diff --git a/signer/core/api.go b/signer/core/api.go index 1387041cc34b..45933284bf36 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -474,7 +474,7 @@ func (api *SignerAPI) Export(ctx context.Context, addr common.Address) (json.Raw return ioutil.ReadFile(wallet.URL().Path) } -// Imports tries to import the given keyJSON in the local keystore. The keyJSON data is expected to be +// Import tries to import the given keyJSON in the local keystore. The keyJSON data is expected to be // in web3 keystore format. It will decrypt the keyJSON with the given passphrase and on successful // decryption it will encrypt the key with the given newPassphrase and store it in the keystore. func (api *SignerAPI) Import(ctx context.Context, keyJSON json.RawMessage) (Account, error) { diff --git a/signer/core/cliui.go b/signer/core/cliui.go index 0d9b5f3d36d4..2f969669c2d4 100644 --- a/signer/core/cliui.go +++ b/signer/core/cliui.go @@ -13,6 +13,7 @@ // // You should have received a copy of the GNU General Public License // along with go-ethereum. If not, see . + package core import ( diff --git a/signer/core/types.go b/signer/core/types.go index 8386bd44e798..2acc0a4f4a11 100644 --- a/signer/core/types.go +++ b/signer/core/types.go @@ -73,8 +73,8 @@ type SendTxArgs struct { Input *hexutil.Bytes `json:"input"` } -func (t SendTxArgs) String() string { - s, err := json.Marshal(t) +func (args SendTxArgs) String() string { + s, err := json.Marshal(args) if err == nil { return string(s) } diff --git a/signer/core/validation.go b/signer/core/validation.go index 97bb3b685098..288456df878b 100644 --- a/signer/core/validation.go +++ b/signer/core/validation.go @@ -128,7 +128,7 @@ func (v *Validator) validate(msgs *ValidationMessages, txargs *SendTxArgs, metho if len(data) == 0 { if txargs.Value.ToInt().Cmp(big.NewInt(0)) > 0 { // Sending ether into black hole - return errors.New(`Tx will create contract with value but empty code!`) + return errors.New("Tx will create contract with value but empty code!") } // No value submitted at least msgs.crit("Tx will create contract with empty code!") diff --git a/signer/rules/rules.go b/signer/rules/rules.go index fa270436df3e..711e2dddec0b 100644 --- a/signer/rules/rules.go +++ b/signer/rules/rules.go @@ -46,17 +46,17 @@ func consoleOutput(call otto.FunctionCall) otto.Value { return otto.Value{} } -// rulesetUi provides an implementation of SignerUI that evaluates a javascript +// rulesetUI provides an implementation of SignerUI that evaluates a javascript // file for each defined UI-method -type rulesetUi struct { +type rulesetUI struct { next core.SignerUI // The next handler, for manual processing storage storage.Storage credentials storage.Storage jsRules string // The rules to use } -func NewRuleEvaluator(next core.SignerUI, jsbackend, credentialsBackend storage.Storage) (*rulesetUi, error) { - c := &rulesetUi{ +func NewRuleEvaluator(next core.SignerUI, jsbackend, credentialsBackend storage.Storage) (*rulesetUI, error) { + c := &rulesetUI{ next: next, storage: jsbackend, credentials: credentialsBackend, @@ -66,11 +66,11 @@ func NewRuleEvaluator(next core.SignerUI, jsbackend, credentialsBackend storage. return c, nil } -func (r *rulesetUi) Init(javascriptRules string) error { +func (r *rulesetUI) Init(javascriptRules string) error { r.jsRules = javascriptRules return nil } -func (r *rulesetUi) execute(jsfunc string, jsarg interface{}) (otto.Value, error) { +func (r *rulesetUI) execute(jsfunc string, jsarg interface{}) (otto.Value, error) { // Instantiate a fresh vm engine every time vm := otto.New() @@ -115,7 +115,7 @@ func (r *rulesetUi) execute(jsfunc string, jsarg interface{}) (otto.Value, error return vm.Run(call) } -func (r *rulesetUi) checkApproval(jsfunc string, jsarg []byte, err error) (bool, error) { +func (r *rulesetUI) checkApproval(jsfunc string, jsarg []byte, err error) (bool, error) { if err != nil { return false, err } @@ -139,7 +139,7 @@ func (r *rulesetUi) checkApproval(jsfunc string, jsarg []byte, err error) (bool, return false, fmt.Errorf("Unknown response") } -func (r *rulesetUi) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { +func (r *rulesetUI) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { jsonreq, err := json.Marshal(request) approved, err := r.checkApproval("ApproveTx", jsonreq, err) if err != nil { @@ -158,11 +158,11 @@ func (r *rulesetUi) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, return core.SignTxResponse{Approved: false}, err } -func (r *rulesetUi) lookupPassword(address common.Address) string { +func (r *rulesetUI) lookupPassword(address common.Address) string { return r.credentials.Get(strings.ToLower(address.String())) } -func (r *rulesetUi) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { +func (r *rulesetUI) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { jsonreq, err := json.Marshal(request) approved, err := r.checkApproval("ApproveSignData", jsonreq, err) if err != nil { @@ -175,7 +175,7 @@ func (r *rulesetUi) ApproveSignData(request *core.SignDataRequest) (core.SignDat return core.SignDataResponse{Approved: false, Password: ""}, err } -func (r *rulesetUi) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { +func (r *rulesetUI) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { jsonreq, err := json.Marshal(request) approved, err := r.checkApproval("ApproveExport", jsonreq, err) if err != nil { @@ -188,13 +188,13 @@ func (r *rulesetUi) ApproveExport(request *core.ExportRequest) (core.ExportRespo return core.ExportResponse{Approved: false}, err } -func (r *rulesetUi) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { +func (r *rulesetUI) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { // This cannot be handled by rules, requires setting a password // dispatch to next return r.next.ApproveImport(request) } -func (r *rulesetUi) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { +func (r *rulesetUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { jsonreq, err := json.Marshal(request) approved, err := r.checkApproval("ApproveListing", jsonreq, err) if err != nil { @@ -207,22 +207,22 @@ func (r *rulesetUi) ApproveListing(request *core.ListRequest) (core.ListResponse return core.ListResponse{}, err } -func (r *rulesetUi) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { +func (r *rulesetUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { // This cannot be handled by rules, requires setting a password // dispatch to next return r.next.ApproveNewAccount(request) } -func (r *rulesetUi) ShowError(message string) { +func (r *rulesetUI) ShowError(message string) { log.Error(message) r.next.ShowError(message) } -func (r *rulesetUi) ShowInfo(message string) { +func (r *rulesetUI) ShowInfo(message string) { log.Info(message) r.next.ShowInfo(message) } -func (r *rulesetUi) OnSignerStartup(info core.StartupInfo) { +func (r *rulesetUI) OnSignerStartup(info core.StartupInfo) { jsonInfo, err := json.Marshal(info) if err != nil { log.Warn("failed marshalling data", "data", info) @@ -235,7 +235,7 @@ func (r *rulesetUi) OnSignerStartup(info core.StartupInfo) { } } -func (r *rulesetUi) OnApprovedTx(tx ethapi.SignTransactionResult) { +func (r *rulesetUI) OnApprovedTx(tx ethapi.SignTransactionResult) { jsonTx, err := json.Marshal(tx) if err != nil { log.Warn("failed marshalling transaction", "tx", tx) diff --git a/signer/rules/rules_test.go b/signer/rules/rules_test.go index 6a0403500408..b6060eba7345 100644 --- a/signer/rules/rules_test.go +++ b/signer/rules/rules_test.go @@ -33,18 +33,18 @@ import ( const JS = ` /** -This is an example implementation of a Javascript rule file. +This is an example implementation of a Javascript rule file. -When the signer receives a request over the external API, the corresponding method is evaluated. -Three things can happen: +When the signer receives a request over the external API, the corresponding method is evaluated. +Three things can happen: -1. The method returns "Approve". This means the operation is permitted. -2. The method returns "Reject". This means the operation is rejected. +1. The method returns "Approve". This means the operation is permitted. +2. The method returns "Reject". This means the operation is rejected. 3. Anything else; other return values [*], method not implemented or exception occurred during processing. This means -that the operation will continue to manual processing, via the regular UI method chosen by the user. +that the operation will continue to manual processing, via the regular UI method chosen by the user. -[*] Note: Future version of the ruleset may use more complex json-based returnvalues, making it possible to not -only respond Approve/Reject/Manual, but also modify responses. For example, choose to list only one, but not all +[*] Note: Future version of the ruleset may use more complex json-based returnvalues, making it possible to not +only respond Approve/Reject/Manual, but also modify responses. For example, choose to list only one, but not all accounts in a list-request. The points above will continue to hold for non-json based responses ("Approve"/"Reject"). **/ @@ -72,49 +72,49 @@ func mixAddr(a string) (*common.MixedcaseAddress, error) { return common.NewMixedcaseAddressFromString(a) } -type alwaysDenyUi struct{} +type alwaysDenyUI struct{} -func (alwaysDenyUi) OnSignerStartup(info core.StartupInfo) { +func (alwaysDenyUI) OnSignerStartup(info core.StartupInfo) { } -func (alwaysDenyUi) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { +func (alwaysDenyUI) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { return core.SignTxResponse{Transaction: request.Transaction, Approved: false, Password: ""}, nil } -func (alwaysDenyUi) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { +func (alwaysDenyUI) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { return core.SignDataResponse{Approved: false, Password: ""}, nil } -func (alwaysDenyUi) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { +func (alwaysDenyUI) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { return core.ExportResponse{Approved: false}, nil } -func (alwaysDenyUi) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { +func (alwaysDenyUI) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { return core.ImportResponse{Approved: false, OldPassword: "", NewPassword: ""}, nil } -func (alwaysDenyUi) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { +func (alwaysDenyUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { return core.ListResponse{Accounts: nil}, nil } -func (alwaysDenyUi) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { +func (alwaysDenyUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { return core.NewAccountResponse{Approved: false, Password: ""}, nil } -func (alwaysDenyUi) ShowError(message string) { +func (alwaysDenyUI) ShowError(message string) { panic("implement me") } -func (alwaysDenyUi) ShowInfo(message string) { +func (alwaysDenyUI) ShowInfo(message string) { panic("implement me") } -func (alwaysDenyUi) OnApprovedTx(tx ethapi.SignTransactionResult) { +func (alwaysDenyUI) OnApprovedTx(tx ethapi.SignTransactionResult) { panic("implement me") } -func initRuleEngine(js string) (*rulesetUi, error) { - r, err := NewRuleEvaluator(&alwaysDenyUi{}, storage.NewEphemeralStorage(), storage.NewEphemeralStorage()) +func initRuleEngine(js string) (*rulesetUI, error) { + r, err := NewRuleEvaluator(&alwaysDenyUI{}, storage.NewEphemeralStorage(), storage.NewEphemeralStorage()) if err != nil { return nil, fmt.Errorf("failed to create js engine: %v", err) } @@ -196,59 +196,59 @@ func TestSignTxRequest(t *testing.T) { } } -type dummyUi struct { +type dummyUI struct { calls []string } -func (d *dummyUi) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { +func (d *dummyUI) ApproveTx(request *core.SignTxRequest) (core.SignTxResponse, error) { d.calls = append(d.calls, "ApproveTx") return core.SignTxResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { +func (d *dummyUI) ApproveSignData(request *core.SignDataRequest) (core.SignDataResponse, error) { d.calls = append(d.calls, "ApproveSignData") return core.SignDataResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { +func (d *dummyUI) ApproveExport(request *core.ExportRequest) (core.ExportResponse, error) { d.calls = append(d.calls, "ApproveExport") return core.ExportResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { +func (d *dummyUI) ApproveImport(request *core.ImportRequest) (core.ImportResponse, error) { d.calls = append(d.calls, "ApproveImport") return core.ImportResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { +func (d *dummyUI) ApproveListing(request *core.ListRequest) (core.ListResponse, error) { d.calls = append(d.calls, "ApproveListing") return core.ListResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { +func (d *dummyUI) ApproveNewAccount(request *core.NewAccountRequest) (core.NewAccountResponse, error) { d.calls = append(d.calls, "ApproveNewAccount") return core.NewAccountResponse{}, core.ErrRequestDenied } -func (d *dummyUi) ShowError(message string) { +func (d *dummyUI) ShowError(message string) { d.calls = append(d.calls, "ShowError") } -func (d *dummyUi) ShowInfo(message string) { +func (d *dummyUI) ShowInfo(message string) { d.calls = append(d.calls, "ShowInfo") } -func (d *dummyUi) OnApprovedTx(tx ethapi.SignTransactionResult) { +func (d *dummyUI) OnApprovedTx(tx ethapi.SignTransactionResult) { d.calls = append(d.calls, "OnApprovedTx") } -func (d *dummyUi) OnSignerStartup(info core.StartupInfo) { +func (d *dummyUI) OnSignerStartup(info core.StartupInfo) { } //TestForwarding tests that the rule-engine correctly dispatches requests to the next caller func TestForwarding(t *testing.T) { js := "" - ui := &dummyUi{make([]string, 0)} + ui := &dummyUI{make([]string, 0)} jsBackend := storage.NewEphemeralStorage() credBackend := storage.NewEphemeralStorage() r, err := NewRuleEvaluator(ui, jsBackend, credBackend) @@ -308,22 +308,22 @@ func TestStorage(t *testing.T) { function testStorage(){ storage.Put("mykey", "myvalue") a = storage.Get("mykey") - + storage.Put("mykey", ["a", "list"]) // Should result in "a,list" a += storage.Get("mykey") - + storage.Put("mykey", {"an": "object"}) // Should result in "[object Object]" a += storage.Get("mykey") - + storage.Put("mykey", JSON.stringify({"an": "object"})) // Should result in '{"an":"object"}' a += storage.Get("mykey") a += storage.Get("missingkey") //Missing keys should result in empty string storage.Put("","missing key==noop") // Can't store with 0-length key a += storage.Get("") // Should result in '' - + var b = new BigNumber(2) var c = new BigNumber(16)//"0xf0",16) var d = b.plus(c) @@ -361,7 +361,7 @@ const ExampleTxWindow = ` if(str.slice(0,2) == "0x"){ return new BigNumber(str.slice(2),16)} return new BigNumber(str) } - + // Time window: 1 week var window = 1000* 3600*24*7; @@ -370,7 +370,7 @@ const ExampleTxWindow = ` function isLimitOk(transaction){ var value = big(transaction.value) - // Start of our window function + // Start of our window function var windowstart = new Date().getTime() - window; var txs = []; @@ -382,17 +382,17 @@ const ExampleTxWindow = ` // First, remove all that have passed out of the time-window var newtxs = txs.filter(function(tx){return tx.tstamp > windowstart}); console.log(txs, newtxs.length); - + // Secondly, aggregate the current sum sum = new BigNumber(0) sum = newtxs.reduce(function(agg, tx){ return big(tx.value).plus(agg)}, sum); console.log("ApproveTx > Sum so far", sum); console.log("ApproveTx > Requested", value.toNumber()); - + // Would we exceed weekly limit ? return sum.plus(value).lt(limit) - + } function ApproveTx(r){ console.log(r) @@ -405,14 +405,14 @@ const ExampleTxWindow = ` /** * OnApprovedTx(str) is called when a transaction has been approved and signed. The parameter - * 'response_str' contains the return value that will be sent to the external caller. - * The return value from this method is ignore - the reason for having this callback is to allow the - * ruleset to keep track of approved transactions. + * 'response_str' contains the return value that will be sent to the external caller. + * The return value from this method is ignore - the reason for having this callback is to allow the + * ruleset to keep track of approved transactions. * - * When implementing rate-limited rules, this callback should be used. + * When implementing rate-limited rules, this callback should be used. * If a rule responds with neither 'Approve' nor 'Reject' - the tx goes to manual processing. If the user * then accepts the transaction, this method will be called. - * + * * TLDR; Use this method to keep track of signed transactions, instead of using the data in ApproveTx. */ function OnApprovedTx(resp){ diff --git a/signer/storage/aes_gcm_storage.go b/signer/storage/aes_gcm_storage.go index 1ac3475588a3..225276667fad 100644 --- a/signer/storage/aes_gcm_storage.go +++ b/signer/storage/aes_gcm_storage.go @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with go-ethereum. If not, see . // + package storage import ( @@ -106,10 +107,8 @@ func (s *AESEncryptedStorage) readEncryptedStorage() (map[string]storedCredentia if os.IsNotExist(err) { // Doesn't exist yet return creds, nil - - } else { - log.Warn("Failed to read encrypted storage", "err", err, "file", s.filename) } + log.Warn("Failed to read encrypted storage", "err", err, "file", s.filename) } if err = json.Unmarshal(raw, &creds); err != nil { log.Warn("Failed to unmarshal encrypted storage", "err", err, "file", s.filename)