Interface Analyzer (#8923)

BUILD.bazel

@@ -120,6 +120,7 @@ nogo(
         "//tools/analyzers/shadowpredecl:go_tool_library",
         "//tools/analyzers/nop:go_tool_library",
         "//tools/analyzers/slicedirect:go_tool_library",
+        "//tools/analyzers/interfacechecker:go_tool_library",
         "//tools/analyzers/ineffassign:go_tool_library",
         "//tools/analyzers/properpermissions:go_tool_library",
     ] + select({

beacon-chain/blockchain/head.go

@@ -113,7 +113,7 @@ func (s *Service) saveHead(ctx context.Context, headRoot [32]byte) error {
 	if err != nil {
 		return errors.Wrap(err, "could not retrieve head state in DB")
 	}
-	if newHeadState == nil {
+	if newHeadState == nil || newHeadState.IsNil() {
 		return errors.New("cannot save nil head state")
 	}
 
@@ -262,7 +262,7 @@ func (s *Service) cacheJustifiedStateBalances(ctx context.Context, justifiedRoot
 			return err
 		}
 	}
-	if justifiedState == nil {
+	if justifiedState == nil || justifiedState.IsNil() {
 		return errors.New("justified state can't be nil")
 	}
 

beacon-chain/blockchain/info.go

@@ -39,7 +39,7 @@ func (s *Service) TreeHandler(w http.ResponseWriter, r *http.Request) {
 		log.WithError(err).Error("Could not get head state")
 		return
 	}
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		if _, err := w.Write([]byte("Unavailable during initial syncing")); err != nil {
 			log.WithError(err).Error("Failed to render p2p info page")
 		}

beacon-chain/blockchain/process_attestation_helpers.go

@@ -29,7 +29,7 @@ func (s *Service) getAttPreState(ctx context.Context, c *ethpb.Checkpoint) (ifac
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get cached checkpoint state")
 	}
-	if cachedState != nil {
+	if cachedState != nil && !cachedState.IsNil() {
 		return cachedState, nil
 	}
 

beacon-chain/blockchain/process_block.go

@@ -203,7 +203,7 @@ func (s *Service) onBlockBatch(ctx context.Context, blks []*ethpb.SignedBeaconBl
 	if err != nil {
 		return nil, nil, err
 	}
-	if preState == nil {
+	if preState == nil || preState.IsNil() {
 		return nil, nil, fmt.Errorf("nil pre state for slot %d", b.Slot)
 	}
 

beacon-chain/blockchain/process_block_helpers.go

@@ -39,7 +39,7 @@ func (s *Service) getBlockPreState(ctx context.Context, b *ethpb.BeaconBlock) (i
 	if err != nil {
 		return nil, errors.Wrapf(err, "could not get pre state for slot %d", b.Slot)
 	}
-	if preState == nil {
+	if preState == nil || preState.IsNil() {
 		return nil, errors.Wrapf(err, "nil pre state for slot %d", b.Slot)
 	}
 

beacon-chain/blockchain/service.go

@@ -135,7 +135,7 @@ func (s *Service) Start() {
 	attestationProcessorSubscribed := make(chan struct{}, 1)
 
 	// If the chain has already been initialized, simply start the block processing routine.
-	if beaconState != nil {
+	if beaconState != nil && !beaconState.IsNil() {
 		log.Info("Blockchain data already exists in DB, initializing...")
 		s.genesisTime = time.Unix(int64(beaconState.GenesisTime()), 0)
 		s.cfg.OpsService.SetGenesisTime(beaconState.GenesisTime())

beacon-chain/core/blocks/eth1_data.go

@@ -21,7 +21,7 @@ import (
 //    if state.eth1_data_votes.count(body.eth1_data) * 2 > EPOCHS_PER_ETH1_VOTING_PERIOD * SLOTS_PER_EPOCH:
 //        state.eth1_data = body.eth1_data
 func ProcessEth1DataInBlock(_ context.Context, beaconState iface.BeaconState, eth1Data *ethpb.Eth1Data) (iface.BeaconState, error) {
-	if beaconState == nil {
+	if beaconState == nil || beaconState.IsNil() {
 		return nil, errors.New("nil state")
 	}
 	if err := beaconState.AppendEth1DataVotes(eth1Data); err != nil {

beacon-chain/core/state/transition.go

@@ -179,7 +179,7 @@ func ProcessSlotsUsingNextSlotCache(
 	}
 	// If the next slot state is not nil (i.e. cache hit).
 	// We replace next slot state with parent state.
-	if nextSlotState != nil {
+	if nextSlotState != nil && !nextSlotState.IsNil() {
 		parentState = nextSlotState
 	}
 
@@ -208,7 +208,7 @@ func ProcessSlotsUsingNextSlotCache(
 func ProcessSlots(ctx context.Context, state iface.BeaconState, slot types.Slot) (iface.BeaconState, error) {
 	ctx, span := trace.StartSpan(ctx, "core.state.ProcessSlots")
 	defer span.End()
-	if state == nil {
+	if state == nil || state.IsNil() {
 		return nil, errors.New("nil state")
 	}
 	span.AddAttributes(trace.Int64Attribute("slots", int64(slot)-int64(state.Slot())))
@@ -399,7 +399,7 @@ func ProcessEpochPrecompute(ctx context.Context, state iface.BeaconState) (iface
 	defer span.End()
 	span.AddAttributes(trace.Int64Attribute("epoch", int64(helpers.CurrentEpoch(state))))
 
-	if state == nil {
+	if state == nil || state.IsNil() {
 		return nil, errors.New("nil state")
 	}
 	vp, bp, err := precompute.New(ctx, state)

beacon-chain/core/state/transition_no_verify_sig.go

@@ -121,7 +121,7 @@ func CalculateStateRoot(
 		traceutil.AnnotateError(span, ctx.Err())
 		return [32]byte{}, ctx.Err()
 	}
-	if state == nil {
+	if state == nil || state.IsNil() {
 		return [32]byte{}, errors.New("nil state")
 	}
 	if signed == nil || signed.Block == nil {

beacon-chain/db/kv/genesis.go

@@ -70,7 +70,7 @@ func (s *Store) LoadGenesis(ctx context.Context, r io.Reader) error {
 	}
 	// If some different genesis state existed already, return an error. The same genesis state is
 	// considered a no-op.
-	if existing != nil {
+	if existing != nil && !existing.IsNil() {
 		a, err := existing.HashTreeRoot(ctx)
 		if err != nil {
 			return err
@@ -108,7 +108,7 @@ func (s *Store) EnsureEmbeddedGenesis(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	if gs != nil {
+	if gs != nil && !gs.IsNil() {
 		return s.SaveGenesisData(ctx, gs)
 	}
 	return nil

beacon-chain/db/kv/state.go

@@ -294,7 +294,7 @@ func (s *Store) HighestSlotStatesBelow(ctx context.Context, slot types.Slot) ([]
 			return nil, err
 		}
 	}
-	if st == nil {
+	if st == nil || st.IsNil() {
 		st, err = s.GenesisState(ctx)
 		if err != nil {
 			return nil, err

beacon-chain/p2p/gossip_scoring_params.go

@@ -113,7 +113,7 @@ func (s *Service) retrieveActiveValidators() (uint64, error) {
 		if err != nil {
 			return 0, err
 		}
-		if genState == nil {
+		if genState == nil || genState.IsNil() {
 			return 0, errors.New("no genesis state exists")
 		}
 		activeVals, err := helpers.ActiveValidatorCount(genState, helpers.CurrentEpoch(genState))
@@ -128,7 +128,7 @@ func (s *Service) retrieveActiveValidators() (uint64, error) {
 	if err != nil {
 		return 0, err
 	}
-	if bState == nil {
+	if bState == nil || bState.IsNil() {
 		return 0, errors.Errorf("no state with root %#x exists", rt)
 	}
 	activeVals, err := helpers.ActiveValidatorCount(bState, helpers.CurrentEpoch(bState))

beacon-chain/powchain/deposit.go

@@ -17,7 +17,7 @@ func (s *Service) processDeposit(ctx context.Context, eth1Data *ethpb.Eth1Data,
 	if err != nil {
 		return errors.Wrap(err, "could not process pre-genesis deposits")
 	}
-	if beaconState != nil {
+	if beaconState != nil && !beaconState.IsNil() {
 		s.preGenesisState = beaconState
 	}
 	return nil

beacon-chain/powchain/service.go

@@ -247,7 +247,7 @@ func (s *Service) Start() {
 		if err != nil {
 			log.Fatal(err)
 		}
-		if genState == nil {
+		if genState == nil || genState.IsNil() {
 			log.Fatal("cannot create genesis state: no eth1 http endpoint defined")
 		}
 	}
@@ -611,7 +611,7 @@ func (s *Service) initDepositCaches(ctx context.Context, ctrs []*protodb.Deposit
 		if err != nil {
 			return errors.Wrap(err, "could not get finalized state")
 		}
-		if fState == nil {
+		if fState == nil || fState.IsNil() {
 			return errors.Errorf("finalized state with root %#x does not exist in the db", rt)
 		}
 		// Set deposit index to the one in the current archived state.
@@ -1014,7 +1014,7 @@ func (s *Service) ensureValidPowchainData(ctx context.Context) error {
 		return err
 	}
 	// Exit early if no genesis state is saved.
-	if genState == nil {
+	if genState == nil || genState.IsNil() {
 		return nil
 	}
 	eth1Data, err := s.cfg.BeaconDB.PowchainData(ctx)

beacon-chain/rpc/beacon/validators_stream.go

@@ -99,7 +99,7 @@ func (bs *Server) StreamValidatorsInfo(stream ethpb.BeaconChain_StreamValidators
 	if err != nil {
 		return status.Error(codes.Internal, "Could not access head state")
 	}
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		return status.Error(codes.Internal, "Not ready to serve information")
 	}
 
@@ -260,7 +260,7 @@ func (is *infostream) generateValidatorsInfo(pubKeys [][]byte) ([]*ethpb.Validat
 	if err != nil {
 		return nil, status.Error(codes.Internal, "Could not access head state")
 	}
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		return nil, status.Error(codes.Internal, "Not ready to serve information")
 	}
 	epoch := types.Epoch(headState.Slot() / params.BeaconConfig().SlotsPerEpoch)
@@ -450,7 +450,7 @@ func (is *infostream) handleBlockProcessed() {
 		log.Warn("Could not access head state for infostream")
 		return
 	}
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		// We aren't ready to serve information
 		return
 	}

beacon-chain/rpc/validator/attester.go

@@ -90,7 +90,7 @@ func (vs *Server) GetAttestationData(ctx context.Context, req *ethpb.Attestation
 			return nil, status.Errorf(codes.Internal, "Could not get historical head state: %v", err)
 		}
 	}
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		return nil, status.Error(codes.Internal, "Could not lookup parent state from head.")
 	}
 

beacon-chain/rpc/validator/server.go

@@ -154,7 +154,7 @@ func (vs *Server) WaitForChainStart(_ *emptypb.Empty, stream ethpb.BeaconNodeVal
 	if err != nil {
 		return status.Errorf(codes.Internal, "Could not retrieve head state: %v", err)
 	}
-	if head != nil {
+	if head != nil && !head.IsNil() {
 		res := &ethpb.ChainStartResponse{
 			Started:               true,
 			GenesisTime:           head.GenesisTime(),

beacon-chain/rpc/validator/status.go

@@ -219,7 +219,7 @@ func (vs *Server) validatorStatus(
 }
 
 func statusForPubKey(headState iface.ReadOnlyBeaconState, pubKey []byte) (ethpb.ValidatorStatus, types.ValidatorIndex, error) {
-	if headState == nil {
+	if headState == nil || headState.IsNil() {
 		return ethpb.ValidatorStatus_UNKNOWN_STATUS, 0, errors.New("head state does not exist")
 	}
 	idx, ok := headState.ValidatorIndexByPubkey(bytesutil.ToBytes48(pubKey))

beacon-chain/state/interface/phase0.go

@@ -43,6 +43,7 @@ type ReadOnlyBeaconState interface {
 	Slashings() []uint64
 	FieldReferencesCount() map[string]uint64
 	MarshalSSZ() ([]byte, error)
+	IsNil() bool
 }
 
 // WriteOnlyBeaconState defines a struct which only has write access to beacon state methods.

beacon-chain/state/stateV0/state_test.go

@@ -94,3 +94,14 @@ func TestBeaconState_NoDeadlock(t *testing.T) {
 	// Test will not terminate in the event of a deadlock.
 	wg.Wait()
 }
+
+func TestStateTrie_IsNil(t *testing.T) {
+	var emptyState *BeaconState
+	assert.Equal(t, true, emptyState.IsNil())
+
+	emptyProto := &BeaconState{state: nil}
+	assert.Equal(t, true, emptyProto.IsNil())
+
+	nonNilState := &BeaconState{state: &pb.BeaconState{}}
+	assert.Equal(t, false, nonNilState.IsNil())
+}

beacon-chain/state/stateV0/state_trie.go

@@ -379,6 +379,12 @@ func (b *BeaconState) FieldReferencesCount() map[string]uint64 {
 	return refMap
 }
 
+// IsNil checks if the state and the underlying proto
+// object are nil.
+func (b *BeaconState) IsNil() bool {
+	return b == nil || b.state == nil
+}
+
 func (b *BeaconState) rootSelector(ctx context.Context, field fieldIndex) ([32]byte, error) {
 	ctx, span := trace.StartSpan(ctx, "beaconState.rootSelector")
 	defer span.End()

beacon-chain/state/stategen/getter.go

@@ -78,7 +78,7 @@ func (s *State) StateByRootInitialSync(ctx context.Context, blockRoot [32]byte)
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get ancestor state")
 	}
-	if startState == nil {
+	if startState == nil || startState.IsNil() {
 		return nil, errUnknownState
 	}
 	summary, err := s.stateSummary(ctx, blockRoot)
@@ -149,7 +149,7 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (iface.
 
 	// First, it checks if the state exists in hot state cache.
 	cachedState := s.hotStateCache.get(blockRoot)
-	if cachedState != nil {
+	if cachedState != nil && !cachedState.IsNil() {
 		return cachedState, nil
 	}
 
@@ -179,7 +179,7 @@ func (s *State) loadStateByRoot(ctx context.Context, blockRoot [32]byte) (iface.
 	if err != nil {
 		return nil, errors.Wrap(err, "could not get ancestor state")
 	}
-	if startState == nil {
+	if startState == nil || startState.IsNil() {
 		return nil, errUnknownBoundaryState
 	}
 

beacon-chain/state/stategen/replay.go

@@ -156,7 +156,7 @@ func executeStateTransitionStateGen(
 func processSlotsStateGen(ctx context.Context, state iface.BeaconState, slot types.Slot) (iface.BeaconState, error) {
 	ctx, span := trace.StartSpan(ctx, "stategen.ProcessSlotsStateGen")
 	defer span.End()
-	if state == nil {
+	if state == nil || state.IsNil() {
 		return nil, errUnknownState
 	}
 

beacon-chain/state/stategen/service.go

@@ -101,7 +101,7 @@ func (s *State) Resume(ctx context.Context) (iface.BeaconState, error) {
 	if err != nil {
 		return nil, err
 	}
-	if fState == nil {
+	if fState == nil || fState.IsNil() {
 		return nil, errors.New("finalized state not found in disk")
 	}
 

nogo_config.json

@@ -84,6 +84,13 @@
       ".*_test\\.go": "Only tests"
     }
   },
+  "interfacechecker": {
+    "exclude_files": {
+      "external/.*": "Third party code",
+      "rules_go_work-.*": "Third party code",
+      ".*/.*_test\\.go": "Tests are OK to ignore this check for"
+    }
+  },
   "cryptorand": {
     "only_files": {
       "beacon-chain/.*": "",

tools/analyzers/interfacechecker/BUILD.bazel

@@ -0,0 +1,26 @@
+load("@prysm//tools/go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_tool_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["analyzer.go"],
+    importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/interfacechecker",
+    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 = "interfacechecker",
+    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",
+    ],
+)

tools/analyzers/interfacechecker/analyzer.go

@@ -0,0 +1,97 @@
+// Package interfacechecker implements a static analyzer to prevent incorrect conditional checks on select interfaces.
+package interfacechecker
+
+import (
+	"errors"
+	"go/ast"
+	"go/token"
+	"go/types"
+	"strings"
+
+	"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 = "Enforce usage of proper conditional check for interfaces"
+
+// Analyzer runs static analysis.
+var Analyzer = &analysis.Analyzer{
+	Name:     "interfacechecker",
+	Doc:      Doc,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+// These are the selected interfaces that we want to parse through and check nilness for.
+var selectedInterfaces = []string{
+	"interfaces.SignedBeaconBlock",
+	"interface.BeaconState",
+	"interface.ReadOnlyBeaconState",
+	"interface.WriteOnlyBeaconState",
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspection, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, errors.New("analyzer is not type *inspector.Inspector")
+	}
+
+	nodeFilter := []ast.Node{
+		(*ast.IfStmt)(nil),
+	}
+
+	inspection.Preorder(nodeFilter, func(node ast.Node) {
+		stmt, ok := node.(*ast.IfStmt)
+		if !ok {
+			return
+		}
+		exp, ok := stmt.Cond.(*ast.BinaryExpr)
+		if !ok {
+			return
+		}
+		handleConditionalExpression(exp, pass)
+	})
+
+	return nil, nil
+}
+
+func handleConditionalExpression(exp *ast.BinaryExpr, pass *analysis.Pass) {
+	identX, ok := exp.X.(*ast.Ident)
+	if !ok {
+		return
+	}
+	identY, ok := exp.Y.(*ast.Ident)
+	if !ok {
+		return
+	}
+	typeMap := pass.TypesInfo.Types
+	if _, ok := typeMap[identX].Type.(*types.Slice); ok {
+		return
+	}
+	if _, ok := typeMap[identY].Type.(*types.Slice); ok {
+		return
+	}
+	for _, iface := range selectedInterfaces {
+		xIsIface := strings.Contains(typeMap[identX].Type.String(), iface)
+		xIsNil := typeMap[identX].IsNil()
+		yisIface := strings.Contains(typeMap[identY].Type.String(), iface)
+		yIsNil := typeMap[identY].IsNil()
+		// Exit early if neither are of the desired interface
+		if !xIsIface && !yisIface {
+			continue
+		}
+		if xIsIface && yIsNil {
+			reportFailure(identX.Pos(), pass)
+		}
+		if yisIface && xIsNil {
+			reportFailure(identY.Pos(), pass)
+		}
+	}
+}
+
+func reportFailure(pos token.Pos, pass *analysis.Pass) {
+	pass.Reportf(pos, "A single nilness check is being performed on an interface"+
+		", this check needs another accompanying nilness check on the underlying object for the interface.")
+}

tools/interop/export-genesis/main.go

@@ -34,7 +34,7 @@ func main() {
 	if err != nil {
 		panic(err)
 	}
-	if gs == nil {
+	if gs == nil || gs.IsNil() {
 		panic("nil genesis state")
 	}
 	b, err := gs.MarshalSSZ()