diff --git a/lib/blockchain.go b/lib/blockchain.go index 1ed21f172..51c561fb0 100644 --- a/lib/blockchain.go +++ b/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. diff --git a/lib/pos_blockchain.go b/lib/pos_blockchain.go index 48c6256d9..8a0165efc 100644 --- a/lib/pos_blockchain.go +++ b/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 } diff --git a/lib/pos_blockchain_test.go b/lib/pos_blockchain_test.go index 0a334addf..8fdc2d3a8 100644 --- a/lib/pos_blockchain_test.go +++ b/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.