btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.2k stars 2.36k forks source link

blockchain: fix inconsistent utxocache and database on reorg #2123

Closed kcalvinalvin closed 6 months ago

kcalvinalvin commented 7 months ago

During reorgs, the assumption being made was that the utxocache is ignored and we're directly accessing the database for utxo set modifications.

However, after a block disconnect, the BlockChain.chainLock, the lock that prevents modifications to the utxo set, was being freed for the callback functions for the blockchain notifications. https://github.com/btcsuite/btcd/blob/13152b35e191385a874294a9dbc902e48b1d71b0/blockchain/chain.go#L853-L858

If a caller calls either of the two methods below, the utxocache could be loaded with utxos that are fetched from the database:

https://github.com/btcsuite/btcd/blob/13152b35e191385a874294a9dbc902e48b1d71b0/blockchain/utxoviewpoint.go#L687-L693

https://github.com/btcsuite/btcd/blob/13152b35e191385a874294a9dbc902e48b1d71b0/blockchain/utxoviewpoint.go#L722-L732

Since the UTXOs fetched here are considered unmodified and therefore wouldn't be saved in the database when the utxoCache is flushed at the end of a reorganization, if a caller makes a call to the above two functions during a reorg, they could be mistakenly think that a utxo exists when it was removed from the utxo set.

Example:

There's a chain like so where a utxo created in b1001 is spent in b1002. Blocks b1002 and b1001 are being disconnected out by a longer chain.

b1000() -> b1001(b1000.tx[1].out[0]) -> b1002(b1001.tx[1].out[0])

Mempool tries to fetch b1001.tx[1].out[0] three times in total. 2, 8 and 11. At 6 the utxo b1001.tx[1].out[0] get loaded up to the cache. The cache never gets reset and that utxo gets fetched at 12. When the reorg finishes, the mempool can no longer fetch b1001.tx[1].out[0]

         database                 UTXO cache                 BlockChain                                     Mempool

1.     {b1002.tx[0].out[0]}       {}                         chainLock.Lock() (for reorg)
2.     {b1002.tx[0].out[0]}       {}                                                                        Try to acquire lock (fetch output b1001.tx[1].out[0])
4.     {b1001.tx[1].out[0]}       {}                         disconnect b1002                               Try to acquire lock (fetch output b1001.tx[1].out[0])
5.     {}                         {}                         chainLock.Unlock() (subscriber callback)       Acquired lock (fetch output b1001.tx[1].out[0])
6.     {}                         {b1001.tx[1].out[0]}                                                      Fetched b1001.tx[1].out[0] (should happen)
7.     {}                         {b1001.tx[1].out[0]}       chainLock.Lock()   (finished callback)
8.     {}                         {b1001.tx[1].out[0]}                                                      Try to acquire lock (fetch output b1001.tx[1].out[0])
9.     {}                         {b1001.tx[1].out[0]}                                                      Try to acquire lock (fetch output b1001.tx[1].out[0])
10.    {b1000.tx[1].out[0]}       {b1001.tx[1].out[0]}       disconnect b1001                               Try to acquire lock (fetch output b1001.tx[1].out[0])
11.    {b1000.tx[1].out[0]}       {b1001.tx[1].out[0]}       chainLock.Unlock() (subscriber callback)       Acquired lock (fetch output b1001.tx[1].out[0])
12.    {b1000.tx[1].out[0]}       {b1001.tx[1].out[0]}                                                      Fetched b1001.tx[1].out[0] (shouldn't happen)
13.    {b1000.tx[1].out[0]}       {b1001.tx[1].out[0]}       chainLock.Lock()   (finished callback)
14.    {b1000.tx[1].out[0]}       {}                         Flush UTXO set
15.    {b1000.tx[1].out[0]}       {}                                                                        Try to acquire lock (fetch output b1001.tx[1].out[0])
16.    {b1000.tx[1].out[0]}       {}                                                                        Acquired lock (fetch output b1001.tx[1].out[0])
17.    {b1000.tx[1].out[0]}       {}                                                                        Did not find b1001.tx[1].out[0] (should happen)
coveralls commented 7 months ago

Pull Request Test Coverage Report for Build 8184050728

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
blockchain/fullblocktests/generate.go 55 57 96.49%
blockchain/chain.go 14 18 77.78%
<!-- Total: 72 78 92.31% -->
Files with Coverage Reduction New Missed Lines %
blockchain/chainio.go 2 60.99%
connmgr/connmanager.go 2 86.27%
blockchain/utxoviewpoint.go 9 72.54%
peer/peer.go 9 73.94%
<!-- Total: 22 -->
Totals Coverage Status
Change from base Build 8117173129: 0.003%
Covered Lines: 29309
Relevant Lines: 51619

💛 - Coveralls