Compare commits

...

4 commits

Author SHA1 Message Date
Hennadii Stepanov
d6b6ae10be
Merge 1e7cdb04fd into 66aa6a47bd 2025-01-08 20:44:38 +01:00
glozow
66aa6a47bd
Merge bitcoin/bitcoin#30391: BlockAssembler: return selected packages virtual size and fee
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
7c123c08dd  miner: add package feerate vector to CBlockTemplate (ismaelsadeeq)

Pull request description:

  This PR enables `BlockAssembler` to add all selected packages' fee and virtual size to a vector, and then return the vector as a member of `CBlockTemplate` struct.

  This PR is the first step in the https://github.com/bitcoin/bitcoin/issues/30392 project.

  The packages' vsize and fee are used in #30157 to select a percentile fee rate of the top block in the mempool.

ACKs for top commit:
  rkrux:
    tACK 7c123c08dd
  ryanofsky:
    Code review ACK 7c123c08dd. Changes since last review are rebasing due to a test conflict, giving the new field a better name and description, resolving the test conflict, and renaming a lot of test variables. The actual code change is still one-line change.
  glozow:
    reACK 7c123c08dd

Tree-SHA512: 767b0b3d4273cf1589fd2068d729a66c7414c0f9574b15989fbe293f8c85cd6c641dd783cde55bfabab32cd047d7d8a071d6897b06ed4295c0d071e588de0861
2025-01-08 13:01:23 -05:00
ismaelsadeeq
7c123c08dd
miner: add package feerate vector to CBlockTemplate
- The package feerates are ordered by the sequence in which
  packages are selected for inclusion in the block template.

- The commit also tests this new behaviour.

Co-authored-by: willcl-ark <will@256k1.dev>
2025-01-07 15:29:17 -05:00
Hennadii Stepanov
1e7cdb04fd
depends: Avoid using helper variables in toolchain file
Using helper variables has two issues:
1. They contaminate the global namespace of the main build script.
2. They can be used as `set(var)`, effectively exposing a cache variable
`var`, which makes the toolchain file susceptible to the build
environment.
2024-11-24 13:48:12 +00:00
4 changed files with 38 additions and 17 deletions

View file

@ -97,50 +97,46 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND NOT CMAKE_HOST_APPLE)
endif()
# Set configuration options for the main build system.
set(qt_packages @qt_packages@)
if("${qt_packages}" STREQUAL "")
# The depends/Makefile can generate values with "not-set"
# semantics as empty strings or strings containing only spaces.
# Therefore, MATCHES must be used rather than STREQUAL.
if("@qt_packages@" MATCHES "^[ ]*$")
set(BUILD_GUI OFF CACHE BOOL "")
else()
set(BUILD_GUI ON CACHE BOOL "")
endif()
set(qrencode_packages @qrencode_packages@)
if("${qrencode_packages}" STREQUAL "")
if("@qrencode_packages@" MATCHES "^[ ]*$")
set(WITH_QRENCODE OFF CACHE BOOL "")
else()
set(WITH_QRENCODE ON CACHE BOOL "")
endif()
set(zmq_packages @zmq_packages@)
if("${zmq_packages}" STREQUAL "")
if("@zmq_packages@" MATCHES "^[ ]*$")
set(WITH_ZMQ OFF CACHE BOOL "")
else()
set(WITH_ZMQ ON CACHE BOOL "")
endif()
set(wallet_packages @wallet_packages@)
if("${wallet_packages}" STREQUAL "")
if("@wallet_packages@" MATCHES "^[ ]*$")
set(ENABLE_WALLET OFF CACHE BOOL "")
else()
set(ENABLE_WALLET ON CACHE BOOL "")
endif()
set(bdb_packages @bdb_packages@)
if("${wallet_packages}" STREQUAL "" OR "${bdb_packages}" STREQUAL "")
if("@wallet_packages@" MATCHES "^[ ]*$" OR "@bdb_packages@" MATCHES "^[ ]*$")
set(WITH_BDB OFF CACHE BOOL "")
else()
set(WITH_BDB ON CACHE BOOL "")
endif()
set(sqlite_packages @sqlite_packages@)
if("${wallet_packages}" STREQUAL "" OR "${sqlite_packages}" STREQUAL "")
if("@wallet_packages@" MATCHES "^[ ]*$" OR "@sqlite_packages@" MATCHES "^[ ]*$")
set(WITH_SQLITE OFF CACHE BOOL "")
else()
set(WITH_SQLITE ON CACHE BOOL "")
endif()
set(usdt_packages @usdt_packages@)
if("${usdt_packages}" STREQUAL "")
if("@usdt_packages@" MATCHES "^[ ]*$")
set(WITH_USDT OFF CACHE BOOL "")
else()
set(WITH_USDT ON CACHE BOOL "")

View file

@ -421,6 +421,7 @@ void BlockAssembler::addPackageTxs(int& nPackagesSelected, int& nDescendantsUpda
}
++nPackagesSelected;
pblocktemplate->m_package_feerates.emplace_back(packageFees, static_cast<int32_t>(packageSize));
// Update transactions that depend on each of these
nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);

View file

@ -10,6 +10,7 @@
#include <policy/policy.h>
#include <primitives/block.h>
#include <txmempool.h>
#include <util/feefrac.h>
#include <memory>
#include <optional>
@ -39,6 +40,9 @@ struct CBlockTemplate
std::vector<CAmount> vTxFees;
std::vector<int64_t> vTxSigOpsCost;
std::vector<unsigned char> vchCoinbaseCommitment;
/* A vector of package fee rates, ordered by the sequence in which
* packages are selected for inclusion in the block template.*/
std::vector<FeeFrac> m_package_feerates;
};
// Container for tracking updates to ancestor feerate as we include (parent)

View file

@ -16,6 +16,7 @@
#include <txmempool.h>
#include <uint256.h>
#include <util/check.h>
#include <util/feefrac.h>
#include <util/strencodings.h>
#include <util/time.h>
#include <util/translation.h>
@ -25,6 +26,7 @@
#include <test/util/setup_common.h>
#include <memory>
#include <vector>
#include <boost/test/unit_test.hpp>
@ -123,19 +125,22 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
tx.vout[0].nValue = 5000000000LL - 1000;
// This tx has a low fee: 1000 satoshis
Txid hashParentTx = tx.GetHash(); // save this txid for later use
AddToMempool(tx_mempool, entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
const auto parent_tx{entry.Fee(1000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
AddToMempool(tx_mempool, parent_tx);
// This tx has a medium fee: 10000 satoshis
tx.vin[0].prevout.hash = txFirst[1]->GetHash();
tx.vout[0].nValue = 5000000000LL - 10000;
Txid hashMediumFeeTx = tx.GetHash();
AddToMempool(tx_mempool, entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx));
const auto medium_fee_tx{entry.Fee(10000).Time(Now<NodeSeconds>()).SpendsCoinbase(true).FromTx(tx)};
AddToMempool(tx_mempool, medium_fee_tx);
// This tx has a high fee, but depends on the first transaction
tx.vin[0].prevout.hash = hashParentTx;
tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 50k satoshi fee
Txid hashHighFeeTx = tx.GetHash();
AddToMempool(tx_mempool, entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx));
const auto high_fee_tx{entry.Fee(50000).Time(Now<NodeSeconds>()).SpendsCoinbase(false).FromTx(tx)};
AddToMempool(tx_mempool, high_fee_tx);
std::unique_ptr<BlockTemplate> block_template = mining->createNewBlock(options);
BOOST_REQUIRE(block_template);
@ -145,6 +150,21 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const
BOOST_CHECK(block.vtx[2]->GetHash() == hashHighFeeTx);
BOOST_CHECK(block.vtx[3]->GetHash() == hashMediumFeeTx);
// Test the inclusion of package feerates in the block template and ensure they are sequential.
const auto block_package_feerates = BlockAssembler{m_node.chainman->ActiveChainstate(), &tx_mempool, options}.CreateNewBlock()->m_package_feerates;
BOOST_CHECK(block_package_feerates.size() == 2);
// parent_tx and high_fee_tx are added to the block as a package.
const auto combined_txs_fee = parent_tx.GetFee() + high_fee_tx.GetFee();
const auto combined_txs_size = parent_tx.GetTxSize() + high_fee_tx.GetTxSize();
FeeFrac package_feefrac{combined_txs_fee, combined_txs_size};
// The package should be added first.
BOOST_CHECK(block_package_feerates[0] == package_feefrac);
// The medium_fee_tx should be added next.
FeeFrac medium_tx_feefrac{medium_fee_tx.GetFee(), medium_fee_tx.GetTxSize()};
BOOST_CHECK(block_package_feerates[1] == medium_tx_feefrac);
// Test that a package below the block min tx fee doesn't get included
tx.vin[0].prevout.hash = hashHighFeeTx;
tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee