diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index ad299e2b6..485bca84f 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -11,6 +11,7 @@ program](https://hackerone.com/tendermint). ### BREAKING CHANGES: - CLI/RPC/Config + - [rpc] \#3471 Paginate `/validators` response (default: 30 vals per page) - [rpc] \#3188 Remove `BlockMeta` in `ResultBlock` in favor of `BlockId` for `/block` - [rpc] `/block_results` response format updated (see RPC docs for details) @@ -28,54 +29,57 @@ program](https://hackerone.com/tendermint). } } ``` - - [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) Remove `#event` suffix from the ID in event responses. + - [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) Remove `#event` suffix from the ID in event responses. `{"jsonrpc": "2.0", "id": 0, "result": ...}` - - [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) Switch to integer IDs instead of `json-client-XYZ` + - [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) Switch to integer IDs instead of `json-client-XYZ` ``` id=0 method=/subscribe id=0 result=... id=1 method=/abci_query id=1 result=... ``` - * ID is unique for each request; - * Request.ID is now optional. Notification is a Request without an ID. Previously ID="" or ID=0 were considered as notifications. + - ID is unique for each request; + - Request.ID is now optional. Notification is a Request without an ID. Previously ID="" or ID=0 were considered as notifications. - Apps - Go API - - [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error. + + - [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error. - [rpc/client] \#3471 `Validators` now requires two more args: `page` and `perPage` - [libs/common] \#3262 Make error the last parameter of `Task` (@PSalant726) + - [libs/common] \#3862 Remove `errors.go` from `libs/common` - Blockchain Protocol + - [abci] \#2521 Remove `TotalTxs` and `NumTxs` from `Header` - [types] [\#4151](https://github.com/tendermint/tendermint/pull/4151) Enforce ordering of votes in DuplicateVoteEvidence to be lexicographically sorted on BlockID - P2P Protocol - - [p2p] [\3668](https://github.com/tendermint/tendermint/pull/3668) Make `SecretConnection` non-malleable + - [p2p][\3668](https://github.com/tendermint/tendermint/pull/3668) Make `SecretConnection` non-malleable ### FEATURES: ### IMPROVEMENTS: -- [mempool] [\#4083](https://github.com/tendermint/tendermint/pull/4083) Added TxInfo parameter to CheckTx(), and removed CheckTxWithInfo() (@erikgrinaker) -- [mempool] [\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker) -- [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker) -- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead. -- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values. -- [p2p] [\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr) -- [rpc] [\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface. +- [mempool][\#4083](https://github.com/tendermint/tendermint/pull/4083) Added TxInfo parameter to CheckTx(), and removed CheckTxWithInfo() (@erikgrinaker) +- [mempool][\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker) +- [tools][\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker) +- [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead. +- [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values. +- [p2p][\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr) +- [rpc][\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface. - [privval] Add `SignerDialerEndpointRetryWaitInterval` option (@cosmostuba) - [crypto] Add `RegisterKeyType` to amino to allow external key types registration (@austinabell) - [rpc] \#3188 Added `block_size` to `BlockMeta` this is reflected in `/blockchain` - [types] \#2521 Add `NumTxs` to `BlockMeta` and `EventDataNewBlockHeader` -- [docs] [\#4111](https://github.com/tendermint/tendermint/issues/4111) Replaced dead whitepaper link in README.md +- [docs][\#4111](https://github.com/tendermint/tendermint/issues/4111) Replaced dead whitepaper link in README.md ### BUG FIXES: -- [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Refresh `tm-monitor` health when validator count is updated (@erikgrinaker) -- [state] [\#4104](https://github.com/tendermint/tendermint/pull/4104) txindex/kv: Fsync data to disk immediately after receiving it (@guagualvcha) -- [state] [\#4095](https://github.com/tendermint/tendermint/pull/4095) txindex/kv: Return an error if there's one when the user searches for a tx (hash=X) (@hsyis) -- [rpc/lib] [\#4051](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon) -- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) JSONRPCClient: validate that Response.ID matches Request.ID -- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) WSClient: check for unsolicited responses +- [tools][\#4023](https://github.com/tendermint/tendermint/issues/4023) Refresh `tm-monitor` health when validator count is updated (@erikgrinaker) +- [state][\#4104](https://github.com/tendermint/tendermint/pull/4104) txindex/kv: Fsync data to disk immediately after receiving it (@guagualvcha) +- [state][\#4095](https://github.com/tendermint/tendermint/pull/4095) txindex/kv: Return an error if there's one when the user searches for a tx (hash=X) (@hsyis) +- [rpc/lib][\#4051](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon) +- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) JSONRPCClient: validate that Response.ID matches Request.ID +- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) WSClient: check for unsolicited responses diff --git a/libs/common/async.go b/libs/common/async.go index 76a9ec9c7..7f9c4e872 100644 --- a/libs/common/async.go +++ b/libs/common/async.go @@ -2,6 +2,8 @@ package common import ( "sync/atomic" + + "github.com/pkg/errors" ) //---------------------------------------- @@ -142,7 +144,7 @@ func Parallel(tasks ...Task) (trs *TaskResultSet, ok bool) { if pnk := recover(); pnk != nil { atomic.AddInt32(numPanics, 1) // Send panic to taskResultCh. - taskResultCh <- TaskResult{nil, ErrorWrap(pnk, "Panic in task")} + taskResultCh <- TaskResult{nil, errors.Errorf("panic in task %v", pnk)} // Closing taskResultCh lets trs.Wait() work. close(taskResultCh) // Decrement waitgroup. diff --git a/libs/common/async_test.go b/libs/common/async_test.go index dc90d380c..276fe886e 100644 --- a/libs/common/async_test.go +++ b/libs/common/async_test.go @@ -1,12 +1,12 @@ package common import ( - "errors" "fmt" "sync/atomic" "testing" "time" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) @@ -125,20 +125,21 @@ func TestParallelRecover(t *testing.T) { // Verify task #0, #1, #2. checkResult(t, taskResultSet, 0, 0, nil, nil) checkResult(t, taskResultSet, 1, 1, errors.New("some error"), nil) - checkResult(t, taskResultSet, 2, nil, nil, 2) + checkResult(t, taskResultSet, 2, nil, nil, errors.Errorf("panic in task %v", 2).Error()) } // Wait for result -func checkResult(t *testing.T, taskResultSet *TaskResultSet, index int, val interface{}, err error, pnk interface{}) { +func checkResult(t *testing.T, taskResultSet *TaskResultSet, index int, + val interface{}, err error, pnk interface{}) { taskResult, ok := taskResultSet.LatestResult(index) taskName := fmt.Sprintf("Task #%v", index) assert.True(t, ok, "TaskResultCh unexpectedly closed for %v", taskName) assert.Equal(t, val, taskResult.Value, taskName) switch { case err != nil: - assert.Equal(t, err, taskResult.Error, taskName) + assert.Equal(t, err.Error(), taskResult.Error.Error(), taskName) case pnk != nil: - assert.Equal(t, pnk, taskResult.Error.(Error).Data(), taskName) + assert.Equal(t, pnk, taskResult.Error.Error(), taskName) default: assert.Nil(t, taskResult.Error, taskName) } diff --git a/libs/common/errors.go b/libs/common/errors.go deleted file mode 100644 index aacfbe274..000000000 --- a/libs/common/errors.go +++ /dev/null @@ -1,214 +0,0 @@ -package common - -import ( - "fmt" - "runtime" -) - -//---------------------------------------- -// Convenience method. - -func ErrorWrap(cause interface{}, format string, args ...interface{}) Error { - if causeCmnError, ok := cause.(*cmnError); ok { //nolint:gocritic - msg := fmt.Sprintf(format, args...) - return causeCmnError.Stacktrace().Trace(1, msg) - } else if cause == nil { - return newCmnError(FmtError{format, args}).Stacktrace() - } else { - // NOTE: causeCmnError is a typed nil here. - msg := fmt.Sprintf(format, args...) - return newCmnError(cause).Stacktrace().Trace(1, msg) - } -} - -//---------------------------------------- -// Error & cmnError - -/* - -Usage with arbitrary error data: - -```go - // Error construction - type MyError struct{} - var err1 error = NewErrorWithData(MyError{}, "my message") - ... - // Wrapping - var err2 error = ErrorWrap(err1, "another message") - if (err1 != err2) { panic("should be the same") - ... - // Error handling - switch err2.Data().(type){ - case MyError: ... - default: ... - } -``` -*/ -type Error interface { - Error() string - Stacktrace() Error - Trace(offset int, format string, args ...interface{}) Error - Data() interface{} -} - -// New Error with formatted message. -// The Error's Data will be a FmtError type. -func NewError(format string, args ...interface{}) Error { - err := FmtError{format, args} - return newCmnError(err) -} - -// New Error with specified data. -func NewErrorWithData(data interface{}) Error { - return newCmnError(data) -} - -type cmnError struct { - data interface{} // associated data - msgtraces []msgtraceItem // all messages traced - stacktrace []uintptr // first stack trace -} - -var _ Error = &cmnError{} - -// NOTE: do not expose. -func newCmnError(data interface{}) *cmnError { - return &cmnError{ - data: data, - msgtraces: nil, - stacktrace: nil, - } -} - -// Implements error. -func (err *cmnError) Error() string { - return fmt.Sprintf("%v", err) -} - -// Captures a stacktrace if one was not already captured. -func (err *cmnError) Stacktrace() Error { - if err.stacktrace == nil { - var offset = 3 - var depth = 32 - err.stacktrace = captureStacktrace(offset, depth) - } - return err -} - -// Add tracing information with msg. -// Set n=0 unless wrapped with some function, then n > 0. -func (err *cmnError) Trace(offset int, format string, args ...interface{}) Error { - msg := fmt.Sprintf(format, args...) - return err.doTrace(msg, offset) -} - -// Return the "data" of this error. -// Data could be used for error handling/switching, -// or for holding general error/debug information. -func (err *cmnError) Data() interface{} { - return err.data -} - -func (err *cmnError) doTrace(msg string, n int) Error { - pc, _, _, _ := runtime.Caller(n + 2) // +1 for doTrace(). +1 for the caller. - // Include file & line number & msg. - // Do not include the whole stack trace. - err.msgtraces = append(err.msgtraces, msgtraceItem{ - pc: pc, - msg: msg, - }) - return err -} - -func (err *cmnError) Format(s fmt.State, verb rune) { - switch verb { - case 'p': - s.Write([]byte(fmt.Sprintf("%p", &err))) - default: - if s.Flag('#') { - s.Write([]byte("--= Error =--\n")) - // Write data. - s.Write([]byte(fmt.Sprintf("Data: %#v\n", err.data))) - // Write msg trace items. - s.Write([]byte(fmt.Sprintf("Msg Traces:\n"))) - for i, msgtrace := range err.msgtraces { - s.Write([]byte(fmt.Sprintf(" %4d %s\n", i, msgtrace.String()))) - } - // Write stack trace. - if err.stacktrace != nil { - s.Write([]byte(fmt.Sprintf("Stack Trace:\n"))) - for i, pc := range err.stacktrace { - fnc := runtime.FuncForPC(pc) - file, line := fnc.FileLine(pc) - s.Write([]byte(fmt.Sprintf(" %4d %s:%d\n", i, file, line))) - } - } - s.Write([]byte("--= /Error =--\n")) - } else { - // Write msg. - s.Write([]byte(fmt.Sprintf("%v", err.data))) - } - } -} - -//---------------------------------------- -// stacktrace & msgtraceItem - -func captureStacktrace(offset int, depth int) []uintptr { - var pcs = make([]uintptr, depth) - n := runtime.Callers(offset, pcs) - return pcs[0:n] -} - -type msgtraceItem struct { - pc uintptr - msg string -} - -func (mti msgtraceItem) String() string { - fnc := runtime.FuncForPC(mti.pc) - file, line := fnc.FileLine(mti.pc) - return fmt.Sprintf("%s:%d - %s", - file, line, - mti.msg, - ) -} - -//---------------------------------------- -// fmt error - -/* - -FmtError is the data type for NewError() (e.g. NewError().Data().(FmtError)) -Theoretically it could be used to switch on the format string. - -```go - // Error construction - var err1 error = NewError("invalid username %v", "BOB") - var err2 error = NewError("another kind of error") - ... - // Error handling - switch err1.Data().(cmn.FmtError).Format() { - case "invalid username %v": ... - case "another kind of error": ... - default: ... - } -``` -*/ -type FmtError struct { - format string - args []interface{} -} - -func (fe FmtError) Error() string { - return fmt.Sprintf(fe.format, fe.args...) -} - -func (fe FmtError) String() string { - return fmt.Sprintf("FmtError{format:%v,args:%v}", - fe.format, fe.args) -} - -func (fe FmtError) Format() string { - return fe.format -} diff --git a/libs/common/errors_test.go b/libs/common/errors_test.go deleted file mode 100644 index b85936dd5..000000000 --- a/libs/common/errors_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package common - -import ( - fmt "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestErrorPanic(t *testing.T) { - type pnk struct { - msg string - } - - capturePanic := func() (err Error) { - defer func() { - if r := recover(); r != nil { - err = ErrorWrap(r, "This is the message in ErrorWrap(r, message).") - } - }() - panic(pnk{"something"}) - } - - var err = capturePanic() - - assert.Equal(t, pnk{"something"}, err.Data()) - assert.Equal(t, "{something}", fmt.Sprintf("%v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), "This is the message in ErrorWrap(r, message).") - assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0") -} - -func TestErrorWrapSomething(t *testing.T) { - - var err = ErrorWrap("something", "formatter%v%v", 0, 1) - - assert.Equal(t, "something", err.Data()) - assert.Equal(t, "something", fmt.Sprintf("%v", err)) - assert.Regexp(t, `formatter01\n`, fmt.Sprintf("%#v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0") -} - -func TestErrorWrapNothing(t *testing.T) { - - var err = ErrorWrap(nil, "formatter%v%v", 0, 1) - - assert.Equal(t, - FmtError{"formatter%v%v", []interface{}{0, 1}}, - err.Data()) - assert.Equal(t, "formatter01", fmt.Sprintf("%v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`) - assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0") -} - -func TestErrorNewError(t *testing.T) { - - var err = NewError("formatter%v%v", 0, 1) - - assert.Equal(t, - FmtError{"formatter%v%v", []interface{}{0, 1}}, - err.Data()) - assert.Equal(t, "formatter01", fmt.Sprintf("%v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`) - assert.NotContains(t, fmt.Sprintf("%#v", err), "Stack Trace") -} - -func TestErrorNewErrorWithStacktrace(t *testing.T) { - - var err = NewError("formatter%v%v", 0, 1).Stacktrace() - - assert.Equal(t, - FmtError{"formatter%v%v", []interface{}{0, 1}}, - err.Data()) - assert.Equal(t, "formatter01", fmt.Sprintf("%v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`) - assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0") -} - -func TestErrorNewErrorWithTrace(t *testing.T) { - - var err = NewError("formatter%v%v", 0, 1) - err.Trace(0, "trace %v", 1) - err.Trace(0, "trace %v", 2) - err.Trace(0, "trace %v", 3) - - assert.Equal(t, - FmtError{"formatter%v%v", []interface{}{0, 1}}, - err.Data()) - assert.Equal(t, "formatter01", fmt.Sprintf("%v", err)) - assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`) - dump := fmt.Sprintf("%#v", err) - assert.NotContains(t, dump, "Stack Trace") - assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 1`, dump) - assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 2`, dump) - assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 3`, dump) -} - -func TestErrorWrapError(t *testing.T) { - var err1 error = NewError("my message") - var err2 error = ErrorWrap(err1, "another message") - assert.Equal(t, err1, err2) -} diff --git a/lite/base_verifier.go b/lite/base_verifier.go index 9fe583b0f..6a2a50ab5 100644 --- a/lite/base_verifier.go +++ b/lite/base_verifier.go @@ -5,7 +5,6 @@ import ( "github.com/pkg/errors" - cmn "github.com/tendermint/tendermint/libs/common" lerr "github.com/tendermint/tendermint/lite/errors" "github.com/tendermint/tendermint/types" ) @@ -46,13 +45,13 @@ func (bv *BaseVerifier) Verify(signedHeader types.SignedHeader) error { // We can't verify commits for a different chain. if signedHeader.ChainID != bv.chainID { - return cmn.NewError("BaseVerifier chainID is %v, cannot verify chainID %v", + return errors.Errorf("BaseVerifier chainID is %v, cannot verify chainID %v", bv.chainID, signedHeader.ChainID) } // We can't verify commits older than bv.height. if signedHeader.Height < bv.height { - return cmn.NewError("BaseVerifier height is %v, cannot verify height %v", + return errors.Errorf("BaseVerifier height is %v, cannot verify height %v", bv.height, signedHeader.Height) } diff --git a/lite/proxy/query.go b/lite/proxy/query.go index 518a6a235..5acb8f424 100644 --- a/lite/proxy/query.go +++ b/lite/proxy/query.go @@ -24,7 +24,7 @@ func GetWithProof(prt *merkle.ProofRuntime, key []byte, reqHeight int64, node rp val cmn.HexBytes, height int64, proof *merkle.Proof, err error) { if reqHeight < 0 { - err = cmn.NewError("Height cannot be negative") + err = errors.New("Height cannot be negative") return } @@ -54,7 +54,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts // Validate the response, e.g. height. if resp.IsErr() { - err = cmn.NewError("Query error for key %d: %d", key, resp.Code) + err = errors.Errorf("Query error for key %d: %d", key, resp.Code) return nil, err } @@ -62,7 +62,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts return nil, lerr.ErrEmptyTree() } if resp.Height == 0 { - return nil, cmn.NewError("Height returned is zero") + return nil, errors.New("Height returned is zero") } // AppHash for height H is in header H+1