Update Get Block Root function (#1339)

beacon-chain/core/blocks/block.go

@@ -53,36 +53,35 @@ func IsSlotValid(slot uint64, genesisTime time.Time) bool {
 	return clock.Now().After(validTimeThreshold)
 }
 
-// BlockRoot returns the block hash from input slot, the block hashes
-// are stored in BeaconState.
-//
+// BlockRoot returns the block root stored in the BeaconState for a given slot.
+// It returns an error if the requested block root is not within the BeaconState.
 // Spec pseudocode definition:
-//   def get_block_root(state: BeaconState, slot: int) -> Hash32:
-//     """
-//     Returns the block hash at a recent ``slot``.
-//     """
-//     earliest_slot_in_array = state.slot - len(state.latest_block_roots)
-//     assert earliest_slot_in_array <= slot < state.slot
-//     return state.latest_block_roots[slot - earliest_slot_in_array]
+// 	def get_block_root(state: BeaconState, slot: int) -> Hash32:
+//		"""
+//		returns the block root at a recent ``slot``.
+//		"""
+//		assert state.slot <= slot + LATEST_BLOCK_ROOTS_LENGTH
+//		assert slot < state.slot
+//		return state.latest_block_roots[slot % LATEST_BLOCK_ROOTS_LENGTH]
 func BlockRoot(state *pb.BeaconState, slot uint64) ([]byte, error) {
+	//	Check to see if the requested block root lies within LatestBlockRootHash32S
+	//	and if not generate error.
 	var earliestSlot uint64
-
-	// If the state slot is less than the length of state block root list, then
-	// the earliestSlot would result in a negative number. Therefore we should
-	// default earliestSlot = 0 in this case.
+	var previousSlot uint64
 	if state.Slot > uint64(len(state.LatestBlockRootHash32S)) {
 		earliestSlot = state.Slot - uint64(len(state.LatestBlockRootHash32S))
+	} else {
+		earliestSlot = 0
 	}
-
-	if slot < earliestSlot || slot >= state.Slot {
-		return []byte{}, fmt.Errorf("slot %d out of bounds: %d <= slot < %d",
+	previousSlot = state.Slot - 1
+	if state.Slot > slot+uint64(len(state.LatestBlockRootHash32S)) || slot >= state.Slot {
+		return []byte{}, fmt.Errorf("slot %d is not within expected range of %d to %d",
 			slot,
 			earliestSlot,
-			state.Slot,
+			previousSlot,
 		)
 	}
-
-	return state.LatestBlockRootHash32S[slot-earliestSlot], nil
+	return state.LatestBlockRootHash32S[slot%uint64(len(state.LatestBlockRootHash32S))], nil
 }
 
 // ProcessBlockRoots processes the previous block root into the state, by appending it

beacon-chain/core/blocks/block_test.go

@@ -40,7 +40,7 @@ func TestGenesisBlock(t *testing.T) {
 
 func TestBlockRootAtSlot_OK(t *testing.T) {
 	if params.BeaconConfig().EpochLength != 64 {
-		t.Errorf("EpochLength should be 64 for these tests to pass")
+		t.Errorf("epochLength should be 64 for these tests to pass")
 	}
 	var blockRoots [][]byte
 
@@ -73,22 +73,22 @@ func TestBlockRootAtSlot_OK(t *testing.T) {
 		}, {
 			slot:         2999,
 			stateSlot:    3000,
-			expectedRoot: []byte{127},
+			expectedRoot: []byte{55},
 		}, {
 			slot:         2873,
 			stateSlot:    3000,
-			expectedRoot: []byte{1},
+			expectedRoot: []byte{57},
 		},
 	}
 	for _, tt := range tests {
 		state.Slot = tt.stateSlot
 		result, err := BlockRoot(state, tt.slot)
 		if err != nil {
-			t.Errorf("Failed to get block root at slot %d: %v", tt.slot, err)
+			t.Errorf("failed to get block root at slot %d: %v", tt.slot, err)
 		}
 		if !bytes.Equal(result, tt.expectedRoot) {
 			t.Errorf(
-				"Result block root was an unexpected value. Wanted %d, got %d",
+				"result block root was an unexpected value. Wanted %d, got %d",
 				tt.expectedRoot,
 				result,
 			)
@@ -98,25 +98,36 @@ func TestBlockRootAtSlot_OK(t *testing.T) {
 
 func TestBlockRootAtSlot_OutOfBounds(t *testing.T) {
 	if params.BeaconConfig().EpochLength != 64 {
-		t.Errorf("EpochLength should be 64 for these tests to pass")
+		t.Errorf("epochLength should be 64 for these tests to pass")
 	}
 
-	state := &pb.BeaconState{}
+	var blockRoots [][]byte
+
+	for i := uint64(0); i < params.BeaconConfig().EpochLength*2; i++ {
+		blockRoots = append(blockRoots, []byte{byte(i)})
+	}
+	state := &pb.BeaconState{
+		LatestBlockRootHash32S: blockRoots,
+	}
 
 	tests := []struct {
 		slot        uint64
+		stateSlot   uint64
 		expectedErr string
 	}{
 		{
 			slot:        1000,
-			expectedErr: "slot 1000 out of bounds: 0 <= slot < 0",
+			stateSlot:   500,
+			expectedErr: "slot 1000 is not within expected range of 372 to 499",
 		},
 		{
 			slot:        129,
-			expectedErr: "slot 129 out of bounds: 0 <= slot < 0",
+			stateSlot:   400,
+			expectedErr: "slot 129 is not within expected range of 272 to 399",
 		},
 	}
 	for _, tt := range tests {
+		state.Slot = tt.stateSlot
 		_, err := BlockRoot(state, tt.slot)
 		if err != nil && err.Error() != tt.expectedErr {
 			t.Errorf("Expected error \"%s\" got \"%v\"", tt.expectedErr, err)
@@ -148,8 +159,8 @@ func TestProcessBlockRoots(t *testing.T) {
 	expectedRoot := hashutil.MerkleRoot(expectedHashes)
 
 	if !bytes.Equal(newState.BatchedBlockRootHash32S[0], expectedRoot[:]) {
-		t.Errorf("Saved merkle root is not equal to expected merkle root"+
-			"\n Expected %#x but got %#x", expectedRoot, newState.BatchedBlockRootHash32S[0])
+		t.Errorf("saved merkle root is not equal to expected merkle root"+
+			"\n expected %#x but got %#x", expectedRoot, newState.BatchedBlockRootHash32S[0])
 	}
 }
 
@@ -161,11 +172,11 @@ func TestForkVersion(t *testing.T) {
 	}
 
 	if ForkVersion(forkData, 9) != 2 {
-		t.Errorf("Fork Version not equal to 2 %d", ForkVersion(forkData, 9))
+		t.Errorf("fork Version not equal to 2 %d", ForkVersion(forkData, 9))
 	}
 
 	if ForkVersion(forkData, 11) != 3 {
-		t.Errorf("Fork Version not equal to 3 %d", ForkVersion(forkData, 11))
+		t.Errorf("fork Version not equal to 3 %d", ForkVersion(forkData, 11))
 	}
 }
 
@@ -179,11 +190,11 @@ func TestDomainVersion(t *testing.T) {
 	constant := uint64(math.Pow(2, 32))
 
 	if DomainVersion(forkData, 9, 2) != 2*constant+2 {
-		t.Errorf("Incorrect domain version %d", DomainVersion(forkData, 9, 2))
+		t.Errorf("incorrect domain version %d", DomainVersion(forkData, 9, 2))
 	}
 
 	if DomainVersion(forkData, 11, 3) != 3*constant+3 {
-		t.Errorf("Incorrect domain version %d", DomainVersion(forkData, 11, 3))
+		t.Errorf("incorrect domain version %d", DomainVersion(forkData, 11, 3))
 	}
 }
 
@@ -243,20 +254,20 @@ func TestDecodeDepositAmountAndTimeStamp(t *testing.T) {
 	for _, tt := range tests {
 		data, err := EncodeDepositData(tt.depositData, tt.amount, tt.timestamp)
 		if err != nil {
-			t.Fatalf("Could not encode data %v", err)
+			t.Fatalf("could not encode data %v", err)
 		}
 
 		decAmount, decTimestamp, err := DecodeDepositAmountAndTimeStamp(data)
 		if err != nil {
-			t.Fatalf("Could not decode data %v", err)
+			t.Fatalf("could not decode data %v", err)
 		}
 
 		if tt.amount != decAmount {
-			t.Errorf("Decoded amount not equal to given amount, %d : %d", decAmount, tt.amount)
+			t.Errorf("decoded amount not equal to given amount, %d : %d", decAmount, tt.amount)
 		}
 
 		if tt.timestamp != decTimestamp {
-			t.Errorf("Decoded timestamp not equal to given timestamp, %d : %d", decTimestamp, tt.timestamp)
+			t.Errorf("decoded timestamp not equal to given timestamp, %d : %d", decTimestamp, tt.timestamp)
 		}
 	}
 }

beacon-chain/core/epoch/epoch_operations_test.go

@@ -259,12 +259,12 @@ func TestHeadAttestationsOk(t *testing.T) {
 
 	prevAttestations := []*pb.PendingAttestationRecord{
 		{Data: &pb.AttestationData{Slot: 1, BeaconBlockRootHash32: []byte{'A'}}},
-		{Data: &pb.AttestationData{Slot: 2, BeaconBlockRootHash32: []byte{'B'}}},
-		{Data: &pb.AttestationData{Slot: 3, BeaconBlockRootHash32: []byte{'C'}}},
-		{Data: &pb.AttestationData{Slot: 4, BeaconBlockRootHash32: []byte{'D'}}},
+		{Data: &pb.AttestationData{Slot: 2, BeaconBlockRootHash32: []byte{'A'}}},
+		{Data: &pb.AttestationData{Slot: 3, BeaconBlockRootHash32: []byte{'A'}}},
+		{Data: &pb.AttestationData{Slot: 4, BeaconBlockRootHash32: []byte{'A'}}},
 	}
 
-	state := &pb.BeaconState{Slot: 5, LatestBlockRootHash32S: [][]byte{{'A'}, {'X'}, {'C'}, {'Y'}}}
+	state := &pb.BeaconState{Slot: 5, LatestBlockRootHash32S: [][]byte{{'A'}, {'A'}, {'A'}, {'A'}}}
 
 	headAttestations, err := PrevHeadAttestations(state, prevAttestations)
 	if err != nil {
@@ -274,15 +274,15 @@ func TestHeadAttestationsOk(t *testing.T) {
 	if headAttestations[0].Data.Slot != 1 {
 		t.Errorf("headAttestations[0] wanted slot 1, got slot %d", headAttestations[0].Data.Slot)
 	}
-	if headAttestations[1].Data.Slot != 3 {
-		t.Errorf("headAttestations[1] wanted slot 3, got slot %d", headAttestations[1].Data.Slot)
+	if headAttestations[1].Data.Slot != 2 {
+		t.Errorf("headAttestations[1] wanted slot 2, got slot %d", headAttestations[1].Data.Slot)
 	}
 	if !bytes.Equal([]byte{'A'}, headAttestations[0].Data.BeaconBlockRootHash32) {
 		t.Errorf("headAttestations[0] wanted hash [A], got slot %v",
 			headAttestations[0].Data.BeaconBlockRootHash32)
 	}
-	if !bytes.Equal([]byte{'C'}, headAttestations[1].Data.BeaconBlockRootHash32) {
-		t.Errorf("headAttestations[1] wanted hash [C], got slot %v",
+	if !bytes.Equal([]byte{'A'}, headAttestations[1].Data.BeaconBlockRootHash32) {
+		t.Errorf("headAttestations[1] wanted hash [A], got slot %v",
 			headAttestations[1].Data.BeaconBlockRootHash32)
 	}
 }

beacon-chain/core/state/transition_test.go

@@ -479,14 +479,14 @@ func TestProcessEpoch_InactiveConditions(t *testing.T) {
 
 func TestProcessEpoch_CantGetBoundaryAttestation(t *testing.T) {
 	state := &pb.BeaconState{
-		Slot: 1,
+		Slot: 5,
 		LatestAttestations: []*pb.PendingAttestationRecord{
-			{Data: &pb.AttestationData{}},
+			{Data: &pb.AttestationData{Slot: 4}},
 		}}
 
 	want := fmt.Sprintf(
-		"could not get current boundary attestations: slot %d out of bounds: %d <= slot < %d",
-		state.LatestAttestations[0].Data.Slot, state.Slot, state.Slot,
+		"could not get current boundary attestations: slot %d is not within expected range of %d to %d",
+		0, state.Slot, state.Slot-1,
 	)
 	if _, err := ProcessEpoch(state); !strings.Contains(err.Error(), want) {
 		t.Errorf("Expected: %s, received: %v", want, err)
@@ -559,13 +559,14 @@ func TestProcessEpoch_CantGetPrevValidatorIndices(t *testing.T) {
 
 func TestProcessEpoch_CantProcessCurrentBoundaryAttestations(t *testing.T) {
 	state := &pb.BeaconState{
+		Slot: 100,
 		LatestAttestations: []*pb.PendingAttestationRecord{
 			{Data: &pb.AttestationData{}},
 		}}
 
 	want := fmt.Sprintf(
-		"could not get current boundary attestations: slot %d out of bounds: %d <= slot < %d",
-		state.LatestAttestations[0].Data.Slot, state.Slot, state.Slot,
+		"could not get prev boundary attestations: slot %d is not within expected range of %d to %d",
+		0, state.Slot, state.Slot-1,
 	)
 	if _, err := ProcessEpoch(state); !strings.Contains(err.Error(), want) {
 		t.Errorf("Expected: %s, received: %v", want, err)