Merge #19069: refactor: replace pointers by references within tx_verify.{h,cpp}
b00266fe0c
refactor: replace pointers by references within tx_verify.{h,cpp} (Sebastian Falbesoner) Pull request description: This PR gets rid of another unnecessary use of raw pointers, similar to PR #19053 (see also issue #19062 where useful commands for finding potential candidates are listed) but in the tx verification module. For the functions `CalculateSequenceLocks()` and `SequenceLocks()`, the `prevHeights` vector parameter type is changed to be passed as a reference. Note that there were no checks for null pointers -- if one would pass `nullptr` to one of the functions, the following line would immediately lead to a crash:dcacea096e/src/consensus/tx_verify.cpp (L32)
ACKs for top commit: Empact: Code Review ACKb00266fe0c
Tree-SHA512: 0eb71591467905434082029128bdca4df94988c372af40dca325654f6c002c72a00c73776cb5e72d6de2b2f218649211a5dbf19300a2e01f1841d6034e0f01e0
This commit is contained in:
commit
a79bca2f1f
4 changed files with 11 additions and 11 deletions
|
@ -27,9 +27,9 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
|
std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block)
|
||||||
{
|
{
|
||||||
assert(prevHeights->size() == tx.vin.size());
|
assert(prevHeights.size() == tx.vin.size());
|
||||||
|
|
||||||
// Will be set to the equivalent height- and time-based nLockTime
|
// Will be set to the equivalent height- and time-based nLockTime
|
||||||
// values that would be necessary to satisfy all relative lock-
|
// values that would be necessary to satisfy all relative lock-
|
||||||
|
@ -59,11 +59,11 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
|
||||||
// consensus-enforced meaning at this point.
|
// consensus-enforced meaning at this point.
|
||||||
if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLE_FLAG) {
|
if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_DISABLE_FLAG) {
|
||||||
// The height of this input is not relevant for sequence locks
|
// The height of this input is not relevant for sequence locks
|
||||||
(*prevHeights)[txinIndex] = 0;
|
prevHeights[txinIndex] = 0;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
int nCoinHeight = (*prevHeights)[txinIndex];
|
int nCoinHeight = prevHeights[txinIndex];
|
||||||
|
|
||||||
if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) {
|
if (txin.nSequence & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) {
|
||||||
int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight-1, 0))->GetMedianTimePast();
|
int64_t nCoinTime = block.GetAncestor(std::max(nCoinHeight-1, 0))->GetMedianTimePast();
|
||||||
|
@ -99,7 +99,7 @@ bool EvaluateSequenceLocks(const CBlockIndex& block, std::pair<int, int64_t> loc
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block)
|
bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block)
|
||||||
{
|
{
|
||||||
return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block));
|
return EvaluateSequenceLocks(block, CalculateSequenceLocks(tx, flags, prevHeights, block));
|
||||||
}
|
}
|
||||||
|
|
|
@ -66,13 +66,13 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime);
|
||||||
* Also removes from the vector of input heights any entries which did not
|
* Also removes from the vector of input heights any entries which did not
|
||||||
* correspond to sequence locked inputs as they do not affect the calculation.
|
* correspond to sequence locked inputs as they do not affect the calculation.
|
||||||
*/
|
*/
|
||||||
std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
|
std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block);
|
||||||
|
|
||||||
bool EvaluateSequenceLocks(const CBlockIndex& block, std::pair<int, int64_t> lockPair);
|
bool EvaluateSequenceLocks(const CBlockIndex& block, std::pair<int, int64_t> lockPair);
|
||||||
/**
|
/**
|
||||||
* Check if transaction is final per BIP 68 sequence numbers and can be included in a block.
|
* Check if transaction is final per BIP 68 sequence numbers and can be included in a block.
|
||||||
* Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed.
|
* Consensus critical. Takes as input a list of heights at which tx's inputs (in order) confirmed.
|
||||||
*/
|
*/
|
||||||
bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>* prevHeights, const CBlockIndex& block);
|
bool SequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block);
|
||||||
|
|
||||||
#endif // BITCOIN_CONSENSUS_TX_VERIFY_H
|
#endif // BITCOIN_CONSENSUS_TX_VERIFY_H
|
||||||
|
|
|
@ -448,7 +448,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
|
||||||
m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
|
m_node.mempool->addUnchecked(entry.Fee(HIGHFEE).Time(GetTime()).SpendsCoinbase(true).FromTx(tx));
|
||||||
BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes
|
BOOST_CHECK(CheckFinalTx(CTransaction(tx), flags)); // Locktime passes
|
||||||
BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail
|
BOOST_CHECK(!TestSequenceLocks(CTransaction(tx), flags)); // Sequence locks fail
|
||||||
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, &prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 2))); // Sequence locks pass on 2nd block
|
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 2))); // Sequence locks pass on 2nd block
|
||||||
|
|
||||||
// relative time locked
|
// relative time locked
|
||||||
tx.vin[0].prevout.hash = txFirst[1]->GetHash();
|
tx.vin[0].prevout.hash = txFirst[1]->GetHash();
|
||||||
|
@ -461,7 +461,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
|
||||||
|
|
||||||
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
|
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
|
||||||
::ChainActive().Tip()->GetAncestor(::ChainActive().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
|
::ChainActive().Tip()->GetAncestor(::ChainActive().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
|
||||||
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, &prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 1))); // Sequence locks pass 512 seconds later
|
BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(::ChainActive().Tip()->nHeight + 1))); // Sequence locks pass 512 seconds later
|
||||||
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
|
for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
|
||||||
::ChainActive().Tip()->GetAncestor(::ChainActive().Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP
|
::ChainActive().Tip()->GetAncestor(::ChainActive().Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP
|
||||||
|
|
||||||
|
|
|
@ -302,7 +302,7 @@ bool CheckSequenceLocks(const CTxMemPool& pool, const CTransaction& tx, int flag
|
||||||
prevheights[txinIndex] = coin.nHeight;
|
prevheights[txinIndex] = coin.nHeight;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
lockPair = CalculateSequenceLocks(tx, flags, &prevheights, index);
|
lockPair = CalculateSequenceLocks(tx, flags, prevheights, index);
|
||||||
if (lp) {
|
if (lp) {
|
||||||
lp->height = lockPair.first;
|
lp->height = lockPair.first;
|
||||||
lp->time = lockPair.second;
|
lp->time = lockPair.second;
|
||||||
|
@ -2172,7 +2172,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
|
||||||
prevheights[j] = view.AccessCoin(tx.vin[j].prevout).nHeight;
|
prevheights[j] = view.AccessCoin(tx.vin[j].prevout).nHeight;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!SequenceLocks(tx, nLockTimeFlags, &prevheights, *pindex)) {
|
if (!SequenceLocks(tx, nLockTimeFlags, prevheights, *pindex)) {
|
||||||
LogPrintf("ERROR: %s: contains a non-BIP68-final transaction\n", __func__);
|
LogPrintf("ERROR: %s: contains a non-BIP68-final transaction\n", __func__);
|
||||||
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-nonfinal");
|
return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-nonfinal");
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue