diff --git a/blockdag/blocknode.go b/blockdag/blocknode.go index 8e501b40b..a11cb0e23 100644 --- a/blockdag/blocknode.go +++ b/blockdag/blocknode.go @@ -101,6 +101,9 @@ type blockNode struct { // only be accessed using the concurrent-safe NodeStatus method on // blockIndex once the node has been added to the global index. status blockStatus + + // isFinalized determines whether the node is below the finality point. + isFinalized bool } // initBlockNode initializes a block node from the given header and parent nodes. diff --git a/blockdag/dag.go b/blockdag/dag.go index 519304728..645c486f9 100644 --- a/blockdag/dag.go +++ b/blockdag/dag.go @@ -721,27 +721,60 @@ func (dag *BlockDAG) checkFinalityRules(newNode *blockNode) error { // updateFinalityPoint updates the dag's last finality point if necessary. func (dag *BlockDAG) updateFinalityPoint() { selectedTip := dag.selectedTip() - var newFinalityPoint *blockNode // if the selected tip is the genesis block - it should be the new finality point if selectedTip.isGenesis() { - newFinalityPoint = selectedTip - } else { - // We are looking for a new finality point only if the new block's finality score is higher - // by 2 than the existing finality point's - if selectedTip.finalityScore() < dag.lastFinalityPoint.finalityScore()+2 { - return - } + dag.lastFinalityPoint = selectedTip + return + } + // We are looking for a new finality point only if the new block's finality score is higher + // by 2 than the existing finality point's + if selectedTip.finalityScore() < dag.lastFinalityPoint.finalityScore()+2 { + return + } - var currentNode *blockNode - for currentNode = selectedTip.selectedParent; ; currentNode = currentNode.selectedParent { - // We look for the first node in the selected parent chain that has a higher finality score than the last finality point. - if currentNode.selectedParent.finalityScore() == dag.lastFinalityPoint.finalityScore() { - break + var currentNode *blockNode + for currentNode = selectedTip.selectedParent; ; currentNode = currentNode.selectedParent { + // We look for the first node in the selected parent chain that has a higher finality score than the last finality point. + if currentNode.selectedParent.finalityScore() == dag.lastFinalityPoint.finalityScore() { + break + } + } + dag.lastFinalityPoint = currentNode + spawn(func() { + dag.finalizeNodesBelowFinalityPoint(true) + }) +} + +func (dag *BlockDAG) finalizeNodesBelowFinalityPoint(deleteDiffData bool) { + queue := make([]*blockNode, 0, len(dag.lastFinalityPoint.parents)) + for _, parent := range dag.lastFinalityPoint.parents { + queue = append(queue, parent) + } + var blockHashesToDelete []*daghash.Hash + if deleteDiffData { + blockHashesToDelete = make([]*daghash.Hash, 0, FinalityInterval) + } + for len(queue) > 0 { + var current *blockNode + current, queue = queue[0], queue[1:] + if !current.isFinalized { + current.isFinalized = true + if deleteDiffData { + blockHashesToDelete = append(blockHashesToDelete, current.hash) + } + for _, parent := range current.parents { + queue = append(queue, parent) } } - newFinalityPoint = currentNode } - dag.lastFinalityPoint = newFinalityPoint + if deleteDiffData { + err := dag.db.Update(func(dbTx database.Tx) error { + return dag.utxoDiffStore.removeBlocksDiffData(dbTx, blockHashesToDelete) + }) + if err != nil { + panic(fmt.Sprintf("Error removing diff data from utxoDiffStore: %s", err)) + } + } } // NextBlockCoinbaseTransaction prepares the coinbase transaction for the next mined block diff --git a/blockdag/dag_test.go b/blockdag/dag_test.go index 63f58afff..1bd553474 100644 --- a/blockdag/dag_test.go +++ b/blockdag/dag_test.go @@ -1116,3 +1116,94 @@ func TestAcceptingBlock(t *testing.T) { "Want: nil, got: %s", redChainTipAcceptingBlock.hash) } } + +func TestFinalizeNodesBelowFinalityPoint(t *testing.T) { + testFinalizeNodesBelowFinalityPoint(t, true) + testFinalizeNodesBelowFinalityPoint(t, false) +} + +func testFinalizeNodesBelowFinalityPoint(t *testing.T, deleteDiffData bool) { + params := dagconfig.SimNetParams + params.K = 1 + dag, teardownFunc, err := DAGSetup("testFinalizeNodesBelowFinalityPoint", Config{ + DAGParams: ¶ms, + }) + if err != nil { + t.Fatalf("Failed to setup DAG instance: %v", err) + } + defer teardownFunc() + + blockVersion := int32(0x10000000) + blockTime := dag.genesis.Header().Timestamp + + flushUTXODiffStore := func() { + err := dag.db.Update(func(dbTx database.Tx) error { + return dag.utxoDiffStore.flushToDB(dbTx) + }) + if err != nil { + t.Fatalf("Error flushing utxoDiffStore data to DB: %s", err) + } + dag.utxoDiffStore.clearDirtyEntries() + } + + addNode := func(parent *blockNode) *blockNode { + blockTime = blockTime.Add(time.Second) + node := newTestNode(setFromSlice(parent), blockVersion, 0, blockTime, dag.dagParams.K) + node.updateParentsChildren() + dag.index.AddNode(node) + + // Put dummy diff data in dag.utxoDiffStore + err := dag.utxoDiffStore.setBlockDiff(node, NewUTXODiff()) + if err != nil { + t.Fatalf("setBlockDiff: %s", err) + } + flushUTXODiffStore() + return node + } + + nodes := make([]*blockNode, 0, FinalityInterval) + currentNode := dag.genesis + nodes = append(nodes, currentNode) + for i := 0; i <= FinalityInterval*2; i++ { + currentNode = addNode(currentNode) + nodes = append(nodes, currentNode) + } + + // Manually set the last finality point + dag.lastFinalityPoint = nodes[FinalityInterval-1] + + dag.finalizeNodesBelowFinalityPoint(deleteDiffData) + flushUTXODiffStore() + + for _, node := range nodes[:FinalityInterval-1] { + if !node.isFinalized { + t.Errorf("Node with blue score %d expected to be finalized", node.blueScore) + } + if _, ok := dag.utxoDiffStore.loaded[*node.hash]; deleteDiffData && ok { + t.Errorf("The diff data of node with blue score %d should have been unloaded if deleteDiffData is %T", node.blueScore, deleteDiffData) + } else if !deleteDiffData && !ok { + t.Errorf("The diff data of node with blue score %d shouldn't have been unloaded if deleteDiffData is %T", node.blueScore, deleteDiffData) + } + if diffData, err := dag.utxoDiffStore.diffDataFromDB(node.hash); err != nil { + t.Errorf("diffDataFromDB: %s", err) + } else if deleteDiffData && diffData != nil { + t.Errorf("The diff data of node with blue score %d should have been deleted from the database if deleteDiffData is %T", node.blueScore, deleteDiffData) + } else if !deleteDiffData && diffData == nil { + t.Errorf("The diff data of node with blue score %d shouldn't have been deleted from the database if deleteDiffData is %T", node.blueScore, deleteDiffData) + } + } + + for _, node := range nodes[FinalityInterval-1:] { + if node.isFinalized { + t.Errorf("Node with blue score %d wasn't expected to be finalized", node.blueScore) + } + if _, ok := dag.utxoDiffStore.loaded[*node.hash]; !ok { + t.Errorf("The diff data of node with blue score %d shouldn't have been unloaded", node.blueScore) + } + if diffData, err := dag.utxoDiffStore.diffDataFromDB(node.hash); err != nil { + t.Errorf("diffDataFromDB: %s", err) + } else if diffData == nil { + t.Errorf("The diff data of node with blue score %d shouldn't have been deleted from the database", node.blueScore) + } + } +} diff --git a/blockdag/dagio.go b/blockdag/dagio.go index dd279099a..05698cf5c 100644 --- a/blockdag/dagio.go +++ b/blockdag/dagio.go @@ -554,6 +554,7 @@ func (dag *BlockDAG) initDAGState() error { // Set the last finality point dag.lastFinalityPoint = dag.index.LookupNode(state.LastFinalityPoint) + dag.finalizeNodesBelowFinalityPoint(false) return nil }) diff --git a/blockdag/error.go b/blockdag/error.go index d28223993..4314ed353 100644 --- a/blockdag/error.go +++ b/blockdag/error.go @@ -218,6 +218,10 @@ const ( // ErrSubnetwork indicates that a block doesn't adhere to the subnetwork // registry rules ErrSubnetworkRegistry + + // ErrInvalidParentsRelation indicates that one of the parents of a block + // is also an ancestor of another parent + ErrInvalidParentsRelation ) // Map of ErrorCode values back to their constant names for pretty printing. @@ -265,6 +269,7 @@ var errorCodeStrings = map[ErrorCode]string{ ErrInvalidGas: "ErrInvalidGas", ErrInvalidPayload: "ErrInvalidPayload", ErrInvalidPayloadHash: "ErrInvalidPayloadHash", + ErrInvalidParentsRelation: "ErrInvalidParentsRelation", } // String returns the ErrorCode as a human-readable name. diff --git a/blockdag/error_test.go b/blockdag/error_test.go index ad289ae36..5bb1f0ea3 100644 --- a/blockdag/error_test.go +++ b/blockdag/error_test.go @@ -59,6 +59,7 @@ func TestErrorCodeStringer(t *testing.T) { {ErrInvalidGas, "ErrInvalidGas"}, {ErrInvalidPayload, "ErrInvalidPayload"}, {ErrInvalidPayloadHash, "ErrInvalidPayloadHash"}, + {ErrInvalidParentsRelation, "ErrInvalidParentsRelation"}, {0xffff, "Unknown ErrorCode (65535)"}, } diff --git a/blockdag/external_dag_test.go b/blockdag/external_dag_test.go index 9a69c926f..ec93ef207 100644 --- a/blockdag/external_dag_test.go +++ b/blockdag/external_dag_test.go @@ -2,11 +2,10 @@ package blockdag_test import ( "fmt" + "github.com/daglabs/btcd/util/subnetworkid" "math" "testing" - "github.com/daglabs/btcd/util/subnetworkid" - "github.com/daglabs/btcd/util/daghash" "github.com/daglabs/btcd/util/testtools" @@ -116,7 +115,22 @@ func TestFinality(t *testing.T) { // Here we check that a block with lower blue score than the last finality // point will get rejected - _, err = buildNodeToDag([]*daghash.Hash{genesis.Hash()}) + fakeCoinbaseTx, err := dag.NextBlockCoinbaseTransaction(nil, nil) + if err != nil { + t.Errorf("NextBlockCoinbaseTransaction: %s", err) + } + merkleRoot := blockdag.BuildHashMerkleTreeStore([]*util.Tx{fakeCoinbaseTx}).Root() + beforeFinalityBlock := wire.NewMsgBlock(&wire.BlockHeader{ + Version: 0x10000000, + ParentHashes: []*daghash.Hash{genesis.Hash()}, + HashMerkleRoot: merkleRoot, + AcceptedIDMerkleRoot: &daghash.ZeroHash, + UTXOCommitment: &daghash.ZeroHash, + Timestamp: dag.SelectedTipHeader().Timestamp, + Bits: genesis.MsgBlock().Header.Bits, + }) + beforeFinalityBlock.AddTransaction(fakeCoinbaseTx.MsgTx()) + _, _, err = dag.ProcessBlock(util.NewBlock(beforeFinalityBlock), blockdag.BFNoPoWCheck) if err == nil { t.Errorf("TestFinality: buildNodeToDag expected an error but got ") } @@ -126,7 +140,7 @@ func TestFinality(t *testing.T) { t.Errorf("TestFinality: buildNodeToDag expected an error with code %v but instead got %v", blockdag.ErrFinality, rErr.ErrorCode) } } else { - t.Errorf("TestFinality: buildNodeToDag got unexpected error: %v", rErr) + t.Errorf("TestFinality: buildNodeToDag got unexpected error: %v", err) } // Here we check that a block that doesn't have the last finality point in diff --git a/blockdag/utxodiffstore.go b/blockdag/utxodiffstore.go index 8dec97128..0f4eb709a 100644 --- a/blockdag/utxodiffstore.go +++ b/blockdag/utxodiffstore.go @@ -16,10 +16,11 @@ type blockUTXODiffData struct { } type utxoDiffStore struct { - dag *BlockDAG - dirty map[daghash.Hash]struct{} - loaded map[daghash.Hash]*blockUTXODiffData - mtx sync.RWMutex + dag *BlockDAG + dirty map[daghash.Hash]struct{} + loaded map[daghash.Hash]*blockUTXODiffData + mtx sync.RWMutex + removeMtx sync.Mutex } func newUTXODiffStore(dag *BlockDAG) *utxoDiffStore { @@ -64,6 +65,33 @@ func (diffStore *utxoDiffStore) setBlockDiffChild(node *blockNode, diffChild *bl return nil } +func (diffStore *utxoDiffStore) removeBlocksDiffData(dbTx database.Tx, blockHashes []*daghash.Hash) error { + diffStore.removeMtx.Lock() + defer diffStore.removeMtx.Unlock() + for _, hash := range blockHashes { + err := diffStore.removeBlockDiffData(dbTx, hash) + if err != nil { + return err + } + } + return nil +} + +func (diffStore *utxoDiffStore) removeBlockDiffData(dbTx database.Tx, blockHash *daghash.Hash) error { + // We use RLock here not because we aren't about to write into the map, + // but because RLock has a lower priority than Lock. In order to prevent + // concurrent writes, we employ a second lock around the entirety of + // removeBlocksDiffData. + diffStore.mtx.RLock() + defer diffStore.mtx.RUnlock() + delete(diffStore.loaded, *blockHash) + err := dbRemoveDiffData(dbTx, blockHash) + if err != nil { + return err + } + return nil +} + func (diffStore *utxoDiffStore) setBlockAsDirty(blockHash *daghash.Hash) { diffStore.dirty[*blockHash] = struct{}{} } @@ -164,3 +192,7 @@ func dbStoreDiffData(dbTx database.Tx, hash *daghash.Hash, diffData *blockUTXODi return dbTx.Metadata().Bucket(utxoDiffsBucketName).Put(hash[:], serializedDiffData) } + +func dbRemoveDiffData(dbTx database.Tx, hash *daghash.Hash) error { + return dbTx.Metadata().Bucket(utxoDiffsBucketName).Delete(hash[:]) +} diff --git a/blockdag/utxodiffstore_test.go b/blockdag/utxodiffstore_test.go index ea1dff44d..5c02ac654 100644 --- a/blockdag/utxodiffstore_test.go +++ b/blockdag/utxodiffstore_test.go @@ -2,13 +2,12 @@ package blockdag import ( "fmt" - "reflect" - "testing" - "github.com/daglabs/btcd/dagconfig" "github.com/daglabs/btcd/database" "github.com/daglabs/btcd/util/daghash" "github.com/daglabs/btcd/wire" + "reflect" + "testing" ) func TestUTXODiffStore(t *testing.T) { diff --git a/blockdag/validate.go b/blockdag/validate.go index 219fe714b..59d7aee8a 100644 --- a/blockdag/validate.go +++ b/blockdag/validate.go @@ -604,12 +604,18 @@ func (dag *BlockDAG) validateDifficulty(header *wire.BlockHeader, bluestParent * return nil } -// validateParents validates that no parent is an ancestor of another parent +// validateParents validates that no parent is an ancestor of another parent, and no parent is finalized func validateParents(blockHeader *wire.BlockHeader, parents blockSet) error { minHeight := uint64(math.MaxUint64) queue := newDownHeap() visited := newSet() for _, parent := range parents { + // isFinalized might be false-negative because node finality status is + // updated in a separate goroutine. This is why later the block is + // checked more thoroughly on the finality rules in dag.checkFinalityRules. + if parent.isFinalized { + return ruleError(ErrFinality, fmt.Sprintf("block %s is a finalized parent of block %s", parent.hash, blockHeader.BlockHash())) + } if parent.height < minHeight { minHeight = parent.height } @@ -623,10 +629,10 @@ func validateParents(blockHeader *wire.BlockHeader, parents blockSet) error { for queue.Len() > 0 { current := queue.pop() if parents.contains(current) { - return fmt.Errorf("block %s is both a parent of %s and an"+ + return ruleError(ErrInvalidParentsRelation, fmt.Sprintf("block %s is both a parent of %s and an"+ " ancestor of another parent", current.hash, - blockHeader.BlockHash()) + blockHeader.BlockHash())) } if current.height > minHeight { for _, parent := range current.parents {