Fix some chainstate-init-order bugs. #10758

TheBlueMatt · 2017-07-07T00:06:29Z

This does a number of things to clean up chainstate init order,
fixing some issues as it goes:

Order chainstate init more logically - first all of the
blocktree-related loading, then coinsdb, then
pcoinsTip/chainActive. Only create objects as needed.
More clearly document exactly what is and isn't called in
-reindex and -reindex-chainstate both with comments noting
calls as no-ops and by adding if guards.
Move the writing of fTxIndex to LoadBlockIndex - this fixes a
bug introduced in d6af06d where
InitBlockIndex was writing to fTxIndex which had not yet been
checked (because LoadChainTip hadn't yet initialized the
chainActive, which would otherwise have resulted in
InitBlockIndex being a NOP), allowing you to modify -txindex
without reindex, potentially corrupting your chainstate!
Rename InitBlockIndex to LoadGenesisBlock, which is now a more
natural name for it. Also check mapBlockIndex instead of
chainActive, fixing a bug where we'd write the genesis block out
on every start.
Move LoadGenesisBlock further down in init. This is a more logical
location for it, as it is after all of the blockindex-related
loading and checking, but before any of the UTXO-related loading
and checking.
Give LoadChainTip a return value - allowing it to indicate that
the UTXO DB ran ahead of the block DB. This just provides a nicer
error message instead of the previous mysterious
assert(!setBlockIndexCandidates.empty()) error.
Calls ActivateBestChain in case we just loaded the genesis
block in LoadChainTip, avoiding relying on the ActivateBestChain
in ThreadImport before continuing init process.
Move all of the VerifyDB()-related stuff into a -reindex +
-reindex-chainstate if guard. It couldn't do anything useful
as chainActive.Tip() would be null at this point anyway.

TheBlueMatt · 2017-07-07T00:07:36Z

Note that the two generic "Error initializing block database" strings really should be changed, but as we are already in string freeze for 0.15, they will have to stay as-is for now :(.

TheBlueMatt · 2017-07-07T01:33:28Z

More explicitly fixed the bug that I accidentally fixed by being more explicit - RewindBlockIndex MUST be called even if we start with -reindex-chainstate.

TheBlueMatt · 2017-07-10T17:45:58Z

This needs an 0.15 tag as it fixes some bugs.

TheBlueMatt · 2017-07-16T00:05:32Z

Rebased and added a fix for the issue @sipa pointed out at #10770 (comment)

sipa · 2017-07-16T00:42:53Z

src/validation.cpp

+    }
+
+    if (needs_init) {
+        // Note that LoadBlockIndexDB may have set fReindex if we shut down


I have difficulty parsing this sentence. Can you split it up?

OK, rewrote the comment.

sipa · 2017-07-16T02:22:48Z

@TheBlueMatt When starting with -reindex, and then restart before completing without -reindex, it seems to start over from scratch.

sipa · 2017-07-17T05:18:45Z

ACK

@TheBlueMatt I retract my comment, everything seems to be working like before.

sipa · 2017-07-17T17:48:12Z

I'd like to see this in 0.15 as it at least turns an assert into an error message, in case your chainstate is out of sync with the block chain.

TheBlueMatt · 2017-07-17T17:49:22Z

This (or some subset of it) absolutely has to land for 15, it fixes several regressions currently on master, but its a bugfix, so doesn't need to land today (but isnt small, so should go sooner rather than later).

theuni · 2017-07-19T18:10:32Z

I haven't finished reviewing, but this also fixes a crash when using an invalid -wallet argument. For example: ./bitcoind -wallet='with spaces', which crashes for me on master.

morcos · 2017-07-20T18:33:24Z

utACK c93db42

Strongly advocate for merging this in the near term.
Although I have not gone through testing all sequences, I think this logic is far clearer and more straight forward than the existing logic and there are known bugs in the existing startup sequence.
With all the other changes to databases recently (rewinding, replaying, reindexing), I think it makes sense to have as much testing as possible with this new code rather than waste our time potentially finding more bugs with the old logic.

promag · 2017-07-21T16:03:56Z

In current master running bitcoind -regtest '-wallet=foo%.dat' gives:

./src/bitcoind -regtest '-wallet=foo%.dat'
Error: Invalid characters in -wallet filename
Segmentation fault (core dumped)

With this branch gives

Error: Invalid characters in -wallet filename
bitcoind: scheduler.cpp:200: void SingleThreadedSchedulerClient::EmptyQueue(): Assertion `!m_pscheduler->AreThreadsServicingQueue()' failed.
Aborted (core dumped)

Reporting this here because of c93db42.

TheBlueMatt · 2017-07-23T17:55:30Z

Hmm, I went ahead and added the fix for @promag's race here, feel free to tell me it should be in a different PR. (@theuni note that it was a race, which may explain why you didnt see it).

TheBlueMatt · 2017-07-23T22:11:10Z

@sipa hmm, re: restarting-mid-reindex: your original comment that it was broken was more correct than your later retraction. Fixed and cleaned that stuff up more with more comments :).

promag · 2017-07-23T23:08:22Z

@TheBlueMatt if it's not related with your refactor then IMO a new PR would be best.

TheBlueMatt · 2017-07-24T18:58:45Z

OK, removed the top two commits so this is just what it was that got ACKs. The two commits from the top are in #10919, which should likely also be taken for 15.

sipa · 2017-07-25T01:42:27Z

reutACK c93db429f909e75fbeacd2419fb8434350e3b674

laanwj · 2017-07-27T10:44:06Z

src/validation.cpp

+        if (!ReceivedBlockTransactions(block, state, pindex, blockPos, chainparams.GetConsensus()))
+            return error("LoadBlockIndex(): genesis block not accepted");
+    } catch (const std::runtime_error& e) {
+        return error("LoadBlockIndex(): failed to initialize block database: %s", e.what());


This error is no longer correct, this is "failed to load genesis block" now I guess?

laanwj · 2017-07-27T10:44:57Z

src/validation.cpp

+        CDiskBlockPos blockPos;
+        CValidationState state;
+        if (!FindBlockPos(state, blockPos, nBlockSize+8, 0, block.GetBlockTime()))
+            return error("LoadBlockIndex(): FindBlockPos failed");


Would be better to use error("%s: ...", __func__) here as in other places, but in any case, the function name is wrong now.

laanwj · 2017-07-27T11:19:21Z

utACK apart from error message nits

* Move the writing of fTxIndex to LoadBlockIndex - this fixes a bug introduced in d6af06d where InitBlockIndex was writing to fTxIndex which had not yet been checked (because LoadChainTip hadn't yet initialized the chainActive, which would otherwise have resulted in InitBlockIndex being a NOP), allowing you to modify -txindex without reindex, potentially corrupting your chainstate! * Rename InitBlockIndex to LoadGenesisBlock, which is now a more natural name for it. Also check mapBlockIndex instead of chainActive, fixing a bug where we'd write the genesis block out on every start.

This gives LoadChainTip a return value - allowing it to indicate that the UTXO DB ran ahead of the block DB. This just provides a nicer error message instead of the previous mysterious assert(!setBlockIndexCandidates.empty()) error. This also calls ActivateBestChain in case we just loaded the genesis block in LoadChainTip, avoiding relying on the ActivateBestChain in ThreadImport before continuing init process.

RewindBlockIndex works over both chainActive - disconnecting blocks from the tip that need witness verification - and mapBlockIndex - requiring redownload of blocks missing witness data. It should never have been the case that the second half is skipped if we're about to run -reindex-chainstate.

* Order chainstate init more logically - first all of the blocktree-related loading, then coinsdb, then pcoinsTip/chainActive. Only create objects as needed. * More clearly document exactly what is and isn't called in -reindex and -reindex-chainstate both with comments noting calls as no-ops and by adding if guards. * Move LoadGenesisBlock further down in init. This is a more logical location for it, as it is after all of the blockindex-related loading and checking, but before any of the UTXO-related loading and checking. * Move all of the VerifyDB()-related stuff into a -reindex + -reindex-chainstate if guard. It couldn't do anything useful as chainActive.Tip() would be null at this point anyway.

This was introduced by 3192975. It can be triggered easily when canceling DB upgrade from pre-per-utxo.

TheBlueMatt · 2017-07-27T19:03:20Z

Fixed error printings in LoadGenesisBlock.

morcos · 2017-07-28T02:38:31Z

re-utACK c0025d0

sdaftuar

utACK, left some nits.

Is it worth backporting the RewindBlockIndex bugfix to the 0.13 or 0.14 branch? I could imagine someone upgrading from 0.13.0 to the latest 0.13 release after segwit activates. Or to the latest 0.14 if the 0.15.0 release comes out after activation?

sdaftuar · 2017-07-31T15:38:01Z

src/test/test_bitcoin.cpp

@@ -72,7 +72,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
        pblocktree = new CBlockTreeDB(1 << 20, true);
        pcoinsdbview = new CCoinsViewDB(1 << 23, true);
        pcoinsTip = new CCoinsViewCache(pcoinsdbview);
-        if (!InitBlockIndex(chainparams)) {
+        if (!LoadGenesisBlock(chainparams)) {


The string on the next line ought to be changed to reflect the new function name.

sdaftuar · 2017-07-31T17:28:10Z

src/validation.cpp

-        } catch (const std::runtime_error& e) {
-            return error("LoadBlockIndex(): failed to initialize block database: %s", e.what());
-        }
+    try {


The comment at line 3900 now seems outdated; perhaps it makes more sense to move it to init.cpp:1422?

sdaftuar · 2017-07-31T17:39:50Z

src/validation.cpp

+    // mapBlockIndex. Note that we can't use chainActive here, since it is
+    // set based on the coins db, not the block index db, which is the only
+    // thing loaded at this point.
+    if (mapBlockIndex.count(chainparams.GenesisBlock().GetHash()))


style nit: curly braces for this if

sdaftuar · 2017-07-31T18:04:50Z

src/validation.cpp


    // Load pointer to end of best chain
    BlockMap::iterator it = mapBlockIndex.find(pcoinsTip->GetBestBlock());
    if (it == mapBlockIndex.end())
-        return;
+        return false;


style nit: perhaps add the curly braces here, while you're at it

sdaftuar · 2017-07-31T18:25:25Z

src/init.cpp

@@ -1437,10 +1427,34 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
                    break;
                }

+                // At this point blocktree args are consistent with what's on disk.
+                // If we're not mid-reindex (based on disk + args), add a genesis block on disk.
+                // This is called again in ThreadImport in the reindex completes.


"in the reindex completes" -> "after the reindex completes"?

laanwj · 2017-08-01T10:50:49Z

I'm going to merge this now - sorry @sdaftuar, we should fix your minor nits later, but we have to make some progress with merging for 0.15

c0025d0 Fix segfault when shutting down before fully loading (Matt Corallo) 1385697 Order chainstate init more logically. (Matt Corallo) ff3a219 Call RewindBlockIndex even if we're about to run -reindex-chainstate (Matt Corallo) b0f3249 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo) eda888e Fix some LoadChainTip-related init-order bugs. (Matt Corallo) Pull request description: This does a number of things to clean up chainstate init order, fixing some issues as it goes: * Order chainstate init more logically - first all of the blocktree-related loading, then coinsdb, then pcoinsTip/chainActive. Only create objects as needed. * More clearly document exactly what is and isn't called in -reindex and -reindex-chainstate both with comments noting calls as no-ops and by adding if guards. * Move the writing of fTxIndex to LoadBlockIndex - this fixes a bug introduced in d6af06d where InitBlockIndex was writing to fTxIndex which had not yet been checked (because LoadChainTip hadn't yet initialized the chainActive, which would otherwise have resulted in InitBlockIndex being a NOP), allowing you to modify -txindex without reindex, potentially corrupting your chainstate! * Rename InitBlockIndex to LoadGenesisBlock, which is now a more natural name for it. Also check mapBlockIndex instead of chainActive, fixing a bug where we'd write the genesis block out on every start. * Move LoadGenesisBlock further down in init. This is a more logical location for it, as it is after all of the blockindex-related loading and checking, but before any of the UTXO-related loading and checking. * Give LoadChainTip a return value - allowing it to indicate that the UTXO DB ran ahead of the block DB. This just provides a nicer error message instead of the previous mysterious assert(!setBlockIndexCandidates.empty()) error. * Calls ActivateBestChain in case we just loaded the genesis block in LoadChainTip, avoiding relying on the ActivateBestChain in ThreadImport before continuing init process. * Move all of the VerifyDB()-related stuff into a -reindex + -reindex-chainstate if guard. It couldn't do anything useful as chainActive.Tip() would be null at this point anyway. Tree-SHA512: 3c96ee7ed44f4130bee3479a40c5cd99a619fda5e309c26d60b54feab9f6ec60fabab8cf47a049c9cf15e88999b2edb7f16cbe6819e97273560b201a89d90762

sdaftuar · 2017-08-01T17:22:50Z

@laanwj Agreed, I don't think those nits should have held this up.

e7539f8 Fix some broken init-time prints/constants (Matt Corallo) 13ab353 Check for empty coinsview instead of just-reset coinsview in init (Matt Corallo) fce3f4f Fix resume-of-reindex-after-restart (Matt Corallo) efac91e Always wait for threadGroup to exit in bitcoind shutdown (Matt Corallo) Pull request description: This is a follow-on to #10758 to help move 10758 along. The first fixes a regression in master that was partially fixed in 10758, the second I'm not sure if its a regression or not, but its clearly a bug that should be fixed. Tree-SHA512: aca7b97a97dca66e1a218a33cc6f4aa002292ff1bb0af64e35b81fbaa91b9504f2605375808b43e93a63fc73634ad079b30ef6c9f4ba338d3b5f72d816dfeaff

c0025d0 Fix segfault when shutting down before fully loading (Matt Corallo) 1385697 Order chainstate init more logically. (Matt Corallo) ff3a219 Call RewindBlockIndex even if we're about to run -reindex-chainstate (Matt Corallo) b0f3249 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo) eda888e Fix some LoadChainTip-related init-order bugs. (Matt Corallo) Pull request description: This does a number of things to clean up chainstate init order, fixing some issues as it goes: * Order chainstate init more logically - first all of the blocktree-related loading, then coinsdb, then pcoinsTip/chainActive. Only create objects as needed. * More clearly document exactly what is and isn't called in -reindex and -reindex-chainstate both with comments noting calls as no-ops and by adding if guards. * Move the writing of fTxIndex to LoadBlockIndex - this fixes a bug introduced in d6af06d where InitBlockIndex was writing to fTxIndex which had not yet been checked (because LoadChainTip hadn't yet initialized the chainActive, which would otherwise have resulted in InitBlockIndex being a NOP), allowing you to modify -txindex without reindex, potentially corrupting your chainstate! * Rename InitBlockIndex to LoadGenesisBlock, which is now a more natural name for it. Also check mapBlockIndex instead of chainActive, fixing a bug where we'd write the genesis block out on every start. * Move LoadGenesisBlock further down in init. This is a more logical location for it, as it is after all of the blockindex-related loading and checking, but before any of the UTXO-related loading and checking. * Give LoadChainTip a return value - allowing it to indicate that the UTXO DB ran ahead of the block DB. This just provides a nicer error message instead of the previous mysterious assert(!setBlockIndexCandidates.empty()) error. * Calls ActivateBestChain in case we just loaded the genesis block in LoadChainTip, avoiding relying on the ActivateBestChain in ThreadImport before continuing init process. * Move all of the VerifyDB()-related stuff into a -reindex + -reindex-chainstate if guard. It couldn't do anything useful as chainActive.Tip() would be null at this point anyway. Tree-SHA512: 3c96ee7ed44f4130bee3479a40c5cd99a619fda5e309c26d60b54feab9f6ec60fabab8cf47a049c9cf15e88999b2edb7f16cbe6819e97273560b201a89d90762

e7539f8 Fix some broken init-time prints/constants (Matt Corallo) 13ab353 Check for empty coinsview instead of just-reset coinsview in init (Matt Corallo) fce3f4f Fix resume-of-reindex-after-restart (Matt Corallo) efac91e Always wait for threadGroup to exit in bitcoind shutdown (Matt Corallo) Pull request description: This is a follow-on to bitcoin#10758 to help move 10758 along. The first fixes a regression in master that was partially fixed in 10758, the second I'm not sure if its a regression or not, but its clearly a bug that should be fixed. Tree-SHA512: aca7b97a97dca66e1a218a33cc6f4aa002292ff1bb0af64e35b81fbaa91b9504f2605375808b43e93a63fc73634ad079b30ef6c9f4ba338d3b5f72d816dfeaff

c0025d0 Fix segfault when shutting down before fully loading (Matt Corallo) 1385697 Order chainstate init more logically. (Matt Corallo) ff3a219 Call RewindBlockIndex even if we're about to run -reindex-chainstate (Matt Corallo) b0f3249 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo) eda888e Fix some LoadChainTip-related init-order bugs. (Matt Corallo) Pull request description: This does a number of things to clean up chainstate init order, fixing some issues as it goes: * Order chainstate init more logically - first all of the blocktree-related loading, then coinsdb, then pcoinsTip/chainActive. Only create objects as needed. * More clearly document exactly what is and isn't called in -reindex and -reindex-chainstate both with comments noting calls as no-ops and by adding if guards. * Move the writing of fTxIndex to LoadBlockIndex - this fixes a bug introduced in d6af06d where InitBlockIndex was writing to fTxIndex which had not yet been checked (because LoadChainTip hadn't yet initialized the chainActive, which would otherwise have resulted in InitBlockIndex being a NOP), allowing you to modify -txindex without reindex, potentially corrupting your chainstate! * Rename InitBlockIndex to LoadGenesisBlock, which is now a more natural name for it. Also check mapBlockIndex instead of chainActive, fixing a bug where we'd write the genesis block out on every start. * Move LoadGenesisBlock further down in init. This is a more logical location for it, as it is after all of the blockindex-related loading and checking, but before any of the UTXO-related loading and checking. * Give LoadChainTip a return value - allowing it to indicate that the UTXO DB ran ahead of the block DB. This just provides a nicer error message instead of the previous mysterious assert(!setBlockIndexCandidates.empty()) error. * Calls ActivateBestChain in case we just loaded the genesis block in LoadChainTip, avoiding relying on the ActivateBestChain in ThreadImport before continuing init process. * Move all of the VerifyDB()-related stuff into a -reindex + -reindex-chainstate if guard. It couldn't do anything useful as chainActive.Tip() would be null at this point anyway. Tree-SHA512: 3c96ee7ed44f4130bee3479a40c5cd99a619fda5e309c26d60b54feab9f6ec60fabab8cf47a049c9cf15e88999b2edb7f16cbe6819e97273560b201a89d90762

e7539f8 Fix some broken init-time prints/constants (Matt Corallo) 13ab353 Check for empty coinsview instead of just-reset coinsview in init (Matt Corallo) fce3f4f Fix resume-of-reindex-after-restart (Matt Corallo) efac91e Always wait for threadGroup to exit in bitcoind shutdown (Matt Corallo) Pull request description: This is a follow-on to bitcoin#10758 to help move 10758 along. The first fixes a regression in master that was partially fixed in 10758, the second I'm not sure if its a regression or not, but its clearly a bug that should be fixed. Tree-SHA512: aca7b97a97dca66e1a218a33cc6f4aa002292ff1bb0af64e35b81fbaa91b9504f2605375808b43e93a63fc73634ad079b30ef6c9f4ba338d3b5f72d816dfeaff

4749d52 [Refactoring][BUG] Unchecked LoadGenesisBlock return value (random-zebra) 43cc880 Fix segfault when shutting down before fully loading (Matt Corallo) 5f1f014 Order chainstate init more logically. (random-zebra) 9b87537 More user-friendly error message if UTXO DB runs ahead of block DB (Matt Corallo) 9bcc942 Fix some LoadChainTip-related init-order bugs. (random-zebra) Pull request description: Had this segfault when trying to shut down the wallet before the coins cache is loaded. ``` CCoinsViewCache::DynamicMemoryUsage (this=0x0) at coins.cpp:60 60 return memusage::DynamicUsage(cacheCoins) + (gdb) bt #0 0x0000555555bb8b44 in CCoinsViewCache::DynamicMemoryUsage() const (this=0x0) at coins.cpp:60 #1 0x0000555555996bc1 in FlushStateToDisk(CValidationState&, FlushStateMode) (state=..., mode=mode@entry=FLUSH_STATE_ALWAYS) at validation.cpp:1730 #2 0x0000555555997540 in FlushStateToDisk() () at validation.cpp:1806 #3 0x000055555582516c in PrepareShutdown() () at init.cpp:266 #4 0x0000555555826005 in Shutdown() () at init.cpp:339 ``` This is due to a bug introduced in 64c525b (we should null-check `pcoinsTip` before calling `FlushStateToDisk` at line 266, same as we do at line 283). Upstream fixed it in bitcoin#10758 among few other things. Backported here without ff3a219 (as we don't have `RewindBlockIndex` or `reindex-chainstate` yet). ACKs for top commit: Fuzzbawls: ACK 4749d52 furszy: ACK 4749d52 and merging.. Tree-SHA512: 23f27c4a9422a72794054698313259f89ad22a58dba2809901653e08c964c73e4eaeab872ada42380e76eb46ddd7e57f08b65f22812c791d0c2b274e5f0202a7

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from 61d0d7f to 7bfd19d Compare July 7, 2017 00:06

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch 4 times, most recently from bcd6bb9 to 5925964 Compare July 7, 2017 00:53

fanquake added the Refactoring label Jul 7, 2017

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from 5925964 to 8d5f2c3 Compare July 7, 2017 01:32

TheBlueMatt mentioned this pull request Jul 7, 2017

Use non-atomic flushing with block replay #10148

Merged

TheBlueMatt mentioned this pull request Jul 16, 2017

Drop upgrade-cancel callback registration for a generic "cancelable" #10770

Merged

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from 8d5f2c3 to db8d863 Compare July 16, 2017 00:04

sipa reviewed Jul 16, 2017

View reviewed changes

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from db8d863 to c93db42 Compare July 16, 2017 22:43

sipa added this to the 0.15.0 milestone Jul 17, 2017

sipa mentioned this pull request Jul 18, 2017

Segfault on shutdown after pressing 'q' to postpone utxo db upgrade #10861

Closed

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from 921e3ae to c93db42 Compare July 24, 2017 18:57

TheBlueMatt mentioned this pull request Jul 24, 2017

Fix more init bugs. #10919

Merged

laanwj added the Bug label Jul 25, 2017

laanwj reviewed Jul 27, 2017

View reviewed changes

TheBlueMatt added 5 commits July 27, 2017 15:03

Fix segfault when shutting down before fully loading

c0025d0

This was introduced by 3192975. It can be triggered easily when canceling DB upgrade from pre-per-utxo.

TheBlueMatt force-pushed the 2014-07-nonatomic-flush-fixes branch from c93db42 to c0025d0 Compare July 27, 2017 19:03

sdaftuar approved these changes Jul 31, 2017

View reviewed changes

laanwj merged commit c0025d0 into bitcoin:master Aug 1, 2017

random-zebra mentioned this pull request Mar 2, 2021

[BUG] Fix some chainstate-init-order bugs PIVX-Project/PIVX#2223

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Fix some chainstate-init-order bugs. #10758

Fix some chainstate-init-order bugs. #10758

Uh oh!

Conversation

TheBlueMatt commented Jul 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 7, 2017

Uh oh!

TheBlueMatt commented Jul 7, 2017

Uh oh!

TheBlueMatt commented Jul 10, 2017

Uh oh!

TheBlueMatt commented Jul 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sipa commented Jul 16, 2017

Uh oh!

sipa commented Jul 17, 2017

Uh oh!

sipa commented Jul 17, 2017

Uh oh!

TheBlueMatt commented Jul 17, 2017

Uh oh!

theuni commented Jul 19, 2017

Uh oh!

morcos commented Jul 20, 2017

Uh oh!

promag commented Jul 21, 2017

Uh oh!

TheBlueMatt commented Jul 23, 2017

Uh oh!

TheBlueMatt commented Jul 23, 2017

Uh oh!

promag commented Jul 23, 2017

Uh oh!

TheBlueMatt commented Jul 24, 2017

Uh oh!

sipa commented Jul 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jul 27, 2017

Uh oh!

TheBlueMatt commented Jul 27, 2017

Uh oh!

morcos commented Jul 28, 2017

Uh oh!

sdaftuar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Aug 1, 2017

Uh oh!

sdaftuar commented Aug 1, 2017

Uh oh!

Uh oh!

TheBlueMatt commented Jul 7, 2017 •

edited

Loading