[NOD-83] modify blocks parents children only after validation (#235)

* [NOD-83] Modify block parents' children only after validation

* [NOD-83] Creating CheckConnectBlockTemplateWithLock with RLock

* [NOD-83] Put updateParentsChildren inside updateParents

* [NOD-83] create updateParentsDiffs function
This commit is contained in:
Ori Newman 2019-04-01 13:03:13 +03:00 committed by Evgeny Khirin
parent 62d14bf2bd
commit 20e24e3a23
7 changed files with 28 additions and 55 deletions

View File

@ -62,14 +62,6 @@ func (dag *BlockDAG) maybeAcceptBlock(block *util.Block, flags BehaviorFlags) er
blockHeader := &block.MsgBlock().Header
newNode := newBlockNode(blockHeader, parents, dag.dagParams.K)
newNode.status = statusDataStored
// newBlockNode adds node into children maps of its parents. So it must be
// removed in case of error.
isOk := false
defer func() {
if !isOk {
newNode.detachFromParents()
}
}()
dag.index.AddNode(newNode)
err = dag.index.flushToDB()
@ -91,8 +83,6 @@ func (dag *BlockDAG) maybeAcceptBlock(block *util.Block, flags BehaviorFlags) er
dag.sendNotification(NTBlockAdded, block)
dag.dagLock.Lock()
isOk = true
return nil
}

View File

@ -139,11 +139,6 @@ func initBlockNode(node *blockNode, blockHeader *wire.BlockHeader, parents block
node.timestamp = blockHeader.Timestamp.Unix()
node.hashMerkleRoot = blockHeader.HashMerkleRoot
node.idMerkleRoot = blockHeader.IDMerkleRoot
// update parents to point to new node
for _, p := range parents {
p.children[node.hash] = node
}
}
if len(parents) > 0 {
@ -174,12 +169,10 @@ func newBlockNode(blockHeader *wire.BlockHeader, parents blockSet, phantomK uint
return &node
}
// newBlockNode adds node into children maps of its parents. So it must be
// removed in case of error.
func (node *blockNode) detachFromParents() {
// remove node from parents
for _, p := range node.parents {
delete(p.children, node.hash)
// updateParentsChildren updates the node's parents to point to new node
func (node *blockNode) updateParentsChildren() {
for _, parent := range node.parents {
parent.children.add(node)
}
}

View File

@ -731,24 +731,21 @@ func (dag *BlockDAG) NextBlockFeeTransaction() (*wire.MsgTx, error) {
func (dag *BlockDAG) applyDAGChanges(node *blockNode, block *util.Block, newBlockUTXO UTXOSet, fastAdd bool) (
virtualUTXODiff *UTXODiff, err error) {
// Clone the virtual block so that we don't modify the existing one.
virtualClone := dag.virtual.clone()
if err = node.updateParents(virtualClone, newBlockUTXO); err != nil {
if err = node.updateParents(dag.virtual, newBlockUTXO); err != nil {
return nil, fmt.Errorf("failed updating parents of %s: %s", node, err)
}
// Update the virtual block's children (the DAG tips) to include the new block.
virtualClone.AddTip(node)
// Update the virtual block's parents (the DAG tips) to include the new block.
dag.virtual.AddTip(node)
// Build a UTXO set for the new virtual block
newVirtualUTXO, _, err := virtualClone.blockNode.pastUTXO(virtualClone, dag.db)
newVirtualUTXO, _, err := dag.virtual.blockNode.pastUTXO(dag.virtual, dag.db)
if err != nil {
return nil, fmt.Errorf("could not restore past UTXO for virtual %s: %s", virtualClone, err)
return nil, fmt.Errorf("could not restore past UTXO for virtual %s: %s", dag.virtual, err)
}
// Apply new utxoDiffs to all the tips
err = updateTipsUTXO(virtualClone.parents, virtualClone, newVirtualUTXO)
err = updateTipsUTXO(dag.virtual.parents, dag.virtual, newVirtualUTXO)
if err != nil {
return nil, fmt.Errorf("failed updating the tips' UTXO: %s", err)
}
@ -765,9 +762,6 @@ func (dag *BlockDAG) applyDAGChanges(node *blockNode, block *util.Block, newBloc
}
dag.index.SetStatusFlags(node, statusValid)
// It is now safe to apply the new virtual block
dag.virtual = virtualClone
// And now we can update the finality point of the DAG (if required)
dag.lastFinalityPoint = dag.newFinalityPoint(node)
@ -960,6 +954,12 @@ func (node *blockNode) restoreUTXO(virtual *virtualBlock) (UTXOSet, error) {
// updateParents adds this block to the children sets of its parents
// and updates the diff of any parent whose DiffChild is this block
func (node *blockNode) updateParents(virtual *virtualBlock, newBlockUTXO UTXOSet) error {
node.updateParentsChildren()
return node.updateParentsDiffs(virtual, newBlockUTXO)
}
// updateParentsDiffs updates the diff of any parent whose DiffChild is this block
func (node *blockNode) updateParentsDiffs(virtual *virtualBlock, newBlockUTXO UTXOSet) error {
virtualDiffFromNewBlock, err := virtual.utxoSet.diffFrom(newBlockUTXO)
if err != nil {
return err

View File

@ -1183,14 +1183,21 @@ func countSpentOutputs(block *util.Block) int {
return numSpent
}
// CheckConnectBlockTemplate fully validates that connecting the passed block to
// CheckConnectBlockTemplateWithLock fully validates that connecting the passed block to
// the DAG does not violate any consensus rules, aside from the proof of
// work requirement. The block must connect to the current tip of the main dag.
//
// This function is safe for concurrent access.
func (dag *BlockDAG) CheckConnectBlockTemplateWithLock(block *util.Block) error {
dag.dagLock.RLock()
defer dag.dagLock.RUnlock()
return dag.CheckConnectBlockTemplate(block)
}
// CheckConnectBlockTemplate fully validates that connecting the passed block to
// the DAG does not violate any consensus rules, aside from the proof of
// work requirement. The block must connect to the current tip of the main dag.
func (dag *BlockDAG) CheckConnectBlockTemplate(block *util.Block) error {
dag.dagLock.Lock()
defer dag.dagLock.Unlock()
// Skip the proof of work check as this is just a block template.
flags := BFNoPoWCheck
@ -1222,7 +1229,6 @@ func (dag *BlockDAG) CheckConnectBlockTemplate(block *util.Block) error {
}
templateNode := newBlockNode(&header, dag.virtual.tips(), dag.dagParams.K)
defer templateNode.detachFromParents()
_, err = dag.checkConnectToPastUTXO(templateNode,
dag.UTXOSet(), block.Transactions(), false)

View File

@ -121,11 +121,6 @@ func TestCheckConnectBlockTemplate(t *testing.T) {
"block 4: %v", err)
}
blockNode3 := dag.index.LookupNode(blocks[3].Hash())
if blockNode3.children.containsHash(blocks[4].Hash()) {
t.Errorf("Block 4 wasn't successfully detached as a child from block3")
}
// Block 3a should fail to connect since does not build on chain tip.
err = dag.CheckConnectBlockTemplate(blocks[5])
if err == nil {

View File

@ -385,15 +385,7 @@ func NewBlkTmplGenerator(policy *Policy, params *dagconfig.Params,
// ----------------------------------- --
func (g *BlkTmplGenerator) NewBlockTemplate(payToAddress util.Address) (*BlockTemplate, error) {
g.dag.RLock()
isDagLocked := true
// We need to read-unlock the DAG before calling CheckConnectBlockTemplate
// Therefore the deferred function is only relevant in cases where an
// error is returned before calling CheckConnectBlockTemplate
defer func() {
if isDagLocked {
g.dag.RUnlock()
}
}()
defer g.dag.RUnlock()
// Extend the most recently known best block.
nextBlockHeight := g.dag.Height() + 1
@ -683,9 +675,6 @@ func (g *BlkTmplGenerator) NewBlockTemplate(payToAddress util.Address) (*BlockTe
block := util.NewBlock(&msgBlock)
block.SetHeight(nextBlockHeight)
g.dag.RUnlock()
isDagLocked = false
if err := g.dag.CheckConnectBlockTemplate(block); err != nil {
return nil, err
}

View File

@ -2153,7 +2153,7 @@ func handleGetBlockTemplateProposal(s *Server, request *btcjson.TemplateRequest)
return "bad-parentblk", nil
}
if err := s.cfg.DAG.CheckConnectBlockTemplate(block); err != nil {
if err := s.cfg.DAG.CheckConnectBlockTemplateWithLock(block); err != nil {
if _, ok := err.(blockdag.RuleError); !ok {
errStr := fmt.Sprintf("Failed to process block proposal: %s", err)
log.Error(errStr)