Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction #9499

TheBlueMatt · 2017-01-09T19:44:58Z

This takes the rather obvious step of keeping a circular buffer of CTransactionRefs which is populated with orphan transactions, transactions replaced out of mempool, and rejected transactions and uses them to populate compact blocks.

It defaults to 100 transactions in size, the same as the orphan map, which, in the worst case, would be about 40MB+memory overhead (this would be 100 400K-weight witness-only-ish transactions replaced, same as the orphan map).

It limits rejected transactions to 100KB in memory usage before storing.

In previous tests, @gmaxwell had indicated that if we had an infinite-sized buffer here we could save a ton of compact-block-round-trips, so in non-adversarial conditions even a small buffer should help at least sometimes.

TheBlueMatt · 2017-01-09T19:46:11Z

Most of the diff here is changing the signature of AcceptToMemoryPool trivially....

TheBlueMatt · 2017-01-09T19:51:42Z

To clarify: only rejected transactions are limited based on memory-usage (to 100KB), everything else relies on the standardness limits - 400K weight, which would be 400KB of witness data, max.

gmaxwell · 2017-01-09T20:32:57Z

Concept ACK. nice implementation.

You should probably consider adding an extra count "% from memory (% hits against extra)," to the successful reconstruction log entry.

The count limit is unfortunate, merely 100 entries but 40MB max? you could easily keep count of the worst case memory usage (ignoring the deduplication) and then just target 4MB-- lots less memory and a higher hitrate no doubt. Ultimately reject will need to be separately limited (too easy for a single trouble maker to blow it out, and not just an attacker: an older peer without a fee filter) somehow but this should be fine for now.

TheBlueMatt · 2017-01-09T21:17:38Z

Indeed, doing smarter limiting would be nice, but this should do for a bare-bones help-the-default-case win for 0.14 (if it turns out to do enough and the fast progress we've been making on review continues through the end of the week). If we prefer it's not really any additional review burden to do a memory-based limit across the entire list.

…

On January 9, 2017 3:33:01 PM EST, Gregory Maxwell ***@***.***> wrote: Concept ACK. nice implementation. You should probably consider adding an extra count "% from memory (% hits against extra)," to the successful reconstruction log entry. The count limit is unfortunate, merely 100 entries but 40MB max? you could easily keep count of the worst case memory usage (ignoring the deduplication) and then just target 4MB-- lots less memory and a higher hitrate no doubt. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #9499 (comment)

instagibbs

very nice, and plenty of room for improvement on top later

utACK, running now with an extra commit on top to see efficacy: instagibbs@fcb8ace

instagibbs · 2017-01-10T16:12:22Z

src/net_processing.cpp

@@ -59,6 +59,9 @@ map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(cs_main);
 map<COutPoint, set<map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(cs_main);
 void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

+static size_t vOrphanAndRecentlyRemovedTxnIt = 0;


technically this is for rejects as well too now... maybe vExtraTxnIt and same for the vector below

gmaxwell · 2017-01-10T18:54:02Z

It's working for me. I added logging for transactions that come from the extrapool

LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from memory (%lu extra hits), and %lu txn requested\n", hash.ToString(), prefilled_count, mempool_count, extra_count, vtx_missing.size());

Please add that, I know extra can't be counted completely precisely in the presence of collisions but it's very useful information.

Some stats:

$ grep essfu ~/.bitcoin/debug.log | tail -n 72 | awk '{aa=int(substr($15,2))>0; bb=$19>0; if (aa && !bb) {saved+=1} else if (!aa && !bb) {noextra+=1} else {roundtrip+=1} } END {print saved/NR " " noextra/NR " " roundtrip/NR}'
0.180556 0.527778 0.291667

So the extra increased no-roundtrip from 52% to 68%... in the last 72 blocks here. Though I see there are at least a couple blocks where extra was just too small and would have been successful if it were larger, I can tell because <5 were missed and grepped the logs showed that I had previously received them. I think thats a big enough improvement to justify doing it, but the size should be increased eventually (presumably via better memory management).

TheBlueMatt · 2017-01-10T19:50:14Z

Renamed vOrphanAndRecentlyRemovedTxn -> vExtraTxnForCompact, no other changes

morcos · 2017-01-10T21:28:39Z

src/net_processing.cpp

@@ -1741,7 +1741,10 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
                // See https://github.com/bitcoin/bitcoin/issues/8279 for details.
                assert(recentRejects);
                recentRejects->insert(tx.GetHash());
-            }
+                if (RecursiveDynamicUsage(*ptx) < 100000)


A quick examination shows 80-90% of transactions not accepted to the mempool are due to rate limited free transaction. What do you think about also adding to this check and the one below && state.GetRejectCode() != REJECT_INSUFFICIENTFEE

You could be throwing away some transactions that would help blocks reconstruct, but I get close to 100 "not accepted" transactions an hour and it seems like I'd rather have 5 hours of non-free transactions rather than 1 hour of all.

It might be more interesting to distinguish REJECT_INSUFFICIENTFEE from rejection because it didn't even meet the static minimum minfee.

I was trying to avoid any complication here, but if you think something simple is worth it I'm ok with adding something.

Yep understood, my guess was that this was simple and a decent improvement but @gmaxwell has a point in that failed replacements could also be REJECT_INSUFFICIENTFEE and those might not be things you want to throw away. So up to you if you can simply disambiguate.

I propose leaving it alone for now, in the long run we'll want some kind of smarter queue.

sdaftuar · 2017-01-11T10:25:30Z

So the extra increased no-roundtrip from 52% to 68%... in the last 72 blocks here.

[Edited: my results I posted were wrong... recalculating.]

gmaxwell · 2017-01-11T19:08:36Z

My last 72 blocks: 0.125 0.625 0.25 (so .62 to .75)

instagibbs · 2017-01-11T19:12:59Z

(piling on) with a 1000 transaction buffer: 0.0833333 0.597222 0.319444 (~8% absolute improvement)

update: up to 20% RTT saved now.

theuni · 2017-01-11T23:46:49Z

src/blockencodings.cpp

@@ -47,7 +47,7 @@ uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const uint256& txhash) const {



-ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock) {
+ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector<std::pair<uint256, CTransactionRef>>& extra_txn) {


const? Or was this intended to remove the matches?

morcos · 2017-01-12T01:51:36Z

1000 tx buffer I got
0.19 0.65 0.16

Definitely worth merging I say..

ACK 863edb4 , but please also add the extra printing

instagibbs · 2017-01-12T19:54:43Z

tACK 863edb4 (agree with additional printing)

TheBlueMatt · 2017-01-12T20:20:31Z

Made the second parameter to InitData const and added the requested debug print.

luke-jr · 2017-01-14T19:06:10Z

src/blockencodings.cpp

+        // the extra risk.
+        if (mempool_count == shorttxids.size())
+            break;
+    }


A lot of code duplication here that would probably be better shared, but maybe not critical for 0.14.

Yea, I thought about it, but to avoid having two vectors I decided against it. Will fix in a follow-up pr for post-0.14.

luke-jr · 2017-01-14T19:07:44Z

I wonder if it might be better to simply cache whether transactions we discard are valid or invalid, rather than the full transaction data. Then hypothetically we can judge the entire block as valid/invalid quickly, even though we still have more data to fetch. But this idea sounds like a lot of work...

TheBlueMatt · 2017-01-14T19:59:51Z

@luke-jr we tried that before...and would have broken consensus had it been shipped, so, yes, that is "a lot of work".

gmaxwell · 2017-01-16T06:56:14Z

ACK.

sipa · 2017-01-14T21:53:54Z

src/blockencodings.cpp

+                have_txn[idit->second]  = true;
+                mempool_count++;
+            } else {
+                // If we find two mempool txn that match the short id, just request it.


Maybe code duplication is acceptable, but at least adapt the comments, or delete them (it's not about mempool txn at this point).

Note that the comment had already been mostly adapted, with an extra few lines added at the end to clarify that if there is a conflict between extra and mempool, it is ignored only if the transactions are identical.

sipa · 2017-01-17T03:39:50Z

src/blockencodings.cpp

@@ -130,6 +131,35 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c
        if (mempool_count == shorttxids.size())
            break;
    }
+    }


Maybe add a comment explaining that if a short txid appears in both the mempool and in the extra_txn vector, we still assume the mempool one is the correct one, and don't treat it as a collision. As the mempool has much better chance for matching with actual blocks, this is justified.

This isnt true? This is only true if the witness hash (and thus, the txn) are identical.

In fact, there's already a comment indicating this.

Seems I was misreading the code, and interpreting the early-exit mempool_count == shorttxids.size() as the way the loop always successfully ends.

sipa · 2017-01-17T03:41:10Z

src/net_processing.cpp

-            }
+                if (RecursiveDynamicUsage(*ptx) < 100000)
+                    AddToCompactExtraTransactions(ptx);
+            } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000)


This branch looks like it will never be taken, as every time the condition holds, the previous branch should have matches as well.

Nevermind. I'm confused by the the braces. Can you instead please add braces around all the indented sections?

OK, braces added.

luke-jr

utACK c735540 edded80 1531652 93380c5 7f8c8ca b55b416 fac4c78

Not so sure about 863edb4

Disclaimer: Reviewed across 3 sittings

luke-jr · 2017-01-14T20:35:58Z

src/net_processing.cpp

@@ -586,6 +589,17 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals)
 // mapOrphanTransactions
 //

+void AddToCompactExtraTransactions(const CTransactionRef& tx)
+{


AssertLockHeld(cs_main);?

Backed this out - some of our orphan map test stuff doesn't take cs_main, so it would fail in test_bitcoin (though all of the actual-bitcoind runs succeeded). This should be as a part of a separate PR to add cs_main and AssertLockHelds to all of the orphan handling functions.

luke-jr · 2017-01-14T20:36:27Z

src/net_processing.cpp

@@ -59,6 +59,9 @@ map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(cs_main);
 map<COutPoint, set<map<uint256, COrphanTx>::iterator, IteratorComparator>> mapOrphanTransactionsByPrev GUARDED_BY(cs_main);
 void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

+static size_t vExtraTxnForCompactIt = 0;


I think this can/should be moved into AddToCompactExtraTransactions itself?

Leaving it outside makes it clear that you cannot simply insert into ExtraTxn, you need to do something special (and, technically, avoids a hidden atomic operation).

luke-jr · 2017-01-17T19:24:25Z

src/net_processing.cpp

+                if (RecursiveDynamicUsage(*ptx) < 100000) {
+                    AddToCompactExtraTransactions(ptx);
+                }
+            } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) {


Shouldn't we at least check that it's valid?

is valid how? The CValidationState was likely not ever set if there were mempool conflicts (which is exactly the case we want here)

There are a few cases of invalid transactions we could detect; eg, script fails.

Yes, but we set state.Invalid for txn-mempool-conflict, and I super dont want a huge list of which cases of invalid we care about and which we dont here.

Hmm, that sounds like something we probably shouldn't be doing (although off-topic for this PR). How about using the ban score?

Lets revisit this after we fix DoS banning and have a more reasonable API to access :).

luke-jr · 2017-01-17T19:24:32Z

src/net_processing.cpp

+                if (RecursiveDynamicUsage(*ptx) < 100000) {
+                    AddToCompactExtraTransactions(ptx);
+                }
+            } else if (tx.HasWitness() && RecursiveDynamicUsage(*ptx) < 100000) {


What stops someone from flooding peers with the same transaction triggering this case and making the extra-pool useless?

There isnt actually much you can do to prevent this - the goal is to capture some number of conflicting transactions and keep them around, and without some complicated rate-limiting logic you're not gonna win that fight.

Fair enough. I guess worst case it simply behaves the same as if this feature was never added.

Yea, this is no worse than before, and in many cases is a big win...its also written to be an easy review and merge, with the opportunity of cleaning it up and making it more clever in the future.

luke-jr · 2017-01-17T20:10:36Z

utACK 863edb4 too (and the formatting commits after my review left off)

instagibbs · 2017-01-18T15:11:38Z

re-ACK c594580

…or compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo)

jnewbery

Apologies for the post-merge review!

jnewbery · 2017-01-19T21:53:20Z

src/blockencodings.cpp

+        // Though ideally we'd continue scanning for the two-txn-match-shortid case,
+        // the performance win of an early exit here is too good to pass up and worth
+        // the extra risk.
+        if (mempool_count == shorttxids.size())


I don't think this early exit is worth it. The only way we could hit this is if we didn't find all of the shortid transactions in the mempool and we found the remaining shortid transactions in vExtraTxnForCompact. If that's the case, what do we gain/lose from early exit:

gain: we won't have to scan through the < 100 transactions in vExtraTxnForCompact. That's an almost negligable win.

lose: if there is a second transaction in vExtraTxnForCompact with the same shortid, we run the risk of a READ_STATUS_FAILED in FillBlock.

Looping through the GetShortID calls is nontrivial for large-ish mempools, and collisions should be ~1 in a million, so I think its still probably worth it.

Plus the intention would be to significantly increase the size of the extra pool in 0.15, assuming smarter memory management.

Understood. When the extra pool size is 100 transactions this isn't worth it, but if the extra pool is made much larger in 0.15, it makes sense.

jnewbery · 2017-01-19T22:00:33Z

src/blockencodings.cpp

+                if (txn_available[idit->second] &&
+                        txn_available[idit->second]->GetWitnessHash() != extra_txn[i].second->GetWitnessHash()) {
+                    txn_available[idit->second].reset();
+                    mempool_count--;


These counts are wrong if there's a triple shortid collision. Same for the mempool count above.

If there's a shortid collision between a transaction in the mempool and in vExtraTxnForCompact, I believe extra_count will underflow.

Indeed, looks like the counting will provide spurious results sometimes...want to open a new pr to fix it?

Sure. I'll take a look tomorrow.

…d txn for compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo)

* Merge bitcoin#8068: Compact Blocks 48efec8 Fix some minor compact block issues that came up in review (Matt Corallo) ccd06b9 Elaborate bucket size math (Pieter Wuille) 0d4cb48 Use vTxHashes to optimize InitData significantly (Matt Corallo) 8119026 Provide a flat list of txid/terators to txn in CTxMemPool (Matt Corallo) 678ee97 Add BIP 152 to implemented BIPs list (Matt Corallo) 56ba516 Add reconstruction debug logging (Matt Corallo) 2f34a2e Get our "best three" peers to announce blocks using cmpctblocks (Matt Corallo) 927f8ee Add ability to fetch CNode by NodeId (Matt Corallo) d25cd3e Add receiver-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 9c837d5 Add sender-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 00c4078 Add protocol messages for short-ids blocks (Matt Corallo) e3b2222 Add some blockencodings tests (Matt Corallo) f4f8f14 Add TestMemPoolEntryHelper::FromTx version for CTransaction (Matt Corallo) 85ad31e Add partial-block block encodings API (Matt Corallo) 5249dac Add COMPACTSIZE wrapper similar to VARINT for serialization (Matt Corallo) cbda71c Move context-required checks from CheckBlockHeader to Contextual... (Matt Corallo) 7c29ec9 If AcceptBlockHeader returns true, pindex will be set. (Matt Corallo) 96806c3 Stop trimming when mapTx is empty (Pieter Wuille) * Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks 1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar) * Merge bitcoin#8418: Add tests for compact blocks 45c7ddd Add p2p test for BIP 152 (compact blocks) (Suhas Daftuar) 9a22a6c Add support for compactblocks to mininode (Suhas Daftuar) a8689fd Tests: refactor compact size serialization in mininode (Suhas Daftuar) 9c8593d Implement SipHash in Python (Pieter Wuille) 56c87e9 Allow changing BIP9 parameters on regtest (Suhas Daftuar) * Merge bitcoin#8505: Trivial: Fix typos in various files 1aacfc2 various typos (leijurv) * Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup a159f25 Remove redundand (and shadowing) declaration (Pavel Janík) cce3024 Do not shadow local variable, cleanup (Pavel Janík) * Merge bitcoin#8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8854: [qa] Fix race condition in p2p-compactblocks test b5fd666 [qa] Fix race condition in p2p-compactblocks test (Suhas Daftuar) * Merge bitcoin#8393: Support for compact blocks together with segwit 27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2 (Suhas Daftuar) 422fac6 [qa] Add support for compactblocks v2 to mininode (Suhas Daftuar) f5b9b8f [qa] Fix bug in mininode witness deserialization (Suhas Daftuar) 6aa28ab Use cmpctblock type 2 for segwit-enabled transfer (Pieter Wuille) be7555f Fix overly-prescriptive p2p-segwit test for new fetch logic (Matt Corallo) 06128da Make GetFetchFlags always request witness objects from witness peers (Matt Corallo) * Merge bitcoin#8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py b55d941 [qa] Fix race condition in sendheaders.py (Suhas Daftuar) 6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8904: [qa] Fix compact block shortids for a test case 4cdece4 [qa] Fix compact block shortids for a test case (Dagur Valberg Johannsson) * Merge bitcoin#8637: Compact Block Tweaks (rebase of bitcoin#8235) 3ac6de0 Align constant names for maximum compact block / blocktxn depth (Pieter Wuille) b2e93a3 Add cmpctblock to debug help list (instagibbs) fe998e9 More agressively filter compact block requests (Matt Corallo) 02a337d Dont remove a "preferred" cmpctblock peer if they provide a block (Matt Corallo) * Merge bitcoin#8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ 6f2f639 Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ (Jorge Timón) * Merge bitcoin#8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock 72ca7d9 Don't hold cs_main when calling ProcessNewBlock from a cmpctblock (Matt Corallo) * Merge bitcoin#8995: Add missing cs_main lock to ::GETBLOCKTXN processing dfe7906 Add missing cs_main lock to ::GETBLOCKTXN processing (Matt Corallo) * Merge bitcoin#8515: A few mempool removal optimizations 0334430 Add some missing includes (Pieter Wuille) 4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille) 51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille) f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille) * Merge bitcoin#9026: Fix handling of invalid compact blocks d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar) 88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar) c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar) * Merge bitcoin#9039: Various serialization simplifcations and optimizations d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille) 25a211a Add optimized CSizeComputer serializers (Pieter Wuille) a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille) a603925 Avoid -Wshadow errors (Pieter Wuille) 5284721 Get rid of nType and nVersion (Pieter Wuille) 657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille) fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille) c2c5d42 Make streams' read and write return void (Pieter Wuille) 50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille) * Merge bitcoin#9058: Fixes for p2p-compactblocks.py test timeouts on travis (bitcoin#8842) dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky) 55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky) 47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky) * Merge bitcoin#9160: [trivial] Fix hungarian variable name ec34648 [trivial] Fix hungarian variable name (Russell Yanofsky) * Merge bitcoin#9159: [qa] Wait for specific block announcement in p2p-compactblocks dfa44d1 [qa] Wait for specific block announcement in p2p-compactblocks (Russell Yanofsky) * Merge bitcoin#9125: Make CBlock a vector of shared_ptr of CTransactions b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille) 1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille) da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille) 0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille) * Merge bitcoin#8872: Remove block-request logic from INV message processing 037159c Remove block-request logic from INV message processing (Matt Corallo) 3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo) d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews) * Merge bitcoin#9199: Always drop the least preferred HB peer when adding a new one. ca8549d Always drop the least preferred HB peer when adding a new one. (Gregory Maxwell) * Merge bitcoin#9233: Fix some typos 15fa95d Fix some typos (fsb4000) * Merge bitcoin#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) 76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo) e736772 Move network-msg-processing code out of main to its own file (Matt Corallo) 87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo) * Merge bitcoin#9014: Fix block-connection performance regression dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo) 2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo) 2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo) ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo) fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo) 6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo) * Merge bitcoin#9240: Remove txConflicted a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos) bf663f8 remove external usage of mempool conflict tracking (Alex Morcos) * Merge bitcoin#9344: Do not run functions with necessary side-effects in assert() da9cdd2 Do not run functions with necessary side-effects in assert() (Gregory Maxwell) * Merge bitcoin#9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock a13fa4c Remove unused CDiskBlockPos* argument from ProcessNewBlock (Matt Corallo) * Merge bitcoin#9352: Attempt reconstruction from all compact block announcements 813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar) 7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar) * Merge bitcoin#9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling) bd02bdd Release cs_main before processing cmpctblock as header (Suhas Daftuar) 680b0c0 Release cs_main before calling ProcessNewBlock (cmpctblock handling) (Suhas Daftuar) * Merge bitcoin#9283: A few more CTransactionRef optimizations 91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille) 6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille) 62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille) c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille) * Merge bitcoin#9375: Relay compact block messages prior to full block connection 02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo) * Merge bitcoin#9486: Make peer=%d log prints consistent e6111b2 Make peer id logging consistent ("peer=%d" instead of "peer %d") (Matt Corallo) * Merge bitcoin#9400: Set peers as HB peers upon full block validation d4781ac Set peers as HB peers upon full block validation (Gregory Sanders) * Merge bitcoin#9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo) * Merge bitcoin#9587: Do not shadow local variable named `tx`. 44f2baa Do not shadow local variable named `tx`. (Pavel Janík) * Merge bitcoin#9510: [trivial] Fix typos in comments cc16d99 [trivial] Fix typos in comments (practicalswift) * Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer. dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery) * Fix using of AcceptToMemoryPool in PrivateSend code * add `override` * fSupportsDesiredCmpctVersion * bring back tx ressurection in DisconnectTip * Fix delayed headers * Remove unused CConnman::FindNode overload * Fix typos and comments * Fix minor code differences * Don't use rejection cache for corrupted transactions Partly based on bitcoin#8525 * Backport missed cs_main locking changes Missed from bitcoin@58a215c * Backport missed comments and mapBlockSource.emplace call Missed from two commits: bitcoin@88c3549 bitcoin@7c98ce5 * Add CheckPeerHeaders() helper and check in (nCount == 0) too

48efec8 Fix some minor compact block issues that came up in review (Matt Corallo) ccd06b9 Elaborate bucket size math (Pieter Wuille) 0d4cb48 Use vTxHashes to optimize InitData significantly (Matt Corallo) 8119026 Provide a flat list of txid/terators to txn in CTxMemPool (Matt Corallo) 678ee97 Add BIP 152 to implemented BIPs list (Matt Corallo) 56ba516 Add reconstruction debug logging (Matt Corallo) 2f34a2e Get our "best three" peers to announce blocks using cmpctblocks (Matt Corallo) 927f8ee Add ability to fetch CNode by NodeId (Matt Corallo) d25cd3e Add receiver-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 9c837d5 Add sender-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 00c4078 Add protocol messages for short-ids blocks (Matt Corallo) e3b2222 Add some blockencodings tests (Matt Corallo) f4f8f14 Add TestMemPoolEntryHelper::FromTx version for CTransaction (Matt Corallo) 85ad31e Add partial-block block encodings API (Matt Corallo) 5249dac Add COMPACTSIZE wrapper similar to VARINT for serialization (Matt Corallo) cbda71c Move context-required checks from CheckBlockHeader to Contextual... (Matt Corallo) 7c29ec9 If AcceptBlockHeader returns true, pindex will be set. (Matt Corallo) 96806c3 Stop trimming when mapTx is empty (Pieter Wuille) * Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks 1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar) * Merge bitcoin#8418: Add tests for compact blocks 45c7ddd Add p2p test for BIP 152 (compact blocks) (Suhas Daftuar) 9a22a6c Add support for compactblocks to mininode (Suhas Daftuar) a8689fd Tests: refactor compact size serialization in mininode (Suhas Daftuar) 9c8593d Implement SipHash in Python (Pieter Wuille) 56c87e9 Allow changing BIP9 parameters on regtest (Suhas Daftuar) * Merge bitcoin#8505: Trivial: Fix typos in various files 1aacfc2 various typos (leijurv) * Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup a159f25 Remove redundand (and shadowing) declaration (Pavel Janík) cce3024 Do not shadow local variable, cleanup (Pavel Janík) * Merge bitcoin#8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8854: [qa] Fix race condition in p2p-compactblocks test b5fd666 [qa] Fix race condition in p2p-compactblocks test (Suhas Daftuar) * Merge bitcoin#8393: Support for compact blocks together with segwit 27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2 (Suhas Daftuar) 422fac6 [qa] Add support for compactblocks v2 to mininode (Suhas Daftuar) f5b9b8f [qa] Fix bug in mininode witness deserialization (Suhas Daftuar) 6aa28ab Use cmpctblock type 2 for segwit-enabled transfer (Pieter Wuille) be7555f Fix overly-prescriptive p2p-segwit test for new fetch logic (Matt Corallo) 06128da Make GetFetchFlags always request witness objects from witness peers (Matt Corallo) * Merge bitcoin#8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py b55d941 [qa] Fix race condition in sendheaders.py (Suhas Daftuar) 6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8904: [qa] Fix compact block shortids for a test case 4cdece4 [qa] Fix compact block shortids for a test case (Dagur Valberg Johannsson) * Merge bitcoin#8637: Compact Block Tweaks (rebase of bitcoin#8235) 3ac6de0 Align constant names for maximum compact block / blocktxn depth (Pieter Wuille) b2e93a3 Add cmpctblock to debug help list (instagibbs) fe998e9 More agressively filter compact block requests (Matt Corallo) 02a337d Dont remove a "preferred" cmpctblock peer if they provide a block (Matt Corallo) * Merge bitcoin#8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ 6f2f639 Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ (Jorge Timón) * Merge bitcoin#8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock 72ca7d9 Don't hold cs_main when calling ProcessNewBlock from a cmpctblock (Matt Corallo) * Merge bitcoin#8995: Add missing cs_main lock to ::GETBLOCKTXN processing dfe7906 Add missing cs_main lock to ::GETBLOCKTXN processing (Matt Corallo) * Merge bitcoin#8515: A few mempool removal optimizations 0334430 Add some missing includes (Pieter Wuille) 4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille) 51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille) f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille) * Merge bitcoin#9026: Fix handling of invalid compact blocks d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar) 88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar) c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar) * Merge bitcoin#9039: Various serialization simplifcations and optimizations d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille) 25a211a Add optimized CSizeComputer serializers (Pieter Wuille) a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille) a603925 Avoid -Wshadow errors (Pieter Wuille) 5284721 Get rid of nType and nVersion (Pieter Wuille) 657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille) fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille) c2c5d42 Make streams' read and write return void (Pieter Wuille) 50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille) * Merge bitcoin#9058: Fixes for p2p-compactblocks.py test timeouts on travis (bitcoin#8842) dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky) 55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky) 47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky) * Merge bitcoin#9160: [trivial] Fix hungarian variable name ec34648 [trivial] Fix hungarian variable name (Russell Yanofsky) * Merge bitcoin#9159: [qa] Wait for specific block announcement in p2p-compactblocks dfa44d1 [qa] Wait for specific block announcement in p2p-compactblocks (Russell Yanofsky) * Merge bitcoin#9125: Make CBlock a vector of shared_ptr of CTransactions b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille) 1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille) da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille) 0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille) * Merge bitcoin#8872: Remove block-request logic from INV message processing 037159c Remove block-request logic from INV message processing (Matt Corallo) 3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo) d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews) * Merge bitcoin#9199: Always drop the least preferred HB peer when adding a new one. ca8549d Always drop the least preferred HB peer when adding a new one. (Gregory Maxwell) * Merge bitcoin#9233: Fix some typos 15fa95d Fix some typos (fsb4000) * Merge bitcoin#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) 76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo) e736772 Move network-msg-processing code out of main to its own file (Matt Corallo) 87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo) * Merge bitcoin#9014: Fix block-connection performance regression dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo) 2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo) 2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo) ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo) fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo) 6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo) * Merge bitcoin#9240: Remove txConflicted a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos) bf663f8 remove external usage of mempool conflict tracking (Alex Morcos) * Merge bitcoin#9344: Do not run functions with necessary side-effects in assert() da9cdd2 Do not run functions with necessary side-effects in assert() (Gregory Maxwell) * Merge bitcoin#9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock a13fa4c Remove unused CDiskBlockPos* argument from ProcessNewBlock (Matt Corallo) * Merge bitcoin#9352: Attempt reconstruction from all compact block announcements 813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar) 7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar) * Merge bitcoin#9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling) bd02bdd Release cs_main before processing cmpctblock as header (Suhas Daftuar) 680b0c0 Release cs_main before calling ProcessNewBlock (cmpctblock handling) (Suhas Daftuar) * Merge bitcoin#9283: A few more CTransactionRef optimizations 91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille) 6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille) 62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille) c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille) * Merge bitcoin#9375: Relay compact block messages prior to full block connection 02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo) * Merge bitcoin#9486: Make peer=%d log prints consistent e6111b2 Make peer id logging consistent ("peer=%d" instead of "peer %d") (Matt Corallo) * Merge bitcoin#9400: Set peers as HB peers upon full block validation d4781ac Set peers as HB peers upon full block validation (Gregory Sanders) * Merge bitcoin#9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo) * Merge bitcoin#9587: Do not shadow local variable named `tx`. 44f2baa Do not shadow local variable named `tx`. (Pavel Janík) * Merge bitcoin#9510: [trivial] Fix typos in comments cc16d99 [trivial] Fix typos in comments (practicalswift) * Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer. dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery) * Fix using of AcceptToMemoryPool in PrivateSend code * add `override` * fSupportsDesiredCmpctVersion * bring back tx ressurection in DisconnectTip * Fix delayed headers * Remove unused CConnman::FindNode overload * Fix typos and comments * Fix minor code differences * Don't use rejection cache for corrupted transactions Partly based on bitcoin#8525 * Backport missed cs_main locking changes Missed from bitcoin/bitcoin@58a215c * Backport missed comments and mapBlockSource.emplace call Missed from two commits: bitcoin/bitcoin@88c3549 bitcoin/bitcoin@7c98ce5 * Add CheckPeerHeaders() helper and check in (nCount == 0) too

TheBlueMatt added 2 commits January 9, 2017 14:15

Move ORPHAN constants from validation.h to net_processing.h

c735540

Make ATMP optionally return the CTransactionRefs it replaced

edded80

TheBlueMatt force-pushed the 2016-12-recent-tx-cache-cmpct branch from 53ba7e4 to 03b5047 Compare January 9, 2017 20:12

TheBlueMatt force-pushed the 2016-12-recent-tx-cache-cmpct branch from 03b5047 to ac926a3 Compare January 10, 2017 00:59

instagibbs approved these changes Jan 10, 2017

View reviewed changes

TheBlueMatt added 4 commits January 10, 2017 14:48

Keep shared_ptrs to recently-replaced txn for compact blocks

1531652

Use replaced transactions in compact block reconstruction

93380c5

Consider all orphan txn for compact-block-extra-txn cache

7f8c8ca

Consider all (<100k memusage) txn for compact-block-extra-txn cache

863edb4

TheBlueMatt force-pushed the 2016-12-recent-tx-cache-cmpct branch from ac926a3 to 863edb4 Compare January 10, 2017 19:48

morcos reviewed Jan 10, 2017

View reviewed changes

theuni reviewed Jan 11, 2017

View reviewed changes

laanwj added this to the 0.14.0 milestone Jan 12, 2017

TheBlueMatt added 2 commits January 12, 2017 12:19

Add extra_count lower bound to compact reconstruction debug print

b55b416

Make PartiallyDownloadedBlock::InitData's second param const

fac4c78

luke-jr reviewed Jan 14, 2017

View reviewed changes

sipa reviewed Jan 17, 2017

View reviewed changes

TheBlueMatt added 2 commits January 16, 2017 22:58

Clarify comment about mempool/extra conflicts

1ccfe9b

Add braces around AddToCompactExtraTransactions

c594580

luke-jr reviewed Jan 17, 2017

View reviewed changes

TheBlueMatt force-pushed the 2016-12-recent-tx-cache-cmpct branch from 853b6c0 to c594580 Compare January 17, 2017 20:29

laanwj merged commit c594580 into bitcoin:master Jan 19, 2017

paveljanik mentioned this pull request Jan 19, 2017

Do not shadow local variable named tx. #9587

Merged

jnewbery reviewed Jan 19, 2017

View reviewed changes

ryanofsky mentioned this pull request Feb 28, 2017

Improve DisconnectTip performance #9208

Merged

gladcow mentioned this pull request Mar 1, 2018

Backport compact blocks functionality from bitcoin dashpay/dash#1966

Merged

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

		@@ -47,7 +47,7 @@ uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const uint256& txhash) const {



		ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock) {
		ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& cmpctblock, std::vector<std::pair<uint256, CTransactionRef>>& extra_txn) {

Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction #9499

Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction #9499

Uh oh!

Conversation

TheBlueMatt commented Jan 9, 2017

Uh oh!

TheBlueMatt commented Jan 9, 2017

Uh oh!

TheBlueMatt commented Jan 9, 2017

Uh oh!

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

Uh oh!

Uh oh!

TheBlueMatt commented Jan 9, 2017 via email

Uh oh!

instagibbs left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmaxwell commented Jan 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jan 10, 2017

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!

sdaftuar commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmaxwell commented Jan 11, 2017

Uh oh!

instagibbs commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morcos commented Jan 12, 2017

Uh oh!

instagibbs commented Jan 12, 2017

Uh oh!

TheBlueMatt commented Jan 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Jan 14, 2017

Uh oh!

TheBlueMatt commented Jan 14, 2017

Uh oh!

gmaxwell commented Jan 16, 2017

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!

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!

gmaxwell commented Jan 9, 2017 •

edited

Loading

instagibbs left a comment •

edited

Loading

gmaxwell commented Jan 10, 2017 •

edited

Loading

sdaftuar commented Jan 11, 2017 •

edited

Loading

instagibbs commented Jan 11, 2017 •

edited

Loading