Refactor block file logic #15118

jimpo · 2019-01-07T06:32:58Z

This cleans up and refactors block file helpers so that they may be used by the block filter indexer. Per design discussion about storing BIP 157 block filters, it has been suggested that they are stored in the same way as block and undo data. This refactor is sufficient to simplify file operations for this use case, though in the future perhaps more pruning-related logic ought to be moved into the new classes.

The basic abstraction is a FlatFileSeq which manages access to a sequence of numbered files into which raw data is written.

Empact · 2019-01-07T07:04:02Z

src/init.cpp

@@ -1631,8 +1631,8 @@ bool AppInitMain(InitInterfaces& interfaces)

    // ********************************************************* Step 11: import blocks

-    if (!CheckDiskSpace() && !CheckDiskSpace(0, true))
-        return false;
+    if (!CheckDiskSpace(GetDataDir()) && !CheckDiskSpace(GetBlocksDir()))


Should this not be ||? /cc @jonasschnelli 386a6b6

Yeah, seems like it to me.

src/util/system.cpp

src/flatfile.h

Empact · 2019-01-07T07:34:40Z

src/flatfile.h

+     *
+     * @param[in] pos The first unwritten position in the file to be flushed.
+     * @param[in] finalize True if no more data will be written to this file.
+     * @return True on success, false on failure.


nit: true

Start of sentence True is fine IMO.

src/validation.cpp

src/flatfile.h

Empact · 2019-01-07T07:48:32Z

Concept ACK hooray objects!

promag

Concept ACK.

Code review up to ad59122.

src/validation.cpp

promag · 2019-01-07T09:27:42Z

src/validation.cpp

@@ -3024,7 +3028,7 @@ static bool FindBlockPos(CDiskBlockPos &pos, unsigned int nAddSize, unsigned int
                }
            }
            else


Commit 2af8b71

Care add {}?

I added the braces after the code is moved to FlatFileSeq::Allocate.

promag · 2019-01-07T09:30:07Z

src/flatfile.cpp

+#include <tinyformat.h>
+
+FlatFileSeq::FlatFileSeq(fs::path dir, const char* prefix, size_t chunk_size)
+    : m_dir(std::move(dir)), m_prefix(prefix), m_chunk_size(chunk_size)


Commit 88cf780

nit, initialize one per line?

I don't see this in the style guide?

You also don't see to declare class members in multiple lines, but fair enough.

+1 on one per line as it makes diffs prettier when adding/removing stuff, but I'm fine as it is too, since it's only 3 vars.

src/flatfile.h

promag · 2019-01-07T09:33:03Z

src/flatfile.h

+#ifndef BITCOIN_FLATFILE_H
+#define BITCOIN_FLATFILE_H
+
+#include <chain.h>


Commit 88cf780

Prefer forward declaration of CDiskBlockPos?

Good point. Though CDiskBlockPos is moved into this header later on, so I don't think it's necessary to change in the earlier commit.

src/validation.cpp

promag · 2019-01-07T09:41:36Z

src/flatfile.cpp

@@ -13,3 +14,26 @@ fs::path FlatFileSeq::FileName(const CDiskBlockPos& pos) const
 {
    return m_dir / strprintf("%s%05u.dat", m_prefix, pos.nFile);
 }
+
+FILE* FlatFileSeq::Open(const CDiskBlockPos& pos, bool fReadOnly)


Commit ad59122

Just asking for a follow up, could this be changed to std::fstream?

Maybe... but a lot of file utilities like FileCommit, AllocateFileRange, etc would have to be modified as well. I think the better approach would be to return a CAutoFile and define the move constructor/assignment operator properly. And probably move it to a different module, like in util.

jamesob · 2019-01-08T17:37:04Z

src/validation.cpp

@@ -310,6 +311,8 @@ static void FindFilesToPruneManual(std::set<int>& setFilesToPrune, int nManualPr
 static void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight);
 bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsViewCache &inputs, bool fScriptChecks, unsigned int flags, bool cacheSigStore, bool cacheFullScriptStore, PrecomputedTransactionData& txdata, std::vector<CScriptCheck> *pvChecks = nullptr);
 static FILE* OpenUndoFile(const CDiskBlockPos &pos, bool fReadOnly = false);
+static FlatFileSeq BlockFileSeq();
+static FlatFileSeq UndoFileSeq();


Any reason not to just have these on-hand as global objects instead of having to reconstruct an object for each use through these functions? Obviously there isn't much of a performance concern given these calls happen before disk IO, but I'm just curious.

They are constructed with GetDataDir/GetBlocksDir which are not static, so the initialization logic would get a bit tricky. Could be OK with lazy evaluation or something, but didn't seem worth it. Open to suggestions.

Ah, okay. Like I said, no big deal AFAICT.

src/validation.cpp

jamesob

All commits so far reviewed (up to 697d4c3fc). Great change aside from the bug I've noted above. This does a nice job of cleaning up and deduplicating flatfile data access code, and adds some test coverage. When taking into account the use for BIP157 filter storage (forthcoming), this is a very valuable patch.

jamesob · 2019-01-08T18:48:43Z

src/test/flatfile_tests.cpp

+                      "payments to be sent directly from one party to another without going "
+                      "through a financial institution.");
+    std::string line2("Digital signatures provide part of the solution, but the main benefits are "
+                      "lost if a trusted third party is still required to prevent double-spending.");


src/flatfile.h

src/flatfile.cpp

DrahtBot · 2019-01-09T22:48:36Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15340 (gui: Introduce bilingual GUI error messages by hebasto)
#13868 (Remove unused fScriptChecks parameter from CheckInputs by Empact)
#13746 (-masterdatadir for datadir bootstrapping by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

jamesob

tACK 9b2d9ee, though the last two commits are in need of fixup.

I read over the code once more since the last rebase and feedback commits. To test, I ran a reindex using this branch on a datadir with a preexisting chain of height 522,000 that had been built without these changes. The reindex completed successfully. I started a recent version of master (12b3010) and saw that block verification succeeded and startup ran as normal.

I'll reACK after squash.

jamesob · 2019-01-24T15:36:08Z

src/flatfile.h

+    FlatFilePos(int nFileIn, unsigned int nPosIn) {
+        nFile = nFileIn;
+        nPos = nPosIn;
+    }


I know this is kind of a move, but worth using an initializer list?

kallewoof

utACK with some nits

kallewoof · 2019-01-30T07:27:19Z

src/flatfile.cpp

@@ -0,0 +1,91 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2019 The Bitcoin Core developers


Copyright looked off to me initially, but I guess if you move code, the copyright stays the same. (I was expecting "2019" only).

kallewoof · 2019-01-30T07:29:02Z

src/flatfile.cpp

+#include <tinyformat.h>
+
+FlatFileSeq::FlatFileSeq(fs::path dir, const char* prefix, size_t chunk_size)
+    : m_dir(std::move(dir)), m_prefix(prefix), m_chunk_size(chunk_size)


+1 on one per line as it makes diffs prettier when adding/removing stuff, but I'm fine as it is too, since it's only 3 vars.

kallewoof · 2019-01-30T07:33:37Z

src/flatfile.h

+     *
+     * @param[in] pos The first unwritten position in the file to be flushed.
+     * @param[in] finalize True if no more data will be written to this file.
+     * @return True on success, false on failure.


Start of sentence True is fine IMO.

kallewoof · 2019-01-30T07:35:20Z

src/flatfile.cpp

+FILE* FlatFileSeq::Open(const FlatFilePos& pos, bool read_only)
+{
+    if (pos.IsNull())
+        return nullptr;


This is a move I believe, so feel free to ignore: add {} or remove newline. (Also applies to code 4 lines down.)

Yeah, it's a move. Would rather not change.

kallewoof · 2019-01-30T07:38:40Z

src/flatfile.cpp

+        return nullptr;
+    }
+    if (pos.nPos) {
+        if (fseek(file, pos.nPos, SEEK_SET)) {


Can do single if here.

if (pos.nPos && fseek(file, pos.nPos, SEEK_SET)) {

kallewoof · 2019-01-30T07:45:56Z

src/util/system.h

@@ -71,6 +71,7 @@ void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length);
 bool RenameOver(fs::path src, fs::path dest);
 bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false);
 bool DirIsWritable(const fs::path& directory);
+bool CheckDiskSpace(const fs::path& dir, uint64_t nAdditionalBytes = 0);


Nit/style: naming convention, additional_bytes?

This is just moving code out of validation.{h,cpp}. Would rather not also rename vars.

kallewoof · 2019-01-30T07:52:12Z

src/test/flatfile_tests.cpp

+
+    BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 99), 2, out_of_space), 101);
+    BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 99))), 200);
+    BOOST_CHECK(!out_of_space);


Would adding

uint64_t too_big = 1024 * 1024 + fs::space(data_dir).available; BOOST_CHECK_EQUAL(seq.Allocate(FlatFilePos(0, 0), too_big, out_of_space), 0); BOOST_CHECK_EQUAL(fs::file_size(seq.FileName(FlatFilePos(0, 0))), 200); BOOST_CHECK(out_of_space);

be overkill?

I couldn't get this test passing on Travis... tried setting too_big to a petabyte as well, and there was still some issue.

The reason is that size_t is 32 bit (edit: on certain systems) and there is more than 2^32 bytes available on the system, resulting in an overflow. The Allocate method should probably take a uint64_t to avoid this.

kallewoof · 2019-01-30T07:56:43Z

src/util/system.cpp

+{
+    constexpr uint64_t nMinDiskSpace = 52428800;
+    uint64_t nFreeBytesAvailable = fs::space(dir).available;
+    return nFreeBytesAvailable >= nMinDiskSpace + nAdditionalBytes;


Nit/style: var_names

This is just moving code out of validation.{h,cpp}. Would rather not also rename vars.

kallewoof · 2019-01-30T08:04:41Z

src/validation.cpp

+            return AbortNode("Disk space is low!", _("Error: Disk space is low!"));
+        }
+        if (bytes_allocated != 0 && fPruneMode) {
+            fCheckForPruning = true;


Can also do

fCheckForPruning |= bytes_allocated != 0 && fPruneMode;

and remove the if stuff.

kallewoof · 2019-01-30T08:05:26Z

src/validation.cpp

+        return state.Error("out of disk space");
+    }
+    if (bytes_allocated != 0 && fPruneMode) {
+        fCheckForPruning = true;


Can get rid of if case here as described above.

etscrivner · 2019-01-31T20:11:46Z

src/flatfile.cpp

+
+FlatFileSeq::FlatFileSeq(fs::path dir, const char* prefix, size_t chunk_size)
+    : m_dir(std::move(dir)), m_prefix(prefix), m_chunk_size(chunk_size)
+{}


Should this raise an error if chunk_size = 0?

etscrivner · 2019-01-31T20:22:12Z

src/flatfile.cpp

+    return file;
+}
+
+size_t FlatFileSeq::Allocate(const FlatFilePos& pos, size_t add_size, bool& out_of_space)


Should add_size = 0 be allowed? Seems like this would always cause n_new_chunks = n_old_chunks thereby causing the function to return 0.

Returning 0 seems like the right thing to do there. Probably better than throwing an exception or something.

etscrivner · 2019-01-31T20:35:03Z

src/util/system.cpp

@@ -135,6 +135,13 @@ bool DirIsWritable(const fs::path& directory)
    return true;
 }

+bool CheckDiskSpace(const fs::path& dir, uint64_t nAdditionalBytes)
+{
+    constexpr uint64_t nMinDiskSpace = 52428800;


It would be good to add an additional comment above this indicating its approximate size. Previously this comment was present:

// Check for nMinDiskSpace bytes (currently 50MB)

etscrivner · 2019-01-31T20:38:04Z

src/flatfile.cpp

+        size_t new_size = n_new_chunks * m_chunk_size;
+        size_t inc_size = new_size - old_size;
+
+        if (CheckDiskSpace(m_dir, inc_size)) {


Any concerns about the type mismatch here between inc_size (size_t) and the second argument to CheckDiskSpace which is a uint64_t?

Do you get a compiler warning? If not, the implicit cast ought to be fine.

No compiler warning, maybe just a note for the future that the sizes of the integer types should probably be reconciled at some point.

ryanofsky

utACK 2e476e9

ryanofsky · 2019-02-20T21:10:06Z

src/validation.cpp

@@ -3016,21 +3016,13 @@ static bool FindBlockPos(CDiskBlockPos &pos, unsigned int nAddSize, unsigned int
        vinfoBlockFile[nFile].nSize += nAddSize;

    if (!fKnown) {
-        unsigned int nOldChunks = (pos.nPos + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;
-        unsigned int nNewChunks = (vinfoBlockFile[nFile].nSize + BLOCKFILE_CHUNK_SIZE - 1) / BLOCKFILE_CHUNK_SIZE;


In commit "validation: Refactor block file pre-allocation into FlatFileSeq." (72d6932)

Note (just for reference because I found this function confusing): In the new code this line is replaced with:

unsigned int n_new_chunks = (pos.nPos + add_size + m_chunk_size - 1) / m_chunk_size;

which is equivalent because vinfoBlockFile[nFile].nSize == pos.nPos + add_size in the !fKnown case due to:

pos.nPos = vinfoBlockFile[nFile].nSize; ... vinfoBlockFile[nFile].nSize += nAddSize;

above

Probably the right move would be to add some comments in the new code, to reduce confusion to future readers?

I'm happy to add comments anywhere you think it would make the new code more clear, but I don't want to add comments because the old code was unnecessarily confusing and this is a slight change for anyone who happened to be familiar with it. I personally find the new Allocate function clear enough as is (including the n_new_chunks calculation), but of course I wrote it...

A PR comment like the one above seems like a fine way to address this sort of thing IMO.

ryanofsky · 2019-02-20T21:13:37Z

src/validation.cpp

+            return AbortNode("Disk space is low!", _("Error: Disk space is low!"));
+        }
+        if (bytes_allocated != 0 && fPruneMode) {
+            fCheckForPruning = true;


In commit "validation: Refactor block file pre-allocation into FlatFileSeq." (72d6932)

Probably doesn't make any difference, but previously the fCheckForPruning = true setting would happen before return AbortNode, but now it will be skipped if there is no disk space.

Same comment applies in FindUndoPos below.

ryanofsky · 2019-02-20T21:30:04Z

src/flatfile.cpp

+        fclose(file);
+        return error("%s: failed to truncate file %d", __func__, pos.nFile);
+    }
+    if (!FileCommit(file)) {


In commit "validation: Refactor file flush logic into FlatFileSeq." (5bc3ece)

Probably ok, but previous code would still call FileCommit when TruncateFile failed, and now it is skipped.

gwillen

utACK 2e476e9, all comments are nits.

src/validation.cpp

src/flatfile.cpp

src/validation.cpp

-BEGIN VERIFY SCRIPT- sed -i 's/CDiskBlockPos/FlatFilePos/g' $(git ls-files 'src/*.h' 'src/*.cpp') -END VERIFY SCRIPT-

laanwj · 2019-03-02T22:21:08Z

utACK 04cca33

04cca33 Style cleanup. (Jim Posen) 4c01e4e flatfile: Unit tests for FlatFileSeq methods. (Jim Posen) 65a489e scripted-diff: Rename CBlockDiskPos to FlatFilePos. (Jim Posen) d6d8a78 Move CDiskBlockPos from chain to flatfile. (Jim Posen) e038093 validation: Refactor file flush logic into FlatFileSeq. (Jim Posen) 992404b validation: Refactor block file pre-allocation into FlatFileSeq. (Jim Posen) e2d2abb validation: Refactor OpenDiskFile into method on FlatFileSeq. (Jim Posen) 9183d6e validation: Extract basic block file logic into FlatFileSeq class. (Jim Posen) 62e7add util: Move CheckDiskSpace to util. (Jim Posen) Pull request description: This cleans up and refactors block file helpers so that they may be used by the block filter indexer. Per [design discussion](#14121 (comment)) about storing BIP 157 block filters, it has been suggested that they are stored in the same way as block and undo data. This refactor is sufficient to simplify file operations for this use case, though in the future perhaps more pruning-related logic ought to be moved into the new classes. The basic abstraction is a `FlatFileSeq` which manages access to a sequence of numbered files into which raw data is written. Tree-SHA512: b2108756777f2dad8964a1a2ef2764486e708a4a4a8cfac47b5de8bcb0625388964438eb096b10cfd9ea39212c299b5cb32fa943e768db2333cf49ea7def157e

…tem.h 3e48fc1 Renamed and moved util.h/cpp files to util/system.h & util/system.cpp (furszy) 7954cef Style cleanup. (Jim Posen) 48801a9 Use common SetDataDir method to create temp directory in tests. (winder) 06464d3 flatfile: Unit tests for FlatFileSeq methods. (Jim Posen) bbb9a90 scripted-diff: Rename CBlockDiskPos to FlatFilePos. (Jim Posen) 472857a Move CDiskBlockPos from chain to flatfile. (Jim Posen) 7fa47bb validation: Refactor file flush logic into FlatFileSeq. (Jim Posen) bf09076 validation: Refactor block file pre-allocation into FlatFileSeq. (Jim Posen) c813080 validation: Refactor OpenDiskFile into method on FlatFileSeq. (Jim Posen) 2619fe8 validation: Extract basic block file logic into FlatFileSeq class. (Jim Posen) e65acf0 util: Move CheckDiskSpace to util. (Jim Posen) Pull request description: This is part of the block logic refactoring and encapsulation work coming from upstream (work started in #2326 and #2328 due the possible blocks db corruption issue). Needed for two future works: (1) faster block validation using a new cache structure and (2) the future possible custom BIP157 (new sync protocol that supersedes BIP37 for light clients -- pending research needed after finishing v6.0 priorities --) Essentially, it's encapsulating the sequenced files open/read/write functionalities inside a new `flatfile` module, and extending the unit test coverage to validate its correct behavior. Adapted the following PRs: * bitcoin#15118. * bitcoin#13145. * Plus added `util.h/cpp` files mv to `util/system.h` & `util/system.cpp`. ACKs for top commit: random-zebra: utACK 3e48fc1 Tree-SHA512: c21ffb6ede054a279f9792c1cbe645b948078bd6bc607d32438f0601fd4df3650c0a34b349e46b4ea69b48f9e6c7bb18d258e139c6e1a47452ac9ea4f3bbee25

merge bitcoin#15118, bitcoin#14172, bitcoin#15623, bitcoin#14121, partial bitcoin#13743, partial bitcoin#15280: block filters

jimpo changed the title ~~Flatfile~~ Refactor block file logic Jan 7, 2019

fanquake added the Refactoring label Jan 7, 2019

Empact reviewed Jan 7, 2019

View reviewed changes

src/util/system.cpp Outdated Show resolved Hide resolved

jimpo force-pushed the flatfile branch from 9ab64b2 to 13583e8 Compare January 7, 2019 07:20

Empact reviewed Jan 7, 2019

View reviewed changes

src/flatfile.h Outdated Show resolved Hide resolved

Empact reviewed Jan 7, 2019

View reviewed changes

src/validation.cpp Show resolved Hide resolved

Empact reviewed Jan 7, 2019

View reviewed changes

src/flatfile.h Show resolved Hide resolved

promag reviewed Jan 7, 2019

View reviewed changes

Empact mentioned this pull request Jan 8, 2019

Fail AppInitMain if either disk space check fails #15124

Merged

jamesob reviewed Jan 8, 2019

View reviewed changes

jimpo force-pushed the flatfile branch 2 times, most recently from a5d21e5 to fbbf962 Compare January 9, 2019 20:28

practicalswift reviewed Jan 9, 2019

View reviewed changes

src/flatfile.h Show resolved Hide resolved

practicalswift reviewed Jan 9, 2019

View reviewed changes

src/flatfile.cpp Outdated Show resolved Hide resolved

jimpo force-pushed the flatfile branch from fbbf962 to 9b2d9ee Compare January 24, 2019 19:21

jamesob approved these changes Jan 25, 2019

View reviewed changes

kallewoof reviewed Jan 30, 2019

View reviewed changes

etscrivner reviewed Jan 31, 2019

View reviewed changes

jimpo force-pushed the flatfile branch 4 times, most recently from 010d0cc to 9131aef Compare February 5, 2019 07:19

DrahtBot added the Needs rebase label Feb 13, 2019

jimpo force-pushed the flatfile branch from 3003e2d to 2e476e9 Compare February 13, 2019 19:29

DrahtBot removed the Needs rebase label Feb 13, 2019

ryanofsky approved these changes Feb 20, 2019

View reviewed changes

gwillen reviewed Feb 22, 2019

View reviewed changes

src/validation.cpp Outdated Show resolved Hide resolved

src/validation.cpp Outdated Show resolved Hide resolved

src/flatfile.cpp Show resolved Hide resolved

src/validation.cpp Outdated Show resolved Hide resolved

jimpo force-pushed the flatfile branch from 2e476e9 to dc10b9b Compare February 22, 2019 19:38

jimpo added 9 commits February 22, 2019 17:38

util: Move CheckDiskSpace to util.

62e7add

validation: Extract basic block file logic into FlatFileSeq class.

9183d6e

validation: Refactor OpenDiskFile into method on FlatFileSeq.

e2d2abb

validation: Refactor block file pre-allocation into FlatFileSeq.

992404b

validation: Refactor file flush logic into FlatFileSeq.

e038093

Move CDiskBlockPos from chain to flatfile.

d6d8a78

scripted-diff: Rename CBlockDiskPos to FlatFilePos.

65a489e

-BEGIN VERIFY SCRIPT- sed -i 's/CDiskBlockPos/FlatFilePos/g' $(git ls-files 'src/*.h' 'src/*.cpp') -END VERIFY SCRIPT-

flatfile: Unit tests for FlatFileSeq methods.

4c01e4e

Style cleanup.

04cca33

jimpo force-pushed the flatfile branch from dc10b9b to 04cca33 Compare February 23, 2019 01:39

laanwj merged commit 04cca33 into bitcoin:master Mar 2, 2019

Empact mentioned this pull request Mar 4, 2019

Remove unused fScriptChecks parameter from CheckInputs #13868

Merged

jimpo deleted the flatfile branch March 5, 2019 00:04

furszy mentioned this pull request Apr 23, 2021

Refactor block file logic + util.h/cpp moved to util/system.h PIVX-Project/PIVX#2336

Merged

kwvg added a commit to kwvg/dash that referenced this pull request Aug 2, 2021

bitcoin#15118: Refactor block file logic

9bfa2b7

kwvg added a commit to kwvg/dash that referenced this pull request Aug 2, 2021

bitcoin#15118: Refactor block file logic

b1d481e

kwvg added a commit to kwvg/dash that referenced this pull request Aug 8, 2021

bitcoin#15118: Refactor block file logic

d921f4f

UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Aug 11, 2021

bitcoin#15118: Refactor block file logic

db810bd

kwvg added a commit to kwvg/dash that referenced this pull request Aug 12, 2021

bitcoin#15118: Refactor block file logic

4d3e3d7

UdjinM6 added a commit to dashpay/dash that referenced this pull request Aug 13, 2021

Merge pull request #4314 from kittywhiskers/blockfilters

607bd4b

merge bitcoin#15118, bitcoin#14172, bitcoin#15623, bitcoin#14121, partial bitcoin#13743, partial bitcoin#15280: block filters

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

		@@ -0,0 +1,91 @@
		// Copyright (c) 2009-2010 Satoshi Nakamoto
		// Copyright (c) 2009-2019 The Bitcoin Core developers

Refactor block file logic #15118

Refactor block file logic #15118

Uh oh!

Conversation

jimpo commented Jan 7, 2019

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Empact commented Jan 7, 2019

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

jamesob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

jamesob left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kallewoof 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!

DrahtBot commented Jan 9, 2019 •

edited

Loading

kallewoof Feb 14, 2019 •

edited

Loading