WIP: Z/fix reorg logic

lib/blockchain.go

@@ -245,6 +245,11 @@ func (nn *BlockNode) GetEncoderType() EncoderType {
 }
 
 func (nn *BlockNode) GetParent(blockIndex *BlockIndex) *BlockNode {
+	// No header or no parent hash => no parent.
+	if nn.Header == nil || nn.Header.PrevBlockHash == nil {
+		return nil
+	}
+
 	// If we don't have a parent, try to get it from the block index. We
 	// no longer have a guarantee that we have set the parent node since
 	// we no longer keep the entire block index in memory.

lib/pos_blockchain.go

@@ -1673,6 +1673,12 @@ func (bc *Blockchain) tryApplyNewTip(blockNode *BlockNode, currentView uint64, l
 		connectedBlockHashes,
 		disconnectedBlockHashes,
 	)
+
+	// Flush any cached UtxoViews that correspond to blocks we just disconnected.
+	// (Prevents stale views from being reused after a re-org.)
+	for _, h := range uniqueDisconnectedBlockHashes {
+		bc.blockViewCache.Delete(h)
+	}
 	return true, uniqueConnectedBlockHashes, uniqueDisconnectedBlockHashes, nil
 }
 
@@ -1686,6 +1692,24 @@ func (bc *Blockchain) shouldReorg(blockNode *BlockNode, currentView uint64) bool
 	if chainTip.Hash.IsEqual(blockNode.Header.PrevBlockHash) {
 		return false
 	}
+
+	// Re-orgs must NEVER discard committed history.
+	committedTip, exists := bc.GetCommittedTip()
+	if !exists {
+		// No committed history yet; fall back to view-based rule.
+		return blockNode.Header.ProposedInView >= currentView
+	}
+	// Determine the fork-point (the parent of the candidate).
+	parent := blockNode.GetParent(bc.blockIndex)
+	if parent == nil {
+		return false // cannot safely determine fork point
+	}
+	// If the fork-point height is below the committed tip height we would
+	// detach a committed block disallowed.
+	if parent.Height < committedTip.Height {
+		return false
+	}
+
 	// If the block is proposed in a view less than the current view, there's no need to reorg.
 	return blockNode.Header.ProposedInView >= currentView
 }
@@ -1702,6 +1726,11 @@ func (bc *Blockchain) addTipBlockToBestChain(blockNode *BlockNode) {
 func (bc *Blockchain) removeTipBlockFromBestChain() *BlockNode {
 	// Remove the last block from the best chain.
 	lastBlock := bc.blockIndex.GetTip()
+	// TODO: This should be an error, for now while debugging we'll just log it.
+	if lastBlock.IsCommitted() {
+		fmt.Printf("BUG: attempted to remove committed block %v\n", lastBlock.Hash)
+	}
+
 	bc.blockIndex.setTip(lastBlock.GetParent(bc.blockIndex))
 	return lastBlock
 }

lib/pos_blockchain_test.go

@@ -1269,6 +1269,79 @@ func TestShouldReorg(t *testing.T) {
 	require.True(t, bc.shouldReorg(newBlock, 2))
 }
 
+// TestShouldReorgCommittedSafety verifies that a re-org which would
+// detach committed history is rejected, and that a re-org starting at
+// the committed tip is still allowed.
+func TestShouldReorgCommittedSafety(t *testing.T) {
+	bc, _, _ := NewTestBlockchain(t)
+
+	// Build mock chain:
+	h0 := NewBlockHash(RandomBytes(32)) // genesis (committed implicitly)
+	h1 := NewBlockHash(RandomBytes(32))
+	h2 := NewBlockHash(RandomBytes(32))
+
+	genesis := &BlockNode{
+		Hash:   h0,
+		Height: 0,
+		Status: StatusBlockStored | StatusBlockValidated | StatusBlockCommitted,
+		Header: &MsgDeSoHeader{Height: 0, ProposedInView: 0},
+	}
+	committed := &BlockNode{
+		Hash:   h1,
+		Height: 1,
+		Status: StatusBlockStored | StatusBlockValidated | StatusBlockCommitted,
+		Header: &MsgDeSoHeader{
+			Height:         1,
+			ProposedInView: 1,
+			PrevBlockHash:  h0,
+		},
+	}
+	uncommitted := &BlockNode{
+		Hash:   h2,
+		Height: 2,
+		Status: StatusBlockStored | StatusBlockValidated,
+		Header: &MsgDeSoHeader{
+			Height:         2,
+			ProposedInView: 2,
+			PrevBlockHash:  h1,
+		},
+	}
+
+	// Load into block-index and make uncommitted the tip.
+	require.NoError(t, bc.blockIndex.setBlockIndexFromMap(map[BlockHash]*BlockNode{
+		*h0: genesis,
+		*h1: committed,
+		*h2: uncommitted,
+	}))
+	bc.addTipBlockToBestChain(genesis)
+	bc.addTipBlockToBestChain(committed)
+	bc.addTipBlockToBestChain(uncommitted)
+
+	// --- Candidate that forks _below_ committed tip (parent = genesis) ---
+	badFork := &BlockNode{
+		Hash:   NewBlockHash(RandomBytes(32)),
+		Height: 2,
+		Header: &MsgDeSoHeader{
+			Height:         2,
+			ProposedInView: 3,
+			PrevBlockHash:  h0, // fork-point below committed tip
+		},
+	}
+	require.False(t, bc.shouldReorg(badFork, 3), "re-org that removes committed blocks must be rejected")
+
+	// --- Candidate that forks exactly AT committed tip (parent = h1) ---
+	goodFork := &BlockNode{
+		Hash:   NewBlockHash(RandomBytes(32)),
+		Height: 3,
+		Header: &MsgDeSoHeader{
+			Height:         3,
+			ProposedInView: 4,
+			PrevBlockHash:  h1, // fork-point is committed tip itself
+		},
+	}
+	require.True(t, bc.shouldReorg(goodFork, 4), "re-org starting at committed tip should be allowed")
+}
+
 // TestTryApplyNewTip tests that tryApplyNewTip works as expected.
 // It tests the following cases:
 // 1. Simple reorg. Just replacing the uncommitted tip.