[Integration] Fixes finalized block height with block state bug

integration/tests/common/block_state.go

@@ -23,65 +23,89 @@ type BlockState struct {
 	highestSealed     *messages.BlockProposal
 }
 
-// TODO refactor to remove deep indentation
+func NewBlockState() *BlockState {
+	return &BlockState{
+		Mutex:             sync.Mutex{},
+		blocksByID:        make(map[flow.Identifier]*messages.BlockProposal),
+		finalizedByHeight: make(map[uint64]*messages.BlockProposal),
+	}
+}
+
 func (bs *BlockState) Add(b *messages.BlockProposal) {
 	bs.Lock()
 	defer bs.Unlock()
 
-	if bs.blocksByID == nil {
-		bs.blocksByID = make(map[flow.Identifier]*messages.BlockProposal) // TODO: initialize this map in constructor
-	}
 	bs.blocksByID[b.Header.ID()] = b
-	bs.highestProposed = b.Header.Height
+	if b.Header.Height > bs.highestProposed {
+		bs.highestProposed = b.Header.Height
+	}
 
-	// add confirmations
-	confirmsHeight := b.Header.Height - 3
-	if b.Header.Height >= 3 && confirmsHeight > bs.highestFinalized {
-		if bs.finalizedByHeight == nil {
-			bs.finalizedByHeight = make(map[uint64]*messages.BlockProposal) // TODO: initialize this map in constructor
-		}
-		// put all ancestors into `finalizedByHeight`
-		ancestor, ok := b, true
-		for ancestor.Header.Height > bs.highestFinalized {
-			h := ancestor.Header.Height
+	if b.Header.Height < 3 {
+		return
+	}
 
-			// if ancestor is confirmed put it into the finalized map
-			if h <= confirmsHeight {
+	confirmsHeight := b.Header.Height - uint64(3)
+	if confirmsHeight < bs.highestFinalized {
+		return
+	}
 
-				finalized := ancestor
-				bs.finalizedByHeight[h] = finalized
+	bs.processAncestors(b, confirmsHeight)
+	bs.updateHighestFinalizedHeight()
+}
 
-				// if ancestor confirms a heigher height than highestFinalized, increase highestFinalized
-				if h > bs.highestFinalized {
-					bs.highestFinalized = h
+// processAncestors checks whether ancestors of block are within the confirming height, and finalizes
+// them if that is the case.
+// It also processes the seals of blocks being finalized.
+func (bs *BlockState) processAncestors(b *messages.BlockProposal, confirmsHeight uint64) {
+	// puts this block proposal and all ancestors into `finalizedByHeight`
+	ancestor, ok := b, true
+	for ancestor.Header.Height > bs.highestFinalized {
+		h := ancestor.Header.Height
+
+		// if ancestor is confirmed put it into the finalized map
+		if h <= confirmsHeight {
+
+			finalized := ancestor
+			bs.finalizedByHeight[h] = finalized
+
+			// update last sealed height
+			for _, seal := range finalized.Payload.Seals {
+				sealed, ok := bs.blocksByID[seal.BlockID]
+				if !ok {
+					continue
 				}
 
-				// update last sealed height
-				for _, seal := range finalized.Payload.Seals {
-					sealed, ok := bs.blocksByID[seal.BlockID]
-					if !ok {
-						continue
-					}
-
-					if bs.highestSealed == nil ||
-						sealed.Header.Height > bs.highestSealed.Header.Height {
-						bs.highestSealed = sealed
-					}
+				if bs.highestSealed == nil ||
+					sealed.Header.Height > bs.highestSealed.Header.Height {
+					bs.highestSealed = sealed
 				}
-
 			}
 
-			// find parent
-			ancestor, ok = bs.blocksByID[ancestor.Header.ParentID]
+		}
 
-			// stop if parent not found
-			if !ok {
-				return
-			}
+		// find parent
+		ancestor, ok = bs.blocksByID[ancestor.Header.ParentID]
+
+		// stop if parent not found
+		if !ok {
+			return
 		}
 	}
 }
 
+// updateHighestFinalizedHeight moves forward the highestFinalized height for the newly finalized blocks.
+func (bs *BlockState) updateHighestFinalizedHeight() {
+	for {
+		// checks whether next height has been finalized and updates highest finalized height
+		// if that is the case.
+		if _, ok := bs.finalizedByHeight[bs.highestFinalized+1]; !ok {
+			return
+		}
+
+		bs.highestFinalized++
+	}
+}
+
 // WaitForFirstFinalized waits until the first block is finalized
 func (bs *BlockState) WaitForFirstFinalized(t *testing.T) *messages.BlockProposal {
 	require.Eventually(t, func() bool {

integration/tests/common/testnet_state_tracker.go

@@ -15,7 +15,7 @@ import (
 
 type TestnetStateTracker struct {
 	ghostTracking bool
-	BlockState    BlockState
+	BlockState    *BlockState
 	ReceiptState  ReceiptState
 	ApprovalState ResultApprovalState
 	MsgState      MsgState
@@ -25,7 +25,7 @@ type TestnetStateTracker struct {
 // be used to stop tracking
 func (tst *TestnetStateTracker) Track(t *testing.T, ctx context.Context, ghost *client.GhostClient) {
 	// reset the state for in between tests
-	tst.BlockState = BlockState{}
+	tst.BlockState = NewBlockState()
 	tst.ReceiptState = ReceiptState{}
 
 	var reader *client.FlowMessageStreamReader

integration/tests/execution/chunk_data_pack_test.go

@@ -49,7 +49,7 @@ func (gs *ChunkDataPacksSuite) TestVerificationNodesRequestChunkDataPacks() {
 	require.NoError(gs.T(), err, "could not deploy counter")
 
 	// wait until we see a different state commitment for a finalized block, call that block blockB
-	blockB, _ := common.WaitUntilFinalizedStateCommitmentChanged(gs.T(), &gs.BlockState, &gs.ReceiptState)
+	blockB, _ := common.WaitUntilFinalizedStateCommitmentChanged(gs.T(), gs.BlockState, &gs.ReceiptState)
 	gs.T().Logf("got blockB height %v ID %v", blockB.Header.Height, blockB.Header.ID())
 
 	// wait for execution receipt for blockB from execution node 1

integration/tests/execution/failing_tx_reverted_test.go

@@ -37,7 +37,7 @@ func (s *FailingTxRevertedSuite) TestExecutionFailingTxReverted() {
 	require.NoError(s.T(), err, "could not deploy counter")
 
 	// wait until we see a different state commitment for a finalized block, call that block blockB
-	blockB, erBlockB := common.WaitUntilFinalizedStateCommitmentChanged(s.T(), &s.BlockState, &s.ReceiptState)
+	blockB, erBlockB := common.WaitUntilFinalizedStateCommitmentChanged(s.T(), s.BlockState, &s.ReceiptState)
 	s.T().Logf("got blockB height %v ID %v", blockB.Header.Height, blockB.Header.ID())
 
 	// final states

integration/tests/execution/state_sync_test.go

@@ -41,7 +41,7 @@ func (s *StateSyncSuite) TestStateSyncAfterNetworkPartition() {
 	require.NoError(s.T(), err, "could not deploy counter")
 
 	// wait until we see a different state commitment for a finalized block, call that block blockB
-	blockB, _ := common.WaitUntilFinalizedStateCommitmentChanged(s.T(), &s.BlockState, &s.ReceiptState)
+	blockB, _ := common.WaitUntilFinalizedStateCommitmentChanged(s.T(), s.BlockState, &s.ReceiptState)
 	s.T().Logf("got blockB height %v ID %v", blockB.Header.Height, blockB.Header.ID())
 
 	// wait for execution receipt for blockB from execution node 1