Added Comments/Documentation on Recent Block Hashes of ActiveState (#533)

* Add some clarifying comments to calculateNewBlockHash

* missing )

* words

beacon-chain/types/active_state.go

@@ -21,7 +21,7 @@ type ActiveState struct {
 
 // NewGenesisActiveState initializes the active state for slot 0.
 func NewGenesisActiveState() *ActiveState {
-	// Bootstrap recent block hashes to all 0s for first 2 cycles (128 slots).
+	// Bootstrap recent block hashes to all 0s for first 2 cycles.
 	var recentBlockHashes [][]byte
 	for i := 0; i < 2*params.CycleLength; i++ {
 		recentBlockHashes = append(recentBlockHashes, make([]byte, 0, 32))
@@ -102,15 +102,39 @@ func (a *ActiveState) calculateNewAttestations(add []*pb.AggregatedAttestation,
 	return update
 }
 
+// calculateNewBlockHashes builds a new slice of recent block hashes with the
+// provided block and the parent slot number.
+//
+// The algorithm is:
+//   1) shift the array by block.SlotNumber - parentSlot (i.e. truncate the
+//     first by the number of slots that have occurred between the block and
+//     its parent).
+//
+//   2) fill the array with the block hash for all values between the parent
+//     slot and the block slot.
+//
+// Computation of the active state hash depends on this feature that slots with
+// missing blocks have the block hash of the next block hash in the chain.
+//
+// For example, if we have a segment of recent block hashes that look like this
+//   [0xF, 0x7, 0x0, 0x0, 0x5]
+//
+// Where 0x0 is an empty or missing hash where no block was produced in the
+// alloted slot. When storing the list (or at least when computing the hash of
+// the active state), the list should be backfilled as such:
+//
+//   [0xF, 0x7, 0x5, 0x5, 0x5]
+//
+// This method does not mutate the active state.
 func (a *ActiveState) calculateNewBlockHashes(block *Block, parentSlot uint64) ([][]byte, error) {
 	hash, err := block.Hash()
 	if err != nil {
 		return nil, err
 	}
 
-	dist := block.SlotNumber() - parentSlot
+	distance := block.SlotNumber() - parentSlot
 	existing := a.data.RecentBlockHashes
-	update := existing[dist:]
+	update := existing[distance:]
 	for len(update) < 2*params.CycleLength {
 		update = append(update, hash[:])
 	}

beacon-chain/types/active_state_test.go

@@ -1,13 +1,15 @@
 package types
 
 import (
+	"bytes"
+	"reflect"
 	"testing"
 
 	"github.com/prysmaticlabs/prysm/beacon-chain/params"
 	pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1"
 )
 
-func TestGenesisActiveState(t *testing.T) {
+func TestGenesisActiveState_HashEquality(t *testing.T) {
 	aState1 := NewGenesisActiveState()
 	aState2 := NewGenesisActiveState()
 
@@ -23,6 +25,26 @@ func TestGenesisActiveState(t *testing.T) {
 	}
 }
 
+func TestGenesisActiveState_InitializesRecentBlockHashes(t *testing.T) {
+	as := NewGenesisActiveState()
+	want, got := len(as.data.RecentBlockHashes), 2*params.CycleLength
+	if want != got {
+		t.Errorf("Wrong number of recent block hashes. Got: %d Want: %d", got, want)
+	}
+
+	want = cap(as.data.RecentBlockHashes)
+	if want != got {
+		t.Errorf("The slice underlying array capacity is wrong. Got: %d Want: %d", got, want)
+	}
+
+	zero := make([]byte, 0, 32)
+	for _, h := range as.data.RecentBlockHashes {
+		if !bytes.Equal(h, zero) {
+			t.Errorf("Unexpected non-zero hash data: %v", h)
+		}
+	}
+}
+
 func TestUpdateAttestations(t *testing.T) {
 	aState := NewGenesisActiveState()
 
@@ -112,6 +134,42 @@ func TestUpdateRecentBlockHashes(t *testing.T) {
 	}
 }
 
+func TestCalculateNewBlockHashes_DoesNotMutateData(t *testing.T) {
+	interestingData := [][]byte{
+		[]byte("hello"),
+		[]byte("world"),
+		[]byte("block"),
+		[]byte("hash"),
+	}
+
+	s := NewGenesisActiveState()
+	for i, d := range interestingData {
+		s.data.RecentBlockHashes[i] = d
+	}
+	original := make([][]byte, 2*params.CycleLength)
+	copy(original, s.data.RecentBlockHashes)
+
+	if !reflect.DeepEqual(s.data.RecentBlockHashes, original) {
+		t.Fatal("setup data should be equal!")
+	}
+
+	block := &Block{
+		data: &pb.BeaconBlock{
+			SlotNumber: 2,
+		},
+	}
+
+	result, _ := s.calculateNewBlockHashes(block, 0 /*parentSlot*/)
+
+	if !reflect.DeepEqual(s.data.RecentBlockHashes, original) {
+		t.Error("data has mutated from the original")
+	}
+
+	if reflect.DeepEqual(result, original) {
+		t.Error("the resulting data did not change from the original")
+	}
+}
+
 func TestBlockVoteCacheNoAttestations(t *testing.T) {
 	aState := NewGenesisActiveState()
 	cState, err := NewGenesisCrystallizedState()