diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 06611a4a2f4..cc411730b10 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -243,11 +243,11 @@ std::unique_ptr BlockAssembler::CreateNewBlock() return std::move(pblocktemplate); } -bool BlockAssembler::TestChunkBlockLimits(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const +bool BlockAssembler::TestChunkBlockLimits(int64_t chunk_weight, int64_t chunk_sigops_cost) const { // block_max_weight has been flattened before block assembly limit checks. Assert(m_options.block_max_weight); - if (nBlockWeight + chunk_feerate.size >= *m_options.block_max_weight) { + if (nBlockWeight + chunk_weight >= m_options.block_max_weight) { return false; } if (nBlockSigOpsCost + chunk_sigops_cost >= MAX_BLOCK_SIGOPS_COST) { @@ -310,12 +310,14 @@ void BlockAssembler::addChunks() } int64_t chunk_sig_ops = 0; + int64_t chunk_weight = 0; for (const auto& tx : selected_transactions) { chunk_sig_ops += tx.get().GetSigOpCost(); + chunk_weight += tx.get().GetTxWeight(); } // Check to see if this chunk will fit. - if (!TestChunkBlockLimits(chunk_feerate, chunk_sig_ops) || !TestChunkTransactions(selected_transactions)) { + if (!TestChunkBlockLimits(chunk_weight, chunk_sig_ops) || !TestChunkTransactions(selected_transactions)) { // This chunk won't fit, so we skip it and will try the next best one. m_mempool->SkipBuilderChunk(); ++nConsecutiveFailed; diff --git a/src/node/miner.h b/src/node/miner.h index 7b5702ac594..a23f7cbeee0 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -108,7 +108,7 @@ private: // helper functions for addChunks() /** Test if a new chunk would "fit" in the block */ - bool TestChunkBlockLimits(FeePerWeight chunk_feerate, int64_t chunk_sigops_cost) const; + bool TestChunkBlockLimits(int64_t chunk_weight, int64_t chunk_sigops_cost) const; /** Perform locktime checks on each transaction in a chunk: * This check should always succeed, and is here * only as an extra check in case of a bug */ diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 90df73b4567..f4cbde4d43a 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -61,6 +61,7 @@ struct MinerTestingSetup : public TestingSetup { void TestPackageSelection(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); void TestBasicMining(const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); void TestPrioritisedMining(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void TestSigOpsAdjustedWeightChunkLimit(const CScript& scriptPubKey, const std::vector& txFirst) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool TestSequenceLocks(const CTransaction& tx, CTxMemPool& tx_mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { CCoinsViewMemPool view_mempool{&m_node.chainman->ActiveChainstate().CoinsTip(), tx_mempool}; @@ -320,6 +321,22 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const BOOST_CHECK(block.vtx[8]->GetHash() == hashLowFeeTx2); } +// One sigop-dense tx spending `input`: num_outputs bare CHECKMULTISIG outputs, +// i.e. 20 * num_outputs legacy sigops (OP_NOP forces the inaccurate max-20 count). +static CMutableTransaction CreateBigSigOpsTx(const COutPoint& input, unsigned int num_outputs) +{ + CMutableTransaction tx; + tx.vin.resize(1); + tx.vin[0].prevout = input; + tx.vin[0].scriptSig = CScript() << OP_1; + tx.vout.resize(num_outputs); + for (auto& out : tx.vout) { + out.nValue = 0; + out.scriptPubKey = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; + } + return tx; +} + std::vector CreateBigSigOpsCluster(const CTransactionRef& first_tx) { std::vector ret; @@ -346,22 +363,36 @@ std::vector CreateBigSigOpsCluster(const CTransactionRef& first // Tx2-51 has 400 sigops: 1 input, 20 CHECKMULTISIG outputs // Total: 1000 CHECKMULTISIG + 1 for (unsigned int i = 0; i < 50; ++i) { - auto tx2 = tx; - tx2.vin.resize(1); - tx2.vin[0].prevout.hash = parent_tx->GetHash(); - tx2.vin[0].prevout.n = i; - tx2.vin[0].scriptSig = CScript() << OP_1; - tx2.vout.resize(20); - tx2.vout[0].nValue = parent_tx->vout[i].nValue - CENT; - for (auto &out : tx2.vout) { - out.nValue = 0; - out.scriptPubKey = CScript() << OP_0 << OP_0 << OP_0 << OP_NOP << OP_CHECKMULTISIG << OP_1; - } - ret.push_back(MakeTransactionRef(tx2)); + ret.push_back(MakeTransactionRef(CreateBigSigOpsTx(COutPoint{parent_tx->GetHash(), i}, /*num_outputs=*/20))); } return ret; } +void MinerTestingSetup::TestSigOpsAdjustedWeightChunkLimit(const CScript& scriptPubKey, const std::vector& txFirst) +{ + auto mining{MakeMining()}; + BOOST_REQUIRE(mining); + + CTxMemPool& tx_mempool{MakeMempool()}; + LOCK(tx_mempool.cs); + TestMemPoolEntryHelper entry; + + const auto tx{CreateBigSigOpsTx(COutPoint{txFirst[0]->GetHash(), 0}, /*num_outputs=*/50)}; + const auto sigop_entry{entry.Fee(COIN).SpendsCoinbase(true).SigOpsCost(GetLegacySigOpCount(CTransaction(tx)) * WITNESS_SCALE_FACTOR).FromTx(tx)}; + BOOST_REQUIRE(sigop_entry.GetAdjustedWeight() > sigop_entry.GetTxWeight()); + BOOST_REQUIRE(sigop_entry.GetSigOpCost() < MAX_BLOCK_SIGOPS_COST); + TryAddToMempool(tx_mempool, sigop_entry); + + BlockCreateOptions options{ + // +1 because TestChunkBlockLimits rejects on >= (exact fit doesn't count). + .block_max_weight = DEFAULT_BLOCK_RESERVED_WEIGHT + sigop_entry.GetTxWeight() + 1, + .coinbase_output_script = scriptPubKey, + }; + const CBlock block{mining->createNewBlock(options, /*cooldown=*/false)->getBlock()}; + BOOST_CHECK_EQUAL(block.vtx.size(), 2U); + BOOST_CHECK(block.vtx[1]->GetHash() == tx.GetHash()); +} + void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std::vector& txFirst, int baseheight) { FakeNodeClock clock{}; @@ -933,6 +964,8 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) m_node.chainman->ActiveChain().Tip()->nHeight--; TestPrioritisedMining(scriptPubKey, txFirst); + + TestSigOpsAdjustedWeightChunkLimit(scriptPubKey, txFirst); } BOOST_AUTO_TEST_SUITE_END()