Fix parent attestation check (#700)

beacon-chain/types/block.go

@@ -176,6 +176,12 @@ func (b *Block) IsValid(
 		return false
 	}
 
+	if enableAttestationValidity {
+		if !b.doesParentProposerExist(cState, parentSlot) || !b.areAttestationsValid(db, aState, cState, parentSlot) {
+			return false
+		}
+	}
+
 	_, proposerIndex, err := casper.ProposerShardAndIndex(
 		cState.ShardAndCommitteesForSlots(),
 		cState.LastStateRecalculationSlot(),
@@ -185,20 +191,6 @@ func (b *Block) IsValid(
 		return false
 	}
 
-	if enableAttestationValidity {
-		// verify proposer from last slot is in the first attestation object in AggregatedAttestation.
-		if isBitSet, err := bitutil.CheckBit(b.Attestations()[0].AttesterBitfield, int(proposerIndex)); !isBitSet {
-			log.Errorf("Can not locate proposer in the first attestation of AttestionRecord %v", err)
-			return false
-		}
-
-		for index, attestation := range b.Attestations() {
-			if !b.isAttestationValid(index, db, aState, cState, parentSlot) {
-				log.Errorf("attestation invalid: %v", attestation)
-				return false
-			}
-		}
-	}
 	cStateProposerRandaoSeed := cState.Validators()[proposerIndex].RandaoCommitment
 	blockRandaoReveal := b.RandaoReveal()
 	isSimulatedBlock := bytes.Equal(blockRandaoReveal[:], params.GetConfig().SimulatedBlockRandao[:])
@@ -210,6 +202,36 @@ func (b *Block) IsValid(
 	return true
 }
 
+func (b *Block) areAttestationsValid(db beaconDB, aState *ActiveState, cState *CrystallizedState, parentSlot uint64) bool {
+	for index, attestation := range b.Attestations() {
+		if !b.isAttestationValid(index, db, aState, cState, parentSlot) {
+			log.Errorf("attestation invalid: %v", attestation)
+			return false
+		}
+	}
+
+	return true
+}
+
+func (b *Block) doesParentProposerExist(cState *CrystallizedState, parentSlot uint64) bool {
+	_, parentProposerIndex, err := casper.ProposerShardAndIndex(
+		cState.ShardAndCommitteesForSlots(),
+		cState.LastStateRecalculationSlot(),
+		parentSlot)
+	if err != nil {
+		log.Errorf("Cannot get proposer index: %v", err)
+		return false
+	}
+
+	// verify proposer from last slot is in the first attestation object in AggregatedAttestation.
+	if isBitSet, err := bitutil.CheckBit(b.Attestations()[0].AttesterBitfield, int(parentProposerIndex)); !isBitSet {
+		log.Errorf("Can not locate proposer in the first attestation of AttestionRecord %v", err)
+		return false
+	}
+
+	return true
+}
+
 // UpdateAncestorHashes updates the skip list of ancestor block hashes.
 // i'th item is 2**i'th ancestor for i = 0, ..., 31.
 func UpdateAncestorHashes(parentAncestorHashes [][32]byte, parentSlotNum uint64, parentHash [32]byte) [][32]byte {

beacon-chain/types/block_test.go

@@ -81,7 +81,6 @@ func TestGenesisBlock(t *testing.T) {
 
 func TestBlockValidity(t *testing.T) {
 	cState, err := NewGenesisCrystallizedState(nil)
-
 	if err != nil {
 		t.Fatalf("failed to generate crystallized state: %v", err)
 	}
@@ -111,7 +110,7 @@ func TestBlockValidity(t *testing.T) {
 		},
 	})
 
-	parentSlot := uint64(1)
+	parentSlot := uint64(0)
 	db := &mockDB{}
 
 	if !b.isAttestationValid(0, db, aState, cState, parentSlot) {
@@ -122,10 +121,61 @@ func TestBlockValidity(t *testing.T) {
 	if !b.IsValid(db, aState, cState, parentSlot, false, genesisTime) {
 		t.Fatalf("failed block validation")
 	}
-	if !b.IsValid(db, aState, cState, parentSlot, true, genesisTime) {
-		t.Fatalf("failed block validation")
+}
+
+func TestBlockValidityNoParentProposer(t *testing.T) {
+	cState, err := NewGenesisCrystallizedState(nil)
+	if err != nil {
+		t.Fatalf("failed to generate crystallized state: %v", err)
 	}
 
+	recentBlockHashes := make([][]byte, 2*params.GetConfig().CycleLength)
+	for i := 0; i < 2*int(params.GetConfig().CycleLength); i++ {
+		recentBlockHashes = append(recentBlockHashes, make([]byte, 32))
+	}
+
+	aState := NewActiveState(&pb.ActiveState{
+		RecentBlockHashes: recentBlockHashes,
+	}, make(map[[32]byte]*utils.VoteCache))
+	parentSlot := uint64(1)
+	db := &mockDB{}
+
+	// Test case with invalid RANDAO reveal.
+	badRandaoBlock := NewBlock(&pb.BeaconBlock{
+		Slot:         2,
+		RandaoReveal: []byte{'B'},
+		Attestations: []*pb.AggregatedAttestation{
+			{
+				Slot:             0,
+				Shard:            1,
+				JustifiedSlot:    0,
+				AttesterBitfield: []byte{64, 0},
+			},
+		},
+	})
+	genesisTime := params.GetConfig().GenesisTime
+	if badRandaoBlock.IsValid(db, aState, cState, parentSlot, false, genesisTime) {
+		t.Fatalf("should have failed doesParentProposerExist")
+	}
+}
+
+func TestBlockValidityInvalidRandao(t *testing.T) {
+	cState, err := NewGenesisCrystallizedState(nil)
+	if err != nil {
+		t.Fatalf("failed to generate crystallized state: %v", err)
+	}
+
+	recentBlockHashes := make([][]byte, 2*params.GetConfig().CycleLength)
+	for i := 0; i < 2*int(params.GetConfig().CycleLength); i++ {
+		recentBlockHashes = append(recentBlockHashes, make([]byte, 32))
+	}
+
+	aState := NewActiveState(&pb.ActiveState{
+		RecentBlockHashes: recentBlockHashes,
+	}, make(map[[32]byte]*utils.VoteCache))
+	parentSlot := uint64(0)
+	db := &mockDB{}
+
 	// Test case with invalid RANDAO reveal.
 	badRandaoBlock := NewBlock(&pb.BeaconBlock{
 		Slot:         1,
@@ -139,6 +189,7 @@ func TestBlockValidity(t *testing.T) {
 			},
 		},
 	})
+	genesisTime := params.GetConfig().GenesisTime
 	if badRandaoBlock.IsValid(db, aState, cState, parentSlot, false, genesisTime) {
 		t.Fatalf("should have failed with invalid RANDAO")
 	}