Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: fix warning flagging the use of DeepEqual on error #23624

Merged
merged 7 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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[:])
}

Copy link
Contributor

@holiman holiman Sep 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this whole thing needed? I'm thinking we can replace it with

var ErrGenesisMismatch = errors.New("genesis mismatch")

And in the places where we now instantiate it, we simply do

return fmt.Errorf("%w: database has %x, new %x", ErrGenesisMismatch, have, new)

And then we can just do the generic errors.Is for it, instead of implementing custom Is support

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reasons explained previously, this method is not useful for any production use of the error.

// SetupGenesisBlock writes or updates the genesis block in db.
// The block that will be used is:
//
Expand Down
3 changes: 2 additions & 1 deletion core/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package core

import (
"errors"
"math/big"
"reflect"
"testing"
Expand Down Expand Up @@ -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))
}
Expand Down
4 changes: 2 additions & 2 deletions p2p/discover/v5wire/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ import (
"bytes"
"crypto/ecdsa"
"encoding/hex"
"errors"
"flag"
"fmt"
"io/ioutil"
"net"
"os"
"path/filepath"
"reflect"
"strings"
"testing"

Expand Down Expand Up @@ -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))
}
}
Expand Down
12 changes: 7 additions & 5 deletions p2p/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()}},
Expand All @@ -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:]}},
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,15 @@ 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.
//
Expand Down
4 changes: 2 additions & 2 deletions rlp/raw_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ package rlp

import (
"bytes"
"errors"
"io"
"reflect"
"testing"
"testing/quick"
)
Expand Down Expand Up @@ -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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions rpc/websocket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down