Skip to content

Commit 1b417c2

Browse files
sipasipa
committed
Use cmpctblock type 2 for segwit-enabled transfer
1 parent 0df9ea4 commit 1b417c2

File tree

5 files changed

+32
-27
lines changed

5 files changed

+32
-27
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: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,8 @@ struct CNodeState {
297297
bool fProvidesHeaderAndIDs;
298298
//! Whether this peer can give us witnesses
299299
bool fHaveWitness;
300+
//! Whether this peer wants witnesses in cmpctblocks/blocktxns
301+
bool fWantCmpctWitness;
300302

301303
CNodeState() {
302304
fCurrentlyConnected = false;
@@ -317,6 +319,7 @@ struct CNodeState {
317319
fPreferHeaderAndIDs = false;
318320
fProvidesHeaderAndIDs = false;
319321
fHaveWitness = false;
322+
fWantCmpctWitness = false;
320323
}
321324
};
322325

@@ -476,16 +479,16 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) {
476479
}
477480

478481
void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pfrom) {
479-
if (nLocalServices & NODE_WITNESS) {
480-
// Don't ever request compact blocks when segwit is enabled.
482+
if (nLocalServices & ~pfrom->nServices & NODE_WITNESS) {
483+
// Never ask from peers who can't provide witnesses.
481484
return;
482485
}
483486
if (nodestate->fProvidesHeaderAndIDs) {
484487
BOOST_FOREACH(const NodeId nodeid, lNodesAnnouncingHeaderAndIDs)
485488
if (nodeid == pfrom->GetId())
486489
return;
487490
bool fAnnounceUsingCMPCTBLOCK = false;
488-
uint64_t nCMPCTBLOCKVersion = 1;
491+
uint64_t nCMPCTBLOCKVersion = (nLocalServices & NODE_WITNESS) ? 2 : 1;
489492
if (lNodesAnnouncingHeaderAndIDs.size() >= 3) {
490493
// As per BIP152, we only get 3 of our peers to announce
491494
// blocks using compact encodings.
@@ -4817,11 +4820,12 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
48174820
// they wont have a useful mempool to match against a compact block,
48184821
// and we dont feel like constructing the object for them, so
48194822
// instead we respond with the full, non-compact block.
4823+
bool fPeerWantsWitness = State(pfrom->GetId())->fWantCmpctWitness;
48204824
if (mi->second->nHeight >= chainActive.Height() - 10) {
4821-
CBlockHeaderAndShortTxIDs cmpctblock(block);
4822-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
4825+
CBlockHeaderAndShortTxIDs cmpctblock(block, fPeerWantsWitness);
4826+
pfrom->PushMessageWithFlag(fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
48234827
} else
4824-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, block);
4828+
pfrom->PushMessageWithFlag(fPeerWantsWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCK, block);
48254829
}
48264830

48274831
// Trigger the peer node to send a getblocks request for the next batch of inventory
@@ -5087,13 +5091,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
50875091
pfrom->PushMessage(NetMsgType::SENDHEADERS);
50885092
}
50895093
if (pfrom->nVersion >= SHORT_IDS_BLOCKS_VERSION) {
5090-
// Tell our peer we are willing to provide version-1 cmpctblocks
5094+
// Tell our peer we are willing to provide version 1 or 2 cmpctblocks
50915095
// However, we do not request new block announcements using
50925096
// cmpctblock messages.
50935097
// We send this to non-NODE NETWORK peers as well, because
50945098
// they may wish to request compact blocks from us
50955099
bool fAnnounceUsingCMPCTBLOCK = false;
5096-
uint64_t nCMPCTBLOCKVersion = 1;
5100+
uint64_t nCMPCTBLOCKVersion = (pfrom->nServices & nLocalServices & NODE_WITNESS) ? 2 : 1;
50975101
pfrom->PushMessage(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion);
50985102
}
50995103
}
@@ -5173,12 +5177,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
51735177
else if (strCommand == NetMsgType::SENDCMPCT)
51745178
{
51755179
bool fAnnounceUsingCMPCTBLOCK = false;
5176-
uint64_t nCMPCTBLOCKVersion = 1;
5180+
uint64_t nCMPCTBLOCKVersion = 0;
51775181
vRecv >> fAnnounceUsingCMPCTBLOCK >> nCMPCTBLOCKVersion;
5178-
if (nCMPCTBLOCKVersion == 1) {
5182+
if (nCMPCTBLOCKVersion == 1 || nCMPCTBLOCKVersion == 2) {
51795183
LOCK(cs_main);
5180-
State(pfrom->GetId())->fProvidesHeaderAndIDs = true;
5184+
State(pfrom->GetId())->fProvidesHeaderAndIDs = !(nLocalServices & NODE_WITNESS) || (nCMPCTBLOCKVersion == 2);
51815185
State(pfrom->GetId())->fPreferHeaderAndIDs = fAnnounceUsingCMPCTBLOCK;
5186+
State(pfrom->GetId())->fWantCmpctWitness = (nCMPCTBLOCKVersion == 2);
51825187
}
51835188
}
51845189

@@ -5236,7 +5241,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
52365241
nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER &&
52375242
(!IsWitnessEnabled(chainActive.Tip(), chainparams.GetConsensus()) || State(pfrom->GetId())->fHaveWitness)) {
52385243
inv.type |= nFetchFlags;
5239-
if (nodestate->fProvidesHeaderAndIDs && !(nLocalServices & NODE_WITNESS))
5244+
if (nodestate->fProvidesHeaderAndIDs)
52405245
vToFetch.push_back(CInv(MSG_CMPCT_BLOCK, inv.hash));
52415246
else
52425247
vToFetch.push_back(inv);
@@ -5365,7 +5370,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
53655370
}
53665371
resp.txn[i] = block.vtx[req.indexes[i]];
53675372
}
5368-
pfrom->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCKTXN, resp);
5373+
pfrom->PushMessageWithFlag(State(pfrom->GetId())->fWantCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::BLOCKTXN, resp);
53695374
}
53705375

53715376

@@ -5625,7 +5630,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56255630
// We requested this block for some reason, but our mempool will probably be useless
56265631
// so we just grab the block via normal getdata
56275632
std::vector<CInv> vInv(1);
5628-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5633+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
56295634
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
56305635
return true;
56315636
}
@@ -5663,7 +5668,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56635668
} else if (status == READ_STATUS_FAILED) {
56645669
// Duplicate txindexes, the block is now in-flight, so just request it
56655670
std::vector<CInv> vInv(1);
5666-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5671+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
56675672
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
56685673
return true;
56695674
}
@@ -5690,7 +5695,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
56905695
// We requested this block, but its far into the future, so our
56915696
// mempool will probably be useless - request the block normally
56925697
std::vector<CInv> vInv(1);
5693-
vInv[0] = CInv(MSG_BLOCK, cmpctblock.header.GetHash());
5698+
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(pfrom, pindex->pprev, chainparams.GetConsensus()), cmpctblock.header.GetHash());
56945699
pfrom->PushMessage(NetMsgType::GETDATA, vInv);
56955700
return true;
56965701
} else {
@@ -5732,7 +5737,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
57325737
} else if (status == READ_STATUS_FAILED) {
57335738
// Might have collided, fall back to getdata now :(
57345739
std::vector<CInv> invs;
5735-
invs.push_back(CInv(MSG_BLOCK, resp.blockhash));
5740+
invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash));
57365741
pfrom->PushMessage(NetMsgType::GETDATA, invs);
57375742
} else {
57385743
CValidationState state;
@@ -5881,7 +5886,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
58815886
pindexLast->GetBlockHash().ToString(), pindexLast->nHeight);
58825887
}
58835888
if (vGetData.size() > 0) {
5884-
if (nodestate->fProvidesHeaderAndIDs && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN) && !(nLocalServices & NODE_WITNESS)) {
5889+
if (nodestate->fProvidesHeaderAndIDs && vGetData.size() == 1 && mapBlocksInFlight.size() == 1 && pindexLast->pprev->IsValid(BLOCK_VALID_CHAIN)) {
58855890
// We seem to be rather well-synced, so it appears pfrom was the first to provide us
58865891
// with this block! Let's get them to announce using compact blocks in the future.
58875892
MaybeSetPeerAsAnnouncingHeaderAndIDs(nodestate, pfrom);
@@ -6504,8 +6509,8 @@ bool SendMessages(CNode* pto)
65046509
//TODO: Shouldn't need to reload block from disk, but requires refactor
65056510
CBlock block;
65066511
assert(ReadBlockFromDisk(block, pBestIndex, consensusParams));
6507-
CBlockHeaderAndShortTxIDs cmpctblock(block);
6508-
pto->PushMessageWithFlag(SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
6512+
CBlockHeaderAndShortTxIDs cmpctblock(block, state.fWantCmpctWitness);
6513+
pto->PushMessageWithFlag(state.fWantCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS, NetMsgType::CMPCTBLOCK, cmpctblock);
65096514
state.pindexBestHeaderSent = pBestIndex;
65106515
} else if (state.fPreferHeaders) {
65116516
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;

0 commit comments

Comments
 (0)