diff --git a/BUILD.bazel b/BUILD.bazel index e1ba6a168..b6a262159 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -105,6 +105,7 @@ nogo( "@org_golang_x_tools//go/analysis/passes/asmdecl:go_tool_library", "//tools/analyzers/maligned:go_tool_library", "//tools/analyzers/roughtime:go_tool_library", + "//tools/analyzers/errcheck:go_tool_library", ], ) diff --git a/beacon-chain/archiver/service.go b/beacon-chain/archiver/service.go index 6c4976184..2645bf3ac 100644 --- a/beacon-chain/archiver/service.go +++ b/beacon-chain/archiver/service.go @@ -147,7 +147,11 @@ func (s *Service) run(ctx context.Context) { select { case event := <-stateChannel: if event.Type == statefeed.BlockProcessed { - data := event.Data.(*statefeed.BlockProcessedData) + data, ok := event.Data.(*statefeed.BlockProcessedData) + if !ok { + log.Error("Event feed data is not type *statefeed.BlockProcessedData") + continue + } log.WithField("headRoot", fmt.Sprintf("%#x", data.BlockRoot)).Debug("Received block processed event") headState, err := s.headFetcher.HeadState(ctx) if err != nil { diff --git a/beacon-chain/blockchain/process_attestation_test.go b/beacon-chain/blockchain/process_attestation_test.go index 3cf4f755e..8d28ca659 100644 --- a/beacon-chain/blockchain/process_attestation_test.go +++ b/beacon-chain/blockchain/process_attestation_test.go @@ -150,6 +150,8 @@ func TestStore_SaveCheckpointState(t *testing.T) { JustificationBits: []byte{0}, Slashings: make([]uint64, params.BeaconConfig().EpochsPerSlashingsVector), FinalizedCheckpoint: ðpb.Checkpoint{Root: bytesutil.PadTo([]byte{'A'}, 32)}, + Validators: []*ethpb.Validator{{PublicKey: bytesutil.PadTo([]byte("foo"), 48)}}, + Balances: []uint64{0}, }) r := [32]byte{'g'} if err := service.beaconDB.SaveState(ctx, s, r); err != nil { diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 9a21c0493..8f84abaae 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -136,7 +136,9 @@ func (s *Service) onBlock(ctx context.Context, signed *ethpb.SignedBeaconBlock) // Prune proto array fork choice nodes, all nodes before finalized check point will // be pruned. - s.forkChoiceStore.Prune(ctx, fRoot) + if err := s.forkChoiceStore.Prune(ctx, fRoot); err != nil { + return nil, errors.Wrap(err, "could not prune proto array fork choice nodes") + } s.prevFinalizedCheckpt = s.finalizedCheckpt s.finalizedCheckpt = postState.FinalizedCheckpoint() diff --git a/beacon-chain/blockchain/service.go b/beacon-chain/blockchain/service.go index fbe169f58..912719d33 100644 --- a/beacon-chain/blockchain/service.go +++ b/beacon-chain/blockchain/service.go @@ -210,7 +210,11 @@ func (s *Service) Start() { select { case event := <-stateChannel: if event.Type == statefeed.ChainStarted { - data := event.Data.(*statefeed.ChainStartedData) + data, ok := event.Data.(*statefeed.ChainStartedData) + if !ok { + log.Error("event data is not type *statefeed.ChainStartedData") + return + } log.WithField("starttime", data.StartTime).Debug("Received chain start event") s.processChainStartTime(ctx, data.StartTime) return diff --git a/beacon-chain/cache/attestation_data.go b/beacon-chain/cache/attestation_data.go index a2996a5a6..1c2e20589 100644 --- a/beacon-chain/cache/attestation_data.go +++ b/beacon-chain/cache/attestation_data.go @@ -148,7 +148,10 @@ func (c *AttestationCache) Put(ctx context.Context, req *ethpb.AttestationDataRe } func wrapperToKey(i interface{}) (string, error) { - w := i.(*attestationReqResWrapper) + w, ok := i.(*attestationReqResWrapper) + if !ok { + return "", errors.New("key is not of type *attestationReqResWrapper") + } if w == nil { return "", errors.New("nil wrapper") } diff --git a/beacon-chain/cache/common.go b/beacon-chain/cache/common.go index 786ac4506..19368007a 100644 --- a/beacon-chain/cache/common.go +++ b/beacon-chain/cache/common.go @@ -14,8 +14,12 @@ var ( // trim the FIFO queue to the maxSize. func trim(queue *cache.FIFO, maxSize int) { for s := len(queue.ListKeys()); s > maxSize; s-- { - // #nosec G104 popProcessNoopFunc never returns an error - _, _ = queue.Pop(popProcessNoopFunc) + _, err := queue.Pop(popProcessNoopFunc) + if err != nil { + // popProcessNoopFunc never returns an error, but we handle this anyway to make linter + // happy. + return + } } } diff --git a/beacon-chain/cache/eth1_data.go b/beacon-chain/cache/eth1_data.go index f35bf7e9e..f053b8b6f 100644 --- a/beacon-chain/cache/eth1_data.go +++ b/beacon-chain/cache/eth1_data.go @@ -125,7 +125,10 @@ func (c *Eth1DataVoteCache) IncrementEth1DataVote(eth1DataHash [32]byte) (uint64 eth1DataVoteCacheHit.Inc() - eInfo, _ := obj.(*Eth1DataVote) + eInfo, ok := obj.(*Eth1DataVote) + if !ok { + return 0, errors.New("cached value is not of type *Eth1DataVote") + } eInfo.VoteCount++ if err := c.eth1DataVoteCache.Add(eInfo); err != nil { diff --git a/beacon-chain/core/epoch/epoch_processing.go b/beacon-chain/core/epoch/epoch_processing.go index e2347c83e..94dff9123 100644 --- a/beacon-chain/core/epoch/epoch_processing.go +++ b/beacon-chain/core/epoch/epoch_processing.go @@ -351,7 +351,11 @@ func unslashedAttestingIndices(state *stateTrie.BeaconState, atts []*pb.PendingA sort.Slice(setIndices, func(i, j int) bool { return setIndices[i] < setIndices[j] }) // Remove the slashed validator indices. for i := 0; i < len(setIndices); i++ { - if v, _ := state.ValidatorAtIndex(setIndices[i]); v != nil && v.Slashed { + v, err := state.ValidatorAtIndex(setIndices[i]) + if err != nil { + return nil, errors.Wrap(err, "failed to look up validator") + } + if v != nil && v.Slashed { setIndices = append(setIndices[:i], setIndices[i+1:]...) } } diff --git a/beacon-chain/core/epoch/precompute/new.go b/beacon-chain/core/epoch/precompute/new.go index 06d150eb5..e0889a6f8 100644 --- a/beacon-chain/core/epoch/precompute/new.go +++ b/beacon-chain/core/epoch/precompute/new.go @@ -3,6 +3,7 @@ package precompute import ( "context" + "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" "github.com/prysmaticlabs/prysm/shared/params" @@ -12,7 +13,7 @@ import ( // New gets called at the beginning of process epoch cycle to return // pre computed instances of validators attesting records and total // balances attested in an epoch. -func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Balance) { +func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Balance, error) { ctx, span := trace.StartSpan(ctx, "precomputeEpoch.New") defer span.End() vp := make([]*Validator, state.NumValidators()) @@ -21,7 +22,7 @@ func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Bala currentEpoch := helpers.CurrentEpoch(state) prevEpoch := helpers.PrevEpoch(state) - state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { + if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { // Was validator withdrawable or slashed withdrawable := currentEpoch >= val.WithdrawableEpoch() p := &Validator{ @@ -46,6 +47,8 @@ func New(ctx context.Context, state *stateTrie.BeaconState) ([]*Validator, *Bala vp[idx] = p return nil - }) - return vp, bp + }); err != nil { + return nil, nil, errors.Wrap(err, "failed to initialize precompute") + } + return vp, bp, nil } diff --git a/beacon-chain/core/epoch/precompute/new_test.go b/beacon-chain/core/epoch/precompute/new_test.go index 101b8bf37..78620cd28 100644 --- a/beacon-chain/core/epoch/precompute/new_test.go +++ b/beacon-chain/core/epoch/precompute/new_test.go @@ -31,7 +31,7 @@ func TestNew(t *testing.T) { t.Fatal(err) } e := params.BeaconConfig().FarFutureEpoch - v, b := precompute.New(context.Background(), s) + v, b, _ := precompute.New(context.Background(), s) if !reflect.DeepEqual(v[0], &precompute.Validator{IsSlashed: true, CurrentEpochEffectiveBalance: 100, InclusionDistance: e, InclusionSlot: e}) { t.Error("Incorrect validator 0 status") diff --git a/beacon-chain/core/epoch/precompute/reward_penalty_test.go b/beacon-chain/core/epoch/precompute/reward_penalty_test.go index e8a601518..465284e01 100644 --- a/beacon-chain/core/epoch/precompute/reward_penalty_test.go +++ b/beacon-chain/core/epoch/precompute/reward_penalty_test.go @@ -35,7 +35,7 @@ func TestProcessRewardsAndPenaltiesPrecompute(t *testing.T) { t.Fatal(err) } - vp, bp := New(context.Background(), state) + vp, bp, _ := New(context.Background(), state) vp, bp, err = ProcessAttestations(context.Background(), state, vp, bp) if err != nil { t.Fatal(err) @@ -88,7 +88,7 @@ func TestAttestationDeltaPrecompute(t *testing.T) { t.Fatal(err) } - vp, bp := New(context.Background(), state) + vp, bp, _ := New(context.Background(), state) vp, bp, err = ProcessAttestations(context.Background(), state, vp, bp) if err != nil { t.Fatal(err) @@ -173,7 +173,7 @@ func TestAttestationDeltas_ZeroEpoch(t *testing.T) { t.Fatal(err) } - vp, bp := New(context.Background(), state) + vp, bp, _ := New(context.Background(), state) vp, bp, err = ProcessAttestations(context.Background(), state, vp, bp) if err != nil { t.Fatal(err) diff --git a/beacon-chain/core/epoch/spectest/justification_and_finalization_test.go b/beacon-chain/core/epoch/spectest/justification_and_finalization_test.go index c2381bd2e..92c344112 100644 --- a/beacon-chain/core/epoch/spectest/justification_and_finalization_test.go +++ b/beacon-chain/core/epoch/spectest/justification_and_finalization_test.go @@ -28,7 +28,7 @@ func runJustificationAndFinalizationTests(t *testing.T, config string) { func processJustificationAndFinalizationPrecomputeWrapper(t *testing.T, state *state.BeaconState) (*state.BeaconState, error) { ctx := context.Background() - vp, bp := precompute.New(ctx, state) + vp, bp, _ := precompute.New(ctx, state) _, bp, err := precompute.ProcessAttestations(ctx, state, vp, bp) if err != nil { t.Fatal(err) diff --git a/beacon-chain/core/epoch/spectest/slashings_test.go b/beacon-chain/core/epoch/spectest/slashings_test.go index 768e8a162..e2db5cefc 100644 --- a/beacon-chain/core/epoch/spectest/slashings_test.go +++ b/beacon-chain/core/epoch/spectest/slashings_test.go @@ -37,7 +37,7 @@ func processSlashingsWrapper(t *testing.T, state *beaconstate.BeaconState) (*bea func processSlashingsPrecomputeWrapper(t *testing.T, state *beaconstate.BeaconState) (*beaconstate.BeaconState, error) { ctx := context.Background() - vp, bp := precompute.New(ctx, state) + vp, bp, _ := precompute.New(ctx, state) _, bp, err := precompute.ProcessAttestations(ctx, state, vp, bp) if err != nil { t.Fatal(err) diff --git a/beacon-chain/core/helpers/committee.go b/beacon-chain/core/helpers/committee.go index eaa90fd29..2485c794e 100644 --- a/beacon-chain/core/helpers/committee.go +++ b/beacon-chain/core/helpers/committee.go @@ -354,12 +354,14 @@ func ShuffledIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint64, erro } indices := make([]uint64, 0, state.NumValidators()) - state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { + if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { if IsActiveValidatorUsingTrie(val, epoch) { indices = append(indices, uint64(idx)) } return nil - }) + }); err != nil { + return nil, err + } return UnshuffleList(indices, seed) } diff --git a/beacon-chain/core/helpers/rewards_penalties.go b/beacon-chain/core/helpers/rewards_penalties.go index 9e411ce52..16bac0ecd 100644 --- a/beacon-chain/core/helpers/rewards_penalties.go +++ b/beacon-chain/core/helpers/rewards_penalties.go @@ -43,12 +43,14 @@ func TotalBalance(state *stateTrie.BeaconState, indices []uint64) uint64 { // return get_total_balance(state, set(get_active_validator_indices(state, get_current_epoch(state)))) func TotalActiveBalance(state *stateTrie.BeaconState) (uint64, error) { total := uint64(0) - state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { + if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { if IsActiveValidatorUsingTrie(val, SlotToEpoch(state.Slot())) { total += val.EffectiveBalance() } return nil - }) + }); err != nil { + return 0, err + } return total, nil } diff --git a/beacon-chain/core/helpers/validators.go b/beacon-chain/core/helpers/validators.go index ac35a29f6..bdb8acd29 100644 --- a/beacon-chain/core/helpers/validators.go +++ b/beacon-chain/core/helpers/validators.go @@ -74,12 +74,14 @@ func ActiveValidatorIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint6 return activeIndices, nil } var indices []uint64 - state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { + if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { if IsActiveValidatorUsingTrie(val, epoch) { indices = append(indices, uint64(idx)) } return nil - }) + }); err != nil { + return nil, err + } if err := UpdateCommitteeCache(state, epoch); err != nil { return nil, errors.Wrap(err, "could not update committee cache") @@ -92,12 +94,14 @@ func ActiveValidatorIndices(state *stateTrie.BeaconState, epoch uint64) ([]uint6 // at the given epoch. func ActiveValidatorCount(state *stateTrie.BeaconState, epoch uint64) (uint64, error) { count := uint64(0) - state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { + if err := state.ReadFromEveryValidator(func(idx int, val *stateTrie.ReadOnlyValidator) error { if IsActiveValidatorUsingTrie(val, epoch) { count++ } return nil - }) + }); err != nil { + return 0, err + } return count, nil } diff --git a/beacon-chain/core/state/BUILD.bazel b/beacon-chain/core/state/BUILD.bazel index 9c616c34b..fe1aa1a56 100644 --- a/beacon-chain/core/state/BUILD.bazel +++ b/beacon-chain/core/state/BUILD.bazel @@ -35,6 +35,7 @@ go_library( "@com_github_pkg_errors//:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", "@com_github_prysmaticlabs_go_ssz//:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", "@io_opencensus_go//trace:go_default_library", ], ) diff --git a/beacon-chain/core/state/transition.go b/beacon-chain/core/state/transition.go index 54457317e..5afff49d6 100644 --- a/beacon-chain/core/state/transition.go +++ b/beacon-chain/core/state/transition.go @@ -21,6 +21,7 @@ import ( "github.com/prysmaticlabs/prysm/shared/mathutil" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/traceutil" + "github.com/sirupsen/logrus" "go.opencensus.io/trace" ) @@ -295,14 +296,21 @@ func ProcessSlots(ctx context.Context, state *stateTrie.BeaconState, slot uint64 } else if err != nil { return nil, err } - defer SkipSlotCache.MarkNotInProgress(key) + defer func() { + if err := SkipSlotCache.MarkNotInProgress(key); err != nil { + traceutil.AnnotateError(span, err) + logrus.WithError(err).Error("Failed to mark skip slot no longer in progress") + } + }() for state.Slot() < slot { if ctx.Err() != nil { traceutil.AnnotateError(span, ctx.Err()) // Cache last best value. if highestSlot < state.Slot() { - SkipSlotCache.Put(ctx, key, state) + if err := SkipSlotCache.Put(ctx, key, state); err != nil { + logrus.WithError(err).Error("Failed to put skip slot cache value") + } } return nil, ctx.Err() } @@ -318,11 +326,17 @@ func ProcessSlots(ctx context.Context, state *stateTrie.BeaconState, slot uint64 return nil, errors.Wrap(err, "could not process epoch with optimizations") } } - state.SetSlot(state.Slot() + 1) + if err := state.SetSlot(state.Slot() + 1); err != nil { + traceutil.AnnotateError(span, err) + return nil, errors.Wrap(err, "failed to increment state slot") + } } if highestSlot < state.Slot() { - SkipSlotCache.Put(ctx, key, state) + if err := SkipSlotCache.Put(ctx, key, state); err != nil { + logrus.WithError(err).Error("Failed to put skip slot cache value") + traceutil.AnnotateError(span, err) + } } return state, nil @@ -605,8 +619,11 @@ func ProcessEpochPrecompute(ctx context.Context, state *stateTrie.BeaconState) ( if state == nil { return nil, errors.New("nil state") } - vp, bp := precompute.New(ctx, state) - vp, bp, err := precompute.ProcessAttestations(ctx, state, vp, bp) + vp, bp, err := precompute.New(ctx, state) + if err != nil { + return nil, err + } + vp, bp, err = precompute.ProcessAttestations(ctx, state, vp, bp) if err != nil { return nil, err } diff --git a/beacon-chain/core/state/transition_test.go b/beacon-chain/core/state/transition_test.go index 9d519493d..984a87b6c 100644 --- a/beacon-chain/core/state/transition_test.go +++ b/beacon-chain/core/state/transition_test.go @@ -531,6 +531,7 @@ func TestProcessEpochPrecompute_CanProcess(t *testing.T) { FinalizedCheckpoint: ðpb.Checkpoint{}, JustificationBits: bitfield.Bitvector4{0x00}, CurrentJustifiedCheckpoint: ðpb.Checkpoint{}, + Validators: []*ethpb.Validator{}, } s, err := beaconstate.InitializeFromProto(base) if err != nil { diff --git a/beacon-chain/db/kv/archived_point.go b/beacon-chain/db/kv/archived_point.go index d108b0795..b421349af 100644 --- a/beacon-chain/db/kv/archived_point.go +++ b/beacon-chain/db/kv/archived_point.go @@ -54,8 +54,7 @@ func (k *Store) LastArchivedIndexRoot(ctx context.Context) [32]byte { defer span.End() var blockRoot []byte - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(archivedIndexRootBucket) lastArchivedIndex := bucket.Get(lastArchivedIndexKey) if lastArchivedIndex == nil { @@ -63,7 +62,9 @@ func (k *Store) LastArchivedIndexRoot(ctx context.Context) [32]byte { } blockRoot = bucket.Get(lastArchivedIndex) return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return bytesutil.ToBytes32(blockRoot) } @@ -75,12 +76,13 @@ func (k *Store) ArchivedPointRoot(ctx context.Context, index uint64) [32]byte { defer span.End() var blockRoot []byte - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(archivedIndexRootBucket) blockRoot = bucket.Get(bytesutil.Uint64ToBytes(index)) return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return bytesutil.ToBytes32(blockRoot) } @@ -90,11 +92,12 @@ func (k *Store) HasArchivedPoint(ctx context.Context, index uint64) bool { ctx, span := trace.StartSpan(ctx, "BeaconDB.HasArchivedPoint") defer span.End() var exists bool - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { iBucket := tx.Bucket(archivedIndexRootBucket) exists = iBucket.Get(bytesutil.Uint64ToBytes(index)) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } diff --git a/beacon-chain/db/kv/attestations.go b/beacon-chain/db/kv/attestations.go index 223365996..764d6c505 100644 --- a/beacon-chain/db/kv/attestations.go +++ b/beacon-chain/db/kv/attestations.go @@ -83,12 +83,13 @@ func (k *Store) HasAttestation(ctx context.Context, attDataRoot [32]byte) bool { ctx, span := trace.StartSpan(ctx, "BeaconDB.HasAttestation") defer span.End() exists := false - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(attestationsBucket) exists = bkt.Get(attDataRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } @@ -281,19 +282,34 @@ func createAttestationIndicesFromFilters(f *filters.QueryFilter) (map[string][]b for k, v := range f.Filters() { switch k { case filters.HeadBlockRoot: - headBlockRoot := v.([]byte) + headBlockRoot, ok := v.([]byte) + if !ok { + return nil, errors.New("headBlockRoot is not type []byte") + } indicesByBucket[string(attestationHeadBlockRootBucket)] = headBlockRoot case filters.SourceRoot: - sourceRoot := v.([]byte) + sourceRoot, ok := v.([]byte) + if !ok { + return nil, errors.New("sourceRoot is not type []byte") + } indicesByBucket[string(attestationSourceRootIndicesBucket)] = sourceRoot case filters.SourceEpoch: - sourceEpoch := v.(uint64) + sourceEpoch, ok := v.(uint64) + if !ok { + return nil, errors.New("sourceEpoch is not type uint64") + } indicesByBucket[string(attestationSourceEpochIndicesBucket)] = bytesutil.Uint64ToBytes(sourceEpoch) case filters.TargetEpoch: - targetEpoch := v.(uint64) + targetEpoch, ok := v.(uint64) + if !ok { + return nil, errors.New("targetEpoch is not type uint64") + } indicesByBucket[string(attestationTargetEpochIndicesBucket)] = bytesutil.Uint64ToBytes(targetEpoch) case filters.TargetRoot: - targetRoot := v.([]byte) + targetRoot, ok := v.([]byte) + if !ok { + return nil, errors.New("targetRoot is not type []byte") + } indicesByBucket[string(attestationTargetRootIndicesBucket)] = targetRoot default: return nil, fmt.Errorf("filter criterion %v not supported for attestations", k) diff --git a/beacon-chain/db/kv/backup.go b/beacon-chain/db/kv/backup.go index ac3b71db1..6880caaa6 100644 --- a/beacon-chain/db/kv/backup.go +++ b/beacon-chain/db/kv/backup.go @@ -39,7 +39,11 @@ func (k *Store) Backup(ctx context.Context) error { if err != nil { panic(err) } - defer copyDB.Close() + defer func() { + if err := copyDB.Close(); err != nil { + logrus.WithError(err).Error("Failed to close destination database") + } + }() return k.db.View(func(tx *bolt.Tx) error { return tx.ForEach(func(name []byte, b *bolt.Bucket) error { diff --git a/beacon-chain/db/kv/blocks.go b/beacon-chain/db/kv/blocks.go index f0cbaccfe..e356648ff 100644 --- a/beacon-chain/db/kv/blocks.go +++ b/beacon-chain/db/kv/blocks.go @@ -120,12 +120,13 @@ func (k *Store) HasBlock(ctx context.Context, blockRoot [32]byte) bool { return true } exists := false - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(blocksBucket) exists = bkt.Get(blockRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } @@ -590,7 +591,10 @@ func createBlockIndicesFromFilters(f *filters.QueryFilter) (map[string][]byte, e for k, v := range f.Filters() { switch k { case filters.ParentRoot: - parentRoot := v.([]byte) + parentRoot, ok := v.([]byte) + if !ok { + return nil, errors.New("parent root is not []byte") + } indicesByBucket[string(blockParentRootIndicesBucket)] = parentRoot case filters.StartSlot: case filters.EndSlot: diff --git a/beacon-chain/db/kv/check_historical_state.go b/beacon-chain/db/kv/check_historical_state.go index 6210cd400..c27c8adfe 100644 --- a/beacon-chain/db/kv/check_historical_state.go +++ b/beacon-chain/db/kv/check_historical_state.go @@ -20,12 +20,14 @@ func (kv *Store) ensureNewStateServiceCompatible(ctx context.Context) error { } var historicalStateDeleted bool - kv.db.View(func(tx *bolt.Tx) error { + if err := kv.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(newStateServiceCompatibleBucket) v := bkt.Get(historicalStateDeletedKey) historicalStateDeleted = len(v) == 1 && v[0] == 0x01 return nil - }) + }); err != nil { + return err + } regenHistoricalStatesConfirmed := false var err error diff --git a/beacon-chain/db/kv/deposit_contract.go b/beacon-chain/db/kv/deposit_contract.go index a05bacd89..577a5403c 100644 --- a/beacon-chain/db/kv/deposit_contract.go +++ b/beacon-chain/db/kv/deposit_contract.go @@ -15,12 +15,13 @@ func (k *Store) DepositContractAddress(ctx context.Context) ([]byte, error) { ctx, span := trace.StartSpan(ctx, "BeaconDB.DepositContractAddress") defer span.End() var addr []byte - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { chainInfo := tx.Bucket(chainMetadataBucket) addr = chainInfo.Get(depositContractAddressKey) return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return addr, nil } diff --git a/beacon-chain/db/kv/operations.go b/beacon-chain/db/kv/operations.go index bd003627a..1610591b7 100644 --- a/beacon-chain/db/kv/operations.go +++ b/beacon-chain/db/kv/operations.go @@ -31,12 +31,13 @@ func (k *Store) HasVoluntaryExit(ctx context.Context, exitRoot [32]byte) bool { ctx, span := trace.StartSpan(ctx, "BeaconDB.HasVoluntaryExit") defer span.End() exists := false - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(voluntaryExitsBucket) exists = bkt.Get(exitRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } diff --git a/beacon-chain/db/kv/regen_historical_states.go b/beacon-chain/db/kv/regen_historical_states.go index d0931578b..a3eebfbf5 100644 --- a/beacon-chain/db/kv/regen_historical_states.go +++ b/beacon-chain/db/kv/regen_historical_states.go @@ -148,7 +148,9 @@ func regenHistoricalStateProcessSlots(ctx context.Context, state *stateTrie.Beac return nil, errors.Wrap(err, "could not process epoch with optimizations") } } - state.SetSlot(state.Slot() + 1) + if err := state.SetSlot(state.Slot() + 1); err != nil { + return nil, err + } } return state, nil } diff --git a/beacon-chain/db/kv/slashings.go b/beacon-chain/db/kv/slashings.go index 6baa160f2..b64de8415 100644 --- a/beacon-chain/db/kv/slashings.go +++ b/beacon-chain/db/kv/slashings.go @@ -31,12 +31,13 @@ func (k *Store) HasProposerSlashing(ctx context.Context, slashingRoot [32]byte) ctx, span := trace.StartSpan(ctx, "BeaconDB.HasProposerSlashing") defer span.End() exists := false - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(proposerSlashingsBucket) exists = bkt.Get(slashingRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } @@ -90,12 +91,13 @@ func (k *Store) HasAttesterSlashing(ctx context.Context, slashingRoot [32]byte) ctx, span := trace.StartSpan(ctx, "BeaconDB.HasAttesterSlashing") defer span.End() exists := false - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bkt := tx.Bucket(attesterSlashingsBucket) exists = bkt.Get(slashingRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } diff --git a/beacon-chain/db/kv/state.go b/beacon-chain/db/kv/state.go index 14d4e6ea3..9a9343d14 100644 --- a/beacon-chain/db/kv/state.go +++ b/beacon-chain/db/kv/state.go @@ -163,12 +163,13 @@ func (k *Store) HasState(ctx context.Context, blockRoot [32]byte) bool { ctx, span := trace.StartSpan(ctx, "BeaconDB.HasState") defer span.End() var exists bool - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(stateBucket) exists = bucket.Get(blockRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } diff --git a/beacon-chain/db/kv/state_summary.go b/beacon-chain/db/kv/state_summary.go index 2f9f2ffa5..9c9718d83 100644 --- a/beacon-chain/db/kv/state_summary.go +++ b/beacon-chain/db/kv/state_summary.go @@ -67,11 +67,12 @@ func (k *Store) HasStateSummary(ctx context.Context, blockRoot [32]byte) bool { ctx, span := trace.StartSpan(ctx, "BeaconDB.HasStateSummary") defer span.End() var exists bool - // #nosec G104. Always returns nil. - k.db.View(func(tx *bolt.Tx) error { + if err := k.db.View(func(tx *bolt.Tx) error { bucket := tx.Bucket(stateSummaryBucket) exists = bucket.Get(blockRoot[:]) != nil return nil - }) + }); err != nil { // This view never returns an error, but we'll handle anyway for sanity. + panic(err) + } return exists } diff --git a/beacon-chain/p2p/connmgr/connmgr.go b/beacon-chain/p2p/connmgr/connmgr.go index 027d2d696..9fd7fa055 100644 --- a/beacon-chain/p2p/connmgr/connmgr.go +++ b/beacon-chain/p2p/connmgr/connmgr.go @@ -222,7 +222,9 @@ func (cm *BasicConnMgr) TrimOpenConns(ctx context.Context) { for _, c := range cm.getConnsToClose(ctx) { log.Info("closing conn: ", c.RemotePeer()) log.Event(ctx, "closeConn", c.RemotePeer()) - c.Close() + if err := c.Close(); err != nil { + log.Errorf("Failed to close connection: %v", err) + } } cm.lastTrim = roughtime.Now() diff --git a/beacon-chain/p2p/service.go b/beacon-chain/p2p/service.go index ce94db065..0f3dfcba6 100644 --- a/beacon-chain/p2p/service.go +++ b/beacon-chain/p2p/service.go @@ -66,11 +66,14 @@ type Service struct { func NewService(cfg *Config) (*Service, error) { var err error ctx, cancel := context.WithCancel(context.Background()) - cache, _ := ristretto.NewCache(&ristretto.Config{ + cache, err := ristretto.NewCache(&ristretto.Config{ NumCounters: 1000, MaxCost: 1000, BufferItems: 64, }) + if err != nil { + return nil, err + } s := &Service{ ctx: ctx, diff --git a/beacon-chain/powchain/block_cache.go b/beacon-chain/powchain/block_cache.go index 39770fb4e..76752bb4b 100644 --- a/beacon-chain/powchain/block_cache.go +++ b/beacon-chain/powchain/block_cache.go @@ -172,7 +172,9 @@ func (b *blockCache) AddBlock(blk *gethTypes.Block) error { func trim(queue *cache.FIFO, maxSize int) { for s := len(queue.ListKeys()); s > maxSize; s-- { // #nosec G104 popProcessNoopFunc never returns an error - _, _ = queue.Pop(popProcessNoopFunc) + if _, err := queue.Pop(popProcessNoopFunc); err != nil { // This never returns an error, but we'll handle anyway for sanity. + panic(err) + } } } diff --git a/beacon-chain/powchain/deposit.go b/beacon-chain/powchain/deposit.go index 5e4eada79..460a46287 100644 --- a/beacon-chain/powchain/deposit.go +++ b/beacon-chain/powchain/deposit.go @@ -9,7 +9,9 @@ import ( func (s *Service) processDeposit(eth1Data *ethpb.Eth1Data, deposit *ethpb.Deposit) error { var err error - s.preGenesisState.SetEth1Data(eth1Data) + if err := s.preGenesisState.SetEth1Data(eth1Data); err != nil { + return err + } s.preGenesisState, err = blocks.ProcessPreGenesisDeposit(context.Background(), s.preGenesisState, deposit) return err } diff --git a/beacon-chain/powchain/log_processing.go b/beacon-chain/powchain/log_processing.go index f57dc3ca9..19b2f019e 100644 --- a/beacon-chain/powchain/log_processing.go +++ b/beacon-chain/powchain/log_processing.go @@ -448,7 +448,10 @@ func (s *Service) checkHeaderRange(start uint64, end uint64, } func (s *Service) checkForChainstart(blockHash [32]byte, blockNumber *big.Int, blockTime uint64) { - valCount, _ := helpers.ActiveValidatorCount(s.preGenesisState, 0) + valCount, err := helpers.ActiveValidatorCount(s.preGenesisState, 0) + if err != nil { + log.WithError(err).Error("Could not determine active validator count from pref genesis state") + } triggered := state.IsValidGenesisState(valCount, s.createGenesisTime(blockTime)) if triggered { s.chainStartData.GenesisTime = s.createGenesisTime(blockTime) diff --git a/beacon-chain/powchain/service.go b/beacon-chain/powchain/service.go index 1dd09a93c..25009296f 100644 --- a/beacon-chain/powchain/service.go +++ b/beacon-chain/powchain/service.go @@ -499,7 +499,9 @@ func (s *Service) batchRequestHeaders(startBlock uint64, endBlock uint64) ([]*ge } for _, h := range headers { if h != nil { - s.blockCache.AddBlock(gethTypes.NewBlockWithHeader(h)) + if err := s.blockCache.AddBlock(gethTypes.NewBlockWithHeader(h)); err != nil { + return nil, err + } } } return headers, nil diff --git a/beacon-chain/rpc/beacon/assignments_test.go b/beacon-chain/rpc/beacon/assignments_test.go index 364c52ba0..34dc53cc3 100644 --- a/beacon-chain/rpc/beacon/assignments_test.go +++ b/beacon-chain/rpc/beacon/assignments_test.go @@ -60,6 +60,7 @@ func TestServer_ListAssignments_NoResults(t *testing.T) { st, err := stateTrie.InitializeFromProto(&pbp2p.BeaconState{ Slot: 0, RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector), + Validators: []*ethpb.Validator{}, }) if err != nil { t.Fatal(err) diff --git a/beacon-chain/rpc/beacon/slashings.go b/beacon-chain/rpc/beacon/slashings.go index f8534acc1..5326cf7b5 100644 --- a/beacon-chain/rpc/beacon/slashings.go +++ b/beacon-chain/rpc/beacon/slashings.go @@ -25,7 +25,9 @@ func (bs *Server) SubmitProposerSlashing( return nil, status.Errorf(codes.Internal, "Could not insert proposer slashing into pool: %v", err) } if featureconfig.Get().BroadcastSlashings { - bs.Broadcaster.Broadcast(ctx, req) + if err := bs.Broadcaster.Broadcast(ctx, req); err != nil { + return nil, err + } } return ðpb.SubmitSlashingResponse{ SlashedIndices: []uint64{req.ProposerIndex}, @@ -47,7 +49,9 @@ func (bs *Server) SubmitAttesterSlashing( return nil, status.Errorf(codes.Internal, "Could not insert attester slashing into pool: %v", err) } if featureconfig.Get().BroadcastSlashings { - bs.Broadcaster.Broadcast(ctx, req) + if err := bs.Broadcaster.Broadcast(ctx, req); err != nil { + return nil, err + } } slashedIndices := sliceutil.IntersectionUint64(req.Attestation_1.AttestingIndices, req.Attestation_2.AttestingIndices) return ðpb.SubmitSlashingResponse{ diff --git a/beacon-chain/rpc/beacon/validators_stream.go b/beacon-chain/rpc/beacon/validators_stream.go index 1153d9db2..da078d458 100644 --- a/beacon-chain/rpc/beacon/validators_stream.go +++ b/beacon-chain/rpc/beacon/validators_stream.go @@ -2,6 +2,7 @@ package beacon import ( "context" + "errors" "fmt" "io" "math/big" @@ -331,7 +332,11 @@ func (is *infostream) generatePendingValidatorInfo(info *ethpb.ValidatorInfo) (* is.eth1DepositsMutex.Lock() if fetchedDeposit, exists := is.eth1Deposits.Get(key); exists { eth1DepositCacheHits.Inc() - deposit = fetchedDeposit.(*eth1Deposit) + var ok bool + deposit, ok = fetchedDeposit.(*eth1Deposit) + if !ok { + return nil, errors.New("cached eth1 deposit is not type *eth1Deposit") + } } else { eth1DepositCacheMisses.Inc() fetchedDeposit, eth1BlockNumber := is.depositFetcher.DepositByPubkey(is.ctx, info.PublicKey) @@ -398,7 +403,10 @@ func (is *infostream) calculateActivationTimeForPendingValidators(res []*ethpb.V // Loop over epochs, roughly simulating progression. for curEpoch := epoch + 1; len(sortedIndices) > 0 && len(pendingValidators) > 0; curEpoch++ { - toProcess, _ := helpers.ValidatorChurnLimit(numAttestingValidators) + toProcess, err := helpers.ValidatorChurnLimit(numAttestingValidators) + if err != nil { + log.WithError(err).Error("Failed to determine validator churn limit") + } if toProcess > uint64(len(sortedIndices)) { toProcess = uint64(len(sortedIndices)) } @@ -491,7 +499,11 @@ func (is *infostream) depositQueueTimestamp(eth1BlockNumber *big.Int) (uint64, e is.eth1BlocktimesMutex.Lock() if cachedTimestamp, exists := is.eth1Blocktimes.Get(key); exists { eth1BlocktimeCacheHits.Inc() - blockTimestamp = cachedTimestamp.(uint64) + var ok bool + blockTimestamp, ok = cachedTimestamp.(uint64) + if !ok { + return 0, errors.New("cached timestamp is not type uint64") + } } else { eth1BlocktimeCacheMisses.Inc() var err error diff --git a/beacon-chain/rpc/service.go b/beacon-chain/rpc/service.go index 62e684518..3981e5566 100644 --- a/beacon-chain/rpc/service.go +++ b/beacon-chain/rpc/service.go @@ -324,7 +324,9 @@ func (s *Service) Stop() error { log.Debug("Initiated graceful stop of gRPC server") } if s.slasherConn != nil { - s.slasherConn.Close() + if err := s.slasherConn.Close(); err != nil { + return err + } } return nil } diff --git a/beacon-chain/rpc/validator/attester.go b/beacon-chain/rpc/validator/attester.go index f813ad395..d5f626e92 100644 --- a/beacon-chain/rpc/validator/attester.go +++ b/beacon-chain/rpc/validator/attester.go @@ -2,6 +2,7 @@ package validator import ( "context" + "errors" "time" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" @@ -19,6 +20,7 @@ import ( "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/roughtime" "github.com/prysmaticlabs/prysm/shared/slotutil" + "github.com/prysmaticlabs/prysm/shared/traceutil" "go.opencensus.io/trace" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -201,7 +203,7 @@ func (vs *Server) ProposeAttestation(ctx context.Context, att *ethpb.Attestation // waitToOneThird waits until one-third of the way through the slot // or the head slot equals to the input slot. func (vs *Server) waitToOneThird(ctx context.Context, slot uint64) { - _, span := trace.StartSpan(ctx, "validator.waitToOneThird") + ctx, span := trace.StartSpan(ctx, "validator.waitToOneThird") defer span.End() // Don't need to wait if current slot is greater than requested slot. @@ -224,7 +226,13 @@ func (vs *Server) waitToOneThird(ctx context.Context, slot uint64) { case event := <-stateChannel: // Node processed a block, check if the processed block is the same as input slot. if event.Type == statefeed.BlockProcessed { - d := event.Data.(*statefeed.BlockProcessedData) + d, ok := event.Data.(*statefeed.BlockProcessedData) + if !ok { + err := errors.New("event feed is not type *statefeed.BlockProcessedData") + traceutil.AnnotateError(span, err) + log.Error(err) + continue + } if slot == d.Slot { return } diff --git a/beacon-chain/rpc/validator/proposer.go b/beacon-chain/rpc/validator/proposer.go index 72f378f6d..69c7dddcc 100644 --- a/beacon-chain/rpc/validator/proposer.go +++ b/beacon-chain/rpc/validator/proposer.go @@ -325,7 +325,9 @@ func (vs *Server) canonicalEth1Data(ctx context.Context, beaconState *stateTrie. var eth1BlockHash [32]byte // Add in current vote, to get accurate vote tally - beaconState.AppendEth1DataVotes(currentVote) + if err := beaconState.AppendEth1DataVotes(currentVote); err != nil { + return nil, nil, errors.Wrap(err, "failed to append eth1 data votes to state") + } hasSupport, err := blocks.Eth1DataHasEnoughSupport(beaconState, currentVote) if err != nil { return nil, nil, errors.Wrap(err, "could not determine if current eth1data vote has enough support") diff --git a/beacon-chain/rpc/validator/server.go b/beacon-chain/rpc/validator/server.go index bd14168c2..2ed7b40ae 100644 --- a/beacon-chain/rpc/validator/server.go +++ b/beacon-chain/rpc/validator/server.go @@ -2,6 +2,7 @@ package validator import ( "context" + "errors" "time" "github.com/prysmaticlabs/prysm/beacon-chain/state/stategen" @@ -171,7 +172,10 @@ func (vs *Server) WaitForChainStart(req *ptypes.Empty, stream ethpb.BeaconNodeVa select { case event := <-stateChannel: if event.Type == statefeed.ChainStarted { - data := event.Data.(*statefeed.ChainStartedData) + data, ok := event.Data.(*statefeed.ChainStartedData) + if !ok { + return errors.New("event data is not type *statefeed.ChainStartData") + } log.WithField("starttime", data.StartTime).Debug("Received chain started event") log.Info("Sending genesis time notification to connected validator clients") res := ðpb.ChainStartResponse{ diff --git a/beacon-chain/state/setters.go b/beacon-chain/state/setters.go index ecba135f2..6b95e901a 100644 --- a/beacon-chain/state/setters.go +++ b/beacon-chain/state/setters.go @@ -43,7 +43,11 @@ func (b *BeaconState) SetFork(val *pbp2p.Fork) error { b.lock.Lock() defer b.lock.Unlock() - b.state.Fork = proto.Clone(val).(*pbp2p.Fork) + fk, ok := proto.Clone(val).(*pbp2p.Fork) + if !ok { + return errors.New("proto.Clone did not return a fork proto") + } + b.state.Fork = fk b.markFieldAsDirty(fork) return nil } diff --git a/beacon-chain/state/stategen/replay.go b/beacon-chain/state/stategen/replay.go index edd93587c..bff3e81a3 100644 --- a/beacon-chain/state/stategen/replay.go +++ b/beacon-chain/state/stategen/replay.go @@ -222,7 +222,9 @@ func processSlotsStateGen(ctx context.Context, state *stateTrie.BeaconState, slo return nil, errors.Wrap(err, "could not process epoch with optimizations") } } - state.SetSlot(state.Slot() + 1) + if err := state.SetSlot(state.Slot() + 1); err != nil { + return nil, err + } } return state, nil diff --git a/beacon-chain/state/stateutil/state_root.go b/beacon-chain/state/stateutil/state_root.go index 244b9d400..7a3ab0148 100644 --- a/beacon-chain/state/stateutil/state_root.go +++ b/beacon-chain/state/stateutil/state_root.go @@ -21,12 +21,15 @@ var nocachedHasher *stateRootHasher var cachedHasher *stateRootHasher func init() { - rootsCache, _ := ristretto.NewCache(&ristretto.Config{ + rootsCache, err := ristretto.NewCache(&ristretto.Config{ NumCounters: cacheSize, // number of keys to track frequency of (1M). MaxCost: 1 << 22, // maximum cost of cache (3MB). // 100,000 roots will take up approximately 3 MB in memory. BufferItems: 64, // number of keys per Get buffer. }) + if err != nil { + panic(err) + } // Temporarily disable roots cache until cache issues can be resolved. cachedHasher = &stateRootHasher{rootsCache: rootsCache} nocachedHasher = &stateRootHasher{} diff --git a/beacon-chain/sync/deadlines.go b/beacon-chain/sync/deadlines.go index 373d5fe4e..11bbab0be 100644 --- a/beacon-chain/sync/deadlines.go +++ b/beacon-chain/sync/deadlines.go @@ -17,11 +17,15 @@ func setRPCStreamDeadlines(stream network.Stream) { func setStreamReadDeadline(stream network.Stream, duration time.Duration) { // libp2p uses the system clock time for determining the deadline so we use // time.Now() instead of the synchronized roughtime.Now(). - stream.SetReadDeadline(time.Now().Add(duration)) + if err := stream.SetReadDeadline(time.Now().Add(duration)); err != nil { + log.WithError(err).Error("Failed to set stream deadline") + } } func setStreamWriteDeadline(stream network.Stream, duration time.Duration) { // libp2p uses the system clock time for determining the deadline so we use // time.Now() instead of the synchronized roughtime.Now(). - stream.SetWriteDeadline(time.Now().Add(duration)) + if err := stream.SetWriteDeadline(time.Now().Add(duration)); err != nil { + log.WithError(err).Error("Failed to set stream deadline") + } } diff --git a/beacon-chain/sync/initial-sync-old/round_robin.go b/beacon-chain/sync/initial-sync-old/round_robin.go index 3d79032fb..e16bb71fe 100644 --- a/beacon-chain/sync/initial-sync-old/round_robin.go +++ b/beacon-chain/sync/initial-sync-old/round_robin.go @@ -312,7 +312,11 @@ func (s *Service) requestBlocks(ctx context.Context, req *p2ppb.BeaconBlocksByRa if err != nil { return nil, errors.Wrap(err, "failed to send request to peer") } - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() resp := make([]*eth.SignedBeaconBlock, 0, req.Count) for { diff --git a/beacon-chain/sync/initial-sync-old/service.go b/beacon-chain/sync/initial-sync-old/service.go index 572fa0d5d..adc89b71e 100644 --- a/beacon-chain/sync/initial-sync-old/service.go +++ b/beacon-chain/sync/initial-sync-old/service.go @@ -86,7 +86,11 @@ func (s *Service) Start() { select { case event := <-stateChannel: if event.Type == statefeed.Initialized { - data := event.Data.(*statefeed.InitializedData) + data, ok := event.Data.(*statefeed.InitializedData) + if !ok { + log.Error("event data is not type *statefeed.InitializedData") + return + } log.WithField("starttime", data.StartTime).Debug("Received state initialized event") genesis = data.StartTime genesisSet = true diff --git a/beacon-chain/sync/initial-sync/blocks_fetcher.go b/beacon-chain/sync/initial-sync/blocks_fetcher.go index 4ca45bf13..dcaad9888 100644 --- a/beacon-chain/sync/initial-sync/blocks_fetcher.go +++ b/beacon-chain/sync/initial-sync/blocks_fetcher.go @@ -383,7 +383,11 @@ func (f *blocksFetcher) requestBlocks( if err != nil { return nil, err } - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() resp := make([]*eth.SignedBeaconBlock, 0, req.Count) for { diff --git a/beacon-chain/sync/initial-sync/blocks_queue.go b/beacon-chain/sync/initial-sync/blocks_queue.go index ffcd28032..ff3b307cc 100644 --- a/beacon-chain/sync/initial-sync/blocks_queue.go +++ b/beacon-chain/sync/initial-sync/blocks_queue.go @@ -25,9 +25,10 @@ const ( ) var ( - errQueueCtxIsDone = errors.New("queue's context is done, reinitialize") - errQueueTakesTooLongToStop = errors.New("queue takes too long to stop") - errNoEpochState = errors.New("epoch state not found") + errQueueCtxIsDone = errors.New("queue's context is done, reinitialize") + errQueueTakesTooLongToStop = errors.New("queue takes too long to stop") + errNoEpochState = errors.New("epoch state not found") + errInputNotFetchRequestParams = errors.New("input data is not type *fetchRequestParams") ) // blocksProvider exposes enough methods for queue to fetch incoming blocks. @@ -221,7 +222,10 @@ func (q *blocksQueue) loop() { // onScheduleEvent is an event called on newly arrived epochs. Transforms state to scheduled. func (q *blocksQueue) onScheduleEvent(ctx context.Context) eventHandlerFn { return func(es *epochState, in interface{}) (stateID, error) { - data := in.(*fetchRequestParams) + data, ok := in.(*fetchRequestParams) + if !ok { + return 0, errInputNotFetchRequestParams + } if err := q.blocksFetcher.scheduleRequest(ctx, data.start, data.count); err != nil { return es.state, err } @@ -236,7 +240,10 @@ func (q *blocksQueue) onDataReceivedEvent(ctx context.Context) eventHandlerFn { return es.state, ctx.Err() } - response := in.(*fetchRequestResponse) + response, ok := in.(*fetchRequestResponse) + if !ok { + return 0, errInputNotFetchRequestParams + } epoch := helpers.SlotToEpoch(response.start) if response.err != nil { // Current window is already too big, re-request previous epochs. @@ -267,7 +274,10 @@ func (q *blocksQueue) onReadyToSendEvent(ctx context.Context) eventHandlerFn { return es.state, ctx.Err() } - data := in.(*fetchRequestParams) + data, ok := in.(*fetchRequestParams) + if !ok { + return 0, errInputNotFetchRequestParams + } epoch := helpers.SlotToEpoch(data.start) ind, ok := q.state.findEpochState(epoch) if !ok { @@ -317,7 +327,10 @@ func (q *blocksQueue) onExtendWindowEvent(ctx context.Context) eventHandlerFn { return es.state, ctx.Err() } - data := in.(*fetchRequestParams) + data, ok := in.(*fetchRequestParams) + if !ok { + return 0, errInputNotFetchRequestParams + } epoch := helpers.SlotToEpoch(data.start) if _, ok := q.state.findEpochState(epoch); !ok { return es.state, errNoEpochState diff --git a/beacon-chain/sync/initial-sync/round_robin.go b/beacon-chain/sync/initial-sync/round_robin.go index d9f29c24c..23ae4a49d 100644 --- a/beacon-chain/sync/initial-sync/round_robin.go +++ b/beacon-chain/sync/initial-sync/round_robin.go @@ -137,7 +137,11 @@ func (s *Service) requestBlocks(ctx context.Context, req *p2ppb.BeaconBlocksByRa if err != nil { return nil, errors.Wrap(err, "failed to send request to peer") } - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() resp := make([]*eth.SignedBeaconBlock, 0, req.Count) for { diff --git a/beacon-chain/sync/initial-sync/service.go b/beacon-chain/sync/initial-sync/service.go index 524570539..6ade46221 100644 --- a/beacon-chain/sync/initial-sync/service.go +++ b/beacon-chain/sync/initial-sync/service.go @@ -86,7 +86,11 @@ func (s *Service) Start() { select { case event := <-stateChannel: if event.Type == statefeed.Initialized { - data := event.Data.(*statefeed.InitializedData) + data, ok := event.Data.(*statefeed.InitializedData) + if !ok { + log.Error("Event feed data is not type *statefeed.InitializedData") + continue + } log.WithField("starttime", data.StartTime).Debug("Received state initialized event") genesis = data.StartTime genesisSet = true diff --git a/beacon-chain/sync/pending_attestations_queue.go b/beacon-chain/sync/pending_attestations_queue.go index 5ba160fa4..0f57cbfc5 100644 --- a/beacon-chain/sync/pending_attestations_queue.go +++ b/beacon-chain/sync/pending_attestations_queue.go @@ -6,6 +6,7 @@ import ( "sync" "time" + "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/shared/bls" @@ -125,7 +126,11 @@ func (s *Service) processPendingAtts(ctx context.Context) error { pid := pids[rand.Int()%len(pids)] targetSlot := helpers.SlotToEpoch(attestations[0].Aggregate.Data.Target.Epoch) for _, p := range pids { - if cs, _ := s.p2p.Peers().ChainState(p); cs != nil && cs.HeadSlot >= targetSlot { + cs, err := s.p2p.Peers().ChainState(p) + if err != nil { + return errors.Wrap(err, "could not get chain state for peer") + } + if cs != nil && cs.HeadSlot >= targetSlot { pid = p break } diff --git a/beacon-chain/sync/pending_blocks_queue.go b/beacon-chain/sync/pending_blocks_queue.go index 5cf9922f3..27bb20b0c 100644 --- a/beacon-chain/sync/pending_blocks_queue.go +++ b/beacon-chain/sync/pending_blocks_queue.go @@ -28,7 +28,9 @@ func (r *Service) processPendingBlocksQueue() { locker := new(sync.Mutex) runutil.RunEvery(r.ctx, processPendingBlocksPeriod, func() { locker.Lock() - r.processPendingBlocks(ctx) + if err := r.processPendingBlocks(ctx); err != nil { + log.WithError(err).Error("Failed to process pending blocks") + } locker.Unlock() }) } @@ -80,7 +82,11 @@ func (r *Service) processPendingBlocks(ctx context.Context) error { // have a head slot newer than the block slot we are requesting. pid := pids[rand.Int()%len(pids)] for _, p := range pids { - if cs, _ := r.p2p.Peers().ChainState(p); cs != nil && cs.HeadSlot >= uint64(s) { + cs, err := r.p2p.Peers().ChainState(p) + if err != nil { + return errors.Wrap(err, "failed to read chain state for peer") + } + if cs != nil && cs.HeadSlot >= uint64(s) { pid = p break } diff --git a/beacon-chain/sync/rpc.go b/beacon-chain/sync/rpc.go index 8e1a9819f..8ab08fbbf 100644 --- a/beacon-chain/sync/rpc.go +++ b/beacon-chain/sync/rpc.go @@ -59,7 +59,11 @@ func (r *Service) registerRPC(topic string, base interface{}, handle rpcHandler) r.p2p.SetStreamHandler(topic, func(stream network.Stream) { ctx, cancel := context.WithTimeout(context.Background(), ttfbTimeout) defer cancel() - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() ctx, span := trace.StartSpan(ctx, "sync.rpc") defer span.End() span.AddAttributes(trace.StringAttribute("topic", topic)) diff --git a/beacon-chain/sync/rpc_beacon_blocks_by_range.go b/beacon-chain/sync/rpc_beacon_blocks_by_range.go index 44a1a3848..08a667ef4 100644 --- a/beacon-chain/sync/rpc_beacon_blocks_by_range.go +++ b/beacon-chain/sync/rpc_beacon_blocks_by_range.go @@ -17,13 +17,20 @@ import ( func (r *Service) beaconBlocksByRangeRPCHandler(ctx context.Context, msg interface{}, stream libp2pcore.Stream) error { ctx, span := trace.StartSpan(ctx, "sync.BeaconBlocksByRangeHandler") defer span.End() - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() setRPCStreamDeadlines(stream) log := log.WithField("handler", "beacon_blocks_by_range") - m := msg.(*pb.BeaconBlocksByRangeRequest) + m, ok := msg.(*pb.BeaconBlocksByRangeRequest) + if !ok { + return errors.New("message is not type *pb.BeaconBlockByRangeRequest") + } startSlot := m.StartSlot endSlot := startSlot + (m.Step * (m.Count - 1)) @@ -42,7 +49,11 @@ func (r *Service) beaconBlocksByRangeRPCHandler(ctx context.Context, msg interfa r.p2p.Peers().IncrementBadResponses(stream.Conn().RemotePeer()) if r.p2p.Peers().IsBad(stream.Conn().RemotePeer()) { log.Debug("Disconnecting bad peer") - defer r.p2p.Disconnect(stream.Conn().RemotePeer()) + defer func() { + if err := r.p2p.Disconnect(stream.Conn().RemotePeer()); err != nil { + log.WithError(err).Error("Failed to disconnect peer") + } + }() } resp, err := r.generateErrorResponse(responseCodeInvalidRequest, rateLimitedError) if err != nil { diff --git a/beacon-chain/sync/rpc_beacon_blocks_by_root.go b/beacon-chain/sync/rpc_beacon_blocks_by_root.go index cbeaa1dc8..77a98a53d 100644 --- a/beacon-chain/sync/rpc_beacon_blocks_by_root.go +++ b/beacon-chain/sync/rpc_beacon_blocks_by_root.go @@ -46,13 +46,20 @@ func (r *Service) sendRecentBeaconBlocksRequest(ctx context.Context, blockRoots // beaconBlocksRootRPCHandler looks up the request blocks from the database from the given block roots. func (r *Service) beaconBlocksRootRPCHandler(ctx context.Context, msg interface{}, stream libp2pcore.Stream) error { - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() setRPCStreamDeadlines(stream) log := log.WithField("handler", "beacon_blocks_by_root") - blockRoots := msg.([][32]byte) + blockRoots, ok := msg.([][32]byte) + if !ok { + return errors.New("message is not type [][32]byte") + } if len(blockRoots) == 0 { resp, err := r.generateErrorResponse(responseCodeInvalidRequest, "no block roots provided in request") if err != nil { @@ -69,7 +76,11 @@ func (r *Service) beaconBlocksRootRPCHandler(ctx context.Context, msg interface{ r.p2p.Peers().IncrementBadResponses(stream.Conn().RemotePeer()) if r.p2p.Peers().IsBad(stream.Conn().RemotePeer()) { log.Debug("Disconnecting bad peer") - defer r.p2p.Disconnect(stream.Conn().RemotePeer()) + defer func() { + if err := r.p2p.Disconnect(stream.Conn().RemotePeer()); err != nil { + log.WithError(err).Error("Failed to disconnect peer") + } + }() } resp, err := r.generateErrorResponse(responseCodeInvalidRequest, rateLimitedError) if err != nil { diff --git a/beacon-chain/sync/rpc_goodbye.go b/beacon-chain/sync/rpc_goodbye.go index 2c1e08ca8..8c31b212a 100644 --- a/beacon-chain/sync/rpc_goodbye.go +++ b/beacon-chain/sync/rpc_goodbye.go @@ -22,7 +22,11 @@ var goodByes = map[uint64]string{ // goodbyeRPCHandler reads the incoming goodbye rpc message from the peer. func (r *Service) goodbyeRPCHandler(ctx context.Context, msg interface{}, stream libp2pcore.Stream) error { - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() setRPCStreamDeadlines(stream) diff --git a/beacon-chain/sync/rpc_status.go b/beacon-chain/sync/rpc_status.go index 48ae6f197..9f2c6d933 100644 --- a/beacon-chain/sync/rpc_status.go +++ b/beacon-chain/sync/rpc_status.go @@ -119,12 +119,19 @@ func (r *Service) removeDisconnectedPeerStatus(ctx context.Context, pid peer.ID) // statusRPCHandler reads the incoming Status RPC from the peer and responds with our version of a status message. // This handler will disconnect any peer that does not match our fork version. func (r *Service) statusRPCHandler(ctx context.Context, msg interface{}, stream libp2pcore.Stream) error { - defer stream.Close() + defer func() { + if err := stream.Close(); err != nil { + log.WithError(err).Error("Failed to close stream") + } + }() ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() setRPCStreamDeadlines(stream) log := log.WithField("handler", "status") - m := msg.(*pb.Status) + m, ok := msg.(*pb.Status) + if !ok { + return errors.New("message is not type *pb.Status") + } if err := r.validateStatusMessage(m, stream); err != nil { log.WithField("peer", stream.Conn().RemotePeer()).Debug("Invalid fork version from peer") @@ -139,7 +146,9 @@ func (r *Service) statusRPCHandler(ctx context.Context, msg interface{}, stream log.WithError(err).Debug("Failed to write to stream") } } - stream.Close() // Close before disconnecting. + if err := stream.Close(); err != nil { // Close before disconnecting. + log.WithError(err).Error("Failed to close stream") + } // Add a short delay to allow the stream to flush before closing the connection. // There is still a chance that the peer won't receive the message. time.Sleep(50 * time.Millisecond) diff --git a/beacon-chain/sync/subscriber.go b/beacon-chain/sync/subscriber.go index b137ff89e..3f93e5d24 100644 --- a/beacon-chain/sync/subscriber.go +++ b/beacon-chain/sync/subscriber.go @@ -49,7 +49,11 @@ func (r *Service) registerSubscribers() { select { case event := <-stateChannel: if event.Type == statefeed.Initialized { - data := event.Data.(*statefeed.InitializedData) + data, ok := event.Data.(*statefeed.InitializedData) + if !ok { + log.Error("Event feed data is not type *statefeed.InitializedData") + return + } log.WithField("starttime", data.StartTime).Debug("Received state initialized event") if data.StartTime.After(roughtime.Now()) { stateSub.Unsubscribe() @@ -243,7 +247,9 @@ func (r *Service) subscribeDynamicWithSubnets( } if !wanted && v != nil { v.Cancel() - r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, k)) + if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, k)); err != nil { + log.WithError(err).Error("Failed to unregister topic validator") + } delete(subscriptions, k) } } @@ -312,7 +318,9 @@ func (r *Service) subscribeDynamic(topicFormat string, determineSubsLen func() i subscriptions, cancelSubs = subscriptions[:wantedSubs-1], subscriptions[wantedSubs:] for i, sub := range cancelSubs { sub.Cancel() - r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)) + if err := r.p2p.PubSub().UnregisterTopicValidator(fmt.Sprintf(topicFormat, i+wantedSubs)); err != nil { + log.WithError(err).Error("Failed to unregister topic validator") + } } } else if len(subscriptions) < wantedSubs { // Increase topics for i := len(subscriptions); i < wantedSubs; i++ { diff --git a/beacon-chain/sync/subscriber_beacon_blocks.go b/beacon-chain/sync/subscriber_beacon_blocks.go index db15f754c..4e7d97a7e 100644 --- a/beacon-chain/sync/subscriber_beacon_blocks.go +++ b/beacon-chain/sync/subscriber_beacon_blocks.go @@ -15,7 +15,10 @@ import ( ) func (r *Service) beaconBlockSubscriber(ctx context.Context, msg proto.Message) error { - signed := msg.(*ethpb.SignedBeaconBlock) + signed, ok := msg.(*ethpb.SignedBeaconBlock) + if !ok { + return errors.New("message is not type *ethpb.SignedBeaconBlock") + } if signed == nil || signed.Block == nil { return errors.New("nil block") diff --git a/beacon-chain/sync/subscriber_committee_index_beacon_attestation.go b/beacon-chain/sync/subscriber_committee_index_beacon_attestation.go index 95385a69c..b44367f52 100644 --- a/beacon-chain/sync/subscriber_committee_index_beacon_attestation.go +++ b/beacon-chain/sync/subscriber_committee_index_beacon_attestation.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/gogo/protobuf/proto" + "github.com/pkg/errors" eth "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/beacon-chain/cache" "github.com/prysmaticlabs/prysm/beacon-chain/core/feed" @@ -19,7 +20,11 @@ func (r *Service) committeeIndexBeaconAttestationSubscriber(ctx context.Context, return fmt.Errorf("message was not type *eth.Attestation, type=%T", msg) } - if exists, _ := r.attPool.HasAggregatedAttestation(a); exists { + exists, err := r.attPool.HasAggregatedAttestation(a) + if err != nil { + return errors.Wrap(err, "failed to determine if attestation pool has this atttestation") + } + if exists { return nil } diff --git a/contracts/deposit-contract/testutils.go b/contracts/deposit-contract/testutils.go index 96ebf5d4a..8e1cd8a12 100644 --- a/contracts/deposit-contract/testutils.go +++ b/contracts/deposit-contract/testutils.go @@ -31,7 +31,10 @@ type TestAccount struct { // Setup creates the simulated backend with the deposit contract deployed func Setup() (*TestAccount, error) { genesis := make(core.GenesisAlloc) - privKey, _ := crypto.GenerateKey() + privKey, err := crypto.GenerateKey() + if err != nil { + return nil, err + } pubKeyECDSA, ok := privKey.Public().(*ecdsa.PublicKey) if !ok { return nil, fmt.Errorf("error casting public key to ECDSA") diff --git a/endtoend/components/validator.go b/endtoend/components/validator.go index dce73824a..cba1cec3b 100644 --- a/endtoend/components/validator.go +++ b/endtoend/components/validator.go @@ -98,7 +98,10 @@ func StartValidators( t.Fatal(err) } - deposits, _, _ := testutil.DeterministicDepositsAndKeys(uint64(validatorNum)) + deposits, _, err := testutil.DeterministicDepositsAndKeys(uint64(validatorNum)) + if err != nil { + t.Fatal(err) + } _, roots, err := testutil.DeterministicDepositTrie(len(deposits)) if err != nil { t.Fatal(err) diff --git a/nogo_config.json b/nogo_config.json index cc5151b24..cd315c941 100644 --- a/nogo_config.json +++ b/nogo_config.json @@ -75,5 +75,16 @@ "beacon-chain/sync/deadlines\\.go": "Libp2p uses time.Now and this file sets a time based deadline in such a way that roughtime cannot be used", "validator/client/grpc_interceptor\\.go": "Uses time.Now() for gRPC duration logging" } + }, + "errcheck": { + "exclude_files": { + "external/.*": "Third party code", + "rules_go_work-.*": "Third party code", + ".*/.*_test\\.go": "TODO(5404): In a follow up PR", + "beacon-chain/p2p/testing/.*\\.go": "TODO(5404): In a follow up PR", + "shared/mock/.*\\.go": "Mocks are OK", + ".*/.*mock\\.go": "Mocks are OK", + ".*/testmain\\.go": "Test runner generated code" + } } } diff --git a/shared/bls/BUILD.bazel b/shared/bls/BUILD.bazel index fd8865b96..0ab26be62 100644 --- a/shared/bls/BUILD.bazel +++ b/shared/bls/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "//shared/params:go_default_library", "@com_github_dgraph_io_ristretto//:go_default_library", "@com_github_pkg_errors//:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", "@herumi_bls_eth_go_binary//:go_default_library", ], ) diff --git a/shared/bls/bls.go b/shared/bls/bls.go index 129b8316b..a334bc77a 100644 --- a/shared/bls/bls.go +++ b/shared/bls/bls.go @@ -14,6 +14,7 @@ import ( "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/hashutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/sirupsen/logrus" ) func init() { @@ -212,8 +213,11 @@ func (s *Signature) VerifyAggregateCommon(pubKeys []*PublicKey, msg [32]byte, do if len(pubKeys) == 0 { return false } - //#nosec G104 - aggregated, _ := pubKeys[0].Copy() + aggregated, err := pubKeys[0].Copy() + if err != nil { + logrus.WithError(err).Error("Failed to copy public key") + return false + } for i := 1; i < len(pubKeys); i++ { aggregated.p.Add(pubKeys[i].p) diff --git a/shared/debug/debug.go b/shared/debug/debug.go index 0787b1ceb..9a804aff7 100644 --- a/shared/debug/debug.go +++ b/shared/debug/debug.go @@ -280,7 +280,11 @@ func writeProfile(name, file string) error { if err != nil { return err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.WithError(err).Error("Failed to close pprof profile file.") + } + }() return p.WriteTo(f, 0) } diff --git a/shared/hashutil/hash.go b/shared/hashutil/hash.go index bc5e445c6..d88ca533e 100644 --- a/shared/hashutil/hash.go +++ b/shared/hashutil/hash.go @@ -25,7 +25,10 @@ var sha256Pool = sync.Pool{New: func() interface{} { // Hash defines a function that returns the sha256 checksum of the data passed in. // https://github.com/ethereum/eth2.0-specs/blob/v0.9.3/specs/core/0_beacon-chain.md#hash func Hash(data []byte) [32]byte { - h := sha256Pool.Get().(hash.Hash) + h, ok := sha256Pool.Get().(hash.Hash) + if !ok { + h = sha256.New() + } defer sha256Pool.Put(h) h.Reset() @@ -49,8 +52,12 @@ func Hash(data []byte) [32]byte { // Note: that this method is only more performant over // hashutil.Hash if the callback is used more than 5 times. func CustomSHA256Hasher() func([]byte) [32]byte { - hasher := sha256Pool.Get().(hash.Hash) - hasher.Reset() + hasher, ok := sha256Pool.Get().(hash.Hash) + if !ok { + hasher = sha256.New() + } else { + hasher.Reset() + } var hash [32]byte return func(data []byte) [32]byte { @@ -76,7 +83,10 @@ var keccak256Pool = sync.Pool{New: func() interface{} { func HashKeccak256(data []byte) [32]byte { var b [32]byte - h := keccak256Pool.Get().(hash.Hash) + h, ok := keccak256Pool.Get().(hash.Hash) + if !ok { + h = sha3.NewLegacyKeccak256() + } defer keccak256Pool.Put(h) h.Reset() diff --git a/shared/keystore/keystore.go b/shared/keystore/keystore.go index f925973ba..637c938b2 100644 --- a/shared/keystore/keystore.go +++ b/shared/keystore/keystore.go @@ -277,7 +277,10 @@ func getKDFKey(cryptoJSON cryptoJSON, auth string) ([]byte, error) { } else if cryptoJSON.KDF == "pbkdf2" { c := ensureInt(cryptoJSON.KDFParams["c"]) - prf := cryptoJSON.KDFParams["prf"].(string) + prf, ok := cryptoJSON.KDFParams["prf"].(string) + if !ok { + return nil, errors.New("KDFParams are not type string") + } if prf != "hmac-sha256" { return nil, fmt.Errorf("unsupported PBKDF2 PRF: %s", prf) } diff --git a/shared/memorypool/memorypool.go b/shared/memorypool/memorypool.go index 5e9eece06..658e041fc 100644 --- a/shared/memorypool/memorypool.go +++ b/shared/memorypool/memorypool.go @@ -33,7 +33,10 @@ func GetDoubleByteSlice(size int) [][]byte { if rawObj == nil { return make([][]byte, size) } - byteSlice := rawObj.([][]byte) + byteSlice, ok := rawObj.([][]byte) + if !ok { + return nil + } if len(byteSlice) >= size { return byteSlice[:size] } @@ -59,7 +62,10 @@ func GetBlockRootsTrie(size int) [][]*[32]byte { if rawObj == nil { return make([][]*[32]byte, size) } - byteSlice := rawObj.([][]*[32]byte) + byteSlice, ok := rawObj.([][]*[32]byte) + if !ok { + return nil + } if len(byteSlice) >= size { return byteSlice[:size] } @@ -85,7 +91,10 @@ func GetStateRootsTrie(size int) [][]*[32]byte { if rawObj == nil { return make([][]*[32]byte, size) } - byteSlice := rawObj.([][]*[32]byte) + byteSlice, ok := rawObj.([][]*[32]byte) + if !ok { + return nil + } if len(byteSlice) >= size { return byteSlice[:size] } @@ -111,7 +120,10 @@ func GetRandaoMixesTrie(size int) [][]*[32]byte { if rawObj == nil { return make([][]*[32]byte, size) } - byteSlice := rawObj.([][]*[32]byte) + byteSlice, ok := rawObj.([][]*[32]byte) + if !ok { + return nil + } if len(byteSlice) >= size { return byteSlice[:size] } diff --git a/shared/prometheus/logrus_collector.go b/shared/prometheus/logrus_collector.go index b79a5a890..3214e69d5 100644 --- a/shared/prometheus/logrus_collector.go +++ b/shared/prometheus/logrus_collector.go @@ -1,6 +1,8 @@ package prometheus import ( + "errors" + "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/sirupsen/logrus" @@ -34,7 +36,10 @@ func NewLogrusCollector() *LogrusCollector { func (hook *LogrusCollector) Fire(entry *logrus.Entry) error { prefix := defaultprefix if prefixValue, ok := entry.Data[prefixKey]; ok { - prefix = prefixValue.(string) + prefix, ok = prefixValue.(string) + if !ok { + return errors.New("prefix is not a string") + } } hook.counterVec.WithLabelValues(entry.Level.String(), prefix).Inc() return nil diff --git a/shared/prometheus/service.go b/shared/prometheus/service.go index 9538a7aff..70e6fe27a 100644 --- a/shared/prometheus/service.go +++ b/shared/prometheus/service.go @@ -90,10 +90,12 @@ func (s *Service) healthzHandler(w http.ResponseWriter, _ *http.Request) { func (s *Service) goroutinezHandler(w http.ResponseWriter, _ *http.Request) { stack := debug.Stack() - // #nosec G104 - w.Write(stack) - // #nosec G104 - pprof.Lookup("goroutine").WriteTo(w, 2) + if _, err := w.Write(stack); err != nil { + log.WithError(err).Error("Failed to write goroutines stack") + } + if err := pprof.Lookup("goroutine").WriteTo(w, 2); err != nil { + log.WithError(err).Error("Failed to write pprof goroutines") + } } // Start the prometheus service. @@ -103,7 +105,9 @@ func (s *Service) Start() { addrParts := strings.Split(s.server.Addr, ":") conn, err := net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%s", addrParts[1]), time.Second) if err == nil { - conn.Close() + if err := conn.Close(); err != nil { + log.WithError(err).Error("Failed to close connection") + } // Something on the port; we cannot use it. log.WithField("address", s.server.Addr).Warn("Port already in use; cannot start prometheus service") } else { diff --git a/slasher/db/kv/block_header.go b/slasher/db/kv/block_header.go index 3c3555689..ee498fdf0 100644 --- a/slasher/db/kv/block_header.go +++ b/slasher/db/kv/block_header.go @@ -10,6 +10,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/shared/bytesutil" "github.com/prysmaticlabs/prysm/shared/params" + "github.com/sirupsen/logrus" bolt "go.etcd.io/bbolt" "go.opencensus.io/trace" ) @@ -52,8 +53,7 @@ func (db *Store) HasBlockHeader(ctx context.Context, epoch uint64, validatorID u defer span.End() prefix := encodeEpochValidatorID(epoch, validatorID) var hasBlockHeader bool - // #nosec G104 - _ = db.view(func(tx *bolt.Tx) error { + if err := db.view(func(tx *bolt.Tx) error { c := tx.Bucket(historicBlockHeadersBucket).Cursor() for k, _ := c.Seek(prefix); k != nil && bytes.HasPrefix(k, prefix); k, _ = c.Next() { hasBlockHeader = true @@ -61,7 +61,9 @@ func (db *Store) HasBlockHeader(ctx context.Context, epoch uint64, validatorID u } hasBlockHeader = false return nil - }) + }); err != nil { + logrus.WithError(err).Error("Failed to lookup block header from DB") + } return hasBlockHeader } diff --git a/tools/analyzers/errcheck/BUILD.bazel b/tools/analyzers/errcheck/BUILD.bazel new file mode 100644 index 000000000..0a35f5841 --- /dev/null +++ b/tools/analyzers/errcheck/BUILD.bazel @@ -0,0 +1,31 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test", "go_tool_library") + +go_library( + name = "go_default_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/errcheck", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_default_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library", + "@org_golang_x_tools//go/ast/inspector:go_default_library", + ], +) + +go_tool_library( + name = "go_tool_library", + srcs = ["analyzer.go"], + importpath = "errcheck", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library", + "@org_golang_x_tools//go/ast/inspector:go_tool_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["embedded_walker_test.go"], + embed = [":go_default_library"], +) diff --git a/tools/analyzers/errcheck/analyzer.go b/tools/analyzers/errcheck/analyzer.go new file mode 100644 index 000000000..8c6bb817e --- /dev/null +++ b/tools/analyzers/errcheck/analyzer.go @@ -0,0 +1,442 @@ +// Package errcheck implements an static analysis analyzer to ensure that errors are handled in go +// code. This analyzer was adapted from https://github.com/kisielk/errcheck (MIT License). +package errcheck + +import ( + "errors" + "fmt" + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Doc explaining the tool. +const Doc = "This tool enforces all errors must be handled and that type assertions test that " + + "the type implements the given interface to prevent runtime panics." + +// Analyzer runs static analysis. +var Analyzer = &analysis.Analyzer{ + Name: "errcheck", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +var exclusions = make(map[string]bool) + +func init() { + for _, exc := range [...]string{ + // bytes + "(*bytes.Buffer).Write", + "(*bytes.Buffer).WriteByte", + "(*bytes.Buffer).WriteRune", + "(*bytes.Buffer).WriteString", + + // fmt + "fmt.Errorf", + "fmt.Print", + "fmt.Printf", + "fmt.Println", + "fmt.Fprint(*bytes.Buffer)", + "fmt.Fprintf(*bytes.Buffer)", + "fmt.Fprintln(*bytes.Buffer)", + "fmt.Fprint(*strings.Builder)", + "fmt.Fprintf(*strings.Builder)", + "fmt.Fprintln(*strings.Builder)", + "fmt.Fprint(os.Stderr)", + "fmt.Fprintf(os.Stderr)", + "fmt.Fprintln(os.Stderr)", + + // math/rand + "math/rand.Read", + "(*math/rand.Rand).Read", + + // hash + "(hash.Hash).Write", + } { + exclusions[exc] = true + } +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + (*ast.ExprStmt)(nil), + (*ast.GoStmt)(nil), + (*ast.DeferStmt)(nil), + (*ast.AssignStmt)(nil), + } + + inspect.Preorder(nodeFilter, func(node ast.Node) { + switch stmt := node.(type) { + case *ast.ExprStmt: + if call, ok := stmt.X.(*ast.CallExpr); ok { + if !ignoreCall(pass, call) && callReturnsError(pass, call) { + reportUnhandledError(pass, call.Lparen, call) + } + } + case *ast.GoStmt: + if !ignoreCall(pass, stmt.Call) && callReturnsError(pass, stmt.Call) { + reportUnhandledError(pass, stmt.Call.Lparen, stmt.Call) + } + case *ast.DeferStmt: + if !ignoreCall(pass, stmt.Call) && callReturnsError(pass, stmt.Call) { + reportUnhandledError(pass, stmt.Call.Lparen, stmt.Call) + } + case *ast.AssignStmt: + if len(stmt.Rhs) == 1 { + // single value on rhs; check against lhs identifiers + if call, ok := stmt.Rhs[0].(*ast.CallExpr); ok { + if ignoreCall(pass, call) { + break + } + isError := errorsByArg(pass, call) + for i := 0; i < len(stmt.Lhs); i++ { + if id, ok := stmt.Lhs[i].(*ast.Ident); ok { + // We shortcut calls to recover() because errorsByArg can't + // check its return types for errors since it returns interface{}. + if id.Name == "_" && (isRecover(pass, call) || isError[i]) { + reportUnhandledError(pass, id.NamePos, call) + } + } + } + } else if assert, ok := stmt.Rhs[0].(*ast.TypeAssertExpr); ok { + if assert.Type == nil { + // type switch + break + } + if len(stmt.Lhs) < 2 { + // assertion result not read + reportUnhandledTypeAssertion(pass, stmt.Rhs[0].Pos()) + } else if id, ok := stmt.Lhs[1].(*ast.Ident); ok && id.Name == "_" { + // assertion result ignored + reportUnhandledTypeAssertion(pass, id.NamePos) + } + } + } else { + // multiple value on rhs; in this case a call can't return + // multiple values. Assume len(stmt.Lhs) == len(stmt.Rhs) + for i := 0; i < len(stmt.Lhs); i++ { + if id, ok := stmt.Lhs[i].(*ast.Ident); ok { + if call, ok := stmt.Rhs[i].(*ast.CallExpr); ok { + if ignoreCall(pass, call) { + continue + } + if id.Name == "_" && callReturnsError(pass, call) { + reportUnhandledError(pass, id.NamePos, call) + } + } else if assert, ok := stmt.Rhs[i].(*ast.TypeAssertExpr); ok { + if assert.Type == nil { + // Shouldn't happen anyway, no multi assignment in type switches + continue + } + reportUnhandledError(pass, id.NamePos, nil) + } + } + } + } + default: + } + }) + + return nil, nil +} + +func reportUnhandledError(pass *analysis.Pass, pos token.Pos, call *ast.CallExpr) { + pass.Reportf(pos, "Unhandled error for function call %s", fullName(pass, call)) +} + +func reportUnhandledTypeAssertion(pass *analysis.Pass, pos token.Pos) { + pass.Reportf(pos, "Unhandled type assertion check. You must test whether or not an "+ + "interface implements the asserted type.") +} + +func fullName(pass *analysis.Pass, call *ast.CallExpr) string { + _, fn, ok := selectorAndFunc(pass, call) + if !ok { + return "" + } + return fn.FullName() +} + +// selectorAndFunc tries to get the selector and function from call expression. +// For example, given the call expression representing "a.b()", the selector +// is "a.b" and the function is "b" itself. +// +// The final return value will be true if it is able to do extract a selector +// from the call and look up the function object it refers to. +// +// If the call does not include a selector (like if it is a plain "f()" function call) +// then the final return value will be false. +func selectorAndFunc(pass *analysis.Pass, call *ast.CallExpr) (*ast.SelectorExpr, *types.Func, bool) { + if call == nil || call.Fun == nil { + return nil, nil, false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return nil, nil, false + } + + fn, ok := pass.TypesInfo.ObjectOf(sel.Sel).(*types.Func) + if !ok { + return nil, nil, false + } + + return sel, fn, true + +} + +func ignoreCall(pass *analysis.Pass, call *ast.CallExpr) bool { + for _, name := range namesForExcludeCheck(pass, call) { + if exclusions[name] { + return true + } + } + return false +} + +var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface) + +func isErrorType(t types.Type) bool { + return types.Implements(t, errorType) +} + +func callReturnsError(pass *analysis.Pass, call *ast.CallExpr) bool { + if isRecover(pass, call) { + return true + } + + for _, isError := range errorsByArg(pass, call) { + if isError { + return true + } + } + + return false +} + +// errorsByArg returns a slice s such that +// len(s) == number of return types of call +// s[i] == true iff return type at position i from left is an error type +func errorsByArg(pass *analysis.Pass, call *ast.CallExpr) []bool { + switch t := pass.TypesInfo.Types[call].Type.(type) { + case *types.Named: + // Single return + return []bool{isErrorType(t)} + case *types.Pointer: + // Single return via pointer + return []bool{isErrorType(t)} + case *types.Tuple: + // Multiple returns + s := make([]bool, t.Len()) + for i := 0; i < t.Len(); i++ { + switch et := t.At(i).Type().(type) { + case *types.Named: + // Single return + s[i] = isErrorType(et) + case *types.Pointer: + // Single return via pointer + s[i] = isErrorType(et) + default: + s[i] = false + } + } + return s + } + return []bool{false} +} + +func isRecover(pass *analysis.Pass, call *ast.CallExpr) bool { + if fun, ok := call.Fun.(*ast.Ident); ok { + if _, ok := pass.TypesInfo.Uses[fun].(*types.Builtin); ok { + return fun.Name == "recover" + } + } + return false +} + +func namesForExcludeCheck(pass *analysis.Pass, call *ast.CallExpr) []string { + sel, fn, ok := selectorAndFunc(pass, call) + if !ok { + return nil + } + + name := fullName(pass, call) + if name == "" { + return nil + } + + // This will be missing for functions without a receiver (like fmt.Printf), + // so just fall back to the the function's fullName in that case. + selection, ok := pass.TypesInfo.Selections[sel] + if !ok { + return []string{name} + } + + // This will return with ok false if the function isn't defined + // on an interface, so just fall back to the fullName. + ts, ok := walkThroughEmbeddedInterfaces(selection) + if !ok { + return []string{name} + } + + result := make([]string, len(ts)) + for i, t := range ts { + // Like in fullName, vendored packages will have /vendor/ in their name, + // thus not matching vendored standard library packages. If we + // want to support vendored stdlib packages, we need to implement + // additional logic here. + result[i] = fmt.Sprintf("(%s).%s", t.String(), fn.Name()) + } + return result +} + +// walkThroughEmbeddedInterfaces returns a slice of Interfaces that +// we need to walk through in order to reach the actual definition, +// in an Interface, of the method selected by the given selection. +// +// false will be returned in the second return value if: +// - the right side of the selection is not a function +// - the actual definition of the function is not in an Interface +// +// The returned slice will contain all the interface types that need +// to be walked through to reach the actual definition. +// +// For example, say we have: +// +// type Inner interface {Method()} +// type Middle interface {Inner} +// type Outer interface {Middle} +// type T struct {Outer} +// type U struct {T} +// type V struct {U} +// +// And then the selector: +// +// V.Method +// +// We'll return [Outer, Middle, Inner] by first walking through the embedded structs +// until we reach the Outer interface, then descending through the embedded interfaces +// until we find the one that actually explicitly defines Method. +func walkThroughEmbeddedInterfaces(sel *types.Selection) ([]types.Type, bool) { + fn, ok := sel.Obj().(*types.Func) + if !ok { + return nil, false + } + + // Start off at the receiver. + currentT := sel.Recv() + + // First, we can walk through any Struct fields provided + // by the selection Index() method. We ignore the last + // index because it would give the method itself. + indexes := sel.Index() + for _, fieldIndex := range indexes[:len(indexes)-1] { + currentT = getTypeAtFieldIndex(currentT, fieldIndex) + } + + // Now currentT is either a type implementing the actual function, + // an Invalid type (if the receiver is a package), or an interface. + // + // If it's not an Interface, then we're done, as this function + // only cares about Interface-defined functions. + // + // If it is an Interface, we potentially need to continue digging until + // we find the Interface that actually explicitly defines the function. + interfaceT, ok := maybeUnname(currentT).(*types.Interface) + if !ok { + return nil, false + } + + // The first interface we pass through is this one we've found. We return the possibly + // wrapping types.Named because it is more useful to work with for callers. + result := []types.Type{currentT} + + // If this interface itself explicitly defines the given method + // then we're done digging. + for !explicitlyDefinesMethod(interfaceT, fn) { + // Otherwise, we find which of the embedded interfaces _does_ + // define the method, add it to our list, and loop. + namedInterfaceT, ok := getEmbeddedInterfaceDefiningMethod(interfaceT, fn) + if !ok { + // This should be impossible as long as we type-checked: either the + // interface or one of its embedded ones must implement the method... + panic(fmt.Sprintf("either %v or one of its embedded interfaces must implement %v", currentT, fn)) + } + result = append(result, namedInterfaceT) + interfaceT, ok = namedInterfaceT.Underlying().(*types.Interface) + if !ok { + panic(fmt.Sprintf("either %v or one of its embedded interfaces must implement %v", currentT, fn)) + } + } + + return result, true +} + +func getTypeAtFieldIndex(startingAt types.Type, fieldIndex int) types.Type { + t := maybeUnname(maybeDereference(startingAt)) + s, ok := t.(*types.Struct) + if !ok { + panic(fmt.Sprintf("cannot get Field of a type that is not a struct, got a %T", t)) + } + + return s.Field(fieldIndex).Type() +} + +// getEmbeddedInterfaceDefiningMethod searches through any embedded interfaces of the +// passed interface searching for one that defines the given function. If found, the +// types.Named wrapping that interface will be returned along with true in the second value. +// +// If no such embedded interface is found, nil and false are returned. +func getEmbeddedInterfaceDefiningMethod(interfaceT *types.Interface, fn *types.Func) (*types.Named, bool) { + for i := 0; i < interfaceT.NumEmbeddeds(); i++ { + embedded := interfaceT.Embedded(i) + if definesMethod(embedded.Underlying().(*types.Interface), fn) { + return embedded, true + } + } + return nil, false +} + +func explicitlyDefinesMethod(interfaceT *types.Interface, fn *types.Func) bool { + for i := 0; i < interfaceT.NumExplicitMethods(); i++ { + if interfaceT.ExplicitMethod(i) == fn { + return true + } + } + return false +} + +func definesMethod(interfaceT *types.Interface, fn *types.Func) bool { + for i := 0; i < interfaceT.NumMethods(); i++ { + if interfaceT.Method(i) == fn { + return true + } + } + return false +} + +func maybeDereference(t types.Type) types.Type { + p, ok := t.(*types.Pointer) + if ok { + return p.Elem() + } + return t +} + +func maybeUnname(t types.Type) types.Type { + n, ok := t.(*types.Named) + if ok { + return n.Underlying() + } + return t +} diff --git a/tools/analyzers/errcheck/embedded_walker_test.go b/tools/analyzers/errcheck/embedded_walker_test.go new file mode 100644 index 000000000..51c13a23b --- /dev/null +++ b/tools/analyzers/errcheck/embedded_walker_test.go @@ -0,0 +1,93 @@ +package errcheck + +import ( + "go/ast" + "go/parser" + "go/token" + "go/types" + "testing" +) + +const commonSrc = ` +package p + +type Inner struct {} +func (Inner) Method() + +type Outer struct {Inner} +type OuterP struct {*Inner} + +type InnerInterface interface { + Method() +} + +type OuterInterface interface {InnerInterface} +type MiddleInterfaceStruct struct {OuterInterface} +type OuterInterfaceStruct struct {MiddleInterfaceStruct} + +var c = ` + +type testCase struct { + selector string + expectedOk bool + expected []string +} + +func TestWalkThroughEmbeddedInterfaces(t *testing.T) { + cases := []testCase{ + testCase{"Inner{}.Method", false, nil}, + testCase{"(&Inner{}).Method", false, nil}, + testCase{"Outer{}.Method", false, nil}, + testCase{"InnerInterface.Method", true, []string{"test.InnerInterface"}}, + testCase{"OuterInterface.Method", true, []string{"test.OuterInterface", "test.InnerInterface"}}, + testCase{"OuterInterfaceStruct.Method", true, []string{"test.OuterInterface", "test.InnerInterface"}}, + } + + for _, c := range cases { + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "test", commonSrc+c.selector, 0) + if err != nil { + t.Fatal(err) + } + + conf := types.Config{} + info := types.Info{ + Selections: make(map[*ast.SelectorExpr]*types.Selection), + } + _, err = conf.Check("test", fset, []*ast.File{f}, &info) + if err != nil { + t.Fatal(err) + } + ast.Inspect(f, func(n ast.Node) bool { + s, ok := n.(*ast.SelectorExpr) + if ok { + selection, ok := info.Selections[s] + if !ok { + t.Fatalf("no Selection!") + } + ts, ok := walkThroughEmbeddedInterfaces(selection) + if ok != c.expectedOk { + t.Errorf("expected ok %v got %v", c.expectedOk, ok) + return false + } + if !ok { + return false + } + + if len(ts) != len(c.expected) { + t.Fatalf("expected %d types, got %d", len(c.expected), len(ts)) + } + + for i, e := range c.expected { + if e != ts[i].String() { + t.Errorf("mismatch at index %d: expected %s got %s", i, e, ts[i]) + } + } + } + + return true + }) + + } + +} diff --git a/tools/analyzers/maligned/analyzer.go b/tools/analyzers/maligned/analyzer.go index f91752673..36a6cae56 100644 --- a/tools/analyzers/maligned/analyzer.go +++ b/tools/analyzers/maligned/analyzer.go @@ -1,6 +1,7 @@ package maligned import ( + "errors" "go/ast" "go/types" @@ -21,7 +22,10 @@ var Analyzer = &analysis.Analyzer{ } func run(pass *analysis.Pass) (interface{}, error) { - inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } nodeFilter := []ast.Node{ (*ast.StructType)(nil), diff --git a/tools/analyzers/roughtime/analyzer.go b/tools/analyzers/roughtime/analyzer.go index df30d5bef..a6d6dbb5f 100644 --- a/tools/analyzers/roughtime/analyzer.go +++ b/tools/analyzers/roughtime/analyzer.go @@ -1,6 +1,7 @@ package roughtime import ( + "errors" "go/ast" "golang.org/x/tools/go/analysis" @@ -22,7 +23,10 @@ var Analyzer = &analysis.Analyzer{ } func run(pass *analysis.Pass) (interface{}, error) { - inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } nodeFilter := []ast.Node{ (*ast.CallExpr)(nil), diff --git a/tools/benchmark-files-gen/main.go b/tools/benchmark-files-gen/main.go index 2df7a1ead..45787fc29 100644 --- a/tools/benchmark-files-gen/main.go +++ b/tools/benchmark-files-gen/main.go @@ -139,7 +139,9 @@ func generateMarshalledFullStateAndBlock() error { } // Temporarily incrementing the beacon state slot here since BeaconProposerIndex is a // function deterministic on beacon state slot. - beaconState.SetSlot(beaconState.Slot() + 1) + if err := beaconState.SetSlot(beaconState.Slot() + 1); err != nil { + return err + } proposerIdx, err := helpers.BeaconProposerIndex(beaconState) if err != nil { return err @@ -149,7 +151,9 @@ func generateMarshalledFullStateAndBlock() error { return err } block.Signature = privs[proposerIdx].Sign(blockRoot[:], domain).Marshal() - beaconState.SetSlot(beaconState.Slot() - 1) + if err := beaconState.SetSlot(beaconState.Slot() - 1); err != nil { + return err + } beaconBytes, err := ssz.Marshal(beaconState) if err != nil { diff --git a/tools/bootnode/bootnode.go b/tools/bootnode/bootnode.go index 0db5518d1..182e6643c 100644 --- a/tools/bootnode/bootnode.go +++ b/tools/bootnode/bootnode.go @@ -16,6 +16,7 @@ import ( "encoding/hex" "flag" "fmt" + "io" "net" "net/http" "os" @@ -169,17 +170,21 @@ func createListener(ipAddr string, port int, cfg discover.Config) *discover.UDPv func (h *handler) httpHandler(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - allNodes := h.listener.AllNodes() - w.Write([]byte("Nodes stored in the table:\n")) - for i, n := range allNodes { - w.Write([]byte(fmt.Sprintf("Node %d\n", i))) - w.Write([]byte(n.String() + "\n")) - w.Write([]byte("Node ID: " + n.ID().String() + "\n")) - w.Write([]byte("IP: " + n.IP().String() + "\n")) - w.Write([]byte(fmt.Sprintf("UDP Port: %d", n.UDP()) + "\n")) - w.Write([]byte(fmt.Sprintf("TCP Port: %d", n.UDP()) + "\n\n")) + write := func(w io.Writer, b []byte) { + if _, err := w.Write(b); err != nil { + log.WithError(err).Error("Failed to write to http response") + } + } + allNodes := h.listener.AllNodes() + write(w, []byte("Nodes stored in the table:\n")) + for i, n := range allNodes { + write(w, []byte(fmt.Sprintf("Node %d\n", i))) + write(w, []byte(n.String()+"\n")) + write(w, []byte("Node ID: "+n.ID().String()+"\n")) + write(w, []byte("IP: "+n.IP().String()+"\n")) + write(w, []byte(fmt.Sprintf("UDP Port: %d", n.UDP())+"\n")) + write(w, []byte(fmt.Sprintf("TCP Port: %d", n.UDP())+"\n\n")) } - } func createLocalNode(privKey *ecdsa.PrivateKey, ipAddr net.IP, port int) (*enode.LocalNode, error) { diff --git a/tools/eth1exporter/BUILD.bazel b/tools/eth1exporter/BUILD.bazel index aa2cd8af3..c049fa5bf 100644 --- a/tools/eth1exporter/BUILD.bazel +++ b/tools/eth1exporter/BUILD.bazel @@ -12,6 +12,7 @@ go_library( "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//ethclient:go_default_library", "@com_github_ethereum_go_ethereum//params:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", "@org_uber_go_automaxprocs//:go_default_library", ], ) @@ -37,6 +38,7 @@ go_image( "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//ethclient:go_default_library", "@com_github_ethereum_go_ethereum//params:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", "@org_uber_go_automaxprocs//:go_default_library", ], ) diff --git a/tools/eth1exporter/main.go b/tools/eth1exporter/main.go index 7491589e4..5e60699b3 100644 --- a/tools/eth1exporter/main.go +++ b/tools/eth1exporter/main.go @@ -17,6 +17,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/params" + "github.com/sirupsen/logrus" _ "go.uber.org/automaxprocs" ) @@ -135,7 +136,9 @@ func MetricsHTTP(w http.ResponseWriter, r *http.Request) { allOut = append(allOut, fmt.Sprintf("%veth_load_seconds %0.2f", *prefix, loadSeconds)) allOut = append(allOut, fmt.Sprintf("%veth_loaded_addresses %v", *prefix, totalLoaded)) allOut = append(allOut, fmt.Sprintf("%veth_total_addresses %v", *prefix, len(allWatching))) - fmt.Fprintln(w, strings.Join(allOut, "\n")) + if _, err := fmt.Fprintln(w, strings.Join(allOut, "\n")); err != nil { + logrus.WithError(err).Error("Failed to write metrics") + } } // ReloadHTTP reloads the addresses from disk. @@ -154,7 +157,11 @@ func OpenAddresses(filename string) error { if err != nil { return err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + panic(err) + } + }() scanner := bufio.NewScanner(file) allWatching = []*Watching{} for scanner.Scan() { diff --git a/tools/interop/convert-keys/main.go b/tools/interop/convert-keys/main.go index 40b1ecf00..db2ed448d 100644 --- a/tools/interop/convert-keys/main.go +++ b/tools/interop/convert-keys/main.go @@ -57,7 +57,11 @@ func main() { if err != nil { log.Fatalf("Failed to create file at %s: %v", os.Args[2], err) } - defer outFile.Close() + defer func() { + if err := outFile.Close(); err != nil { + panic(err) + } + }() if err := keygen.SaveUnencryptedKeysToFile(outFile, out); err != nil { log.Fatalf("Failed to save %v", err) } diff --git a/tools/interop/export-genesis/main.go b/tools/interop/export-genesis/main.go index d8fd9b581..3aa1b4443 100644 --- a/tools/interop/export-genesis/main.go +++ b/tools/interop/export-genesis/main.go @@ -26,7 +26,11 @@ func main() { if err != nil { panic(err) } - defer d.Close() + defer func() { + if err := d.Close(); err != nil { + panic(err) + } + }() gs, err := d.GenesisState(context.Background()) if err != nil { panic(err) diff --git a/validator/keymanager/direct_unencrypted.go b/validator/keymanager/direct_unencrypted.go index a4dd1404d..56f2b3adb 100644 --- a/validator/keymanager/direct_unencrypted.go +++ b/validator/keymanager/direct_unencrypted.go @@ -48,7 +48,11 @@ func NewUnencrypted(input string) (*Unencrypted, string, error) { if err != nil { return nil, unencryptedOptsHelp, err } - defer reader.Close() + defer func() { + if err := reader.Close(); err != nil { + log.WithError(err).Error("Failed to close file reader") + } + }() keyMap, err := unencryptedKeysFromReader(reader) if err != nil {