From c7588cbf7690cd9f047a28efa2dcd8f2435a4e5e Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Fri, 13 Oct 2017 14:36:40 -0700 Subject: [PATCH] blockchain: NodeStatus & Set/UnsetStatusFlags methods on blockIndex. These method allows safe concurrent access to reading and modifying block node statuses. When block statuses get persisted in a later change, the setter methods can be used to mark block nodes as dirty. --- blockchain/accept.go | 4 +-- blockchain/blockindex.go | 70 ++++++++++++++++++++++++++++------------ blockchain/chain.go | 22 ++++++------- blockchain/chainio.go | 4 +-- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/blockchain/accept.go b/blockchain/accept.go index 897c4ddb..963a4fd0 100644 --- a/blockchain/accept.go +++ b/blockchain/accept.go @@ -29,7 +29,7 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags) if prevNode == nil { str := fmt.Sprintf("previous block %s is unknown", prevHash) return false, ruleError(ErrPreviousBlockUnknown, str) - } else if prevNode.KnownInvalid() { + } else if b.index.NodeStatus(prevNode).KnownInvalid() { str := fmt.Sprintf("previous block %s is known to be invalid", prevHash) return false, ruleError(ErrInvalidAncestorBlock, str) } @@ -64,7 +64,7 @@ func (b *BlockChain) maybeAcceptBlock(block *btcutil.Block, flags BehaviorFlags) // block chain (could be either a side chain or the main chain). blockHeader := &block.MsgBlock().Header newNode := newBlockNode(blockHeader, blockHeight) - newNode.status |= statusDataStored + newNode.status = statusDataStored if prevNode != nil { newNode.parent = prevNode newNode.height = blockHeight diff --git a/blockchain/blockindex.go b/blockchain/blockindex.go index 8fa1fb99..642b658e 100644 --- a/blockchain/blockindex.go +++ b/blockchain/blockindex.go @@ -39,6 +39,27 @@ const ( statusNone blockStatus = 0 ) +// HaveData returns whether the full block data is stored in the database. This +// will return false for a block node where only the header is downloaded or +// kept. +func (status blockStatus) HaveData() bool { + return status&statusDataStored != 0 +} + +// KnownValid returns whether the block is known to be valid. This will return +// false for a valid block that has not been fully validated yet. +func (status blockStatus) KnownValid() bool { + return status&statusValid != 0 +} + +// KnownInvalid returns whether the block is known to be invalid. This may be +// because the block itself failed validation or any of its ancestors is +// invalid. This will return false for invalid blocks that have not been proven +// invalid yet. +func (status blockStatus) KnownInvalid() bool { + return status&(statusValidateFailed|statusInvalidAncestor) != 0 +} + // blockNode represents a block within the block chain and is primarily used to // aid in selecting the best chain to be the main chain. The main chain is // stored into the block database. @@ -73,7 +94,10 @@ type blockNode struct { timestamp int64 merkleRoot chainhash.Hash - // status is a bitfield representing the validation state of the block + // status is a bitfield representing the validation state of the block. The + // status field, unlike the other fields, may be written to and so should + // only be accessed using the concurrent-safe NodeStatus method on + // blockIndex once the node has been added to the global index. status blockStatus } @@ -193,20 +217,6 @@ func (node *blockNode) CalcPastMedianTime() time.Time { return time.Unix(medianTimestamp, 0) } -// KnownValid returns whether the block is known to be valid. This will return -// false for a valid block that has not been fully validated yet. -func (node *blockNode) KnownValid() bool { - return node.status&statusValid != 0 -} - -// KnownInvalid returns whether the block is known to be invalid. This may be -// because the block itself failed validation or any of its ancestors is -// invalid. This will return false for invalid blocks that have not been proven -// invalid yet. -func (node *blockNode) KnownInvalid() bool { - return node.status&(statusValidateFailed|statusInvalidAncestor) != 0 -} - // blockIndex provides facilities for keeping track of an in-memory index of the // block chain. Although the name block chain suggests a single chain of // blocks, it is actually a tree-shaped structure where any node can have @@ -265,13 +275,33 @@ func (bi *blockIndex) AddNode(node *blockNode) { bi.Unlock() } -// RemoveNode removes the provided node from the block index. There is no check -// whether another node in the index depends on this one, so it is up to caller -// to avoid that situation. +// NodeStatus provides concurrent-safe access to the status field of a node. // // This function is safe for concurrent access. -func (bi *blockIndex) RemoveNode(node *blockNode) { +func (bi *blockIndex) NodeStatus(node *blockNode) blockStatus { + bi.RLock() + status := node.status + bi.RUnlock() + return status +} + +// SetStatusFlags flips the provided status flags on the block node to on, +// regardless of whether they were on or off previously. This does not unset any +// flags currently on. +// +// This function is safe for concurrent access. +func (bi *blockIndex) SetStatusFlags(node *blockNode, flags blockStatus) { bi.Lock() - delete(bi.index, node.hash) + node.status |= flags + bi.Unlock() +} + +// UnsetStatusFlags flips the provided status flags on the block node to off, +// regardless of whether they were on or off previously. +// +// This function is safe for concurrent access. +func (bi *blockIndex) UnsetStatusFlags(node *blockNode, flags blockStatus) { + bi.Lock() + node.status &^= flags bi.Unlock() } diff --git a/blockchain/chain.go b/blockchain/chain.go index 83bfe883..bd24b1e1 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -503,8 +503,8 @@ func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List // Do not reorganize to a known invalid chain. Ancestors deeper than the // direct parent are checked below but this is a quick check before doing // more unnecessary work. - if node.parent.KnownInvalid() { - node.status |= statusInvalidAncestor + if b.index.NodeStatus(node.parent).KnownInvalid() { + b.index.SetStatusFlags(node, statusInvalidAncestor) return detachNodes, attachNodes } @@ -515,7 +515,7 @@ func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List forkNode := b.bestChain.FindFork(node) invalidChain := false for n := node; n != nil && n != forkNode; n = n.parent { - if n.KnownInvalid() { + if b.index.NodeStatus(n).KnownInvalid() { invalidChain = true break } @@ -529,7 +529,7 @@ func (b *BlockChain) getReorganizeNodes(node *blockNode) (*list.List, *list.List for e := attachNodes.Front(); e != nil; e = next { next = e.Next() n := attachNodes.Remove(e).(*blockNode) - n.status |= statusInvalidAncestor + b.index.SetStatusFlags(n, statusInvalidAncestor) } return detachNodes, attachNodes } @@ -882,7 +882,7 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error // If any previous nodes in attachNodes failed validation, // mark this one as having an invalid ancestor. if validationError != nil { - n.status |= statusInvalidAncestor + b.index.SetStatusFlags(n, statusInvalidAncestor) continue } @@ -902,7 +902,7 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error // Skip checks if node has already been fully validated. Although // checkConnectBlock gets skipped, we still need to update the UTXO // view. - if n.KnownValid() { + if b.index.NodeStatus(n).KnownValid() { err = view.fetchInputUtxos(b.db, block) if err != nil { return err @@ -924,13 +924,13 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error // continue to loop through remaining nodes, marking them as // having an invalid ancestor. if _, ok := err.(RuleError); ok { - n.status |= statusValidateFailed + b.index.SetStatusFlags(n, statusValidateFailed) validationError = err continue } return err } - n.status |= statusValid + b.index.SetStatusFlags(n, statusValid) } if validationError != nil { @@ -1034,7 +1034,7 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla parentHash := &block.MsgBlock().Header.PrevBlock if parentHash.IsEqual(&b.bestChain.Tip().hash) { // Skip checks if node has already been fully validated. - fastAdd = fastAdd || node.KnownValid() + fastAdd = fastAdd || b.index.NodeStatus(node).KnownValid() // Perform several checks to verify the block can be connected // to the main chain without violating any rules and without @@ -1046,11 +1046,11 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla err := b.checkConnectBlock(node, block, view, &stxos) if err != nil { if _, ok := err.(RuleError); ok { - node.status |= statusValidateFailed + b.index.SetStatusFlags(node, statusValidateFailed) } return false, err } - node.status |= statusValid + b.index.SetStatusFlags(node, statusValid) } // In the fast add case the code to check the block connection diff --git a/blockchain/chainio.go b/blockchain/chainio.go index 70277edf..6c629c60 100644 --- a/blockchain/chainio.go +++ b/blockchain/chainio.go @@ -1060,7 +1060,7 @@ func (b *BlockChain) createChainState() error { genesisBlock := btcutil.NewBlock(b.chainParams.GenesisBlock) header := &genesisBlock.MsgBlock().Header node := newBlockNode(header, 0) - node.status |= statusDataStored | statusValid + node.status = statusDataStored | statusValid b.bestChain.SetTip(node) // Add the new node to the index which is used for faster lookups. @@ -1165,7 +1165,7 @@ func (b *BlockChain) initChainState() error { // and add it to the block index. node := &blockNodes[height] initBlockNode(node, header, height) - node.status |= statusDataStored | statusValid + node.status = statusDataStored | statusValid if tip != nil { node.parent = tip node.workSum = node.workSum.Add(tip.workSum,