Fixed Bugs for Proposer Attestation (#539)

beacon-chain/casper/incentives.go

@@ -11,16 +11,21 @@ var log = logrus.WithField("prefix", "casper")
 
 // CalculateRewards adjusts validators balances by applying rewards or penalties
 // based on FFG incentive structure.
-func CalculateRewards(attestations []*pb.AggregatedAttestation, validators []*pb.ValidatorRecord, dynasty uint64, totalDeposit uint64) ([]*pb.ValidatorRecord, error) {
+func CalculateRewards(
+	attestations []*pb.AggregatedAttestation,
+	validators []*pb.ValidatorRecord,
+	dynasty uint64,
+	totalDeposit uint64,
+	totalParticipatedDeposit uint64) ([]*pb.ValidatorRecord, error) {
 	activeValidators := ActiveValidatorIndices(validators, dynasty)
-	attesterDeposits := GetAttestersTotalDeposit(attestations)
+	attesterBitfield := attestations[len(attestations)-1].AttesterBitfield
+	attesterFactor := totalParticipatedDeposit * 3
+	totalFactor := totalDeposit * 2
 
-	attesterFactor := attesterDeposits * 3
-	totalFactor := uint64(totalDeposit * 2)
 	if attesterFactor >= totalFactor {
 		log.Debug("Applying rewards and penalties for the validators from last cycle")
 		for i, attesterIndex := range activeValidators {
-			voted := shared.CheckBit(attestations[len(attestations)-1].AttesterBitfield, int(attesterIndex))
+			voted := shared.CheckBit(attesterBitfield, int(attesterIndex))
 			if voted {
 				validators[i].Balance += params.AttesterReward
 			} else {

beacon-chain/casper/incentives_test.go

@@ -27,7 +27,9 @@ func TestComputeValidatorRewardsAndPenalties(t *testing.T) {
 		testAttesterBitfield,
 		data.Validators,
 		data.CurrentDynasty,
-		data.TotalDeposits)
+		data.TotalDeposits,
+		1000)
+
 	if err != nil {
 		t.Fatalf("could not compute validator rewards and penalties: %v", err)
 	}

beacon-chain/casper/sharding_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestGetShardAndCommitteesForSlots(t *testing.T) {
 	state := &pb.CrystallizedState{
-		LastStateRecalc: 1,
+		LastStateRecalc: 65,
 		ShardAndCommitteesForSlots: []*pb.ShardAndCommitteeArray{
 			{ArrayShardAndCommittee: []*pb.ShardAndCommittee{
 				{ShardId: 1, Committee: []uint32{0, 1, 2, 3, 4}},

beacon-chain/casper/validator.go

@@ -92,22 +92,19 @@ func SampleAttestersAndProposers(seed common.Hash, validators []*pb.ValidatorRec
 	return indices[:int(attesterCount)], indices[len(indices)-1], nil
 }
 
-// GetAttestersTotalDeposit from the pending attestations.
-func GetAttestersTotalDeposit(attestations []*pb.AggregatedAttestation) uint64 {
-	var numOfBits int
-	for _, attestation := range attestations {
-		numOfBits += int(shared.BitSetCount(attestation.AttesterBitfield))
-	}
-	// Assume there's no slashing condition, the following logic will change later phase.
-	return uint64(numOfBits) * params.DefaultBalance
-}
-
 // GetShardAndCommitteesForSlot returns the attester set of a given slot.
-func GetShardAndCommitteesForSlot(shardCommittees []*pb.ShardAndCommitteeArray, lcs uint64, slot uint64) (*pb.ShardAndCommitteeArray, error) {
-	if !(lcs <= slot && slot < lcs+params.CycleLength*2) {
-		return nil, fmt.Errorf("can not return attester set of given slot, input slot %v has to be in between %v and %v", slot, lcs, lcs+params.CycleLength*2)
+func GetShardAndCommitteesForSlot(shardCommittees []*pb.ShardAndCommitteeArray, lastStateRecalc uint64, slot uint64) (*pb.ShardAndCommitteeArray, error) {
+	if lastStateRecalc < params.CycleLength {
+		lastStateRecalc = 0
+	} else {
+		lastStateRecalc = lastStateRecalc - params.CycleLength
 	}
-	return shardCommittees[slot-lcs], nil
+
+	if !(lastStateRecalc <= slot && slot < lastStateRecalc+params.CycleLength*2) {
+		return nil, fmt.Errorf("can not return attester set of given slot, input slot %v has to be in between %v and %v", slot, lastStateRecalc, lastStateRecalc+params.CycleLength*2)
+	}
+
+	return shardCommittees[slot-lastStateRecalc], nil
 }
 
 // AreAttesterBitfieldsValid validates that the length of the attester bitfield matches the attester indices
@@ -137,17 +134,11 @@ func AreAttesterBitfieldsValid(attestation *pb.AggregatedAttestation, attesterIn
 	return true
 }
 
-// GetProposerIndexAndShard returns the index and the shardID of a proposer from a given slot.
-func GetProposerIndexAndShard(shardCommittees []*pb.ShardAndCommitteeArray, lcs uint64, slot uint64) (uint64, uint64, error) {
-	if lcs < params.CycleLength {
-		lcs = 0
-	} else {
-		lcs = lcs - params.CycleLength
-	}
-
+// ProposerShardAndIndex returns the index and the shardID of a proposer from a given slot.
+func ProposerShardAndIndex(shardCommittees []*pb.ShardAndCommitteeArray, lastStateRecalc uint64, slot uint64) (uint64, uint64, error) {
 	slotCommittees, err := GetShardAndCommitteesForSlot(
 		shardCommittees,
-		lcs,
+		lastStateRecalc,
 		slot)
 	if err != nil {
 		return 0, 0, err

beacon-chain/casper/validator_test.go

@@ -192,24 +192,33 @@ func TestAreAttesterBitfieldsValidNoZerofill(t *testing.T) {
 	}
 }
 
-func TestGetProposerIndexAndShard(t *testing.T) {
+func TestProposerShardAndIndex(t *testing.T) {
 	shardCommittees := []*pb.ShardAndCommitteeArray{
 		{ArrayShardAndCommittee: []*pb.ShardAndCommittee{
-			{ShardId: 99, Committee: []uint32{0, 1, 2, 3, 4}},
+			{ShardId: 0, Committee: []uint32{0, 1, 2, 3, 4}},
+			{ShardId: 1, Committee: []uint32{5, 6, 7, 8, 9}},
+		}},
+		{ArrayShardAndCommittee: []*pb.ShardAndCommittee{
+			{ShardId: 2, Committee: []uint32{10, 11, 12, 13, 14}},
+			{ShardId: 3, Committee: []uint32{15, 16, 17, 18, 19}},
+		}},
+		{ArrayShardAndCommittee: []*pb.ShardAndCommittee{
+			{ShardId: 4, Committee: []uint32{20, 21, 22, 23, 24}},
+			{ShardId: 5, Committee: []uint32{25, 26, 27, 28, 29}},
 		}},
 	}
-	if _, _, err := GetProposerIndexAndShard(shardCommittees, 100, 0); err == nil {
-		t.Error("GetProposerIndexAndShard should have failed with invalid lcs")
+	if _, _, err := ProposerShardAndIndex(shardCommittees, 100, 0); err == nil {
+		t.Error("ProposerShardAndIndex should have failed with invalid lcs")
 	}
-	shardID, index, err := GetProposerIndexAndShard(shardCommittees, 0, 0)
+	shardID, index, err := ProposerShardAndIndex(shardCommittees, 128, 64)
 	if err != nil {
-		t.Fatalf("GetProposerIndexAndShard failed with %v", err)
+		t.Fatalf("ProposerShardAndIndex failed with %v", err)
 	}
-	if shardID != 99 {
-		t.Errorf("Invalid shard ID. Wanted 99, got %d", shardID)
+	if shardID != 0 {
+		t.Errorf("Invalid shard ID. Wanted 0, got %d", shardID)
 	}
-	if index != 0 {
-		t.Errorf("Invalid proposer index. Wanted 0, got %d", index)
+	if index != 4 {
+		t.Errorf("Invalid proposer index. Wanted 4, got %d", index)
 	}
 }
 

beacon-chain/sync/service.go

@@ -176,12 +176,13 @@ func (ss *Service) run() {
 				log.Errorf("Failed to get parent slot: %v", err)
 				continue
 			}
-			proposerShardID, _, err := casper.GetProposerIndexAndShard(cState.ShardAndCommitteesForSlots(), cState.LastStateRecalc(), parentSlot)
+			proposerShardID, _, err := casper.ProposerShardAndIndex(cState.ShardAndCommitteesForSlots(), cState.LastStateRecalc(), parentSlot)
 			if err != nil {
 				log.Errorf("Failed to get proposer shard ID: %v", err)
 				continue
 			}
-			if err := attestation.VerifyAttestation(proposerShardID); err != nil {
+			// TODO(#258): stubbing public key with empty 32 bytes.
+			if err := attestation.VerifyProposerAttestation([32]byte{}, proposerShardID); err != nil {
 				log.Errorf("Failed to verify proposer attestation: %v", err)
 				continue
 			}

beacon-chain/types/attestation.go

@@ -123,9 +123,9 @@ func (a *Attestation) AggregateSig() []uint64 {
 	return a.data.AggregateSig
 }
 
-// VerifyAttestation verifies the proposer's attestation of the block.
+// VerifyProposerAttestation verifies the proposer's attestation of the block.
 // Proposers broadcast the attestation along with the block to its peers.
-func (a *Attestation) VerifyAttestation(proposerShardID uint64) error {
+func (a *Attestation) VerifyProposerAttestation(pubKey [32]byte, proposerShardID uint64) error {
 
 	// Verify the attestation attached with block response.
 	// Get proposer index and shardID.
@@ -137,9 +137,10 @@ func (a *Attestation) VerifyAttestation(proposerShardID uint64) error {
 		proposerShardID,
 		a.JustifiedSlotNumber())
 
-	log.Infof("Constructed attestation message for incoming block 0x%x", attestationMsg)
+	log.Infof("Constructing attestation message for incoming block 0x%x", attestationMsg)
 
 	// TODO(#258): use attestationMsg to verify against signature and public key. Return error if incorrect.
+	log.Infof("Verifying attestation with public key 0x%x", pubKey)
 
 	log.Info("successfully verified attestation with incoming block")
 	return nil

beacon-chain/types/attestation_test.go

@@ -52,7 +52,7 @@ func TestAttestation(t *testing.T) {
 	if !bytes.Equal(attestation.ShardBlockHash(), []byte{0}) {
 		t.Errorf("mismatched shard block hash")
 	}
-	if err := attestation.VerifyAttestation(0); err != nil {
+	if err := attestation.VerifyProposerAttestation([32]byte{}, 0); err != nil {
 		t.Errorf("verify attestation failed: %v", err)
 	}
 }

beacon-chain/types/block.go

@@ -164,9 +164,8 @@ func (b *Block) IsValid(aState *ActiveState, cState *CrystallizedState, parentSl
 		return false
 	}
 
-	// verify proposer from last slot is in one of the AggregatedAttestation.
-	var proposerAttested bool
-	_, proposerIndex, err := casper.GetProposerIndexAndShard(
+	// verify proposer from last slot is in the first attestation object in AggregatedAttestation.
+	_, proposerIndex, err := casper.ProposerShardAndIndex(
 		cState.ShardAndCommitteesForSlots(),
 		cState.LastStateRecalc(),
 		parentSlot)
@@ -174,17 +173,19 @@ func (b *Block) IsValid(aState *ActiveState, cState *CrystallizedState, parentSl
 		log.Errorf("Can not get proposer index %v", err)
 		return false
 	}
+	if !shared.CheckBit(b.Attestations()[0].AttesterBitfield, int(proposerIndex)) {
+		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, aState, cState, parentSlot) {
-			log.Debugf("attestation invalid: %v", attestation)
+			log.Errorf("Attestation invalid: %v", attestation)
 			return false
 		}
-		if shared.BitSetCount(attestation.AttesterBitfield) == 1 && shared.CheckBit(attestation.AttesterBitfield, int(proposerIndex)) {
-			proposerAttested = true
-		}
 	}
 
-	return proposerAttested
+	return true
 }
 
 // isAttestationValid validates an attestation in a block.

beacon-chain/types/crystallized_state.go

@@ -224,6 +224,7 @@ func (c *CrystallizedState) getAttesterIndices(attestation *pb.AggregatedAttesta
 // We also check for dynasty transition and compute for a new dynasty if necessary during this transition.
 func (c *CrystallizedState) NewStateRecalculations(aState *ActiveState, block *Block) (*CrystallizedState, error) {
 	var blockVoteBalance uint64
+	var totalParticipatedDeposits uint64
 	justifiedStreak := c.JustifiedStreak()
 	justifiedSlot := c.LastJustifiedSlot()
 	finalizedSlot := c.LastFinalizedSlot()
@@ -240,9 +241,11 @@ func (c *CrystallizedState) NewStateRecalculations(aState *ActiveState, block *B
 		blockHash := recentBlockHashes[i]
 		if _, ok := blockVoteCache[blockHash]; ok {
 			blockVoteBalance = blockVoteCache[blockHash].VoteTotalDeposit
+			totalParticipatedDeposits += blockVoteCache[blockHash].VoteTotalDeposit
 		} else {
 			blockVoteBalance = 0
 		}
+		// TODO(#542): This should have been total balance of the validators in the slot committee.
 		if 3*blockVoteBalance >= 2*c.TotalDeposits() {
 			if slot > justifiedSlot {
 				justifiedSlot = slot
@@ -263,11 +266,15 @@ func (c *CrystallizedState) NewStateRecalculations(aState *ActiveState, block *B
 	}
 
 	// TODO(471): Update rewards and penalties scheme to align with latest spec.
-	rewardedValidators, _ := casper.CalculateRewards(
-		aState.PendingAttestations(),
-		c.Validators(),
-		c.CurrentDynasty(),
-		c.TotalDeposits())
+	var rewardedValidators []*pb.ValidatorRecord
+	if len(block.Attestations()) != 0 {
+		rewardedValidators, _ = casper.CalculateRewards(
+			block.Attestations(),
+			c.Validators(),
+			c.CurrentDynasty(),
+			c.TotalDeposits(),
+			totalParticipatedDeposits)
+	}
 
 	// Get all active validators and calculate total balance for next cycle.
 	var nextCycleBalance uint64

beacon-chain/types/crystallized_state_test.go

@@ -35,8 +35,23 @@ func TestInitialDeriveCrystallizedState(t *testing.T) {
 		t.Fatalf("Failed to initialize crystallized state: %v", err)
 	}
 
+	var attesterBitfield []byte
+	for len(attesterBitfield)*8 < params.BootstrappedValidatorsCount {
+		attesterBitfield = append(attesterBitfield, byte(0))
+	}
+
 	aState := NewGenesisActiveState()
-	block := NewBlock(nil)
+	block := NewBlock(&pb.BeaconBlock{
+		ParentHash:            []byte{},
+		SlotNumber:            0,
+		ActiveStateHash:       []byte{},
+		CrystallizedStateHash: []byte{},
+		Attestations: []*pb.AggregatedAttestation{{
+			Slot:             0,
+			AttesterBitfield: attesterBitfield,
+			ShardId:          0,
+		}},
+	})
 
 	newCState, err := cState.NewStateRecalculations(aState, block)
 	if err != nil {

shared/testutil/checkbit.go

@@ -9,7 +9,6 @@ func CheckBit(bitfield []byte, index int) bool {
 	} else {
 		chunkLocation++
 	}
-
 	field := bitfield[chunkLocation-1] >> (8 - uint(indexLocation))
 	return field%2 != 0
 }