From 4b13e7969119739114913cb911a5ea5e27a7dede Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 8 Aug 2018 17:49:26 -0700 Subject: [PATCH 1/4] blockchain: in initChainState mark ancestors of best tip as valid if not marked In this commit, we add an additional consistency check within the `initChainState` method. It has been observed that at times, a block wil lbe written to disk (as it's valid), but then the block index isn't updated to reflect this. This can cause btcd to fail to do things like serve cfheaders for valid blocks. To partially remedy this, when we're loading in the index, we assume that all ancestors of the current chain tip are valid, and mark them as such. At the very end, we'll flush the index to ensure the state is fully consistent. Typically this will be a noop, as only dirty elements are flushed. --- blockchain/chainio.go | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/blockchain/chainio.go b/blockchain/chainio.go index 8bd56276..8d76d3ca 100644 --- a/blockchain/chainio.go +++ b/blockchain/chainio.go @@ -1128,7 +1128,7 @@ func (b *BlockChain) initChainState() error { } // Attempt to load the chain state from the database. - return b.db.View(func(dbTx database.Tx) error { + err = b.db.View(func(dbTx database.Tx) error { // Fetch the stored chain state from the database metadata. // When it doesn't exist, it means the database hasn't been // initialized for use with chain yet, so break out now to allow @@ -1221,6 +1221,25 @@ func (b *BlockChain) initChainState() error { return err } + // As a final consistency check, we'll run through all the + // nodes which are ancestors of the current chain tip, and mark + // them as valid if they aren't already marked as such. This + // is a safe assumption as all the block before the current tip + // are valid by definition. + for iterNode := tip; iterNode != nil; iterNode = iterNode.parent { + // If this isn't already marked as valid in the index, then + // we'll mark it as valid now to ensure consistency once + // we're up and running. + if !iterNode.status.KnownValid() { + log.Infof("Block %v (height=%v) ancestor of "+ + "chain tip not marked as valid, "+ + "upgrading to valid for consistency", + iterNode.hash, iterNode.height) + + b.index.SetStatusFlags(iterNode, statusValid) + } + } + // Initialize the state related to the best block. blockSize := uint64(len(blockBytes)) blockWeight := uint64(GetBlockWeight(btcutil.NewBlock(&block))) @@ -1230,6 +1249,14 @@ func (b *BlockChain) initChainState() error { return nil }) + if err != nil { + return err + } + + // As we might have updated the index after it was loaded, we'll + // attempt to flush the index to the DB. This will only result in a + // write if the elements are dirty, so it'll usually be a noop. + return b.index.flushToDB() } // deserializeBlockRow parses a value in the block index bucket into a block From f9722295f8d55c947cdffef275cd96cba1348ebf Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 8 Aug 2018 18:26:22 -0700 Subject: [PATCH 2/4] blockchain: add new flushIndexState function within connectBestChain --- blockchain/chain.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/blockchain/chain.go b/blockchain/chain.go index c49d728b..8c7e7621 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -1085,6 +1085,17 @@ func (b *BlockChain) reorganizeChain(detachNodes, attachNodes *list.List) error func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, flags BehaviorFlags) (bool, error) { fastAdd := flags&BFFastAdd == BFFastAdd + flushIndexState := func() { + // Intentionally ignore errors writing updated node status to DB. If + // it fails to write, it's not the end of the world. If the block is + // valid, we flush in connectBlock and if the block is invalid, the + // worst that can happen is we revalidate the block after a restart. + if writeErr := b.index.flushToDB(); writeErr != nil { + log.Warnf("Error flushing block index changes to disk: %v", + writeErr) + } + } + // We are extending the main (best) chain with a new block. This is the // most common case. parentHash := &block.MsgBlock().Header.PrevBlock @@ -1108,14 +1119,7 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla return false, err } - // Intentionally ignore errors writing updated node status to DB. If - // it fails to write, it's not the end of the world. If the block is - // valid, we flush in connectBlock and if the block is invalid, the - // worst that can happen is we revalidate the block after a restart. - if writeErr := b.index.flushToDB(); writeErr != nil { - log.Warnf("Error flushing block index changes to disk: %v", - writeErr) - } + flushIndexState() if err != nil { return false, err From 598808cfd08a4f440c0b1dd13ce65c99bfa88f22 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 8 Aug 2018 18:30:46 -0700 Subject: [PATCH 3/4] blockchain: mark blocks as invalid if they fail connectBlock --- blockchain/chain.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/blockchain/chain.go b/blockchain/chain.go index 8c7e7621..3528a007 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -1144,6 +1144,17 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla // Connect the block to the main chain. err := b.connectBlock(node, block, view, stxos) if err != nil { + // If we got hit with a rule error, then we'll mark + // that status of the block as invalid and flush the + // index state to disk before returning with the error. + if _, ok := err.(RuleError); ok { + b.index.SetStatusFlags( + node, statusValidateFailed, + ) + } + + flushIndexState() + return false, err } From 69f313436ff96d1cb0a99bb5398e079c90cefd8d Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 8 Aug 2018 18:31:15 -0700 Subject: [PATCH 4/4] blockchain: during fastAdd or if block wasn't already valid, mark as valid in index --- blockchain/chain.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/blockchain/chain.go b/blockchain/chain.go index 3528a007..71c1b2ee 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -1158,6 +1158,14 @@ func (b *BlockChain) connectBestChain(node *blockNode, block *btcutil.Block, fla return false, err } + // If this is fast add, or this block node isn't yet marked as + // valid, then we'll update its status and flush the state to + // disk again. + if fastAdd || !b.index.NodeStatus(node).KnownValid() { + b.index.SetStatusFlags(node, statusValid) + flushIndexState() + } + return true, nil } if fastAdd {