[tests] Test that mempool rejects coinbase transactions #11714

jamesob commented at 8:57 AM on November 18, 2017: member

selection_063

Neither the unit nor functional tests appear to cover rejecting a transaction from acceptance to the mempool on the basis of it being a coinbase. Seems like a decent thing to have a test for.

[tests] Test that mempool rejects coinbase transactions 65e91f5edf

meshcollider commented at 8:59 AM on November 18, 2017: contributor

Concept ACK

fanquake added the label Tests on Nov 18, 2017

laanwj commented at 11:18 AM on November 19, 2017: member

utACK, thanks for adding test coverage

in src/test/txvalidation_tests.cpp:57 in 65e91f5edf

  52 | +    // Check that the validation state reflects the unsuccesful attempt.
  53 | +    BOOST_CHECK(state.IsInvalid());
  54 | +    BOOST_CHECK_EQUAL(state.GetRejectReason(), "coinbase");
  55 | +
  56 | +    int nDoS;
  57 | +    BOOST_CHECK_EQUAL(state.IsInvalid(nDoS), true);

promag commented at 12:48 PM on November 19, 2017:

Call this above:

int nDoS;
BOOST_CHECK(state.IsInvalid(nDoS), true);
BOOST_CHECK_EQUAL(nDoS, 100);
BOOST_CHECK_EQUAL(state.GetRejectReason(), "coinbase");

promag commented at 8:29 PM on November 20, 2017:

@MarcoFalke any reason to not consider the suggestion?

jamesob commented at 9:26 PM on November 20, 2017:

@promag was happy to make the change but didn't have time yet, though it's worth noting that I'm uncertain on what the real difference would be aside from cosmetics. Note that state.IsInvalid with and without an argument call to separate methods, so why not get coverage on both?

MarcoFalke commented at 9:51 AM on November 21, 2017:

To me this seems mostly a cosmetic nit, subject to personal preference of style. For test related pulls I don't see a reason to block them solely based on style nits.

in src/test/txvalidation_tests.cpp:23 in 65e91f5edf

  18 | +/**
  19 | + * Ensure that the mempool won't accept coinbase transactions.
  20 | + */
  21 | +BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
  22 | +{
  23 | +    CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;

promag commented at 12:49 PM on November 19, 2017:

Nit, inline below?

in src/test/txvalidation_tests.cpp:24 in 65e91f5edf

  19 | + * Ensure that the mempool won't accept coinbase transactions.
  20 | + */
  21 | +BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
  22 | +{
  23 | +    CScript scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
  24 | +    CMutableTransaction coinbaseTx;

promag commented at 12:50 PM on November 19, 2017:

Nit, just tx.

promag commented at 12:51 PM on November 19, 2017: member

utACK 65e91f5.

MarcoFalke commented at 8:11 PM on November 20, 2017: member

utACK 65e91f5edf555691d0e5809d441cff7fa63ba722

MarcoFalke merged this on Nov 20, 2017

MarcoFalke closed this on Nov 20, 2017

MarcoFalke referenced this in commit 901ba3e381 on Nov 20, 2017

in src/test/txvalidation_tests.cpp:33 in 65e91f5edf

  28 | +    coinbaseTx.vout.resize(1);
  29 | +    coinbaseTx.vin[0].scriptSig = CScript() << OP_11 << OP_EQUAL;
  30 | +    coinbaseTx.vout[0].nValue = 1 * CENT;
  31 | +    coinbaseTx.vout[0].scriptPubKey = scriptPubKey;
  32 | +
  33 | +    assert(CTransaction(coinbaseTx).IsCoinBase());

MarcoFalke commented at 12:56 PM on November 21, 2017:

nit: In the future use BOOST_REQUIRE?

jasonbcox referenced this in commit c314de9ab5 on Jul 25, 2019

jonspock referenced this in commit 79129ab431 on Sep 5, 2019

jonspock referenced this in commit 21423e9b7a on Sep 6, 2019

proteanx referenced this in commit f7692f6a98 on Sep 6, 2019

PastaPastaPasta referenced this in commit 4cf3d07703 on Jan 17, 2020

PastaPastaPasta referenced this in commit d04b4da3f7 on Jan 17, 2020

PastaPastaPasta referenced this in commit 8029c0a2d4 on Feb 12, 2020

PastaPastaPasta referenced this in commit 19f9cec173 on Feb 12, 2020

PastaPastaPasta referenced this in commit 47215d6c96 on Feb 27, 2020

PastaPastaPasta referenced this in commit a254ed820e on Mar 14, 2020

PastaPastaPasta referenced this in commit 0f768f74d7 on Mar 14, 2020

PastaPastaPasta referenced this in commit 12b08dbbf0 on Mar 14, 2020

PastaPastaPasta referenced this in commit d8d18d6657 on Mar 14, 2020

ckti referenced this in commit 365a88457b on Mar 28, 2021

gades referenced this in commit 48debd428f on Jun 30, 2021

MarcoFalke locked this on Sep 8, 2021