From 79eb9a5230f9dab399d4da708d99f15f29c8ea9a Mon Sep 17 00:00:00 2001 From: Jerry Fireman Date: Tue, 5 Feb 2019 12:55:15 -0500 Subject: [PATCH] Update Get Block Root function (#1339) --- beacon-chain/core/blocks/block.go | 39 +++++++-------- beacon-chain/core/blocks/block_test.go | 49 ++++++++++++------- .../core/epoch/epoch_operations_test.go | 16 +++--- beacon-chain/core/state/transition_test.go | 13 ++--- 4 files changed, 64 insertions(+), 53 deletions(-) diff --git a/beacon-chain/core/blocks/block.go b/beacon-chain/core/blocks/block.go index 6ae8c42c82..4d4a2a23de 100644 --- a/beacon-chain/core/blocks/block.go +++ b/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 diff --git a/beacon-chain/core/blocks/block_test.go b/beacon-chain/core/blocks/block_test.go index 9ec17a26b4..179671905b 100644 --- a/beacon-chain/core/blocks/block_test.go +++ b/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) } } } diff --git a/beacon-chain/core/epoch/epoch_operations_test.go b/beacon-chain/core/epoch/epoch_operations_test.go index f8c9c2c748..e547b6d503 100644 --- a/beacon-chain/core/epoch/epoch_operations_test.go +++ b/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) } } diff --git a/beacon-chain/core/state/transition_test.go b/beacon-chain/core/state/transition_test.go index 6e8e3f5325..cb52a174ac 100644 --- a/beacon-chain/core/state/transition_test.go +++ b/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)