Backport compact blocks functionality from bitcoin #1966

gladcow · 2018-03-01T16:00:32Z

Backport compact blocks functionality (BIP152).

Dash doesn't support SegWit, so Compact Blocks has version 1, but supports not SegWit-related features of Bitcoin Compact Blocks version 2.

Merged BTC PRs:

e9d76a1 Merge bitcoin#8068: Compact Blocks
842bf8d Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks
63c03dd Merge bitcoin#8418: Add tests for compact blocks
ced2d5e Merge bitcoin#8446: [Trivial] BIP9 parameters on regtest cleanup
e753eae Merge bitcoin#8505: Trivial: Fix typos in various files
381d0dd Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup
1c24d5f Merge bitcoin#8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py
6faffb8 Merge bitcoin#8854: [qa] Fix race condition in p2p-compactblocks test
6429cfa Merge bitcoin#8393: Support for compact blocks together with segwit: pick only 27acfc1 commit
d075479 Merge bitcoin#8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py
e2a17e4 Merge bitcoin#8904: [qa] Fix compact block shortids for a test case
0b5a997 Merge bitcoin#8637: Compact Block Tweaks (rebase of bitcoin#8235)
5af9a71 Merge bitcoin#8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/
3cf496d Merge bitcoin#8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock
ced22d0 Merge bitcoin#8995: Add missing cs_main lock to ::GETBLOCKTXN processing
9bdf526 Merge bitcoin#8515: A few mempool removal optimizations
dc6b940 Merge bitcoin#9026: Fix handling of invalid compact blocks
e81df49 Merge bitcoin#9039: Various serialization simplifcations and optimizations
7977a11 Merge bitcoin#9058: Fixes for p2p-compactblocks.py test timeouts on travis (bitcoin#8842)
770364b Merge bitcoin#9160: [trivial] Fix hungarian variable name
9346f84 Merge bitcoin#9075: Decouple peer-processing-logic from block-connection-logic (#3)
44adf68 Merge bitcoin#9159: [qa] Wait for specific block announcement in p2p-compactblocks
7490ae8 Merge bitcoin#9125: Make CBlock a vector of shared_ptr of CTransactions
0c577f2 Merge bitcoin#8872: Remove block-request logic from INV message processing
407d923 Merge bitcoin#9199: Always drop the least preferred HB peer when adding a new one.
7bd1aa5 Merge bitcoin#9233: Fix some typos
dc6dee4 Merge bitcoin#9183: Final Preparation for main.cpp Split
2efcfa5 Merge bitcoin#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp})
d04aeba Merge bitcoin#9014: Fix block-connection performance regression
a1dcf2e Merge bitcoin#9240: Remove txConflicted
82ccac7 Merge bitcoin#9344: Do not run functions with necessary side-effects in assert()
b68685a Merge bitcoin#9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock
a7f7651 Merge bitcoin#9352: Attempt reconstruction from all compact block announcements
7aa7004 Merge bitcoin#9243: Clean up mapArgs and mapMultiArgs Usage
ce5c1f4 Merge bitcoin#9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling)
869781c Merge bitcoin#9283: A few more CTransactionRef optimizations
3908fc4 Merge bitcoin#9375: Relay compact block messages prior to full block connection
f62bc10 Merge bitcoin#9486: Make peer=%d log prints consistent
8a445c5 Merge bitcoin#9400: Set peers as HB peers upon full block validation
9c9af5a Merge bitcoin#9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction
10dc58a Merge bitcoin#9587: Do not shadow local variable named tx.
0fea960 Merge bitcoin#9510: [trivial] Fix typos in comments
09e0c28 Merge bitcoin#9659: Net: Turn some methods and params/variables const
729de15 Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer.
1e92e04 Merge bitcoin#9765: Harden against mistakes handling invalid blocks

UdjinM6

Slightly reviewed up to 52f4a60, see few issues below

UdjinM6 · 2018-03-02T16:38:30Z

src/net_processing.cpp

-void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, const CBlockIndex *pindex = NULL) {
+// returns false, still setting pit, if the block was already in flight from the same peer
+// pit will only be valid as long as the same cs_main lock is being held
+void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const Consensus::Params& consensusParams, const CBlockIndex *pindex = NULL, list<QueuedBlock>::iterator **pit = NULL) {


should return bool

should use explicit std:: prefix (for list here, for map below etc.)

UdjinM6 · 2018-03-02T16:55:59Z

src/protocol.h

+ * Indicates that a node is willing to provide blocks via "cmpctblock" messages.
+ * May indicate that a node prefers to receive new block announcements via a
+ * "cmpctblock" message rather than an "inv", depending on message contents.
+ * @since protocol version 70014 as described by BIP 152


nit: s/70014/70209/g

UdjinM6 · 2018-03-02T17:00:56Z

src/validation.h

@@ -500,6 +500,18 @@ bool CheckBlock(const CBlock& block, CValidationState& state, const Consensus::P
 bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev, int64_t nAdjustedTime);
 bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Consensus::Params& consensusParams, const CBlockIndex *pindexPrev);

+/** Apply the effects of this block (with given index) on the UTXO set represented by coins.


Not sure where this whole block came from but it should be removed I guess

UdjinM6 · 2018-03-02T17:23:22Z

src/blockencodings.cpp

+#include "random.h"
+#include "streams.h"
+#include "txmempool.h"
+#include "main.h"


s/main/validation/

UdjinM6 · 2018-03-05T19:04:16Z

src/net_processing.cpp

+            }
+        }
+        bool fAnnounceUsingCMPCTBLOCK = false;
+        uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1;


todo: should strip out all segwit parts

UdjinM6 · 2018-03-05T19:05:34Z

src/net_processing.cpp

+                    bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams);
+                    assert(ret);
+                    CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantsCmpctWitness);
+                    int nSendFlags = state.fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS;


UdjinM6 · 2018-03-23T15:36:52Z

Probably needs rebase to fix at least some Travis errors

UdjinM6

See inline comments.

(My branch with all fixes applied https://github.com/UdjinM6/dash/commits/pr1966)

UdjinM6 · 2018-03-25T17:23:04Z

src/validation.cpp

-    // UpdateTransactionsFromBlock finds descendants of any transactions in this
-    // block that were added back and cleans up the mempool state.
-    mempool.UpdateTransactionsFromBlock(vHashUpdate);
+


I guess this was removed by mistake, should fix AcceptToMemoryPool args few lines below instead like in https://github.com/bitcoin/bitcoin/pull/9499/files#diff-24efdb00bfbe56b140fb006b562cc70bR2145, the fix UdjinM6@1ba4590

UdjinM6 · 2018-03-25T17:29:34Z

src/net_processing.cpp

@@ -1893,6 +2414,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
            ReadCompactSize(vRecv); // ignore tx count; assume it is 0.
        }

+        if (nCount == 0) {


This part was removed in https://github.com/dashpay/dash/pull/1589/files#diff-eff7adeaec73a769788bb78858815c91L1767, bringing it back is most likely causing issues here. But maybe we could just drop pending headers instead UdjinM6@62ec3e0 @codablock ?

Strange, can't find this comment (which I got as an EMail) in the PR. Yes, this block was moved a few lines down as part of #1589. @UdjinM6: Why revert #1589?
…

On Sun, Mar 25, 2018 at 11:18 PM, UdjinM6 ***@***.***> wrote: In src/net_processing.cpp <#1966 (comment)>: > @@ -1893,6 +2414,11 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr > ReadCompactSize(vRecv); // ignore tx count; assume it is 0. > } > > + if (nCount == 0) { This part was removed in https://github.com/dashpay/ dash/pull/1589/files#diff-eff7adeaec73a769788bb78858815c91L1767, bringing it back is most likely causing issues here. Maybe we should drop pending headers now ***@***.*** <UdjinM6@62ec3e0>? @codablock <https://github.com/codablock> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/dashpay/dash/pull/1966/files/72571c754c43e6bc58c9b3a8ef952eaec6d80fdc#r176947481>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABzAbauHwn52fQyNmQU802ZUp_JPrwb6ks5tiAmWgaJpZM4SYi-b> .

LOL I didn't submitted it till I saw email notification, not sure why github sent notification to you without me submitting the review :D

Re #1589 removal: I'm not sure if it's really helpful anymore, can't seem to reproduce the initial issue after latest backborts. Also, I think it's actually causing #1890 and thus it can be kind of dangerous to have conditions like that on mainnet.

Hmm, I couldn't think of a change from backports that could have fixed the initial issue. While reproducing, did you try a full initial sync on mainnet? The issue should happen when headers are being downloaded and a new block is mined at the same time.

I'm checking this logic now, after @UdjinM6 's fixes all tests are passing, I'll try to test your case with full initial sync.

I found the issue with the fix I proposed earlier - won't work when trying to sync from scratch or when node was offline for too long (>24h on testnet/mainnet). Here is the fix for the fix UdjinM6@08649ba (apply it on top and squash the two together). Sorry 🙈

PS. local tests pass btw, not sure about Travis yet

btw @codablock , do you remember why nCount == 0 was moved down exactly?

I've added the fix (not tested it, compiled only). How do you test it? I've tried to sync it from scratch in testnet and it has worked for me.(

Well, I just dropped blocks/ and chainstate/ in testnet/ and devnet/ and it failed with detected bad peer for initial headers sync, disconnecting peer 0 etc. Reindexing and syncing with latest blocks after few hours of being offline worked fine with old patch before I deleted these folders, so this result was quite surprising to me. Also, I have another folder which was not synced for quite a while and after this failure I tried to sync with that -datadir specified and it failed too. So I started to look a bit deeper :)

@UdjinM6 This was because otherwise we'd never get to the point where the detected bad peer for initial headers sync condition is checked. Starting reviewing now, maybe I can say more later.

UdjinM6 · 2018-03-25T19:34:30Z

src/net_processing.h

@@ -36,6 +39,7 @@ class PeerLogicValidation : public CValidationInterface {
    virtual void SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int nPosInBlock) override;
    virtual void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;
    virtual void BlockChecked(const CBlock& block, const CValidationState& state) override;
+    virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock);


missing override here is causing compile warnings, the fix is obvious UdjinM6@a852074 :)

UdjinM6 · 2018-03-25T19:38:14Z

src/net_processing.cpp

@@ -203,6 +220,9 @@ struct CNodeState {
        nBlocksInFlightValidHeaders = 0;
        fPreferredDownload = false;
        fPreferHeaders = false;
+        fPreferHeaderAndIDs = false;
+        fProvidesHeaderAndIDs = false;
+        fSupportsDesiredCmpctVersion = true; // we have only one version for now


We also have peers that do not (want to) support compact blocks at all so this should be false by default. Should assign true only when expected version is requested via SENDCMPCT i.e. UdjinM6@53b34b1

UdjinM6 · 2018-03-29T15:11:16Z

Tested in tesnet(70209->70208) and devnet(direct connection 70209<->70209) in different configurations (catch up after few hours/few weeks (I have data folder with old blocks)/reindex/sync from scratch) and everything seems to be working as expected) 👍

ACK

UdjinM6 · 2018-03-30T09:41:09Z

src/net.cpp

@@ -335,6 +335,16 @@ CNode* CConnman::FindNode(const CService& addr)
    return NULL;
 }

+//TODO: This is used in only one place in main, and should be removed


This is not used actually, should drop it in both .cpp and .h

PS. might also remove new line added below so that net.* files have no changes at all :)

UdjinM6

Few nits

UdjinM6 · 2018-03-30T10:24:51Z

src/version.h

@@ -43,4 +44,7 @@ static const int FEEFILTER_VERSION = 99999; // disable for now (clarify deployme
 //! DIP0001 was activated in this version
 static const int DIP0001_PROTOCOL_VERSION = 70208;

+//! shord-id-based block download starts with this version


typo: s/shord/short/

UdjinM6 · 2018-03-30T10:27:04Z

src/net_processing.cpp

-        if (pindexLast)
-            UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash());
+        assert(pindexLast);
+        UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash());

        if (nCount == MAX_HEADERS_RESULTS && pindexLast) {


nit: we already check pindexLast in assert, no need to double check here

UdjinM6 · 2018-03-30T10:29:02Z

src/net.cpp

@@ -1807,6 +1807,7 @@ void CConnman::ThreadOpenConnections()
            if (nANow - addr.nLastTry < 600 && nTries < 30)
                continue;

+


nit: pls remove this line

UdjinM6 · 2018-03-30T10:31:56Z

qa/rpc-tests/test_framework/mininode.py


 import dash_hash

 BIP0031_VERSION = 60000
-MY_VERSION = 70208  # current MIN_PEER_PROTO_VERSION
+MY_VERSION = 70209  # current SHORT_IDS_BLOCKS_VERSION to support cmpct blocks? 


nit: pls drop current and ?

codablock

See inline comments.

@UdjinM6 I'd suggest to merge this in squashed mode. This PR contains many already backported PRs but only with compact blocks related changes this time. Seeing these PRs twice in the git history might be more confusing then useful. Also, some commits don't really do what their description says, which is ok as long as the whole PR results in everything being backported. Squashing would hide these mistakes.

In regard to the nCount == 0 thing you struggled with. With the current fix, the detected bad peer for initial headers sync case will not be triggered in case the peer replies with an empty HEADERS message.

I'll do a full diff of bitcoin vs dash now to see if anything was missed.

codablock · 2018-04-04T06:47:53Z

src/protocol.cpp

@@ -74,6 +78,7 @@ static const char* ppszTypeName[] =
    NetMsgType::TX,
    NetMsgType::BLOCK,
    "filtered block", // Should never occur
+    "compact block", // Should never occur


Doesn't this change the numeric inv types of all Dash specific messages?

Yes, it is a wrong place for this string, I'll move it to the end of the list.

codablock · 2018-04-04T06:53:39Z

src/protocol.h

@@ -345,6 +372,9 @@ enum GetDataMsg {
    MSG_GOVERNANCE_OBJECT = 17,
    MSG_GOVERNANCE_OBJECT_VOTE = 18,
    MSG_MASTERNODE_VERIFY = 19,
+    // Nodes may always request a MSG_CMPCT_BLOCK in a getdata, however,
+    // MSG_CMPCT_BLOCK should not appear in any invs except as a part of getdata.
+    MSG_CMPCT_BLOCK = 20, //!< Defined in BIP152


Number Is correct, but it does not point to the correct entries in ppszTypeName.

codablock · 2018-04-04T07:23:42Z

src/net_processing.cpp

-        } else {
-            ProcessNewBlock(chainparams, &block, false, NULL);
-            int nDoS;
-            if (state.IsInvalid(nDoS)) {


This whole blocks seems to be removed by accident?

Hmm, checked a full diff of net_processing.cpp and it looks like it was not removed. Maybe re-added later.

codablock · 2018-04-04T07:52:39Z

src/net_processing.cpp

@@ -397,7 +397,10 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
    }
 }

-void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pfrom, CConnman& connman, CNetMsgMaker& msgMaker) {
+void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pfrom, CConnman& connman) {
+    if (!nodestate->fSupportsDesiredCmpctVersion) {


This commit doesn't do what it says it's doing. See ca8549d
Looks like this change should have been done by some previous commit. I'm ok with keeping it this way as long as we later do a squashed merge (see review comment).

codablock · 2018-04-04T07:54:53Z

src/net_processing.cpp

                    CBlockHeaderAndShortTxIDs cmpctblock(block);
-                    pto->PushMessage(NetMsgType::CMPCTBLOCK, cmpctblock);
+                    connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock));


Not related to what the commit is doing. But as already said in a previous commit, I'm fine with this if we do a squashed merge.

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)

1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar)

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)

1aacfc2 various typos (leijurv)

a159f25 Remove redundand (and shadowing) declaration (Pavel Janík) cce3024 Do not shadow local variable, cleanup (Pavel Janík)

…ks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar)

…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)

44f2baa Do not shadow local variable named `tx`. (Pavel Janík)

cc16d99 [trivial] Fix typos in comments (practicalswift)

dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery)

codablock · 2018-04-04T11:30:57Z

@gladcow Please see https://github.com/codablock/dash/tree/compactblocks_1966
I've added a few commits to fix the stuff I found while doing a full diff. I also added a fix to the nCount == 0 thing. I'm still building and running tests locally however.

EDIT: replaced the nCount == 0 fix with UdjinM6's fix

UdjinM6 · 2018-04-04T11:53:28Z

@codablock https://github.com/codablock/dash/tree/compactblocks_1966 looks good 👍 How about smth like UdjinM6@1464d42 instead of moving (nCount == 0) down again though?

codablock · 2018-04-04T12:05:39Z

@UdjinM6 Yeah, looks better to move it into its own method :) Maybe name it differently though, like "CheckGoodHeadersSyncPeer"...as otherwise it sounds like it's actually checking the headers itself. It would also need a cs_main lock in the case it's called from the nCount == 0 block.

Partly based on bitcoin#8525

Missed from bitcoin@58a215c

Missed from two commits: bitcoin@88c3549 bitcoin@7c98ce5

UdjinM6 · 2018-04-04T12:35:32Z

@codablock agree, see updated version UdjinM6@fd5f8f978

codablock

utACK

* 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

7813eb1 [qa] Overhaul p2p_compactblocks.py (Suhas Daftuar) + extra fixes for pull request dashpay#1966 (compact blocks) Pull request description: Remove tests of: - compactblock behavior in a simulated pre-segwit version of bitcoind This should have been removed a long time ago, as it is not generally necessary for us to test the behavior of old nodes (except perhaps if we want to test that upgrading from an old node to a new one behaves properly) - compactblock behavior during segwit upgrade (ie verifying that network behavior before and after activation was as expected) This is unnecessary to test now that segwit activation has already happened. ACKs for commit 7813eb: jnewbery: utACK 7813eb1 Tree-SHA512: cadf035e6f822fa8cff974ed0c2e88a1d4d7da559b341e574e785fd3d309cc2c98c63bc05479265dc00550ae7b77fc3cbe815caae7f68bcff13a04367dca9b52

gladcow force-pushed the cmpctblk branch from e3cce02 to 52f4a60 Compare March 2, 2018 16:25

UdjinM6 reviewed Mar 2, 2018

View reviewed changes

UdjinM6 reviewed Mar 5, 2018

View reviewed changes

gladcow force-pushed the cmpctblk branch 7 times, most recently from a730fc1 to 3227f79 Compare March 15, 2018 15:21

gladcow force-pushed the cmpctblk branch from 3227f79 to 72571c7 Compare March 24, 2018 06:17

UdjinM6 requested changes Mar 26, 2018

View reviewed changes

gladcow changed the title ~~[WIP] Backport compact blocks functionality from bitcoin~~ Backport compact blocks functionality from bitcoin Mar 27, 2018

gladcow force-pushed the cmpctblk branch 2 times, most recently from 23c6cad to ef24fed Compare March 28, 2018 20:40

UdjinM6 approved these changes Mar 29, 2018

View reviewed changes

UdjinM6 requested a review from codablock March 30, 2018 09:14

UdjinM6 reviewed Mar 30, 2018

View reviewed changes

UdjinM6 added this to the 12.3 milestone Mar 30, 2018

UdjinM6 reviewed Mar 30, 2018

View reviewed changes

codablock suggested changes Apr 4, 2018

View reviewed changes

laanwj added 6 commits April 4, 2018 12:08

Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks

91d0bd9

1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar)

Merge bitcoin#8505: Trivial: Fix typos in various files

5debadc

1aacfc2 various typos (leijurv)

Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup

63c33e2

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-compactbloc…

caae165

…ks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar)

laanwj and others added 11 commits April 4, 2018 12:08

Fix using of AcceptToMemoryPool in PrivateSend code

c728f7d

Merge bitcoin#9587: Do not shadow local variable named tx.

39256a7

44f2baa Do not shadow local variable named `tx`. (Pavel Janík)

Merge bitcoin#9510: [trivial] Fix typos in comments

8e04e55

cc16d99 [trivial] Fix typos in comments (practicalswift)

Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer.

519187c

dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery)

bring back tx ressurection in DisconnectTip

0400258

fSupportsDesiredCmpctVersion

da49fce

add override

ba27cde

Fix delayed headers

1a070be

Remove unused CConnman::FindNode overload

c093912

Fix typos and comments

89b65bd

gladcow force-pushed the cmpctblk branch from 25d1f08 to 89b65bd Compare April 4, 2018 09:09

codablock added 4 commits April 4, 2018 15:30

Fix minor code differences

4e24f73

Don't use rejection cache for corrupted transactions

6791059

Partly based on bitcoin#8525

Backport missed cs_main locking changes

fcbfaa6

Missed from bitcoin@58a215c

Backport missed comments and mapBlockSource.emplace call

3a51596

Missed from two commits: bitcoin@88c3549 bitcoin@7c98ce5

Add CheckPeerHeaders() helper and check in (nCount == 0) too

0bf93ce

codablock approved these changes Apr 10, 2018

View reviewed changes

UdjinM6 merged commit bc45a2f into dashpay:develop Apr 11, 2018

thephez mentioned this pull request Apr 11, 2018

[P2P] Compact block addition dash-docs/dash-docs#59

Closed

5 tasks

		@@ -1807,6 +1807,7 @@ void CConnman::ThreadOpenConnections()
		if (nANow - addr.nLastTry < 600 && nTries < 30)
		continue;

Backport compact blocks functionality from bitcoin #1966

Backport compact blocks functionality from bitcoin #1966

Conversation

gladcow commented Mar 1, 2018 • edited Loading

UdjinM6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UdjinM6 commented Mar 23, 2018

UdjinM6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codablock Mar 26, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UdjinM6 Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UdjinM6 Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UdjinM6 commented Mar 29, 2018 • edited Loading

UdjinM6 Mar 30, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UdjinM6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codablock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codablock commented Apr 4, 2018 • edited Loading

UdjinM6 commented Apr 4, 2018

codablock commented Apr 4, 2018

UdjinM6 commented Apr 4, 2018

codablock left a comment

Choose a reason for hiding this comment

gladcow commented Mar 1, 2018 •

edited

Loading

UdjinM6 Mar 28, 2018 •

edited

Loading

UdjinM6 Mar 28, 2018 •

edited

Loading

UdjinM6 commented Mar 29, 2018 •

edited

Loading

UdjinM6 Mar 30, 2018 •

edited

Loading

codablock commented Apr 4, 2018 •

edited

Loading