The mempool clusters and linearization permit sorting the mempool topologically
without making use of ancestor counts (as long as the graph is not oversized).
Co-authored-by: Pieter Wuille <pieter@wuille.net>
Previously, transaction batches were first sorted by ancestor count and then
feerate, to ensure transactions are announced in a topologically valid order,
while prioritizing higher feerate transactions. Ancestor count is a crude
topological sort criteria, so replace this with linearization order so that the
highest feerate transactions (as would be observed by the mining algorithm) are
relayed before lower feerate ones, in a topologically valid way.
This also fixes a test that only worked due to the ancestor-count-based sort
order.
With a total ordering on mempool transactions, we are now able to calculate a
transaction's mining score at all times. Use this to improve the RBF logic:
- we no longer enforce a "no new unconfirmed parents" rule
- we now require that the mempool's feerate diagram must improve in order
to accept a replacement
- the topology restrictions for conflicts in the package rbf setting have been
eliminated
Revert the temporary change to mempool_ephemeral_dust.py that were previously
made due to RBF validation checks being reordered.
Co-authored-by: Gregory Sanders <gsanders87@gmail.com>, glozow <gloriajzhao@gmail.com>
The addition of a cluster size limit makes the CPFP carveout rule useless,
because carveout cannot be used to bypass the cluster size limit. Remove this
policy rule and update tests to no longer rely on the behavior.
After cluster mempool, the mini_miner will no longer match the miner's block
construction. Eventually mini_miner should be reworked to directly use
linearizations done in the mempool.
Rather than evicting the transactions with the lowest descendant feerate,
instead evict transactions that have the lowest chunk feerate.
Once mining is implemented based on choosing transactions with highest chunk
feerate (see next commit), mining and eviction will be opposites, so that we
will evict the transactions that would be mined last.
Include an adjustment to mempool_tests.cpp due to the additional memory used by
txgraph.
Includes a temporary change to the mempool_ephemeral_dust.py functional test,
due to validation checks being reordered. This change will revert once the RBF
rules are changed in a later commit.
7b544341c0 test: change log rate limit version gate from 299900 to 290100 (Eugene Siegel)
Pull request description:
Change the version gate from 299900 to 290100 for bypassing the log rate limit in case an explicit version is set in the functional test framework.
See discussion here: https://github.com/bitcoin/bitcoin/pull/33225#discussion_r2287838255
ACKs for top commit:
maflcko:
lgtm ACK 7b544341c0
janb84:
ACK 7b544341c0
stickies-v:
ACK 7b544341c0
Tree-SHA512: c07c8741dfdeca87c49748b7082c2ecb829da391908316f35daef7292bc017814a89f04e16e738f3a105541bbc38e4feb5bca3fb6ab718a1dc1de7c70a9c8a58
023cd5a546 txgraph: add SingletonClusterImpl (mem optimization) (Pieter Wuille)
e346250732 txgraph: give Clusters a range of intended tx counts (preparation) (Pieter Wuille)
e93b0f09cc txgraph: abstract out creation of empty Clusters (refactor) (Pieter Wuille)
6baf12621f txgraph: comment fixes (doc fix) (Pieter Wuille)
726b995739 txgraph: make Cluster an abstract class (refactor) (Pieter Wuille)
2602d89edd txgraph: avoid accessing other Cluster internals (refactor) (Pieter Wuille)
04c808ac4c txgraph: expose memory usage estimate function (feature) (Pieter Wuille)
7680bb8fd4 txgraph: keep track of Cluster memory usage (preparation) (Pieter Wuille)
4ba562e5f4 txgraph: keep data structures compact (mem optimization) (Pieter Wuille)
bb5cb222ae depgraph: add memory usage control (feature) (Pieter Wuille)
b1637a90de txgraph: avoid holes in DepGraph positions (mem optimization) (Pieter Wuille)
2b1d302508 txgraph: move some sanity checks from Cluster to TxGraphImpl (refactor) (Pieter Wuille)
d40302fbaf txgraph: Make level of Cluster implicit (optimization) (Pieter Wuille)
Pull request description:
Part of #30289.
This adds a few optimizations to reduce `TxGraph`'s memory usage, and makes sure that dynamic memory it uses doesn't linger after shrinking clusters. Finally, it exposes a function `GetMainMemoryUsage()` to compute `TxGraph`'s approximate memory usage.
It makes the `Cluster` type abstract, with two instances (`SingletonClusterImpl` for 1-transaction clusters, and `GenericClusterImpl` for others).
On my 64-bit system, I obtain the following numbers:
* `SingletonClusterImpl`: 48 bytes, plus 16 bytes malloc overhead in its `unique_ptr`, plus 8-byte pointer in `m_clusters`
* `GenericClusterImpl`: 104 bytes, plus 16 bytes malloc overhead in its `unique_ptr`, plus 8-byte pointer in `m_clusters`, plus 72 bytes malloc overhead inside its vectors and `DepGraph`, plus 40 bytes per transaction in those.
* `TxGraphImpl::Entry`: 72 bytes per transaction
* `TxGraphImpl::ChunkData`: 8 bytes, plus 56 bytes in `std::set` overhead + malloc overhead, all per chunk.
* `TxGraph::Ref`: 16 bytes per transaction
This overall amounts to 200 bytes per cluster, plus 64 bytes per chunk, plus 128 bytes per transaction, but only 224 bytes overall per singleton cluster.
ACKs for top commit:
l0rinc:
code review reACK 023cd5a546
instagibbs:
reACK 023cd5a546
ismaelsadeeq:
reACK 023cd5a546🚢
glozow:
reACK 023cd5a546
Tree-SHA512: c957b27f47318be7c25d71453df2ae9d4e7bf21dab13b6e5e975cca122a221a99b15c584872491225785d276a9165f090675ee0f4460a2775bd3271933e3b246
bc706955d7 ci: expose all ACTIONS_* vars (willcl-ark)
Pull request description:
When using `docker buildx build` in conjunction with the `gha` backend cache type (as we do in our CI) it's important to specify the URL and TOKEN needed to authenticate.
On Cirrus runners this is working with only `ACTIONS_CACHE_URL` and `ACTIONS_RUNTIME_TOKEN`, but this is not enough for the GitHub backend.
Fix this by exporting all `ACTIONS_*` variables.
This fixes docker build layer cache restore/save on forks or where GH-hosted runners are being used, and addresses https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3324707093
ACKs for top commit:
m3dwards:
ACK bc706955d7
maflcko:
lgtm ACK bc706955d7
Tree-SHA512: 13e973bb1c1ca5448dd6c3c176fb5ce39c725886ba2012d3253158205309a7038a1430135b37400e1f2f69408a9d0f4e2b3c5f0515154a593ec382ab7db10266
fa6fd16f36 ci: Use native platform for win-cross task (MarcoFalke)
Pull request description:
Forcing the architecture to amd64 is no longer required. Dropping it should have some benefits:
* Faster CI speed on other arches (riscv64, arm, ...)
* Unlock the CI task to run on riscv64 at all
ACKs for top commit:
hebasto:
ACK fa6fd16f36, tested on Ubuntu 24.04, RISC-V.
Tree-SHA512: 68a3fc90cc22ab085d6946deb106e50b22e06eebc61523a9dcb53b38a50021a19da26cc29e2cd20f4673ffc5cc10f441dacca7cc799782258351609d9fa04969
671b774d1b depends: Use $(package)_file_name when downloading from the fallback (Ava Chow)
Pull request description:
The server hosting the fallbacks uses `make download` so the files are only available with their overridden names rather than the original name on the upstream source. We should therefore also use the overridden name when downloading from the fallback.
Fixes https://github.com/bitcoin-core/bitcoincore.org/issues/1168
ACKs for top commit:
theuni:
utACK 671b774d1b. I was going to PR the same change.
janb84:
ut ACK 671b774d1b
hebasto:
ACK 671b774d1b, tested with the following patch:
Tree-SHA512: ba010adb64900d8d748487cc1a658e2b163872354f4e7b38c4dfc37a14fcb22fec4379a635d2c6788c64dd46bef0d94aa3eb6f522ec700680e886d5468678031
This adds a specialized Cluster implementation for singleton clusters, saving
a significant amount of memory by avoiding the need for m_depgraph, m_mapping,
and m_linearization, and their overheads.
This adds 4 functions to Cluster to help implement Merge() and Split() without
needing access to the internals of the other Cluster. This is a preparation for
a follow-up that will make Clusters a virtual class whose internals are abstracted
away.
