From 24305cda68e936ee2375522e5b59af9f93d4aa99 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Thu, 7 Nov 2019 13:42:25 +0200 Subject: [PATCH] [NOD-385] Change confirmation calculation to be relative to the selected tip (#455) * [NOD-385] Make confirmations be calculated as dag.selectedTip().blueScore - acceptingBlock.blueScore + 2 * [NOD-385] Fix comments * [NOD-385] Make more explicit check in accepting block for selected tip * [NOD-385] Put only non accepted transactions in areTxsInBlock * [NOD-385] fetchSelectedTip only if needed --- apiserver/controllers/transaction.go | 59 +++++++++--- blockdag/dag.go | 45 +++++----- blockdag/dag_test.go | 128 ++++++++++++++++----------- 3 files changed, 145 insertions(+), 87 deletions(-) diff --git a/apiserver/controllers/transaction.go b/apiserver/controllers/transaction.go index 1a7924f29..0105b06bc 100644 --- a/apiserver/controllers/transaction.go +++ b/apiserver/controllers/transaction.go @@ -110,21 +110,42 @@ func GetTransactionsByAddressHandler(address string, skip uint64, limit uint64) return txResponses, nil } -func fetchSelectedTipBlueScore() (uint64, error) { +func fetchSelectedTip() (*dbmodels.Block, error) { db, err := database.DB() if err != nil { - return 0, err + return nil, err } block := &dbmodels.Block{} dbResult := db.Order("blue_score DESC"). Where(&dbmodels.Block{IsChainBlock: true}). - Select("blue_score"). First(block) dbErrors := dbResult.GetErrors() if httpserverutils.HasDBError(dbErrors) { - return 0, httpserverutils.NewErrorFromDBErrors("Some errors were encountered when loading transactions from the database:", dbErrors) + return nil, httpserverutils.NewErrorFromDBErrors("Some errors were encountered when loading transactions from the database:", dbErrors) } - return block.BlueScore, nil + return block, nil +} + +func areTxsInBlock(blockID uint64, txIDs []uint64) (map[uint64]bool, error) { + db, err := database.DB() + if err != nil { + return nil, err + } + transactionBlocks := []*dbmodels.TransactionBlock{} + dbErrors := db. + Where(&dbmodels.TransactionBlock{BlockID: blockID}). + Where("transaction_id in (?)", txIDs). + Find(&transactionBlocks).GetErrors() + + if len(dbErrors) > 0 { + return nil, httpserverutils.NewErrorFromDBErrors("Some errors were encountered when loading UTXOs from the database:", dbErrors) + } + + isInBlock := make(map[uint64]bool) + for _, transactionBlock := range transactionBlocks { + isInBlock[transactionBlock.TransactionID] = true + } + return isInBlock, nil } // GetUTXOsByAddressHandler searches for all UTXOs that belong to a certain address. @@ -145,9 +166,25 @@ func GetUTXOsByAddressHandler(address string) (interface{}, error) { return nil, httpserverutils.NewErrorFromDBErrors("Some errors were encountered when loading UTXOs from the database:", dbErrors) } - selectedTipBlueScore, err := fetchSelectedTipBlueScore() - if err != nil { - return nil, err + nonAcceptedTxIds := make([]uint64, len(transactionOutputs)) + for i, txOut := range transactionOutputs { + if txOut.Transaction.AcceptingBlock == nil { + nonAcceptedTxIds[i] = txOut.TransactionID + } + } + + var selectedTip *dbmodels.Block + var isTxInSelectedTip map[uint64]bool + if len(nonAcceptedTxIds) != 0 { + selectedTip, err = fetchSelectedTip() + if err != nil { + return nil, err + } + + isTxInSelectedTip, err = areTxsInBlock(selectedTip.ID, nonAcceptedTxIds) + if err != nil { + return nil, err + } } UTXOsResponses := make([]*apimodels.TransactionOutputResponse, len(transactionOutputs)) @@ -160,10 +197,12 @@ func GetUTXOsByAddressHandler(address string) (interface{}, error) { var acceptingBlockHash *string var confirmations uint64 acceptingBlockBlueScore := blockdag.UnacceptedBlueScore - if transactionOutput.Transaction.AcceptingBlock != nil { + if isTxInSelectedTip[transactionOutput.ID] { + confirmations = 1 + } else if transactionOutput.Transaction.AcceptingBlock != nil { acceptingBlockHash = btcjson.String(transactionOutput.Transaction.AcceptingBlock.BlockHash) acceptingBlockBlueScore = transactionOutput.Transaction.AcceptingBlock.BlueScore - confirmations = selectedTipBlueScore - acceptingBlockBlueScore + confirmations = selectedTip.BlueScore - acceptingBlockBlueScore + 2 } UTXOsResponses[i] = &apimodels.TransactionOutputResponse{ TransactionID: transactionOutput.Transaction.TransactionID, diff --git a/blockdag/dag.go b/blockdag/dag.go index c427149ac..6d924390b 100644 --- a/blockdag/dag.go +++ b/blockdag/dag.go @@ -1362,9 +1362,13 @@ func (dag *BlockDAG) UTXOCommitment() string { // blockConfirmations returns the current confirmations number of the given node // The confirmations number is defined as follows: -// * If the node is red -> 0 -// * Otherwise -> virtual.blueScore - acceptingBlock.blueScore + 1 +// * If the node is in the selected tip red set -> 0 +// * If the node is the selected tip -> 1 +// * Otherwise -> selectedTip.blueScore - acceptingBlock.blueScore + 2 func (dag *BlockDAG) blockConfirmations(node *blockNode) (uint64, error) { + if node == dag.selectedTip() { + return 1, nil + } acceptingBlock, err := dag.acceptingBlock(node) if err != nil { return 0, err @@ -1375,36 +1379,23 @@ func (dag *BlockDAG) blockConfirmations(node *blockNode) (uint64, error) { return 0, nil } - return dag.virtual.blueScore - acceptingBlock.blueScore + 1, nil + return dag.selectedTip().blueScore - acceptingBlock.blueScore + 2, nil } // acceptingBlock finds the node in the selected-parent chain that had accepted // the given node func (dag *BlockDAG) acceptingBlock(node *blockNode) (*blockNode, error) { - // Explicitly handle the DAG tips - if dag.virtual.tips().contains(node) { - // Return the virtual block if the node is one of the DAG blues - for _, tip := range dag.virtual.blues { - if tip == node { - return &dag.virtual.blockNode, nil - } - } - - // Otherwise, this tip is red and doesn't have an accepting block - return nil, nil - } - // Return an error if the node is the virtual block - if len(node.children) == 0 { - if node == &dag.virtual.blockNode { - return nil, errors.New("cannot get acceptingBlock for virtual") - } - // A childless block that isn't a tip or the virtual can never happen. Panicking - panic(errors.Errorf("got childless block %s that is neither a tip nor the virtual", node.hash)) + if node == &dag.virtual.blockNode { + return nil, errors.New("cannot get acceptingBlock for virtual") } // If the node is a chain-block itself, the accepting block is its chain-child if dag.IsInSelectedParentChain(node.hash) { + if len(node.children) == 0 { + // If the node is the selected tip, it doesn't have an accepting block + return nil, nil + } for _, child := range node.children { if dag.IsInSelectedParentChain(child.hash) { return child, nil @@ -1416,6 +1407,13 @@ func (dag *BlockDAG) acceptingBlock(node *blockNode) (*blockNode, error) { // Find the only chain block that may contain the node in its blues candidateAcceptingBlock := dag.oldestChainBlockWithBlueScoreGreaterThan(node.blueScore) + // if no candidate is found, it means that the node has same or more + // blue score than the selected tip and is found in its anticone, so + // it doesn't have an accepting block + if candidateAcceptingBlock == nil { + return nil, nil + } + // candidateAcceptingBlock is the accepting block only if it actually contains // the node in its blues for _, blue := range candidateAcceptingBlock.blues { @@ -1424,7 +1422,8 @@ func (dag *BlockDAG) acceptingBlock(node *blockNode) (*blockNode, error) { } } - // Otherwise, the node is red and doesn't have an accepting block + // Otherwise, the node is red or in the selected tip anticone, and + // doesn't have an accepting block return nil, nil } diff --git a/blockdag/dag_test.go b/blockdag/dag_test.go index bc9a9ea7b..ab0360c95 100644 --- a/blockdag/dag_test.go +++ b/blockdag/dag_test.go @@ -1042,13 +1042,9 @@ func TestConfirmations(t *testing.T) { if err != nil { t.Fatalf("TestConfirmations: confirmations for tip unexpectedly failed: %s", err) } - acceptingBlock, err := dag.acceptingBlock(tip) - if err != nil { - t.Fatalf("TestConfirmations: dag.acceptingBlock unexpectedly failed: %s", err) - } - expectedConfirmations := uint64(1) - if acceptingBlock == nil { - expectedConfirmations = 0 + expectedConfirmations := uint64(0) + if tip == dag.selectedTip() { + expectedConfirmations = 1 } if tipConfirmations != expectedConfirmations { t.Fatalf("TestConfirmations: unexpected confirmations for tip. "+ @@ -1090,7 +1086,7 @@ func TestConfirmations(t *testing.T) { func TestAcceptingBlock(t *testing.T) { // Create a new database and DAG instance to run tests against. params := dagconfig.SimNetParams - params.K = 1 + params.K = 3 dag, teardownFunc, err := DAGSetup("TestAcceptingBlock", Config{ DAGParams: ¶ms, }) @@ -1105,15 +1101,16 @@ func TestAcceptingBlock(t *testing.T) { if err != nil { t.Fatalf("TestAcceptingBlock: acceptingBlock for genesis block unexpectedly failed: %s", err) } - if genesisAcceptingBlock != &dag.virtual.blockNode { + if genesisAcceptingBlock != nil { t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for genesis block. "+ - "Want: virtual, got: %s", genesisAcceptingBlock.hash) + "Want: nil, got: %s", genesisAcceptingBlock.hash) } - chainBlocks := make([]*blockNode, 5) + numChainBlocks := uint32(10) + chainBlocks := make([]*blockNode, numChainBlocks) chainBlocks[0] = dag.genesis buildNode := buildNodeGenerator(dag.dagParams.K, true) - for i := uint32(1); i <= 4; i++ { + for i := uint32(1); i <= numChainBlocks-1; i++ { chainBlocks[i] = buildNode(setFromSlice(chainBlocks[i-1])) dag.virtual.AddTip(chainBlocks[i]) } @@ -1131,16 +1128,65 @@ func TestAcceptingBlock(t *testing.T) { } } - branchingBlocks := make([]*blockNode, 2) - // Add two branching blocks - branchingBlocks[0] = buildNode(setFromSlice(chainBlocks[1])) - dag.virtual.AddTip(branchingBlocks[0]) - branchingBlocks[1] = buildNode(setFromSlice(branchingBlocks[0])) - dag.virtual.AddTip(branchingBlocks[1]) + // Make sure that the selected tip doesn't have an accepting + tipAcceptingBlock, err := dag.acceptingBlock(chainBlocks[len(chainBlocks)-1]) + if err != nil { + t.Fatalf("TestAcceptingBlock: acceptingBlock for tip unexpectedly failed: %s", err) + } + if tipAcceptingBlock != nil { + t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for tip. "+ + "Want: nil, got: %s", tipAcceptingBlock.hash) + } + + // Generate side-chain of dag.dagParams.K + 1 blocks so its tip + // will be in the blues of the virtual but in the anticone of + // the selected tip. + branchingChainTip := chainBlocks[len(chainBlocks)-2] + for i := uint32(0); i < dag.dagParams.K+1; i++ { + nextBranchingChainTip := buildNode(setFromSlice(branchingChainTip)) + dag.virtual.AddTip(nextBranchingChainTip) + branchingChainTip = nextBranchingChainTip + } + + // Make sure that branchingChainTip is not in the selected parent chain + if dag.IsInSelectedParentChain(branchingChainTip.hash) { + t.Fatalf("TestAcceptingBlock: branchingChainTip wasn't expected to be in the selected parent chain") + } + + // Make sure that branchingChainTip is in the virtual blues + isVirtualBlue := false + for _, virtualBlue := range dag.virtual.blues { + if branchingChainTip == virtualBlue { + isVirtualBlue = true + break + } + } + if !isVirtualBlue { + t.Fatalf("TestAcceptingBlock: redChainBlock was expected to be a virtual blue") + } + + // Make sure that a block that is in the anticone of the selected tip and + // in the blues of the virtual doesn't have an accepting block + branchingChainTipAcceptionBlock, err := dag.acceptingBlock(branchingChainTip) + if err != nil { + t.Fatalf("TestAcceptingBlock: acceptingBlock for red chain block unexpectedly failed: %s", err) + } + if branchingChainTipAcceptionBlock != nil { + t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for branchingChainTipAcceptionBlock. "+ + "Want: nil, got: %s", branchingChainTipAcceptionBlock.hash) + } + + // Add K + 1 branching blocks + intersectionBlock := chainBlocks[1] + sideChainTip := buildNode(setFromSlice(intersectionBlock)) + i := uint32(0) + for ; i < dag.dagParams.K+1; sideChainTip = buildNode(setFromSlice(sideChainTip)) { + dag.virtual.AddTip(sideChainTip) + i++ + } // Make sure that the accepting block of the parent of the branching block didn't change expectedAcceptingBlock := chainBlocks[2] - intersectionBlock := chainBlocks[1] intersectionAcceptingBlock, err := dag.acceptingBlock(intersectionBlock) if err != nil { t.Fatalf("TestAcceptingBlock: acceptingBlock for intersection block unexpectedly failed: %s", err) @@ -1150,44 +1196,18 @@ func TestAcceptingBlock(t *testing.T) { "Want: %s, got: %s", expectedAcceptingBlock.hash, intersectionAcceptingBlock.hash) } - // Make sure that the accepting block of the chain tips is the virtual block - tipAcceptingBlock, err := dag.acceptingBlock(chainBlocks[len(chainBlocks)-1]) - if err != nil { - t.Fatalf("TestAcceptingBlock: acceptingBlock for tip unexpectedly failed: %s", err) - } - if tipAcceptingBlock != &dag.virtual.blockNode { - t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for tip. "+ - "Want: Virtual, got: %s", tipAcceptingBlock.hash) - } + // Make sure that a block that is found in the red set of the selected tip + // doesn't have an accepting block + newTip := buildNode(setFromSlice(sideChainTip, chainBlocks[len(chainBlocks)-1])) + dag.virtual.AddTip(newTip) - // Generate 100 blocks to force the "main" chain to become red - branchingChainTip := branchingBlocks[1] - for i := uint32(0); i < 100; i++ { - nextBranchingChainTip := buildNode(setFromSlice(branchingChainTip)) - dag.virtual.AddTip(nextBranchingChainTip) - branchingChainTip = nextBranchingChainTip - } - - // Make sure that a red block returns nil - redChainBlock := chainBlocks[2] - redChainBlockAcceptionBlock, err := dag.acceptingBlock(redChainBlock) + sideChainTipAcceptingBlock, err := dag.acceptingBlock(sideChainTip) if err != nil { - t.Fatalf("TestAcceptingBlock: acceptingBlock for red chain block unexpectedly failed: %s", err) + t.Fatalf("TestAcceptingBlock: acceptingBlock for sideChainTip unexpectedly failed: %s", err) } - if redChainBlockAcceptionBlock != nil { - t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for red chain block. "+ - "Want: nil, got: %s", redChainBlockAcceptionBlock.hash) - } - - // Make sure that a red tip returns nil - redChainTip := chainBlocks[len(chainBlocks)-1] - redChainTipAcceptingBlock, err := dag.acceptingBlock(redChainTip) - if err != nil { - t.Fatalf("TestAcceptingBlock: acceptingBlock for red chain tip unexpectedly failed: %s", err) - } - if redChainTipAcceptingBlock != nil { - t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for red tip block. "+ - "Want: nil, got: %s", redChainTipAcceptingBlock.hash) + if sideChainTipAcceptingBlock != nil { + t.Fatalf("TestAcceptingBlock: unexpected acceptingBlock for sideChainTip. "+ + "Want: nil, got: %s", intersectionAcceptingBlock.hash) } }