From b0917db4c79d9ec9c06884539697eb96138b5240 Mon Sep 17 00:00:00 2001 From: terence tsao Date: Tue, 15 Sep 2020 20:55:57 -0700 Subject: [PATCH] Core pkg: properly return errs (#7246) * Return errs instead of nils * Update a few more tests Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- beacon-chain/core/helpers/committee.go | 2 +- beacon-chain/core/helpers/committee_test.go | 6 ++++++ beacon-chain/core/helpers/signing_root.go | 2 +- beacon-chain/core/helpers/validators.go | 2 +- beacon-chain/db/kv/regen_historical_states.go | 2 +- beacon-chain/p2p/broadcaster_test.go | 11 ++++++----- beacon-chain/p2p/discovery_test.go | 6 +++--- beacon-chain/p2p/fork_test.go | 3 ++- 8 files changed, 21 insertions(+), 13 deletions(-) diff --git a/beacon-chain/core/helpers/committee.go b/beacon-chain/core/helpers/committee.go index 6d1917778..d0a1f4968 100644 --- a/beacon-chain/core/helpers/committee.go +++ b/beacon-chain/core/helpers/committee.go @@ -350,7 +350,7 @@ func UpdateCommitteeCache(state *stateTrie.BeaconState, epoch uint64) error { func UpdateProposerIndicesInCache(state *stateTrie.BeaconState, epoch uint64) error { indices, err := ActiveValidatorIndices(state, epoch) if err != nil { - return nil + return err } proposerIndices, err := precomputeProposerIndices(state, indices) if err != nil { diff --git a/beacon-chain/core/helpers/committee_test.go b/beacon-chain/core/helpers/committee_test.go index 2de0ae0d1..5aa952986 100644 --- a/beacon-chain/core/helpers/committee_test.go +++ b/beacon-chain/core/helpers/committee_test.go @@ -628,3 +628,9 @@ func TestPrecomputeProposerIndices_Ok(t *testing.T) { } assert.DeepEqual(t, wantedProposerIndices, proposerIndices, "Did not precompute proposer indices correctly") } + +func TestUpdateProposerIndicesInCache_CouldNotGetActiveIndices(t *testing.T) { + err := UpdateProposerIndicesInCache(&beaconstate.BeaconState{}, 0) + want := "nil inner state" + require.ErrorContains(t, want, err) +} diff --git a/beacon-chain/core/helpers/signing_root.go b/beacon-chain/core/helpers/signing_root.go index d5c20c994..333633c81 100644 --- a/beacon-chain/core/helpers/signing_root.go +++ b/beacon-chain/core/helpers/signing_root.go @@ -246,7 +246,7 @@ func computeForkDataRoot(version []byte, root []byte) ([32]byte, error) { func ComputeForkDigest(version []byte, genesisValidatorsRoot []byte) ([4]byte, error) { dataRoot, err := computeForkDataRoot(version, genesisValidatorsRoot) if err != nil { - return [4]byte{}, nil + return [4]byte{}, err } return bytesutil.ToBytes4(dataRoot[:]), nil } diff --git a/beacon-chain/core/helpers/validators.go b/beacon-chain/core/helpers/validators.go index 78dec9263..e1d7445ad 100644 --- a/beacon-chain/core/helpers/validators.go +++ b/beacon-chain/core/helpers/validators.go @@ -250,7 +250,7 @@ func ComputeProposerIndex(bState *stateTrie.BeaconState, activeIndices []uint64, randomByte := hashFunc(b)[i%32] v, err := bState.ValidatorAtIndexReadOnly(candidateIndex) if err != nil { - return 0, nil + return 0, err } effectiveBal := v.EffectiveBalance() diff --git a/beacon-chain/db/kv/regen_historical_states.go b/beacon-chain/db/kv/regen_historical_states.go index ae722b172..7010d6da1 100644 --- a/beacon-chain/db/kv/regen_historical_states.go +++ b/beacon-chain/db/kv/regen_historical_states.go @@ -228,7 +228,7 @@ func (kv *Store) saveArchivedInfo(ctx context.Context, lastBlocksRoot, err := blocks[len(blocks)-1].Block.HashTreeRoot() if err != nil { - return nil + return err } if err := kv.SaveState(ctx, currentState, lastBlocksRoot); err != nil { return err diff --git a/beacon-chain/p2p/broadcaster_test.go b/beacon-chain/p2p/broadcaster_test.go index 20914a4ca..c8e9da2e9 100644 --- a/beacon-chain/p2p/broadcaster_test.go +++ b/beacon-chain/p2p/broadcaster_test.go @@ -10,6 +10,7 @@ import ( "time" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/ethereum/go-ethereum/p2p/discover" "github.com/gogo/protobuf/proto" @@ -41,7 +42,7 @@ func TestService_Broadcast(t *testing.T) { joinedTopics: map[string]*pubsub.Topic{}, cfg: &Config{}, genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), } msg := &testpb.TestSimpleMessage{ @@ -90,7 +91,7 @@ func TestService_Broadcast(t *testing.T) { func TestService_Broadcast_ReturnsErr_TopicNotMapped(t *testing.T) { p := Service{ genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), } assert.ErrorContains(t, ErrMessageNotMapped.Error(), p.Broadcast(context.Background(), &testpb.AddressBook{})) } @@ -152,7 +153,7 @@ func TestService_BroadcastAttestation(t *testing.T) { joinedTopics: map[string]*pubsub.Topic{}, cfg: &Config{}, genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), subnetsLock: make(map[uint64]*sync.RWMutex), subnetsLockLock: sync.Mutex{}, peers: peers.NewStatus(context.Background(), &peers.StatusConfig{ @@ -317,7 +318,7 @@ func TestService_BroadcastAttestationWithDiscoveryAttempts(t *testing.T) { joinedTopics: map[string]*pubsub.Topic{}, cfg: &Config{}, genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), subnetsLock: make(map[uint64]*sync.RWMutex), subnetsLockLock: sync.Mutex{}, peers: peers.NewStatus(context.Background(), &peers.StatusConfig{ @@ -332,7 +333,7 @@ func TestService_BroadcastAttestationWithDiscoveryAttempts(t *testing.T) { joinedTopics: map[string]*pubsub.Topic{}, cfg: &Config{}, genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), subnetsLock: make(map[uint64]*sync.RWMutex), subnetsLockLock: sync.Mutex{}, peers: peers.NewStatus(context.Background(), &peers.StatusConfig{ diff --git a/beacon-chain/p2p/discovery_test.go b/beacon-chain/p2p/discovery_test.go index 40ffa02e4..40f0e6dfe 100644 --- a/beacon-chain/p2p/discovery_test.go +++ b/beacon-chain/p2p/discovery_test.go @@ -50,7 +50,7 @@ func TestCreateListener(t *testing.T) { ipAddr, pkey := createAddrAndPrivKey(t) s := &Service{ genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), cfg: &Config{UDPPort: uint(port)}, } listener, err := s.createListener(ipAddr, pkey) @@ -125,7 +125,7 @@ func TestMultiAddrsConversion_InvalidIPAddr(t *testing.T) { _, pkey := createAddrAndPrivKey(t) s := &Service{ genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), } node, err := s.createLocalNode(pkey, addr, 0, 0) require.NoError(t, err) @@ -142,7 +142,7 @@ func TestMultiAddrConversion_OK(t *testing.T) { UDPPort: 0, }, genesisTime: time.Now(), - genesisValidatorsRoot: []byte{'A'}, + genesisValidatorsRoot: bytesutil.PadTo([]byte{'A'}, 32), } listener, err := s.createListener(ipAddr, pkey) require.NoError(t, err) diff --git a/beacon-chain/p2p/fork_test.go b/beacon-chain/p2p/fork_test.go index cc659d6cc..276838928 100644 --- a/beacon-chain/p2p/fork_test.go +++ b/beacon-chain/p2p/fork_test.go @@ -16,6 +16,7 @@ import ( ma "github.com/multiformats/go-multiaddr" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/p2putils" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/testutil" @@ -261,7 +262,7 @@ func TestAddForkEntry_Genesis(t *testing.T) { require.NoError(t, err) localNode := enode.NewLocalNode(db, pkey) - localNode, err = addForkEntry(localNode, time.Now().Add(10*time.Second), []byte{'A', 'B', 'C', 'D'}) + localNode, err = addForkEntry(localNode, time.Now().Add(10*time.Second), bytesutil.PadTo([]byte{'A', 'B', 'C', 'D'}, 32)) require.NoError(t, err) forkEntry, err := retrieveForkEntry(localNode.Node().Record()) require.NoError(t, err)