From dd55f9adf62ddc6c7cb77b0d0227a39e9ac0093f Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 22 Sep 2021 20:30:35 +0200 Subject: [PATCH 1/7] core: fix warning flagging the use of DeepEqual on error --- core/genesis.go | 9 +++++++++ core/genesis_test.go | 3 ++- params/config.go | 11 +++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/core/genesis.go b/core/genesis.go index 38ace4920bda..25f525da5ccb 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -141,6 +141,15 @@ func (e *GenesisMismatchError) Error() string { return fmt.Sprintf("database contains incompatible genesis (have %x, new %x)", e.Stored, e.New) } +func (e *GenesisMismatchError) Is(target error) bool { + gme, ok := target.(*GenesisMismatchError) + if !ok { + return false + } + return bytes.Equal(e.Stored[:], gme.Stored[:]) && + bytes.Equal(e.New[:], gme.New[:]) +} + // SetupGenesisBlock writes or updates the genesis block in db. // The block that will be used is: // diff --git a/core/genesis_test.go b/core/genesis_test.go index 055be2796c39..b24f70666920 100644 --- a/core/genesis_test.go +++ b/core/genesis_test.go @@ -17,6 +17,7 @@ package core import ( + "errors" "math/big" "reflect" "testing" @@ -160,7 +161,7 @@ func TestSetupGenesis(t *testing.T) { db := rawdb.NewMemoryDatabase() config, hash, err := test.fn(db) // Check the return values. - if !reflect.DeepEqual(err, test.wantErr) { + if !errors.Is(err, test.wantErr) { spew := spew.ConfigState{DisablePointerAddresses: true, DisableCapacities: true} t.Errorf("%s: returned error %#v, want %#v", test.name, spew.NewFormatter(err), spew.NewFormatter(test.wantErr)) } diff --git a/params/config.go b/params/config.go index fdbff930240b..30757d444d58 100644 --- a/params/config.go +++ b/params/config.go @@ -609,6 +609,17 @@ func (err *ConfigCompatError) Error() string { return fmt.Sprintf("mismatching %s in database (have %d, want %d, rewindto %d)", err.What, err.StoredConfig, err.NewConfig, err.RewindTo) } +func (err *ConfigCompatError) Is(target error) bool { + cce, ok := target.(*ConfigCompatError) + if !ok { + return false + } + + return (cce.What == err.What && cce.RewindTo == err.RewindTo && + err.StoredConfig.Cmp(cce.StoredConfig) == 0 && + err.NewConfig.Cmp(cce.NewConfig) == 0) +} + // Rules wraps ChainConfig and is merely syntactic sugar or can be used for functions // that do not have or require information about the block. // From 6a73ac6a667a5849d25b14fd0828d4063b9fe5e5 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Wed, 22 Sep 2021 21:18:26 +0200 Subject: [PATCH 2/7] apply the same change everywhere possible --- eth/tracers/api_test.go | 2 +- event/subscription_test.go | 2 +- p2p/discover/v5wire/encoding_test.go | 4 ++-- p2p/server_test.go | 12 +++++++----- params/config_test.go | 4 ++-- rlp/raw_test.go | 4 ++-- rpc/websocket_test.go | 4 ++-- 7 files changed, 17 insertions(+), 15 deletions(-) diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index 9afd59d596bc..864a9a86bbc1 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -291,7 +291,7 @@ func TestTraceCall(t *testing.T) { t.Errorf("Expect error %v, get nothing", testspec.expectErr) continue } - if !reflect.DeepEqual(err, testspec.expectErr) { + if !errors.Is(err, testspec.expectErr) { t.Errorf("Error mismatch, want %v, get %v", testspec.expectErr, err) } } else { diff --git a/event/subscription_test.go b/event/subscription_test.go index ba081705c44c..343093e62685 100644 --- a/event/subscription_test.go +++ b/event/subscription_test.go @@ -150,7 +150,7 @@ func TestResubscribeWithErrorHandler(t *testing.T) { } expectedSubErrs := []string{"", "err-1", "err-2", "err-3", "err-4", "err-5"} - if !reflect.DeepEqual(subErrs, expectedSubErrs) { + if !errors.Is(subErrs, expectedSubErrs) { t.Fatalf("unexpected subscription errors %v, want %v", subErrs, expectedSubErrs) } } diff --git a/p2p/discover/v5wire/encoding_test.go b/p2p/discover/v5wire/encoding_test.go index b13041d1b9c6..355a8f691399 100644 --- a/p2p/discover/v5wire/encoding_test.go +++ b/p2p/discover/v5wire/encoding_test.go @@ -20,13 +20,13 @@ import ( "bytes" "crypto/ecdsa" "encoding/hex" + "errors" "flag" "fmt" "io/ioutil" "net" "os" "path/filepath" - "reflect" "strings" "testing" @@ -555,7 +555,7 @@ func (n *handshakeTestNode) expectDecode(t *testing.T, ptype byte, p []byte) Pac func (n *handshakeTestNode) expectDecodeErr(t *testing.T, wantErr error, p []byte) { t.Helper() - if _, err := n.decode(p); !reflect.DeepEqual(err, wantErr) { + if _, err := n.decode(p); !errors.Is(err, wantErr) { t.Fatal(fmt.Errorf("(%s) got err %q, want %q", n.ln.ID().TerminalString(), err, wantErr)) } } diff --git a/p2p/server_test.go b/p2p/server_test.go index a5b3190aede2..f6f5700c5efc 100644 --- a/p2p/server_test.go +++ b/p2p/server_test.go @@ -370,6 +370,8 @@ func TestServerSetupConn(t *testing.T) { clientkey, srvkey = newkey(), newkey() clientpub = &clientkey.PublicKey srvpub = &srvkey.PublicKey + fooErr = errors.New("foo") + readErr = errors.New("read error") ) tests := []struct { dontstart bool @@ -387,10 +389,10 @@ func TestServerSetupConn(t *testing.T) { wantCloseErr: errServerStopped, }, { - tt: &setupTransport{pubkey: clientpub, encHandshakeErr: errors.New("read error")}, + tt: &setupTransport{pubkey: clientpub, encHandshakeErr: readErr}, flags: inboundConn, wantCalls: "doEncHandshake,close,", - wantCloseErr: errors.New("read error"), + wantCloseErr: readErr, }, { tt: &setupTransport{pubkey: clientpub, phs: protoHandshake{ID: randomID().Bytes()}}, @@ -400,11 +402,11 @@ func TestServerSetupConn(t *testing.T) { wantCloseErr: DiscUnexpectedIdentity, }, { - tt: &setupTransport{pubkey: clientpub, protoHandshakeErr: errors.New("foo")}, + tt: &setupTransport{pubkey: clientpub, protoHandshakeErr: fooErr}, dialDest: enode.NewV4(clientpub, nil, 0, 0), flags: dynDialedConn, wantCalls: "doEncHandshake,doProtoHandshake,close,", - wantCloseErr: errors.New("foo"), + wantCloseErr: fooErr, }, { tt: &setupTransport{pubkey: srvpub, phs: protoHandshake{ID: crypto.FromECDSAPub(srvpub)[1:]}}, @@ -443,7 +445,7 @@ func TestServerSetupConn(t *testing.T) { } p1, _ := net.Pipe() srv.SetupConn(p1, test.flags, test.dialDest) - if !reflect.DeepEqual(test.tt.closeErr, test.wantCloseErr) { + if !errors.Is(test.tt.closeErr, test.wantCloseErr) { t.Errorf("test %d: close error mismatch: got %q, want %q", i, test.tt.closeErr, test.wantCloseErr) } if test.tt.calls != test.wantCalls { diff --git a/params/config_test.go b/params/config_test.go index 3c8ebaf4a511..d053ddfc95dd 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -17,8 +17,8 @@ package params import ( + "errors" "math/big" - "reflect" "testing" ) @@ -91,7 +91,7 @@ func TestCheckCompatible(t *testing.T) { for _, test := range tests { err := test.stored.CheckCompatible(test.new, test.head) - if !reflect.DeepEqual(err, test.wantErr) { + if !errors.Is(err, test.wantErr) { t.Errorf("error mismatch:\nstored: %v\nnew: %v\nhead: %v\nerr: %v\nwant: %v", test.stored, test.new, test.head, err, test.wantErr) } } diff --git a/rlp/raw_test.go b/rlp/raw_test.go index 185e269d075a..46adff22c5da 100644 --- a/rlp/raw_test.go +++ b/rlp/raw_test.go @@ -18,8 +18,8 @@ package rlp import ( "bytes" + "errors" "io" - "reflect" "testing" "testing/quick" ) @@ -54,7 +54,7 @@ func TestCountValues(t *testing.T) { if count != test.count { t.Errorf("test %d: count mismatch, got %d want %d\ninput: %s", i, count, test.count, test.input) } - if !reflect.DeepEqual(err, test.err) { + if !errors.Is(err, test.err) { t.Errorf("test %d: err mismatch, got %q want %q\ninput: %s", i, err, test.err, test.input) } } diff --git a/rpc/websocket_test.go b/rpc/websocket_test.go index 2486092836bc..cf83b621f171 100644 --- a/rpc/websocket_test.go +++ b/rpc/websocket_test.go @@ -18,13 +18,13 @@ package rpc import ( "context" + "errors" "io" "net" "net/http" "net/http/httptest" "net/http/httputil" "net/url" - "reflect" "strings" "sync/atomic" "testing" @@ -69,7 +69,7 @@ func TestWebsocketOriginCheck(t *testing.T) { t.Fatal("no error for wrong origin") } wantErr := wsHandshakeError{websocket.ErrBadHandshake, "403 Forbidden"} - if !reflect.DeepEqual(err, wantErr) { + if !errors.Is(err, wantErr) { t.Fatalf("wrong error for wrong origin: %q", err) } From 09ffbc1e31848ab49fbb95dba6c7805624357b1a Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Thu, 23 Sep 2021 08:07:13 +0200 Subject: [PATCH 3/7] revert change that was committed by mistake --- event/subscription_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/event/subscription_test.go b/event/subscription_test.go index 343093e62685..ba081705c44c 100644 --- a/event/subscription_test.go +++ b/event/subscription_test.go @@ -150,7 +150,7 @@ func TestResubscribeWithErrorHandler(t *testing.T) { } expectedSubErrs := []string{"", "err-1", "err-2", "err-3", "err-4", "err-5"} - if !errors.Is(subErrs, expectedSubErrs) { + if !reflect.DeepEqual(subErrs, expectedSubErrs) { t.Fatalf("unexpected subscription errors %v, want %v", subErrs, expectedSubErrs) } } From 80e5b407b8b49515b67f0c6127d8aff43df29fd7 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Thu, 23 Sep 2021 10:32:25 +0200 Subject: [PATCH 4/7] fix build error --- eth/tracers/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index 864a9a86bbc1..9afd59d596bc 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -291,7 +291,7 @@ func TestTraceCall(t *testing.T) { t.Errorf("Expect error %v, get nothing", testspec.expectErr) continue } - if !errors.Is(err, testspec.expectErr) { + if !reflect.DeepEqual(err, testspec.expectErr) { t.Errorf("Error mismatch, want %v, get %v", testspec.expectErr, err) } } else { From c0fc80d109214577c4600f3f043fdf260053c2ab Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Thu, 23 Sep 2021 12:55:22 +0200 Subject: [PATCH 5/7] Update config.go --- params/config.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/params/config.go b/params/config.go index 30757d444d58..b864b5234b75 100644 --- a/params/config.go +++ b/params/config.go @@ -610,14 +610,12 @@ func (err *ConfigCompatError) Error() string { } func (err *ConfigCompatError) Is(target error) bool { - cce, ok := target.(*ConfigCompatError) - if !ok { - return false - } - - return (cce.What == err.What && cce.RewindTo == err.RewindTo && - err.StoredConfig.Cmp(cce.StoredConfig) == 0 && - err.NewConfig.Cmp(cce.NewConfig) == 0) + e, ok := target.(*ConfigCompatError) + return ok && + e.What == err.What && + e.RewindTo == err.RewindTo && + e.StoredConfig.Cmp(err.StoredConfig) == 0 && + e.NewConfig.Cmp(err.NewConfig) == 0 } // Rules wraps ChainConfig and is merely syntactic sugar or can be used for functions From ca03e14d876bb07b124ab1588544b2d4954f5d62 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Mon, 18 Oct 2021 15:29:46 +0200 Subject: [PATCH 6/7] revert changes to ConfigCompatError --- params/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/params/config_test.go b/params/config_test.go index d053ddfc95dd..3c8ebaf4a511 100644 --- a/params/config_test.go +++ b/params/config_test.go @@ -17,8 +17,8 @@ package params import ( - "errors" "math/big" + "reflect" "testing" ) @@ -91,7 +91,7 @@ func TestCheckCompatible(t *testing.T) { for _, test := range tests { err := test.stored.CheckCompatible(test.new, test.head) - if !errors.Is(err, test.wantErr) { + if !reflect.DeepEqual(err, test.wantErr) { t.Errorf("error mismatch:\nstored: %v\nnew: %v\nhead: %v\nerr: %v\nwant: %v", test.stored, test.new, test.head, err, test.wantErr) } } From e825fc0b48126af9754c89bb542b708134913330 Mon Sep 17 00:00:00 2001 From: Guillaume Ballet <3272758+gballet@users.noreply.github.com> Date: Tue, 19 Oct 2021 10:56:15 +0200 Subject: [PATCH 7/7] review feedback --- core/genesis.go | 9 --------- core/genesis_test.go | 3 +-- params/config.go | 9 --------- 3 files changed, 1 insertion(+), 20 deletions(-) diff --git a/core/genesis.go b/core/genesis.go index 25f525da5ccb..38ace4920bda 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -141,15 +141,6 @@ func (e *GenesisMismatchError) Error() string { return fmt.Sprintf("database contains incompatible genesis (have %x, new %x)", e.Stored, e.New) } -func (e *GenesisMismatchError) Is(target error) bool { - gme, ok := target.(*GenesisMismatchError) - if !ok { - return false - } - return bytes.Equal(e.Stored[:], gme.Stored[:]) && - bytes.Equal(e.New[:], gme.New[:]) -} - // SetupGenesisBlock writes or updates the genesis block in db. // The block that will be used is: // diff --git a/core/genesis_test.go b/core/genesis_test.go index b24f70666920..055be2796c39 100644 --- a/core/genesis_test.go +++ b/core/genesis_test.go @@ -17,7 +17,6 @@ package core import ( - "errors" "math/big" "reflect" "testing" @@ -161,7 +160,7 @@ func TestSetupGenesis(t *testing.T) { db := rawdb.NewMemoryDatabase() config, hash, err := test.fn(db) // Check the return values. - if !errors.Is(err, test.wantErr) { + if !reflect.DeepEqual(err, test.wantErr) { spew := spew.ConfigState{DisablePointerAddresses: true, DisableCapacities: true} t.Errorf("%s: returned error %#v, want %#v", test.name, spew.NewFormatter(err), spew.NewFormatter(test.wantErr)) } diff --git a/params/config.go b/params/config.go index b864b5234b75..fdbff930240b 100644 --- a/params/config.go +++ b/params/config.go @@ -609,15 +609,6 @@ func (err *ConfigCompatError) Error() string { return fmt.Sprintf("mismatching %s in database (have %d, want %d, rewindto %d)", err.What, err.StoredConfig, err.NewConfig, err.RewindTo) } -func (err *ConfigCompatError) Is(target error) bool { - e, ok := target.(*ConfigCompatError) - return ok && - e.What == err.What && - e.RewindTo == err.RewindTo && - e.StoredConfig.Cmp(err.StoredConfig) == 0 && - e.NewConfig.Cmp(err.NewConfig) == 0 -} - // Rules wraps ChainConfig and is merely syntactic sugar or can be used for functions // that do not have or require information about the block. //