Set peers as HB peers upon full block validation #9400

instagibbs · 2016-12-21T14:02:04Z

We would rather select HB peers to be those that are getting us full valid blocks the fastest, versus merely letting us know about the next valid PoW block header the earliest. Currently a peer can just share a header, withhold the full block, and be selected as a HB peer, denying other less-malicious peers their slot. The attacker could possibly get 3 connections with the node, then block all useful CB propagation gains.

After this fix the worst that can happen is malicious peers are first to give you valid full blocks to stay in the HB slots.

rebroad · 2016-12-21T16:18:33Z

Is it still the case that the first peer to send us the header is the only peer we'll request the block from (with a timeout for non-response of 10 minutes)?

instagibbs · 2016-12-21T16:21:57Z

@rebroad This code doesn't change any inv behavior.

TheBlueMatt · 2016-12-22T08:12:13Z

src/net_processing.cpp

@@ -798,6 +803,11 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
                Misbehaving(it->second.first, nDoS);
        }
    }
+    else if (state.IsValid() && !IsInitialBlockDownload()) {
+        if (it != mapBlockSource.end() && State(it->second.first)) {


nit: maybe move the State() check inside MaybeSet...?

TheBlueMatt · 2016-12-22T08:27:37Z

src/net_processing.cpp

@@ -798,6 +803,11 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
                Misbehaving(it->second.first, nDoS);
        }
    }
+    else if (state.IsValid() && !IsInitialBlockDownload()) {
+        if (it != mapBlockSource.end() && State(it->second.first)) {
+            MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first, *connman);


Ideally this would go in UpdatedBlockTip instead. mapBlockSource wouldn't be available there, but BlockChecked isnt really capturing where we want this - we want to know that this is actually the best currently available block, not just a block that was just checked.

Not really sure how to fix this - maybe just a mapBlocksInFlight.empty() is sufficient, but its still super ugly.

Could we change the lifetime of mapBlockSource to keep it around until then? We could sweep it later.

TheBlueMatt · 2016-12-22T08:54:24Z

On second thought, I suppose I could be convinced that using BlockChecked here was OK if there were very explicit documentation on when it is/isn't called in validationinterface.h and maybe some flags passed in to further disambiguate.

…

On December 21, 2016 8:22:02 AM PST, Gregory Sanders ***@***.***> wrote: @rebroad This code doesn't change any inv behavior. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: #9400 (comment)

TheBlueMatt · 2016-12-23T00:05:15Z

Changing the lifetime is hard, as only BlockChecked is guaranteed to be called if the block's PoW is invalid.

…

On December 22, 2016 5:53:40 AM PST, Gregory Sanders ***@***.***> wrote: instagibbs commented on this pull request. > @@ -798,6 +803,11 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta Misbehaving(it->second.first, nDoS); } } + else if (state.IsValid() && !IsInitialBlockDownload()) { + if (it != mapBlockSource.end() && State(it->second.first)) { + MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first, *connman); Could we change the lifetime of mapBlockSource to keep it around until then? We could sweep it later. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #9400

rebroad · 2016-12-26T02:54:23Z

src/net_processing.cpp

+            fAnnounceUsingCMPCTBLOCK = true;
+            connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetSendVersion()).Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion));
+            lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId());
+            return true;


why return true? it's a void function.

@rebroad It is not a void function https://dev.visucore.com/bitcoin/doxygen/class_c_connman.html#a16958796bb02ef0151b5b5544316c048

I was referring to MaybeSetPeerAsAnnouncingHeaderAndIDs()... was misreading the flow of the code. Sorry.

rebroad · 2016-12-26T17:19:20Z

Not sure if this is behaving as it's supposed to:-

2016-12-26 17:13:12.258123 UpdateTip: new best=00000000000000000153e83f7511f8b104912bc578ad69d5ee5f138bd418a4cf (445158) ver=0x20000000 age=10.2h workage=9.3h tx=448
2016-12-26 17:13:13.525927 send sendcmpct (no announce) ver=1 peer=2
2016-12-26 17:13:13.526175 send sendcmpct (announce) ver=1 peer=9
2016-12-26 17:13:13.583144 UpdateTip: new best=00000000000000000185ee972011605697340a4a4fa8ba778a93bc61a95dc315 (445159) ver=0x20000000 age=9.8h workage=9.2h tx=2823
2016-12-26 17:13:16.007449 UpdateTip: new best=0000000000000000028907ba1fd6f492cdccfd0be79d11c6c43352834964378e (445160) ver=0x20000000 age=9.8h workage=9.0h tx=929
2016-12-26 17:13:16.143575 send sendcmpct (no announce) ver=1 peer=12
2016-12-26 17:13:16.143707 send sendcmpct (announce) ver=1 peer=2
2016-12-26 17:13:16.150024 UpdateTip: new best=0000000000000000034a8dca7fe1801ed65e03aaeff94eeb34236cda2808f19d (445161) ver=0x20000000 age=9.8h workage=8.8h tx=175
2016-12-26 17:13:18.224032 send sendcmpct (no announce) ver=1 peer=8
2016-12-26 17:13:18.224165 send sendcmpct (announce) ver=1 peer=4
2016-12-26 17:13:18.280457 UpdateTip: new best=000000000000000001bfb4c8427fd5d8256d899d60411c339c52b1c87e6df049 (445162) ver=0x20000002 age=9.5h workage=8.7h tx=1608
2016-12-26 17:13:19.570130 UpdateTip: new best=00000000000000000005546a59fcc5f489a6f0ea15111a137ebc9be21b646b8e (445163) ver=0x20000000 age=9.1h workage=8.5h tx=2579
2016-12-26 17:13:20.929863 send sendcmpct (no announce) ver=1 peer=9
2016-12-26 17:13:20.929987 send sendcmpct (announce) ver=1 peer=8
2016-12-26 17:13:20.985947 UpdateTip: new best=0000000000000000003c52ad03a94a8da08e2bce3c76430d798fba39d1083ff7 (445164) ver=0x20000000 age=9.1h workage=8.3h tx=2110

(there are no blocks being received, it's just catching up with the chain using blocks already downloaded quite some time ago)

TheBlueMatt · 2016-12-26T17:39:15Z

@rebroad can you try adding a mapBlocksInFlight.empty() check prior to sending the sendcmpct message in the function? I believe that should (at least mostly) fix the issue.

…

On December 26, 2016 12:19:27 PM EST, R E Broadley ***@***.***> wrote: Not sure if this is behaving as it's supposed to:- ``` 2016-12-26 17:13:12.258123 UpdateTip: new best=00000000000000000153e83f7511f8b104912bc578ad69d5ee5f138bd418a4cf (445158) ver=0x20000000 age=10.2h workage=9.3h tx=448 2016-12-26 17:13:13.525927 send sendcmpct (no announce) ver=1 peer=2 2016-12-26 17:13:13.526175 send sendcmpct (announce) ver=1 peer=9 2016-12-26 17:13:13.583144 UpdateTip: new best=00000000000000000185ee972011605697340a4a4fa8ba778a93bc61a95dc315 (445159) ver=0x20000000 age=9.8h workage=9.2h tx=2823 2016-12-26 17:13:16.007449 UpdateTip: new best=0000000000000000028907ba1fd6f492cdccfd0be79d11c6c43352834964378e (445160) ver=0x20000000 age=9.8h workage=9.0h tx=929 2016-12-26 17:13:16.143575 send sendcmpct (no announce) ver=1 peer=12 2016-12-26 17:13:16.143707 send sendcmpct (announce) ver=1 peer=2 2016-12-26 17:13:16.150024 UpdateTip: new best=0000000000000000034a8dca7fe1801ed65e03aaeff94eeb34236cda2808f19d (445161) ver=0x20000000 age=9.8h workage=8.8h tx=175 2016-12-26 17:13:18.224032 send sendcmpct (no announce) ver=1 peer=8 2016-12-26 17:13:18.224165 send sendcmpct (announce) ver=1 peer=4 2016-12-26 17:13:18.280457 UpdateTip: new best=000000000000000001bfb4c8427fd5d8256d899d60411c339c52b1c87e6df049 (445162) ver=0x20000002 age=9.5h workage=8.7h tx=1608 2016-12-26 17:13:19.570130 UpdateTip: new best=00000000000000000005546a59fcc5f489a6f0ea15111a137ebc9be21b646b8e (445163) ver=0x20000000 age=9.1h workage=8.5h tx=2579 2016-12-26 17:13:20.929863 send sendcmpct (no announce) ver=1 peer=9 2016-12-26 17:13:20.929987 send sendcmpct (announce) ver=1 peer=8 2016-12-26 17:13:20.985947 UpdateTip: new best=0000000000000000003c52ad03a94a8da08e2bce3c76430d798fba39d1083ff7 (445164) ver=0x20000000 age=9.1h workage=8.3h tx=2110 ``` -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #9400 (comment)

instagibbs · 2016-12-26T18:14:51Z

Updated to hopefully fix issue @rebroad

rebroad · 2016-12-27T03:52:10Z

src/net_processing.cpp

@@ -398,7 +398,7 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
 void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman& connman) {
    AssertLockHeld(cs_main);
    CNodeState* nodestate = State(nodeid);
-    if (!nodestate->fSupportsDesiredCmpctVersion) {
+    if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) {


nodestate can be NULL...?

If the peer is disconnected between when it provides the block and when the block is connected it can, sure.

sipa · 2016-12-27T19:04:00Z

utACK bbdf9563b41ce9a7f032e19afbba1f38fabefa7e

morcos · 2016-12-27T20:32:54Z

I think we should have more discussion on whether this is the right logic to implement and not just is the code correct for implementing it. My guess is there is little benefit from this code and its maybe not worth the risk of introducing a change until we have something that will provide a more significant benefit.

It's already the case that the first peer to announce anything to you is the only peer that you will allow to deliver you the full block (barring a full timeout), so for the most part this doesn't change who can be considered for HB peer. The exception to this is peers that are already HB peers and give you cmpctblocks that get optimistically reconstructed. Those peers are already HB so it doesn't help them, but it does prevent any peer who had previously headered you but not yet delivered the block from being considered. But this is a minor roadblock for any attacker (or just inadvertent bad behavior) because as soon as you have a cmpctblock that doesn't optimistically reconstruct, you're back to the base case.

That said I think this is the general right idea, but I'd just prefer to see it in the context of a larger change so that we're not hacking around in this code repeatedly and at risk of introducing another bug. There is definitely an improvement needed in better maintaining good HB peers.

But perhaps I'm wrong and the benefit from this code is worth it on its own?

rebroad · 2016-12-28T02:31:37Z

@morcos completely agree - and I think I'm testing this code only because it potentially compliments other code I have added myself (e.g. requesting blocktxns from more then one node), but without such further changes, this code alone, at this stage, doesn't really provide any benefit from what I can see either, and needs to be combined with other improvements (i.e. more refined versions of what I'm also hackily producing).

Having said that, I don't think using the PR system to share our hacks (that we may often think are more complete than they are) is intrinsically a bad thing, and the review system is designed to help reveal the level of completeness of a PR raised, or dependencies needed, as we are doing.

rebroad · 2016-12-28T11:15:18Z

@instagibbs ok, looking at this debug.log output:-

2016-12-28 09:05:41.928636 recv new best header 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=21s peer=813
2016-12-28 09:05:41.928868 send getdata cmpctblock 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=21s peer=813
2016-12-28 09:05:42.085294 recv best cmpctblock 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=22s size=11814 peer=736
2016-12-28 09:05:42.089384 send getblocktxn 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=22s indexes=1/1933 size=60 peer=736
2016-12-28 09:05:42.479448 recv best cmpctblock 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=22s size=11814 peer=813
2016-12-28 09:05:42.587522 recv blocktxn 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a (445473) age=22s indexes=1 size=256 expected-peer=813 peer=736
2016-12-28 09:05:42.612230 made block 00000000000000000320a325788953e769b47205654f55ea80a4c2e108a1189a size=998115 txs: mempool=1931 req=1

we can see that peer 813 is the first to advertise the block, but if we change the logic to fetch blocktxns from the first peer to provide the compact block, then often the one that provided the header will not be the one to provide the block as it requries an extra round trip to request it, so it'll often not be asked to announce with compact blocks, even though this might mean it then becomes the best block provider - how do we deal with this situation? (Does it need dealing with?) I think the only way to determine if peer 813 would be better would be to measure it's bandwidth and work out whether it would have sent the larger message (the compact block of 11814 bytes) before the other peers were able to.

morcos · 2016-12-28T12:50:06Z

@rebroad are you also running modified logic above? In master, we would not have requested blocktxn from peer 736 in your example.

rebroad · 2016-12-29T07:58:04Z

@morcos Yes, I am running this PR combined with some of my changes - in an aim to make the changes this PR provides more useful, although I think further refinement is still needed as mentioned in my previous comment.

...ah. I see what you are saying now... yes without my changes, peer 813 would have provided the block and therefore been requested to announce as compact blocks going forward, but at the cost of receiving the block more slowly. My proposal in my previous comment was a way in which we could get the block more quickly AND ensure we are requested compact block announcements from the correct peers - assuming both these things are important.

morcos · 2016-12-29T17:27:40Z

src/net_processing.cpp

@@ -798,6 +803,11 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
                Misbehaving(it->second.first, nDoS);
        }
    }
+    else if (state.IsValid() && !IsInitialBlockDownload() && mapBlocksInFlight.empty()) {


I don't think this check of mapBlocksInFlight.empty() will play nicely with #9352 which doesn't MarkBlockReceived until after ProcessNewBlock

perhaps mapBlocksInFlight.count(hash) == mapBlocksInFlight.size() instead?

gmaxwell · 2016-12-30T02:39:12Z

@morcos I think this is the right logic: We should pick for HB usage peers that actually helped us get the block, not one that merely said they would.

Beyond the slightly improved misbehavior resistance, this should reduce a small bias towards non-HB (or non-predicting HB) peers that arises because their announcement has less bytes on the wire, so it transmits somewhat faster even though it results in block recovery somewhat slower. Perhaps it over corrects for it somewhat: in a world without attacks you'd promote to HB status based on time of announcement adjusted by the serialization size ("who would have been first if all were sending CBs?")-- but we don't know the adjustment and we're not in a world without attacks. The net result of the over-correction is just a little hysteresis-- we're more likely to keep our existing HB set unless someone was faster by at least a RTT--, easily justified by the fact that changing HB selection isn't completely free.

Calling maybe on a peer that is already HB is not a no-op: they get moved to the front of the list, which makes them less likely to get dropped.

I do think this is more interesting once we have multiple reconstructions in flight.

morcos · 2016-12-30T14:23:14Z

@gmaxwell @instagibbs Yes, after spending more time reviewing this, I agree it is the right approach, and its simpler than I thought. Still suggest the fix above, but otherwise ACK.

instagibbs · 2016-12-30T19:29:09Z

@morcos fixed and squashed

morcos · 2017-01-14T15:54:44Z

tested ACK d4781ac

I think this would be nice for 0.14

TheBlueMatt · 2017-01-14T20:47:57Z

utACK d4781ac

gmaxwell · 2017-01-15T15:48:24Z

utACK.

d4781ac Set peers as HB peers upon full block validation (Gregory Sanders)

* 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

fanquake added the P2P label Dec 21, 2016

TheBlueMatt reviewed Dec 22, 2016

View reviewed changes

rebroad reviewed Dec 26, 2016

View reviewed changes

instagibbs force-pushed the maybesetfullblock branch from ee43109 to bbdf956 Compare December 26, 2016 18:01

rebroad reviewed Dec 27, 2016

View reviewed changes

morcos reviewed Dec 29, 2016

View reviewed changes

morcos mentioned this pull request Dec 30, 2016

Allow 2 simultaneous block downloads #9447

Closed

Set peers as HB peers upon full block validation

d4781ac

instagibbs force-pushed the maybesetfullblock branch from bbdf956 to d4781ac Compare December 30, 2016 19:06

morcos mentioned this pull request Dec 31, 2016

Relay compact block messages prior to full block connection #9375

Merged

TheBlueMatt mentioned this pull request Jan 14, 2017

Request announcement by cmpctblock AFTER requesting cmpctblock/blocktxn #9315

Closed

TheBlueMatt mentioned this pull request Jan 14, 2017

Clarify assumptions made about when BlockCheck is called #9558

Merged

sipa merged commit d4781ac into bitcoin:master Jan 15, 2017

sipa added a commit that referenced this pull request Jan 15, 2017

Merge #9400: Set peers as HB peers upon full block validation

8a445c5

d4781ac Set peers as HB peers upon full block validation (Gregory Sanders)

gladcow mentioned this pull request Mar 1, 2018

Backport compact blocks functionality from bitcoin dashpay/dash#1966

Merged

gladcow pushed a commit to gladcow/dash that referenced this pull request Apr 4, 2018

Merge bitcoin#9400: Set peers as HB peers upon full block validation

2f8f475

d4781ac Set peers as HB peers upon full block validation (Gregory Sanders)

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set peers as HB peers upon full block validation #9400

Set peers as HB peers upon full block validation #9400

instagibbs commented Dec 21, 2016

rebroad commented Dec 21, 2016

instagibbs commented Dec 21, 2016

TheBlueMatt Dec 22, 2016

TheBlueMatt Dec 22, 2016

instagibbs Dec 22, 2016

TheBlueMatt commented Dec 22, 2016 via email

TheBlueMatt commented Dec 23, 2016 via email •

edited

rebroad Dec 26, 2016

maflcko Dec 26, 2016

rebroad Dec 27, 2016

rebroad commented Dec 26, 2016 •

edited

TheBlueMatt commented Dec 26, 2016 via email

instagibbs commented Dec 26, 2016

rebroad Dec 27, 2016

instagibbs Dec 27, 2016

TheBlueMatt Dec 27, 2016

sipa commented Dec 27, 2016

morcos commented Dec 27, 2016

rebroad commented Dec 28, 2016 •

edited

rebroad commented Dec 28, 2016 •

edited

morcos commented Dec 28, 2016

rebroad commented Dec 29, 2016 •

edited

morcos Dec 29, 2016

morcos Dec 29, 2016

gmaxwell commented Dec 30, 2016 •

edited

morcos commented Dec 30, 2016

instagibbs commented Dec 30, 2016

morcos commented Jan 14, 2017

TheBlueMatt commented Jan 14, 2017

gmaxwell commented Jan 15, 2017

Set peers as HB peers upon full block validation #9400

Set peers as HB peers upon full block validation #9400

Conversation

instagibbs commented Dec 21, 2016

rebroad commented Dec 21, 2016

instagibbs commented Dec 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheBlueMatt commented Dec 22, 2016 via email

TheBlueMatt commented Dec 23, 2016 via email • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rebroad commented Dec 26, 2016 • edited

TheBlueMatt commented Dec 26, 2016 via email

instagibbs commented Dec 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sipa commented Dec 27, 2016

morcos commented Dec 27, 2016

rebroad commented Dec 28, 2016 • edited

rebroad commented Dec 28, 2016 • edited

morcos commented Dec 28, 2016

rebroad commented Dec 29, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmaxwell commented Dec 30, 2016 • edited

morcos commented Dec 30, 2016

instagibbs commented Dec 30, 2016

morcos commented Jan 14, 2017

TheBlueMatt commented Jan 14, 2017

gmaxwell commented Jan 15, 2017

TheBlueMatt commented Dec 23, 2016 via email •

edited

rebroad commented Dec 26, 2016 •

edited

rebroad commented Dec 28, 2016 •

edited

rebroad commented Dec 28, 2016 •

edited

rebroad commented Dec 29, 2016 •

edited

gmaxwell commented Dec 30, 2016 •

edited