Make FillBlock consume txn_available to avoid shared_ptr copies
This commit is contained in:
parent
62607d796c
commit
6713f0f142
3 changed files with 40 additions and 21 deletions
|
@ -142,8 +142,9 @@ bool PartiallyDownloadedBlock::IsTxAvailable(size_t index) const {
|
||||||
return txn_available[index] ? true : false;
|
return txn_available[index] ? true : false;
|
||||||
}
|
}
|
||||||
|
|
||||||
ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) const {
|
ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) {
|
||||||
assert(!header.IsNull());
|
assert(!header.IsNull());
|
||||||
|
uint256 hash = header.GetHash();
|
||||||
block = header;
|
block = header;
|
||||||
block.vtx.resize(txn_available.size());
|
block.vtx.resize(txn_available.size());
|
||||||
|
|
||||||
|
@ -154,8 +155,13 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
|
||||||
return READ_STATUS_INVALID;
|
return READ_STATUS_INVALID;
|
||||||
block.vtx[i] = vtx_missing[tx_missing_offset++];
|
block.vtx[i] = vtx_missing[tx_missing_offset++];
|
||||||
} else
|
} else
|
||||||
block.vtx[i] = txn_available[i];
|
block.vtx[i] = std::move(txn_available[i]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Make sure we can't call FillBlock again.
|
||||||
|
header.SetNull();
|
||||||
|
txn_available.clear();
|
||||||
|
|
||||||
if (vtx_missing.size() != tx_missing_offset)
|
if (vtx_missing.size() != tx_missing_offset)
|
||||||
return READ_STATUS_INVALID;
|
return READ_STATUS_INVALID;
|
||||||
|
|
||||||
|
@ -170,10 +176,10 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
|
||||||
return READ_STATUS_CHECKBLOCK_FAILED;
|
return READ_STATUS_CHECKBLOCK_FAILED;
|
||||||
}
|
}
|
||||||
|
|
||||||
LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", header.GetHash().ToString(), prefilled_count, mempool_count, vtx_missing.size());
|
LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", hash.ToString(), prefilled_count, mempool_count, vtx_missing.size());
|
||||||
if (vtx_missing.size() < 5) {
|
if (vtx_missing.size() < 5) {
|
||||||
for (const auto& tx : vtx_missing)
|
for (const auto& tx : vtx_missing)
|
||||||
LogPrint("cmpctblock", "Reconstructed block %s required tx %s\n", header.GetHash().ToString(), tx->GetHash().ToString());
|
LogPrint("cmpctblock", "Reconstructed block %s required tx %s\n", hash.ToString(), tx->GetHash().ToString());
|
||||||
}
|
}
|
||||||
|
|
||||||
return READ_STATUS_OK;
|
return READ_STATUS_OK;
|
||||||
|
|
|
@ -202,7 +202,7 @@ public:
|
||||||
|
|
||||||
ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock);
|
ReadStatus InitData(const CBlockHeaderAndShortTxIDs& cmpctblock);
|
||||||
bool IsTxAvailable(size_t index) const;
|
bool IsTxAvailable(size_t index) const;
|
||||||
ReadStatus FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing) const;
|
ReadStatus FillBlock(CBlock& block, const std::vector<CTransactionRef>& vtx_missing);
|
||||||
};
|
};
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
|
@ -85,17 +85,23 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
|
||||||
BOOST_CHECK_EQUAL(pool.size(), poolSize - 1);
|
BOOST_CHECK_EQUAL(pool.size(), poolSize - 1);
|
||||||
|
|
||||||
CBlock block2;
|
CBlock block2;
|
||||||
std::vector<CTransactionRef> vtx_missing;
|
{
|
||||||
BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_INVALID); // No transactions
|
PartiallyDownloadedBlock tmp = partialBlock;
|
||||||
|
BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_INVALID); // No transactions
|
||||||
|
partialBlock = tmp;
|
||||||
|
}
|
||||||
|
|
||||||
vtx_missing.push_back(block.vtx[2]); // Wrong transaction
|
// Wrong transaction
|
||||||
partialBlock.FillBlock(block2, vtx_missing); // Current implementation doesn't check txn here, but don't require that
|
{
|
||||||
|
PartiallyDownloadedBlock tmp = partialBlock;
|
||||||
|
partialBlock.FillBlock(block2, {block.vtx[2]}); // Current implementation doesn't check txn here, but don't require that
|
||||||
|
partialBlock = tmp;
|
||||||
|
}
|
||||||
bool mutated;
|
bool mutated;
|
||||||
BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated));
|
BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated));
|
||||||
|
|
||||||
vtx_missing[0] = block.vtx[1];
|
|
||||||
CBlock block3;
|
CBlock block3;
|
||||||
BOOST_CHECK(partialBlock.FillBlock(block3, vtx_missing) == READ_STATUS_OK);
|
BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[1]}) == READ_STATUS_OK);
|
||||||
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString());
|
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString());
|
||||||
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString());
|
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString());
|
||||||
BOOST_CHECK(!mutated);
|
BOOST_CHECK(!mutated);
|
||||||
|
@ -181,17 +187,24 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
||||||
|
|
||||||
CBlock block2;
|
CBlock block2;
|
||||||
std::vector<CTransactionRef> vtx_missing;
|
{
|
||||||
BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_INVALID); // No transactions
|
PartiallyDownloadedBlock tmp = partialBlock;
|
||||||
|
BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_INVALID); // No transactions
|
||||||
|
partialBlock = tmp;
|
||||||
|
}
|
||||||
|
|
||||||
vtx_missing.push_back(block.vtx[1]); // Wrong transaction
|
// Wrong transaction
|
||||||
partialBlock.FillBlock(block2, vtx_missing); // Current implementation doesn't check txn here, but don't require that
|
{
|
||||||
|
PartiallyDownloadedBlock tmp = partialBlock;
|
||||||
|
partialBlock.FillBlock(block2, {block.vtx[1]}); // Current implementation doesn't check txn here, but don't require that
|
||||||
|
partialBlock = tmp;
|
||||||
|
}
|
||||||
bool mutated;
|
bool mutated;
|
||||||
BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated));
|
BOOST_CHECK(block.hashMerkleRoot != BlockMerkleRoot(block2, &mutated));
|
||||||
|
|
||||||
vtx_missing[0] = block.vtx[0];
|
|
||||||
CBlock block3;
|
CBlock block3;
|
||||||
BOOST_CHECK(partialBlock.FillBlock(block3, vtx_missing) == READ_STATUS_OK);
|
PartiallyDownloadedBlock partialBlockCopy = partialBlock;
|
||||||
|
BOOST_CHECK(partialBlock.FillBlock(block3, {block.vtx[0]}) == READ_STATUS_OK);
|
||||||
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString());
|
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block3.GetHash().ToString());
|
||||||
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString());
|
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block3, &mutated).ToString());
|
||||||
BOOST_CHECK(!mutated);
|
BOOST_CHECK(!mutated);
|
||||||
|
@ -200,7 +213,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
|
||||||
block.vtx.clear();
|
block.vtx.clear();
|
||||||
block2.vtx.clear();
|
block2.vtx.clear();
|
||||||
block3.vtx.clear();
|
block3.vtx.clear();
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); // + 1 because of partialBlockCopy.
|
||||||
}
|
}
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
|
||||||
}
|
}
|
||||||
|
@ -240,8 +253,8 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
||||||
|
|
||||||
CBlock block2;
|
CBlock block2;
|
||||||
std::vector<CTransactionRef> vtx_missing;
|
PartiallyDownloadedBlock partialBlockCopy = partialBlock;
|
||||||
BOOST_CHECK(partialBlock.FillBlock(block2, vtx_missing) == READ_STATUS_OK);
|
BOOST_CHECK(partialBlock.FillBlock(block2, {}) == READ_STATUS_OK);
|
||||||
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString());
|
BOOST_CHECK_EQUAL(block.GetHash().ToString(), block2.GetHash().ToString());
|
||||||
bool mutated;
|
bool mutated;
|
||||||
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString());
|
BOOST_CHECK_EQUAL(block.hashMerkleRoot.ToString(), BlockMerkleRoot(block2, &mutated).ToString());
|
||||||
|
@ -250,7 +263,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
|
||||||
txhash = block.vtx[1]->GetHash();
|
txhash = block.vtx[1]->GetHash();
|
||||||
block.vtx.clear();
|
block.vtx.clear();
|
||||||
block2.vtx.clear();
|
block2.vtx.clear();
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 1); // + 1 because of partialBlockCopy.
|
||||||
}
|
}
|
||||||
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
|
BOOST_CHECK_EQUAL(pool.mapTx.find(txhash)->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue