From 226d81f8b708ea1c3a39ec58446e291fe7440fdd Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 21 Feb 2025 09:11:17 +0100 Subject: [PATCH] mining: drop unused -nFees and sigops from CBlockTemplate For the coinbase vTxFees used a dummy value of -nFees. This value was never returned by the RPC or used in a test. Similarly the fist vTxSigOpsCost entry was calculated from the dummy coinbase transaction. Drop both and add code comments to prevent confusion. --- src/interfaces/mining.h | 3 +++ src/node/miner.cpp | 7 ++----- src/node/miner.h | 2 ++ src/rpc/mining.cpp | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 435151c5dda..298a5c92dca 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -34,9 +34,12 @@ public: virtual ~BlockTemplate() = default; virtual CBlockHeader getBlockHeader() = 0; + // Block contains a dummy coinbase transaction that should not be used. virtual CBlock getBlock() = 0; + // Fees per transaction, not including coinbase transaction. virtual std::vector getTxFees() = 0; + // Sigop cost per transaction, not including coinbase transaction. virtual std::vector getTxSigops() = 0; virtual CTransactionRef getCoinbaseTx() = 0; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 33eeaf91fb2..c24debd1e69 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -124,10 +124,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblocktemplate.reset(new CBlockTemplate()); CBlock* const pblock = &pblocktemplate->block; // pointer for convenience - // Add dummy coinbase tx as first transaction + // Add dummy coinbase tx as first transaction. It is skipped by the + // getblocktemplate RPC and mining interface consumers must not use it. pblock->vtx.emplace_back(); - pblocktemplate->vTxFees.push_back(-1); // updated at end - pblocktemplate->vTxSigOpsCost.push_back(-1); // updated at end LOCK(::cs_main); CBlockIndex* pindexPrev = m_chainstate.m_chain.Tip(); @@ -165,7 +164,6 @@ std::unique_ptr BlockAssembler::CreateNewBlock() coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx)); pblocktemplate->vchCoinbaseCommitment = m_chainstate.m_chainman.GenerateCoinbaseCommitment(*pblock, pindexPrev); - pblocktemplate->vTxFees[0] = -nFees; LogPrintf("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d\n", GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost); @@ -174,7 +172,6 @@ std::unique_ptr BlockAssembler::CreateNewBlock() UpdateTime(pblock, chainparams.GetConsensus(), pindexPrev); pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus()); pblock->nNonce = 0; - pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]); BlockValidationState state; if (m_options.test_block_validity && !TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev, diff --git a/src/node/miner.h b/src/node/miner.h index 56101561b11..c09e9eb5d1c 100644 --- a/src/node/miner.h +++ b/src/node/miner.h @@ -37,7 +37,9 @@ static const bool DEFAULT_PRINT_MODIFIED_FEE = false; struct CBlockTemplate { CBlock block; + // Fees per transaction, not including coinbase transaction (unlike CBlock::vtx). std::vector vTxFees; + // Sigops per transaction, not including coinbase transaction (unlike CBlock::vtx). std::vector vTxSigOpsCost; std::vector vchCoinbaseCommitment; /* A vector of package fee rates, ordered by the sequence in which diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b527e686a46..060adee8c66 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -893,7 +893,7 @@ static RPCHelpMan getblocktemplate() } entry.pushKV("depends", std::move(deps)); - int index_in_template = i - 1; + int index_in_template = i - 2; entry.pushKV("fee", tx_fees.at(index_in_template)); int64_t nTxSigOps{tx_sigops.at(index_in_template)}; if (fPreSegWit) {