highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-12 05:32:22 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#36130: test: add tests in transaction_tests.cpp covering live mutants

Browse Source 
5ce3a0b4aa test: cover legacy sigops count CHECKMULTISIG inaccurately (ViniciusCestarii)
a5fc82e2b1 test: cover enforce BIP68 to tx versions higher than 2 (ViniciusCestarii)
bba1d4150e test: cover IsFinalTx requires every input to be SEQUENCE_FINAL (ViniciusCestarii)

Pull request description:

  Kills some live mutants on tx_verify.cpp that affect consensus found with https://github.com/ViniciusCestarii/mutant-harness. They are:

  <details>
  <summary>tx_verify.cpp (killed by 5c35785d6ddda80d5147616342e42d759490e6b9): <code>IsFinalTx</code>: sequence loop returns on the first input instead of requiring all of them</summary>

  ```diff
  diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
  index e580a9d..46009a6 100644
  --- a/src/consensus/tx_verify.cpp
  +++ b/src/consensus/tx_verify.cpp
  @@ -35,11 +35,7 @@ bool IsFinalTx(const CTransaction &tx, int nBlockHeight, int64_t nBlockTime)
       // also check that the spending input's nSequence != SEQUENCE_FINAL,
       // ensuring that an unsatisfied nLockTime value will actually cause
       // IsFinalTx() to return false here:
  -    for (const auto& txin : tx.vin) {
  -        if (!(txin.nSequence == CTxIn::SEQUENCE_FINAL))
  -            return false;
  -    }
  -    return true;
  +    return std::ranges::any_of(tx.vin, [](const CTxIn& txin) { return txin.nSequence == CTxIn::SEQUENCE_FINAL; });
   }

   std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags, std::vector<int>& prevHeights, const CBlockIndex& block)
  ```

  </details>

  <details>
  <summary>tx_verify.cpp (killed by 3ef559d9a5cb79e4721b68427ad679d9f4f6392a): <code>CalculateSequenceLocks</code>: <code>tx.version >= 2</code> -> <code>tx.version == 2</code></summary>

  ```diff
  diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
  index e580a9d..0faaa55 100644
  --- a/src/consensus/tx_verify.cpp
  +++ b/src/consensus/tx_verify.cpp
  @@ -54,7 +54,7 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
       int nMinHeight = -1;
       int64_t nMinTime = -1;

  -    bool fEnforceBIP68 = tx.version >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;
  +    bool fEnforceBIP68 = tx.version == 2 && flags & LOCKTIME_VERIFY_SEQUENCE;

       // Do not enforce sequence numbers as a relative lock time
       // unless we have been instructed to
  ```

  </details>

  <details>
  <summary>tx_verify.cpp (killed by 1944eb409055d88eeaf7888b18a75289c506a943): <code>GetLegacySigOpCount</code>: <code>scriptSig.GetSigOpCount(false)</code> -> <code>GetSigOpCount(true)</code></summary>

  ```diff
  diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp
  index e580a9d..0b98597 100644
  --- a/src/consensus/tx_verify.cpp
  +++ b/src/consensus/tx_verify.cpp
  @@ -120,7 +120,7 @@ unsigned int GetLegacySigOpCount(const CTransaction& tx)
       unsigned int nSigOps = 0;
       for (const auto& txin : tx.vin)
       {
  -        nSigOps += txin.scriptSig.GetSigOpCount(false);
  +        nSigOps += txin.scriptSig.GetSigOpCount(true);
       }
       for (const auto& txout : tx.vout)
       {
  ```

  </details>

  Recommend reviewing per commit.

ACKs for top commit:
  jeanpablojp:
    tACK 5ce3a0b4aa
  instagibbs:
    ACK 5ce3a0b4aa
  brunoerg:
    ACK 5ce3a0b4aa
  sedited:
    ACK 5ce3a0b4aa

Tree-SHA512: 1f5c941638fc2907759e5a8d6faf0669b7b7d03d833b51ad675bed10585dd6b232999aa2a5ecb9e0b58db81b1ec44c9916c680e872608e4fe5ee50e71f6b82b4
This commit is contained in:
merge-script
2026-09-05 14:27:10 +02:00
parent 0b43dea121 5ce3a0b4aa
commit 0f206eed51
1 changed files with 64 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
src/test/transaction_tests.cpp
View File

@@ -6,6 +6,7 @@
#include <test/data/tx_valid.json.h>
#include <test/util/setup_common.h>
#include <chain.h>
#include <checkqueue.h>
#include <clientversion.h>
#include <consensus/amount.h>
@@ -1128,6 +1129,22 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops)
}
}
BOOST_AUTO_TEST_CASE(getlegacysigopcount_inaccurate_test)
{
// Legacy sigops are counted inaccurately in both the scriptSig and the
// scriptPubKey: a CHECKMULTISIG counts as MAX_PUBKEYS_PER_MULTISIG even when the
// preceding OP_N says it takes fewer keys. Counting it accurately would
// undercount, letting a block over the sigop limit through.
const CScript multisig{CScript() << OP_1 << OP_CHECKMULTISIG};
CMutableTransaction mtx;
mtx.vin.emplace_back(COutPoint{}, multisig);
BOOST_CHECK_EQUAL(GetLegacySigOpCount(CTransaction{mtx}), MAX_PUBKEYS_PER_MULTISIG);
mtx.vout.emplace_back(0, multisig);
BOOST_CHECK_EQUAL(GetLegacySigOpCount(CTransaction{mtx}), 2 * MAX_PUBKEYS_PER_MULTISIG);
}
BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test)
{
auto check_invalid{[](CAmount input_value, CAmount output_value, bool coinbase, int spend_height, TxValidationResult expected_result, std::string_view expected_reason) {
@@ -1167,6 +1184,53 @@ BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test)
TxValidationResult::TX_PREMATURE_SPEND, /*expected_reason=*/"bad-txns-premature-spend-of-coinbase");
}
BOOST_AUTO_TEST_CASE(isfinaltx_sequences_test)
{
constexpr int height{100};
// Every transaction here has the same unsatisfied nLockTime, so only the
// sequences decide the outcome.
auto check_final{[](const std::vector<uint32_t>& sequences, bool expected_final) {
CMutableTransaction mtx;
mtx.nLockTime = height;
for (const uint32_t sequence : sequences) {
mtx.vin.emplace_back(COutPoint{}, CScript{}, sequence);
}
BOOST_CHECK_EQUAL(IsFinalTx(CTransaction{mtx}, /*nBlockHeight=*/height, /*nBlockTime=*/0), expected_final);
}};
check_final(/*sequences=*/{CTxIn::SEQUENCE_FINAL, CTxIn::SEQUENCE_FINAL}, /*expected_final=*/true);
// nLockTime is only ignored when every input is SEQUENCE_FINAL
check_final(/*sequences=*/{CTxIn::SEQUENCE_FINAL, CTxIn::MAX_SEQUENCE_NONFINAL}, /*expected_final=*/false);
check_final(/*sequences=*/{CTxIn::MAX_SEQUENCE_NONFINAL, CTxIn::SEQUENCE_FINAL}, /*expected_final=*/false);
}
BOOST_AUTO_TEST_CASE(calculatesequencelocks_tx_version_test)
{
constexpr int coin_height{100};
// A single input with a height-based relative locktime of one block. Only the
// height branch is taken, so the block index is never dereferenced.
auto check_min_height{[](uint32_t version, int expected_min_height) {
CMutableTransaction mtx;
mtx.version = version;
mtx.vin.emplace_back(COutPoint{}, CScript{}, /*nSequenceIn=*/1);
std::vector<int> prev_heights{coin_height};
const CBlockIndex block{};
const auto lock_pair{CalculateSequenceLocks(CTransaction{mtx}, LOCKTIME_VERIFY_SEQUENCE, prev_heights, block)};
BOOST_CHECK_EQUAL(lock_pair.first, expected_min_height);
}};
// BIP68 only applies to versions 2 and up
check_min_height(/*version=*/0, /*expected_min_height=*/-1);
check_min_height(/*version=*/1, /*expected_min_height=*/-1);
check_min_height(/*version=*/2, /*expected_min_height=*/coin_height);
check_min_height(/*version=*/std::numeric_limits<uint32_t>::max(), /*expected_min_height=*/coin_height);
}
BOOST_AUTO_TEST_CASE(getvalueout_out_of_range_throws)
{
CMutableTransaction mtx;