Skip to content

Commit fe1975a

Browse files
sipalaanwj
sipa
authored and
laanwj
committed
Use cmpctblock type 2 for segwit-enabled transfer
Contains version negotiation logic by Matt Corallo and bugfixes by Suhas Daftuar. Github-Pull: #8393 Rebased-From: 6aa28ab
1 parent 611cc50 commit fe1975a

File tree

6 files changed

+64
-30
lines changed

6 files changed

+64
-30
lines changed

‎src/blockencodings.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717

1818
#define MIN_TRANSACTION_BASE_SIZE (::GetSerializeSize(CTransaction(), SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS))
1919

20-
CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block) :
20+
CBlockHeaderAndShortTxIDs::CBlockHeaderAndShortTxIDs(const CBlock& block, bool fUseWTXID) :
2121
nonce(GetRand(std::numeric_limits<uint64_t>::max())),
2222
shorttxids(block.vtx.size() - 1), prefilledtxn(1), header(block) {
2323
FillShortTxIDSelector();
2424
//TODO: Use our mempool prior to block acceptance to predictively fill more than just the coinbase
2525
prefilledtxn[0] = {0, block.vtx[0]};
2626
for (size_t i = 1; i < block.vtx.size(); i++) {
2727
const CTransaction& tx = block.vtx[i];
28-
shorttxids[i - 1] = GetShortID(tx.GetHash());
28+
shorttxids[i - 1] = GetShortID(fUseWTXID ? tx.GetWitnessHash() : tx.GetHash());
2929
}
3030
}
3131

‎src/blockencodings.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ class CBlockHeaderAndShortTxIDs {
146146
// Dummy for deserialization
147147
CBlockHeaderAndShortTxIDs() {}
148148

149-
CBlockHeaderAndShortTxIDs(const CBlock& block);
149+
CBlockHeaderAndShortTxIDs(const CBlock& block, bool fUseWTXID);
150150

151151
uint64_t GetShortID(const uint256& txhash) const;
152152

‎src/main.cpp‎

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,21 @@ struct CNodeState {
292292
bool fPreferHeaders;
293293
//! Whether this peer wants invs or cmpctblocks (when possible) for block announcements.
294294
bool fPreferHeaderAndIDs;
295-
//! Whether this peer will send us cmpctblocks if we request them
295+
/**
296+
* Whether this peer will send us cmpctblocks if we request them.
297+
* This is not used to gate request logic, as we really only care about fSupportsDesiredCmpctVersion,
298+
* but is used as a flag to "lock in" the version of compact blocks (fWantsCmpctWitness) we send.
299+
*/
296300
bool fProvidesHeaderAndIDs;
297301
//! Whether this peer can give us witnesses
298302
bool fHaveWitness;
303+
//! Whether this peer wants witnesses in cmpctblocks/blocktxns
304+
bool fWantsCmpctWitness;
305+
/**
306+
* If we've announced NODE_WITNESS to this peer: whether the peer sends witnesses in cmpctblocks/blocktxns,
307+
* otherwise: whether this peer sends non-witnesses in cmpctblocks/blocktxns.
308+
*/
309+
bool fSupportsDesiredCmpctVersion;
299310

300311
CNodeState() {
301312
fCurrentlyConnected = false;
@@ -316,6 +327,8 @@ struct CNodeState {
316327
fPreferHeaderAndIDs = false;
317328
fProvidesHeaderAndIDs = false;
318329
fHaveWitness = false;
330+
fWantsCmpctWitness = false;
331+
fSupportsDesiredCmpctVersion = false;
319332
}
320333
};
321334

@@ -475,16 +488,16 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
475488
}
476489

477490
void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pfrom) {
478-
if (nLocalServices & NODE_WITNESS) {
479-
// Don't ever request compact blocks when segwit is enabled.
491+
if (!nodestate->fSupportsDesiredCmpctVersion) {
492+
// Never ask from peers who can't provide witnesses.
480493
return;
481494
}
482495
if (nodestate->fProvidesHeaderAndIDs) {
483496
BOOST_FOREACH(const NodeId nodeid, lNodesAnnouncingHeaderAndIDs)
484497
if (nodeid == pfrom->GetId())
485498
return;
486499
bool fAnnounceUsingCMPCTBLOCK = false;
487-
uint64_t nCMPCTBLOCKVersion = 1;
500+
uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1;
488501
if (lNodesAnnouncingHeaderAndIDs.size() >= 3) {
489502
// As per BIP152, we only get 3 of our peers to announce
490503
// blocks using compact encodings.
@@ -4832,11 +4845,12 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
48324845
// they wont have a useful mempool to match against a compact block,
48334846
// and we don't feel like constructing the object for them, so
48344847
// instead we respond with the full, non-compact block.
4848+
bool fPeerWantsWitness = State(pfrom->GetId())->fWantsCmpctWitness;
48354849
if (mi->second->nHeight >= chainActive.Height() - 10) {
4836-
CBlockHeaderAndShortTxIDs cmpctblock(block);
4837-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
4850+
CBlockHeaderAndShortTxIDs cmpctblock(block, fPeerWantsWitness);
4851+
pfrom->PushMessageWithFlag(fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
48384852
} else
4839-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, block);
4853+
pfrom->PushMessageWithFlag(fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, block);
48404854
}
48414855

48424856
// Trigger the peer node to send a getblocks request for the next batch of inventory
@@ -5102,13 +5116,16 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
51025116
pfrom->PushMessage(NetMsgType::SENDHEADERS);
51035117
}
51045118
if (pfrom->nVersion >= SHORT_IDS_BLOCKS_VERSION) {
5105-
// Tell our peer we are willing to provide version-1 cmpctblocks
5119+
// Tell our peer we are willing to provide version 1 or 2 cmpctblocks
51065120
// However, we do not request new block announcements using
51075121
// cmpctblock messages.
51085122
// We send this to non-NODE NETWORK peers as well, because
51095123
// they may wish to request compact blocks from us
51105124
bool fAnnounceUsingCMPCTBLOCK = false;
5111-
uint64_t nCMPCTBLOCKVersion = 1;
5125+
uint64_t nCMPCTBLOCKVersion = 2;
5126+
if (pfrom->GetLocalServices() & NODE_WITNESS)
5127+
pfrom->PushMessage(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion);
5128+
nCMPCTBLOCKVersion = 1;
51125129
pfrom->PushMessage(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion);
51135130
}
51145131
}
@@ -5188,12 +5205,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
51885205
else if (strCommand == NetMsgType::SENDCMPCT)
51895206
{
51905207
bool fAnnounceUsingCMPCTBLOCK = false;
5191-
uint64_t nCMPCTBLOCKVersion = 1;
5208+
uint64_t nCMPCTBLOCKVersion = 0;
51925209
vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion;
5193-
if (nCMPCTBLOCKVersion == 1) {
5210+
if (nCMPCTBLOCKVersion == 1 || ((pfrom->GetLocalServices() & NODE_WITNESS) && nCMPCTBLOCKVersion == 2)) {
51945211
LOCK(cs_main);
5195-
State(pfrom->GetId())->fProvidesHeaderAndIDs = true;
5196-
State(pfrom->GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK;
5212+
// fProvidesHeaderAndIDs is used to "lock in" version of compact blocks we send (fWantsCmpctWitness)
5213+
if (!State(pfrom->GetId())->fProvidesHeaderAndIDs) {
5214+
State(pfrom->GetId())->fProvidesHeaderAndIDs = true;
5215+
State(pfrom->GetId())->fWantsCmpctWitness = nCMPCTBLOCKVersion == 2;
5216+
}
5217+
if (State(pfrom->GetId())->fWantsCmpctWitness == (nCMPCTBLOCKVersion == 2)) // ignore later version announces
5218+
State(pfrom->GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK;
5219+
if (!State(pfrom->GetId())->fSupportsDesiredCmpctVersion) {
5220+
if (pfrom->GetLocalServices() & NODE_WITNESS)
5221+
State(pfrom->GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 2);
5222+
else
5223+
State(pfrom->GetId())->fSupportsDesiredCmpctVersion = (nCMPCTBLOCKVersion == 1);
5224+
}
51975225
}
51985226
}
51995227

@@ -5251,7 +5279,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
52515279
nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER &&
52525280
(!IsWitnessEnabled(chainActive.Tip(), chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) {
52535281
inv.type |= nFetchFlags;
5254-
if (nodestate->fProvidesHeaderAndIDs && !(nLocalServices & NODE_WITNESS))
5282+
if (nodestate->fSupportsDesiredCmpctVersion)
52555283
vToFetch.push_back(CInv(MSG_CMPCT_BLOCK, inv.hash));
52565284
else
52575285
vToFetch.push_back(inv);
@@ -5379,7 +5407,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
53795407
}
53805408
resp.txn[i] = block.vtx[req.indexes[i]];
53815409
}
5382-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCKTXN, resp);
5410+
pfrom->PushMessageWithFlag(State(pfrom->GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCKTXN, resp);
53835411
}
53845412

53855413

@@ -5643,7 +5671,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56435671
// We requested this block for some reason, but our mempool will probably be useless
56445672
// so we just grab the block via normal getdata
56455673
std::vector<CInv> vInv(1);
5646-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5674+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
56475675
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
56485676
}
56495677
return true;
@@ -5655,6 +5683,12 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56555683

56565684
CNodeState *nodestate = State(pfrom->GetId());
56575685

5686+
if (IsWitnessEnabled(pindex->pprev, chainparams.GetConsensus()) && !nodestate->fSupportsDesiredCmpctVersion) {
5687+
// Don't bother trying to process compact blocks from v1 peers
5688+
// after segwit activates.
5689+
return true;
5690+
}
5691+
56585692
// We want to be a bit conservative just to be extra careful about DoS
56595693
// possibilities in compact block processing...
56605694
if (pindex->nHeight <= chainActive.Height() + 2) {
@@ -5681,7 +5715,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56815715
} else if (status == READ_STATUS_FAILED) {
56825716
// Duplicate txindexes, the block is now in-flight, so just request it
56835717
std::vector<CInv> vInv(1);
5684-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5718+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
56855719
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
56865720
return true;
56875721
}
@@ -5708,7 +5742,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
57085742
// We requested this block, but its far into the future, so our
57095743
// mempool will probably be useless - request the block normally
57105744
std::vector<CInv> vInv(1);
5711-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5745+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
57125746
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
57135747
return true;
57145748
} else {
@@ -5750,7 +5784,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
57505784
} else if (status == READ_STATUS_FAILED) {
57515785
// Might have collided, fall back to getdata now :(
57525786
std::vector<CInv> invs;
5753-
invs.push_back(CInv(MSG_BLOCK, resp.blockhash));
5787+
invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash));
57545788
pfrom->PushMessage(NetMsgType::GETDATA, invs);
57555789
} else {
57565790
CValidationState state;
@@ -5899,7 +5933,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
58995933
pindexLast->GetBlockHash().ToString(), pindexLast->nHeight);
59005934
}
59015935
if (vGetData.size() > 0) {
5902-
if (nodestate->fProvidesHeaderAndIDs && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN) && !(nLocalServices & NODE_WITNESS)) {
5936+
if (nodestate->fSupportsDesiredCmpctVersion && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) {
59035937
// We seem to be rather well-synced, so it appears pfrom was the first to provide us
59045938
// with this block! Let's get them to announce using compact blocks in the future.
59055939
MaybeSetPeerAsAnnouncingHeaderAndIDs(nodestate, pfrom);
@@ -6527,8 +6561,8 @@ bool SendMessages(CNode* pto)
65276561
//TODO: Shouldn't need to reload block from disk, but requires refactor
65286562
CBlock block;
65296563
assert(ReadBlockFromDisk(block, pBestIndex, consensusParams));
6530-
CBlockHeaderAndShortTxIDs cmpctblock(block);
6531-
pto->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
6564+
CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantsCmpctWitness);
6565+
pto->PushMessageWithFlag(state.fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
65326566
state.pindexBestHeaderSent = pBestIndex;
65336567
} else if (state.fPreferHeaders) {
65346568
if (vHeaders.size() > 1) {

‎src/test/blockencodings_tests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
6464

6565
// Do a simple ShortTxIDs RT
6666
{
67-
CBlockHeaderAndShortTxIDs shortIDs(block);
67+
CBlockHeaderAndShortTxIDs shortIDs(block, true);
6868

6969
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
7070
stream << shortIDs;
@@ -116,7 +116,7 @@ class TestHeaderAndShortIDs {
116116
stream >> *this;
117117
}
118118
TestHeaderAndShortIDs(const CBlock& block) :
119-
TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs(block)) {}
119+
TestHeaderAndShortIDs(CBlockHeaderAndShortTxIDs(block, true)) {}
120120

121121
uint64_t GetShortID(const uint256& txhash) const {
122122
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
@@ -267,7 +267,7 @@ BOOST_AUTO_TEST_CASE(EmptyBlockRoundTripTest)
267267

268268
// Test simple header round-trip with only coinbase
269269
{
270-
CBlockHeaderAndShortTxIDs shortIDs(block);
270+
CBlockHeaderAndShortTxIDs shortIDs(block, false);
271271

272272
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
273273
stream << shortIDs;

‎src/txmempool.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry,
444444
totalTxSize += entry.GetTxSize();
445445
minerPolicyEstimator->processTransaction(entry, fCurrentEstimate);
446446

447-
vTxHashes.emplace_back(hash, newit);
447+
vTxHashes.emplace_back(tx.GetWitnessHash(), newit);
448448
newit->vTxHashesIdx = vTxHashes.size() - 1;
449449

450450
return true;

‎src/txmempool.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ class CTxMemPool
462462
indexed_transaction_set mapTx;
463463

464464
typedef indexed_transaction_set::nth_index<0>::type::iterator txiter;
465-
std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx hashes/entries in mapTx, in random order
465+
std::vector<std::pair<uint256, txiter> > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order
466466

467467
struct CompareIteratorByHash {
468468
bool operator()(const txiter &a, const txiter &b) const {

0 commit comments

Comments
 (0)