Skip to content

refactor: CFeeRate encapsulates FeeFrac internally #32750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 9, 2025

Conversation

polespinasa
Copy link
Contributor

@polespinasa polespinasa commented Jun 15, 2025

The FeeFrac type represents a fraction, intended to be used for sats/vbyte or sats/WU. It was added to improve accuracy when evaluating fee rates in cluster mempool. [1]
But it can also be used to fix the precision issues that the current CFeeRate class has now.

At the moment, CFeeRate handles the fee rate as satoshis per kilovirtualbyte: CAmount / kvB using an integer.
This PR fix CFeeRate precision issues by encapsulating FeeFrac internally keeping backwards compatibility.

This PR can also be used as a based to use multiple units on RPC calls as detailed in this issue [2].

Some previous discussions:
[1] #30535
[2] #32093

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 15, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32750.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK murchandamus, ismaelsadeeq, theStack, achow101
Concept ACK Eunovo

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

  • param@[in] -> @param[in] [correct Doxygen tag syntax]
  • param@[in] -> @param[in] [correct Doxygen tag syntax]

drahtbot_id_4_m

@polespinasa polespinasa changed the title Refactor CFeeRate to use FeeFrac internally refactor: CFeeRate encapsulates FeeFrac internally Jun 15, 2025
@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Task fuzzer,address,undefined,integer, no depends: https://github.com/bitcoin/bitcoin/runs/44121425336
LLM reason (✨ experimental): The CI failure is caused by an implicit integer conversion that triggered an UndefinedBehaviorSanitizer error during fuzzing.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK I've taken a look into this previously in #30535 (review)

Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can fix the fuzz crash with

diff --git a/src/test/fuzz/fee_rate.cpp b/src/test/fuzz/fee_rate.cpp
index 92616b62bea..f69cb59ab7b 100644
--- a/src/test/fuzz/fee_rate.cpp
+++ b/src/test/fuzz/fee_rate.cpp
@@ -20,7 +20,7 @@ FUZZ_TARGET(fee_rate)
     const CFeeRate fee_rate{satoshis_per_k};
 
     (void)fee_rate.GetFeePerK();
-    const auto bytes = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
+    const auto bytes = fuzzed_data_provider.ConsumeIntegral<int32_t>();
     if (!MultiplicationOverflow(int64_t{bytes}, satoshis_per_k)) {
         (void)fee_rate.GetFee(bytes);
     }

Also the // Compare approximately with CFeeRate test in src/test/fuzz/fee_frac.cpp is now not required.

@polespinasa polespinasa force-pushed the FeeFracWrapper branch 2 times, most recently from fdc3d20 to 417bd07 Compare June 16, 2025 19:42
Copy link
Contributor

@Eunovo Eunovo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

Left some comments

@polespinasa
Copy link
Contributor Author

13efb13 Rebased on top of Master (c584966)

@theStack
Copy link
Contributor

Concept ACK

@polespinasa
Copy link
Contributor Author

db63d5b removes an unused include and some nit on arg names.
990010f modifies the code to use uint instead of ints see #32750 (comment) for context

@polespinasa polespinasa force-pushed the FeeFracWrapper branch 2 times, most recently from cc894b5 to bf63317 Compare June 26, 2025 15:37
@polespinasa
Copy link
Contributor Author

I think I'll drop bf63317 and revert to only db63d5b (unless someone sees a blocking reason and the need to use uint32 instead of int32).

Changing to uint32 needs many changes in FeeFrac (which I would like to avoid touching). In FeeFrac size is defined as an int32_t so using uint32 doesn't give us any "advantage".

Friendly ping @davidgumberg @Eunovo @sipa to know your opinion :)

Co-authored-by: Abubakar Sadiq Ismail <48946461+ismaelsadeeq@users.noreply.github.com>
@polespinasa
Copy link
Contributor Author

d3b8a54
Addressed some of the comments. Mostly nits.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code review, lightly tested ACK d3b8a54

  • Passing negative virtual bytes to GetFee will return -1 as the fee.

From experimenting a bit, it seems to me that before this PR, a negative vsize would wrap from max value, i.e. GetFee(<negative>) would return huge amounts (here with a feerate of 1000 s/kvB):

./test/amount_tests.cpp(40): error: in "amount_tests/GetFeeTest": check feeRate.GetFee(-1) == CAmount(-1) has failed [4294967295 != -1]
./test/amount_tests.cpp(41): error: in "amount_tests/GetFeeTest": check feeRate.GetFee(-5) == CAmount(-1) has failed [4294967291 != -1]

The new behavior of throwing an assertion error on negative vsizes seems like an improvement to me.

./policy/feerate.cpp:22 GetFee: Assertion `virtual_bytes >= 0' failed.
unknown location(0): fatal error: in "amount_tests/GetFeeTest": signal: SIGABRT (application abort requested)

@@ -73,18 +73,21 @@ BOOST_AUTO_TEST_CASE(GetFeeTest)
BOOST_CHECK(CFeeRate(CAmount(-1), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(0), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(1), 0) == CFeeRate(0));
BOOST_CHECK(CFeeRate(CAmount(1), -1000) == CFeeRate(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious whether this test would have passed prior to this change, because the vsize was then expected to be an unsigned integer, and it does.

Copy link
Member

@ismaelsadeeq ismaelsadeeq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK d3b8a54 📦

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-review ACK d3b8a54

Comment on lines -75 to +83
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); }
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.m_feerate.fee, obj.m_feerate.size); }
Copy link
Contributor

@theStack theStack Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

losely related nit: seems like CFeeRate serialization hasn't been used for more than a decade [1] (other than in the fuzz test) and we could as well just remove it; was shortly looking into that as I was worried that this line could change some file format

[1] AFAICT, the last use was removed in #5159

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#22962 ;)

@achow101
Copy link
Member

achow101 commented Aug 9, 2025

ACK d3b8a54

@achow101 achow101 merged commit daca51b into bitcoin:master Aug 9, 2025
19 checks passed
alexanderwiederin added a commit to alexanderwiederin/rust-bitcoinkernel that referenced this pull request Aug 11, 2025
…c2c5afd75

39c2c5afd75 kernel: Fix bitcoin-chainstate for windows
b4c1e8b7b6c kernel: Add Purpose section to header documentation
1209f44eba2 kernel: Allowing reducing exports
bd46b323c51 kernel: Add pure kernel bitcoin-chainstate
61653d71a4c kernel: Add functions to get the block hash from a block
681dba7385d kernel: Add block index utility functions to C header
d3099c8896a kernel: Add function to read block undo data from disk to C header
67ab40ed6b3 kernel: Add functions to read block from disk to C header
f4fa216380c kernel: Add function for copying block data to C header
4d6e283e3d3 kernel: Add functions for the block validation state to C header
77429db87f4 kernel: Add validation interface to C header
b0ffc6d3b77 kernel: Add interrupt function to C header
9f889a77270 kernel: Add import blocks function to C header
762bae19dac kernel: Add chainstate load options for in-memory dbs in C header
d087bd8c463 kernel: Add options for reindexing in C header
323189720f3 kernel: Add block validation to C header
6d994d1d78f kernel: Add chainstate loading when instantiating a ChainstateManager
4390e0a3f84 kernel: Add chainstate manager option for setting worker threads
f28e13fab3b kernel: Add chainstate manager object to C header
a75a4234551 kernel: Add notifications context option to C header
5ca40d94ec5 kernel: Add chain params context option to C header
ad870979956 kernel: Add kernel library context object
3490457e805 kernel: Add logging to kernel library C header
040469a30f3 kernel: Introduce initial kernel C header API
daca51bf80e Merge bitcoin/bitcoin#32750: refactor: CFeeRate encapsulates FeeFrac internally
f679bad6052 Merge bitcoin/bitcoin#33105: validation: detect witness stripping without re-running Script checks
63d604af05f Merge bitcoin/bitcoin#33152: Release: Prepare "Open Transifex translations for v30.0" step
27aefac4250 validation: detect witness stripping without re-running Script checks
2907b58834a policy: introduce a helper to detect whether a transaction spends Segwit outputs
26e9db2df07 Merge bitcoin/bitcoin#31886: cli: return local services in -netinfo
2bb06bcaf28 Merge bitcoin/bitcoin#31679: cmake: Install internal binaries to <prefix>/libexec/
6a2bb0fd835 Merge bitcoin/bitcoin#33151: subtree: update crc32c subtree
656e16aa5e6 qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file
a0eaa449254 Fix typos
b43b8be782b Merge bitcoin/bitcoin#33125: ci: Use mlc `v1` and fix typos
8d4aaaec49c Update Transifex slug for 30.x
8ef8dd6871d Update crc32c subtree to latest upstream master
9a5d29711af Squashed 'src/crc32c/' changes from b60d2b7334..efb8ea04e4
f28a94b40ee ci: update shellcheck to v0.11.0
e46af304416 ci: update mlc to v1
7d60c0eb69b fix typo
49f2f3c89fa doc: fix typos
d818340e7e2 test: Rename shuffled_indeces to shuffled_indices
96f8673b879 doc: fix typos
d767503b6a2 Merge bitcoin/bitcoin#33039: refactor,test: follow-ups to multi-byte block obfuscation
721a051320f test: add coverage for -netinfo header and local services
f7d2db28e90 netinfo: return shortened services, if peers list requested
4489ab526ad netinfo: return local services in the default report
eb073209db9 qa: test witness stripping in p2p_segwit
86e3a0a8cbd refactor: standardize obfuscation memory alignment
13f00345c06 refactor: write `Obfuscation` object when new key is generated in dbwrapper
e5b1b7c5577 refactor: rename `OBFUSCATION_KEY_KEY`
298bf951057 refactor: simplify `Obfuscation::HexKey`
2dea0454254 test: make `obfuscation_serialize` more thorough
a17d8202c36 test: merge xor_roundtrip_random_chunks and xor_bytes_reference
d3b8a54a812 Refactor CFeeRate to use FeeFrac internally
f49840dd902 doc: Fix typo in files.md
f5cf0b1ccc8 bitcoin wrapper: improve help output
c810b168b89 doc: Add description of installed files to files.md
94ffd01a029 doc: Add release notes describing libexec/ binaries
cd97905ebc5 cmake: Move internal binaries from bin/ to libexec/
REVERT: ce8003578e7 kernel: Fix bitcoin-chainstate for windows
REVERT: 9ebe4a83a93 kernel: Add Purpose section to header documentation
REVERT: 10d2432218e kernel: Allowing reducing exports
REVERT: e04b1e1f528 kernel: Add pure kernel bitcoin-chainstate
REVERT: e0f98a533cd kernel: Add functions to get the block hash from a block
REVERT: af1c5e34955 kernel: Add block index utility functions to C header
REVERT: fd60e50badc kernel: Add function to read block undo data from disk to C header
REVERT: f507d230bba kernel: Add functions to read block from disk to C header
REVERT: 1480dd9f41f kernel: Add function for copying block data to C header
REVERT: 26f484b94e6 kernel: Add functions for the block validation state to C header
REVERT: 8c286be14af kernel: Add validation interface to C header
REVERT: 6d733019bba kernel: Add interrupt function to C header
REVERT: c917663b0b5 kernel: Add import blocks function to C header
REVERT: 9c2496406f7 kernel: Add chainstate load options for in-memory dbs in C header
REVERT: 393329899d9 kernel: Add options for reindexing in C header
REVERT: 107156b9362 kernel: Add block validation to C header
REVERT: b83c0d3ba49 kernel: Add chainstate loading when instantiating a ChainstateManager
REVERT: c3279239c75 kernel: Add chainstate manager option for setting worker threads
REVERT: f7a2c3c600e kernel: Add chainstate manager object to C header
REVERT: 5e8b57622c3 kernel: Add notifications context option to C header
REVERT: 56298e76271 kernel: Add chain params context option to C header
REVERT: 9bade37d97d kernel: Add kernel library context object
REVERT: 9859726fbd6 kernel: Add logging to kernel library C header
REVERT: 6dd7a4fcab0 kernel: Introduce initial kernel C header API

git-subtree-dir: libbitcoinkernel-sys/bitcoin
git-subtree-split: 39c2c5afd75e5d455ac2699dcc1c65728e1a5bc5
@hebasto hebasto mentioned this pull request Aug 12, 2025
fanquake added a commit to fanquake/bitcoin that referenced this pull request Aug 12, 2025
Fixes:
```bash
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: symbol __divmodti4 from unsupported version GCC_7.0.0(14)
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: failed IMPORTED_SYMBOLS
```

which is occuring after bitcoin#32750. I can't see any supported distro that is
shipping a new enough glibc (2.31), but a GCC older than 7.0.
alexanderwiederin added a commit to alexanderwiederin/rust-bitcoinkernel that referenced this pull request Aug 12, 2025
…bffb6d65875

abffb6d65875 kernel: Fix bitcoin-chainstate for windows
c9d7b71b7b96 kernel: Add Purpose section to header documentation
88c53c323a4d kernel: Allowing reducing exports
deee70395cef kernel: Add pure kernel bitcoin-chainstate
1a89d2f76bd6 kernel: Add functions to get the block hash from a block
dff6e326c929 kernel: Add block index utility functions to C header
f2a1c5e198fc kernel: Add function to read block undo data from disk to C header
5e0e1cc30026 kernel: Add functions to read block from disk to C header
6a3406caace5 kernel: Add function for copying block data to C header
3ac6249b5b91 kernel: Add functions for the block validation state to C header
366276d61913 kernel: Add validation interface to C header
bc4093175962 kernel: Add interrupt function to C header
70c5d903cb29 kernel: Add import blocks function to C header
2be42e3c1ae1 kernel: Add chainstate load options for in-memory dbs in C header
6be186958803 kernel: Add options for reindexing in C header
33445ec97008 kernel: Add block validation to C header
e63ec151b9da kernel: Add chainstate loading when instantiating a ChainstateManager
171fbb8d118c kernel: Add chainstate manager option for setting worker threads
a00df723ac2f kernel: Add chainstate manager object to C header
40a1f1d46338 kernel: Add notifications context option to C header
506f82bcfb27 kernel: Add chain params context option to C header
c4074d7894fd kernel: Add kernel library context object
54518fdc6f7f kernel: Add logging to kernel library C header
ff4a0ef4132a kernel: Introduce initial kernel C header API
daca51bf80e7 Merge bitcoin/bitcoin#32750: refactor: CFeeRate encapsulates FeeFrac internally
f679bad6052a Merge bitcoin/bitcoin#33105: validation: detect witness stripping without re-running Script checks
63d604af05f8 Merge bitcoin/bitcoin#33152: Release: Prepare "Open Transifex translations for v30.0" step
27aefac42505 validation: detect witness stripping without re-running Script checks
2907b58834ab policy: introduce a helper to detect whether a transaction spends Segwit outputs
26e9db2df076 Merge bitcoin/bitcoin#31886: cli: return local services in -netinfo
2bb06bcaf284 Merge bitcoin/bitcoin#31679: cmake: Install internal binaries to <prefix>/libexec/
6a2bb0fd8359 Merge bitcoin/bitcoin#33151: subtree: update crc32c subtree
656e16aa5e65 qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file
a0eaa4492548 Fix typos
b43b8be782b8 Merge bitcoin/bitcoin#33125: ci: Use mlc `v1` and fix typos
8d4aaaec49c0 Update Transifex slug for 30.x
8ef8dd6871dd Update crc32c subtree to latest upstream master
9a5d29711afc Squashed 'src/crc32c/' changes from b60d2b7334..efb8ea04e4
f28a94b40eea ci: update shellcheck to v0.11.0
e46af3044160 ci: update mlc to v1
7d60c0eb69bf fix typo
49f2f3c89fac doc: fix typos
d818340e7e27 test: Rename shuffled_indeces to shuffled_indices
96f8673b879e doc: fix typos
d767503b6a26 Merge bitcoin/bitcoin#33039: refactor,test: follow-ups to multi-byte block obfuscation
721a051320f2 test: add coverage for -netinfo header and local services
f7d2db28e902 netinfo: return shortened services, if peers list requested
4489ab526add netinfo: return local services in the default report
eb073209db9e qa: test witness stripping in p2p_segwit
86e3a0a8cbd3 refactor: standardize obfuscation memory alignment
13f00345c061 refactor: write `Obfuscation` object when new key is generated in dbwrapper
e5b1b7c5577e refactor: rename `OBFUSCATION_KEY_KEY`
298bf9510578 refactor: simplify `Obfuscation::HexKey`
2dea04542541 test: make `obfuscation_serialize` more thorough
a17d8202c36a test: merge xor_roundtrip_random_chunks and xor_bytes_reference
d3b8a54a8120 Refactor CFeeRate to use FeeFrac internally
f49840dd902c doc: Fix typo in files.md
f5cf0b1ccc8f bitcoin wrapper: improve help output
c810b168b89d doc: Add description of installed files to files.md
94ffd01a0294 doc: Add release notes describing libexec/ binaries
cd97905ebc56 cmake: Move internal binaries from bin/ to libexec/
REVERT: ce8003578e72 kernel: Fix bitcoin-chainstate for windows
REVERT: 9ebe4a83a937 kernel: Add Purpose section to header documentation
REVERT: 10d2432218eb kernel: Allowing reducing exports
REVERT: e04b1e1f5284 kernel: Add pure kernel bitcoin-chainstate
REVERT: e0f98a533cd6 kernel: Add functions to get the block hash from a block
REVERT: af1c5e349550 kernel: Add block index utility functions to C header
REVERT: fd60e50badc5 kernel: Add function to read block undo data from disk to C header
REVERT: f507d230bba3 kernel: Add functions to read block from disk to C header
REVERT: 1480dd9f41fc kernel: Add function for copying block data to C header
REVERT: 26f484b94e66 kernel: Add functions for the block validation state to C header
REVERT: 8c286be14aff kernel: Add validation interface to C header
REVERT: 6d733019bba6 kernel: Add interrupt function to C header
REVERT: c917663b0b51 kernel: Add import blocks function to C header
REVERT: 9c2496406f7e kernel: Add chainstate load options for in-memory dbs in C header
REVERT: 393329899d90 kernel: Add options for reindexing in C header
REVERT: 107156b9362e kernel: Add block validation to C header
REVERT: b83c0d3ba499 kernel: Add chainstate loading when instantiating a ChainstateManager
REVERT: c3279239c75f kernel: Add chainstate manager option for setting worker threads
REVERT: f7a2c3c600e5 kernel: Add chainstate manager object to C header
REVERT: 5e8b57622c33 kernel: Add notifications context option to C header
REVERT: 56298e76271a kernel: Add chain params context option to C header
REVERT: 9bade37d97d7 kernel: Add kernel library context object
REVERT: 9859726fbd6b kernel: Add logging to kernel library C header
REVERT: 6dd7a4fcab08 kernel: Introduce initial kernel C header API

git-subtree-dir: libbitcoinkernel-sys/bitcoin
git-subtree-split: abffb6d6587502371a59d37c043d3d644846c267
Sjors pushed a commit to Sjors/bitcoin that referenced this pull request Aug 12, 2025
Fixes:
```bash
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: symbol __divmodti4 from unsupported version GCC_7.0.0(14)
/distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: failed IMPORTED_SYMBOLS
```

which is occuring after bitcoin#32750. I can't see any supported distro that is
shipping a new enough glibc (2.31), but a GCC older than 7.0.
fanquake added a commit that referenced this pull request Aug 13, 2025
776a163 guix: increase maximum allowed (runtime) GCC to 7 (fanquake)

Pull request description:

  Fixes:
  ```bash
  /distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: symbol __divmodti4 from unsupported version GCC_7.0.0(14)
  /distsrc-base/distsrc-d3b8a54a8120-x86_64-linux-gnu/build/bin/bitcoind: failed IMPORTED_SYMBOLS
  ```

  which is occuring after #32750. I can't see any supported distro that is shipping a new enough glibc (2.31), but a GCC older than 7.0.

  Fixes #33177.

ACKs for top commit:
  hebasto:
    ACK 776a163.

Tree-SHA512: 8e5a77c509eb6164314fdb644ea199916e151eb0c7f48703f3a2bdedf0dea29b7f402ceacb2aaf42ebffba59080cefbb84253b2721047d973a851090447ba3b5
@fanquake
Copy link
Member

As pointed out in #33177, this broke the Guix build. Was fixed in #33178.

fanquake added a commit that referenced this pull request Aug 15, 2025
…ayfee, minrelaytxfee

ba84a25 [doc] update mempool-replacements.md for incremental relay feerate change (glozow)
18720bc [doc] release note for min feerate changes (glozow)
6da5de5 [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB (glozow)
2e515d2 [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit (glozow)
457cfb6 [prep/util] help MockMempoolMinFee handle more precise feerates (glozow)
3eab8b7 [prep/test] replace magic number 1000 with respective feerate vars (glozow)
5f2df0e [miner] lower default -blockmintxfee to 1sat/kvB (glozow)
d6213d6 [doc] assert that default min relay feerate and incremental are the same (glozow)
1fbee5d [test] explicitly check default -minrelaytxfee and -incrementalrelayfee (glozow)
72dc184 [test] RBF rule 4 for various incrementalrelayfee settings (glozow)
85f4988 [test] check bypass of minrelay for various minrelaytxfee settings (glozow)
e5f896b [test] check miner doesn't select 0fee transactions (glozow)

Pull request description:

  ML post for discussion about the general concept, how this impacts the wider ecosystem, philosophy about minimum feerates, etc: https://delvingbitcoin.org/t/changing-the-minimum-relay-feerate/1886

  This PR is inspired by #13922 and #32959 to lower the minimum relay feerate in response to bitcoin's exchange rate changes in the last ~10 years. It lowers the default `-minrelaytxfee` and `-incrementalrelayfee`, and knocks `-blockmintxfee` down to the minimum nonzero setting. Also adds some tests for the settings and pulls in #32750.

  The minimum relay feerate is a DoS protection rule, representing a price on the network bandwidth used to relay transactions that have no PoW. While relay nodes don't all collect fees, the assumption is that if nodes on the network use their resources to relay this transaction, it will reach a miner and the attacker's money will be spent once it is mined. The incremental relay feerate is similar: it's used to price the relay of replacement transactions (the additional fees need to cover the new transactions at this feerate) and evicted transactions (following a trim, the new mempool minimum feerate is the package feerate of what was removed + incremental).

  Also note that many nodes on the network have elected to relay/mine lower feerate transactions. Miners (some say up to 85%) are choosing to mine these low feerate transactions instead of leaving block space unfilled, but these blocks have extremely poor compact block reconstruction rates with nodes that rejected or didn't hear about those transactions earlier.
  - #33106 (comment)
  - https://x.com/caesrcd/status/1947022514267230302
  - https://mempool.space/block/00000000000000000001305770e0aa279dcd8ba8be18c3d5cf736a26f77e06fd
  - https://mempool.space/block/00000000000000000001b491649ec030aa8e003e1f4f9d3b24bb99ba16f91e97
  - https://x.com/mononautical/status/1949452586391855121

  While it wouldn't make sense to loosen DoS restrictions recklessly in response to these events, I think the current price is higher than necessary, and this motivates us changing the default soon. Since the minimum relay feerate defines an amount as too small based on what it costs the attacker, it makes sense to consider BTC's conversion rate to what resources you can buy in the "real world."

  Going off of [this comment](#32959 (comment)) and [this comment](#33106 (comment))
  - Let's say an attacker wants to use/exhaust the network's bandwidth, and has the choice between renting resources from a commercial provider and getting the network to "spam" itself it by sending unconfirmed transactions. We'd like the latter to be more expensive than the former.
  - The bandwidth for relaying a transaction across the network is roughly its serialized size (plus relay overhead) x number of nodes. A 1000vB transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB
  - If the going rate for ec2 bandwidth is 10c/GB, that's like 1-4c per kvB of transaction data
  - Then a 1000vB transaction should pay at least 4c
  - $0.04 USD is 40 satoshis at 100k USD/BTC
  - Baking in some margin for changes in USD/BTC conversion rate, number of nodes (and thus bandwidth), and commercial service costs, I think 50-100 satoshis is on the conservative end but in the right ballpark
  - At least 97% of the recent sub-1sat/vB transactions would be accepted with a new threshold of 0.1sat/vB: #33106 (comment)

  List of feerates that are changed and why:
  - min relay feerate: significant conversion rate changes, see above
  - incremental relay feerate: should follow min relay feerate, see above
  - block minimum feerate: shouldn’t be above min relay feerate, otherwise the node accepts transactions it will never mine. I've knocked it down to the bare minimum of 1sat/kvB. Now that we no longer have coin age priority (removed in v0.15), I think we can leave it to the `CheckFeeRate` policy rule to enforce a minimum entry price, and the block assembly code should just fill up the block with whatever it finds in mempool.

  List of feerates that are not changed and why:
  - dust feerate: this feerate cannot be changed as flexibly as the minrelay feerate. A much longer record of low feerate transactions being mined is needed to motivate a decrease there.
  - maxfeerate (RPC, wallet): I think the conversion rate is relevant as well, but out of scope for this PR
  - minimum feerate returned by fee estimator: should be done later. In the past, we've excluded new policy defaults from fee estimation until we feel confident they represent miner policy (e.g. #9519). Also, the fee estimator itself doesn't have support for sub-1sat/vB yet.
  - all wallet feerates (mintxfee, fallbackfee, discardfee, consolidatefeerate, WALLET_INCREMENTAL_RELAY_FEE, etc.): should be done later. Our standard procedure is to do wallet changes at least 1 release after policy changes.

ACKs for top commit:
  achow101:
    ACK ba84a25
  gmaxwell:
    ACK ba84a25
  jsarenik:
    Tested ACK ba84a25
  darosior:
    ACK ba84a25
  ajtowns:
    ACK ba84a25
  davidgumberg:
    crACK  ba84a25
  w0xlt:
    ACK ba84a25
  caesrcd:
    reACK ba84a25
  ismaelsadeeq:
    re-ACK ba84a25

Tree-SHA512: b4c35e8b506b1184db466551a7e2e48bb1e535972a8dbcaa145ce3a8bfdcc70a8807dc129460f129a9d31024174d34077154a387c32f1a3e6831f6fa5e9c399e
stringintech added a commit to stringintech/go-bitcoinkernel that referenced this pull request Aug 17, 2025
35fced5df7 kernel: Fix bitcoin-chainstate for windows
c164264fd8 kernel: Add Purpose section to header documentation
9096c35c05 kernel: Allowing reducing exports
6d0d4b2507 kernel: Add pure kernel bitcoin-chainstate
ccd85f0333 kernel: Add functions to get the block hash from a block
925f21c37b kernel: Add block index utility functions to C header
b7441841c9 kernel: Add function to read block undo data from disk to C header
bc0e6f098e kernel: Add functions to read block from disk to C header
5120302f96 kernel: Add function for copying block data to C header
718ccee732 kernel: Add functions for the block validation state to C header
eb363ab30e kernel: Add validation interface to C header
246886c6ea kernel: Add interrupt function to C header
f3b34ca457 kernel: Add import blocks function to C header
fe08857d52 kernel: Add chainstate load options for in-memory dbs in C header
f93f171e01 kernel: Add options for reindexing in C header
dca7b4c26e kernel: Add block validation to C header
f031e9ce03 kernel: Add chainstate loading when instantiating a ChainstateManager
3cb99f73ec kernel: Add chainstate manager option for setting worker threads
9454ad8512 kernel: Add chainstate manager object to C header
3bead9ebdd kernel: Add notifications context option to C header
dda805dfb6 kernel: Add chain params context option to C header
ea5334925d kernel: Add kernel library context object
36fafbaef9 kernel: Add logging to kernel library C header
d28eef5cf2 kernel: Introduce initial kernel C header API
daca51bf80 Merge bitcoin/bitcoin#32750: refactor: CFeeRate encapsulates FeeFrac internally
f679bad605 Merge bitcoin/bitcoin#33105: validation: detect witness stripping without re-running Script checks
63d604af05 Merge bitcoin/bitcoin#33152: Release: Prepare "Open Transifex translations for v30.0" step
27aefac425 validation: detect witness stripping without re-running Script checks
2907b58834 policy: introduce a helper to detect whether a transaction spends Segwit outputs
26e9db2df0 Merge bitcoin/bitcoin#31886: cli: return local services in -netinfo
2bb06bcaf2 Merge bitcoin/bitcoin#31679: cmake: Install internal binaries to <prefix>/libexec/
6a2bb0fd83 Merge bitcoin/bitcoin#33151: subtree: update crc32c subtree
656e16aa5e qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file
a0eaa44925 Fix typos
b43b8be782 Merge bitcoin/bitcoin#33125: ci: Use mlc `v1` and fix typos
8d4aaaec49 Update Transifex slug for 30.x
8ef8dd6871 Update crc32c subtree to latest upstream master
9a5d29711a Squashed 'src/crc32c/' changes from b60d2b7334..efb8ea04e4
f28a94b40e ci: update shellcheck to v0.11.0
e46af30441 ci: update mlc to v1
7d60c0eb69 fix typo
49f2f3c89f doc: fix typos
d818340e7e test: Rename shuffled_indeces to shuffled_indices
96f8673b87 doc: fix typos
d767503b6a Merge bitcoin/bitcoin#33039: refactor,test: follow-ups to multi-byte block obfuscation
cf15d45192 Merge bitcoin/bitcoin#33044: contrib: drop use of `PermissionsStartOnly` & `Group=`
d7ed47fb80 Merge bitcoin/bitcoin#33077: kernel: create monolithic kernel static library
38e6ea9f3a Merge bitcoin/bitcoin#33101: cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`
c92115dcb2 Merge bitcoin/bitcoin#33119: rpc: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix
1dab8d0635 Merge bitcoin/bitcoin#33113: refactor: Use immediate lambda to work around GCC bug 117966
45bdbb1317 Merge bitcoin/bitcoin#33122: test: remove duplicated code in test/functional/wallet_migration.py
a45cc17d34 Merge bitcoin/bitcoin#33115: cmake: Switch to generated `ts_files.cmake` file
b90da9c2e9 Merge bitcoin/bitcoin#33138: ci: Pass CI_FAILFAST_TEST_LEAVE_DANGLING into container
fa1d2f6380 ci: Pass CI_FAILFAST_TEST_LEAVE_DANGLING into container
fd813bf863 Merge bitcoin/bitcoin#33002: ci: Only pass documented env vars
9617a42fdb Merge bitcoin/bitcoin#32581: allocators: Apply manual ASan poisoning to `PoolResource`
33e7fc51f4 Merge bitcoin/bitcoin#33133: rpc: fix getpeerinfo ping duration unit docs
721a051320 test: add coverage for -netinfo header and local services
f7d2db28e9 netinfo: return shortened services, if peers list requested
4489ab526a netinfo: return local services in the default report
1252eeb997 rpc: fix getpeerinfo ping duration unit docs
eb073209db qa: test witness stripping in p2p_segwit
6a7c0d3f87 test: refactor to remove duplicated test code
d1b583181d Merge bitcoin/bitcoin#32654: init: make `-blockmaxweight` startup option debug only
50a92cd56f Merge bitcoin/bitcoin#33060: test: Slay BnB Mutants
643bacd124 Merge bitcoin/bitcoin#33058: test: add assertions to SRD max weight test
eeb0b31e3a Merge bitcoin/bitcoin#32941: p2p: TxOrphanage revamp cleanups
c0642e558a [fuzz] fix latency score check in txorphan_protected
0cb1ed2b7c Merge bitcoin/bitcoin#33132: fuzz: txgraph: fix `real_is_optimal` flag propagation in `CommitStaging`
a26fbee38f qt: Translations update
444dcb2f99 fuzz: txgraph: fix `real_is_optimal` flag propagation in `CommitStaging`
83a2216f52 Merge bitcoin/bitcoin#33118: test: fix anti-fee-sniping off-by-one error
3543bfdfec test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify 'spend_vin' is the correct field
e07e2532b4 test: fix anti-fee-sniping off-by-one error
c7a24c3052 ci: Re-enable DEBUG=1 in centos task
3d4d4f0d92 scripted-diff: rename "ann" variables to "latency_score"
3b92448923 [doc] comment fixups for orphanage changes
1384dbaf6d [config] emit warning for -maxorphantx, but allow it to be set
b10c55b298 fix up TxOrphanage lower_bound sanity checks
cfd71c6704 scripted-diff: rename TxOrphanage outpoints index
edb97bb3f1 [logging] add logs for inner loop of LimitOrphans
8a58d0e87d scripted-diff: rename OrphanTxBase to OrphanInfo
cc50f2f0df [cleanup] replace TxOrphanage::Size() with CountUniqueOrphans
ed24e01696 [optimization] Maintain at most 1 reconsiderable announcement per wtxid
af7402ccfa [refactor] make TxOrphanage keep itself trimmed
d1fac25ff3 [doc] 31829 release note
75ed673193 Merge bitcoin/bitcoin#33048: test: reduce runtime of p2p_opportunistic_1p1c.py
ca04eebd72 cmake: Switch to generated `ts_files.cmake` file
95341de6ca cmake, refactor: Move handling of Qt TS files into `locale` directory
24246c3deb Merge bitcoin/bitcoin#31385: package validation: relax the package-not-child-with-unconfirmed-parents rule
b8025b30cc Merge bitcoin/bitcoin#32559: doc: add alpine build instructions
18d1071dd1 init: replace deprecated PermissionsStartOnly systemd directive
1caaf65043 init: remove Group= as it will default to the user's default group
a7bafb3e05 refactor: Use immediate lambda to work around GCC bug 117966
b093a19ae2 cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`
eb59a192d9 cmake, refactor: Encapsulate adding secp256k1 subtree in function
4f27e8ca4d Merge bitcoin/bitcoin#33083: qa: test that we do not disconnect a peer for submitting an invalid compact block
bfc9d95129 Merge bitcoin/bitcoin#33104: test: Perform backup filename checks in migrate_and_get_rpc in wallet_migration.py
8712e074bb Merge bitcoin/bitcoin#33093: refactor: remove unused `ser_writedata16be` and `ser_readdata16be`
5ee4e79669 Merge bitcoin/bitcoin#31244: descriptors: MuSig2
4b80147feb test: Perform backup filename checks in migrate_and_get_rpc
aef2dbb402 Merge bitcoin/bitcoin#33099: ci: allow for any libc++ intrumentation & use it for TSAN
8283af13fe Merge bitcoin/bitcoin#32584: depends: hard-code necessary c(xx)flags rather than setting them per-host
547c64814d Merge bitcoin/bitcoin#32987: init: [gui] Avoid UB/crash in InitAndLoadChainstate
e6bfd95d50 Merge bitcoin-core/gui#881: Move `FreespaceChecker` class into its own module
8a94cf8efe Merge bitcoin/bitcoin#30635: rpc: add optional blockhash to waitfornewblock, unhide wait methods in help
dc78ed2140 Merge bitcoin/bitcoin#33005: refactor: GenTxid type safety followups
3cb65ffa83 Merge bitcoin/bitcoin#33100: ci: remove `ninja-build` from MSAN jobs
7aa5b67132 ci: remove DEBUG_LOCKORDER from TSAN job
b09af2ce50 ci: instrument libc++ in TSAN job
6653cafd0b ci: allow libc++ instrumentation other than msan
3333d3f75f ci: Only pass documented env vars
3fe3fdb02b Merge bitcoin/bitcoin#33102: fuzz: cover BanMan::IsDiscouraged
c2ed576d2c fuzz: cover BanMan::IsDiscouraged
3a03f07560 qt: Avoid header circular dependency
cab6736b70 ci: remove ninja-build from MSAN jobs
0431a690c3 cleanup: remove unused `ser_writedata16be` and `ser_readdata16be`
00604296e1 Merge bitcoin/bitcoin#32866: doc: add note for watch-only wallet migration
91058877ff Merge bitcoin/bitcoin#32273: wallet: Fix relative path backup during migration.
6b99670e3c Merge bitcoin/bitcoin#33075: doc: Add legacy wallet removal release notes
2cef200340 Merge bitcoin/bitcoin#28944: wallet, rpc: add anti-fee-sniping to `send` and `sendall`
932e993b37 Merge bitcoin/bitcoin#33073: guix: warn SOURCE_DATE_EPOCH set in guix-codesign
0bed946e5d Merge bitcoin/bitcoin#33079: ci: limit max stack size to 512 KiB
28ec91c30e Merge bitcoin/bitcoin#33088: doc: move `cmake -B build -LH` up in Unix build docs
c157438116 qa: test that we do disconnect upon a second invalid compact block being announced
2f410ad78c Merge bitcoin/bitcoin#32263: cluster mempool: add TxGraph work controls
6757052fc4 doc: move `cmake -B build -LH` up in Unix build docs
9954d6c833 depends: hard-code necessary c(xx)flags rather than setting them per-host
953c90d764 Merge bitcoin/bitcoin#33086: contrib: [tracing] fix pointer argument handling in mempool_monitor.py
5888b4a2a5 doc: add note for watch-only wallet migration
3b23f95e34 ci: limit max stack size to 512 KiB
2931a87477 ci: limit stack size to 512kb in native macOS jobs
3724e9b40a Merge bitcoin/bitcoin#32973: validation: docs and cleanups for MemPoolAccept coins views
0ce041ea88 tracing: fix pointer argument handling in mempool_monitor.py
25884bd896 qt, refactor: Move `FreespaceChecker` class into its own module
fb2dcbb160 qa: test cached failure for compact block
f12d8b104e qa: test a compact block with an invalid transaction
d6c37b28a7 qa: remove unnecessary tx removal from compact block
321984705d Merge bitcoin/bitcoin#32279: [IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline
94b39ce738 refactor: Change `m_tx_inventory_to_send` from `std::set<GenTxid>` to `std::set<Wtxid>`
fa45ccc15d doc: Add legacy wallet removal release notes
2a97ff466d Merge bitcoin/bitcoin#29954: RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`
fd068257e0 Merge bitcoin/bitcoin#33065: rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`
9cafdf8941 Merge bitcoin/bitcoin#33064: test: fix RPC coverage check
fdbade6f8d kernel: create monolithic kernel static library
c8309198f8 Merge bitcoin/bitcoin#33070: doc/zmq: fix unix socket path example
1bed0f734b guix: warn SOURCE_DATE_EPOCH set in guix-codesign
cc33e45789 test: improve assertion for SRD max weight test
1c10b7351e RPC: Return permitbaremultisig and maxdatacarriersize in getmempoolinfo
e83699a626 doc/zmq: fix unix socket path example
8aed477c33 test: fix RPC coverage check
2630b64f81 test: add abortrescan RPC test
75a5c8258e Merge bitcoin/bitcoin#33063: util: Revert "common: Close non-std fds before exec in RunCommandJSON"
d5104cfbae prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline
52121506b2 test: assert `CScript` allocation characteristics
65ac7f6d4d refactor: modernize `CScriptBase` definition
756da2a994 refactor: extract `STATIC_SIZE` constant to prevector
251d020846 init, wallet: replace hardcoded output types with `FormatAllOutputTypes`
3b188b8b3d Merge bitcoin/bitcoin#31576: test: Move `script_assets_tests` into its own suite
2e97541396 Merge bitcoin/bitcoin#32944: wallet: Remove `upgradewallet` RPC
b08041cac8 Merge bitcoin/bitcoin#32845: rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs
a3cf623364 test: Test max_selection_weight edge cases
57fe8acc8a test: Check max_weight_exceeded error
e3ba0757a9 rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`
fc162299f0 Merge bitcoin/bitcoin#32994: p2p: rename GetAddresses -> GetAddressesUnsafe
633d8ea17b Merge bitcoin/bitcoin#32970: ci: Enable more shellcheck
faa1c3e80d Revert "Merge bitcoin/bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON"
6cdc5a90cf Merge bitcoin/bitcoin#32967: log: [refactor] Use info level for init logs
443c32a3e6 Merge bitcoin/bitcoin#32822: fuzz: Make process_message(s) more deterministic
face8123fd log: [refactor] Use info level for init logs
fa183761cb log: Remove function name from init logs
5ad79b2035 Merge bitcoin/bitcoin#32593: wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC
ea17a9423f [doc] release note for relaxing requirement of all unconfirmed parents present
12f48d5ed3 test: add chained 1p1c propagation test
525be56741 [unit test] package submission 2p1c with 1 parent missing
f24771af05 relax child-with-unconfirmed-parents rule
e17fb86382 Merge bitcoin/bitcoin#32888: ci: Use optimized Debug build type in test-each-commit
fd3d80c209 Merge bitcoin/bitcoin#33047: test: check proper OP_2ROT behavior
1119ac51f0 Merge bitcoin/bitcoin#33040: doc: update headers and remove manual TOCs
e2f2df0ead Merge bitcoin/bitcoin#32984: wallet: Set migrated wallet name only on success
16f7b43b68 Merge bitcoin/bitcoin#33049: doc: Fix typos in asmap README
b59dc21847 doc: Fix typos in asmap README
ca38cf701d doc: fix a few obvious typos in the affected files
ddab466e0d doc: remove manual TOCs
26a3730711 doc: unify `developer-notes` and `productivity` header styles
eb13718448 Merge bitcoin/bitcoin#31179: RPC: Add reserve member function to `UniValue` and use it in `blockToJSON` function
86e3a0a8cb refactor: standardize obfuscation memory alignment
13f00345c0 refactor: write `Obfuscation` object when new key is generated in dbwrapper
eb65f57f31 [test] setmocktime instead of waiting in 1p1c tests
70772dd469 [test] cut the number of transactions involved in 1p1c DoS tests
b94c6356a2 test: check proper OP_2ROT behavior
73e754bd01 Merge bitcoin/bitcoin#33001: test: Do not pass tests on unhandled exceptions
a9819b0e9d refactor: Change `FindTxForGetData` to take GenTxid instead of CInv
d588575ed1 refactor: miscellaneous GenTxid followups
cfb859e82e Merge bitcoin/bitcoin#33037: doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)
afd3b34dc5 Merge bitcoin/bitcoin#33004: Enable `-natpmp` by default
49bbf9ff28 Merge bitcoin/bitcoin#33036: Update secp256k1 subtree to latest master
c5c1960f93 doc: Add release notes for changes in RPCs
90fd5acbe5 rpc, test: Fix error message in getdescriptoractivity
39fef1d203 test: Add missing logging info for each test
53ac704efd rpc, test: Fix error message in unloadwallet
1fc3a8e8e7 rpc, test: Add EnsureUniqueWalletName tests
900bb53905 Merge bitcoin/bitcoin#32990: wallet: remove outdated `pszSkip` arg of database `Rewrite` func
c8ec423719 Merge bitcoin/bitcoin#33020: test: delete commented-out tests and add a test case in wallet_signer
09f004bd9f Merge bitcoin/bitcoin#32945: tests: speed up coins_tests by parallelizing
5d98fc7559 Merge bitcoin/bitcoin#33030: test: check tx is final when there is no locktime
e5b1b7c557 refactor: rename `OBFUSCATION_KEY_KEY`
298bf95105 refactor: simplify `Obfuscation::HexKey`
2dea045425 test: make `obfuscation_serialize` more thorough
a17d8202c3 test: merge xor_roundtrip_random_chunks and xor_bytes_reference
b635bc0896 rpc, util: Add EnsureUniqueWalletName
da318fe53f test: delete commented out tests
6d80e999a0 test: external signer returns invalid JSON response
065e42976a test: IsFinalTx returns true when there is no locktime
1cb2399703 doc: clarify the GetAddresses/GetAddressesUnsafe documentation
e5a7dfd79f p2p: rename GetAddresses -> GetAddressesUnsafe
faa2f3b1af doc: Add release notes for 32521 (MAX_TX_LEGACY_SIGOPS)
336b8be37b Update secp256k1 subtree to latest master
5600e6fc4b Squashed 'src/secp256k1/' changes from 4187a46649..b9313c6e1a
06ab3a394a tests: speed up coins_tests by parallelizing
7129c9ea8e Merge bitcoin/bitcoin#32827: mempool: Avoid needless vtx iteration during IBD
11c6a864c9 Merge bitcoin/bitcoin#33007: test: fix `ReadTopologicalSet` unsigned integer overflow
9bc33432e2 Merge bitcoin/bitcoin#32999: ci: Use APT_LLVM_V in msan task
5878f35446 Merge bitcoin/bitcoin#31144: [IBD] multi-byte block obfuscation
249889bee6 orphanage: avoid vtx iteration when no orphans
41ad2be434 mempool: Avoid expensive loop in `removeForBlock` during IBD
e9edd43a95 Merge bitcoin/bitcoin#32521: policy: make pathological transactions packed with legacy sigops non-standard
80067ac111 Merge bitcoin/bitcoin#31829: p2p: improve TxOrphanage denial of service bounds
31c4e77a25 test: fix ReadTopologicalSet unsigned integer overflow
672c85cb1e Merge bitcoin/bitcoin#32868: test: refactor: overhaul block hash determination for `CBlock{,Header}` objects
fa1a14a13a fuzz: Reset chainman state in process_message(s) targets
fa9a3de09b fuzz: DisableNextWrite
aeeeeec9f7 fuzz: Reset dirty connman state in process_message(s) targets
fa11eea405 fuzz: Avoid non-determinism in process_message(s) target (PeerMan)
faa3e68411 test: Log KeyboardInterrupt as exception
fac90e5261 test: Check that the GUI interactive reindex works
b2d07f872c Add release notes for -natpmp enabled by default
3fc660d267 mapport: turn -natpmp to on by default
fa30b34026 test: Do not pass tests on unhandled exceptions
96da68a38f qa: functional test a transaction running into the legacy sigop limit
367147954d qa: unit test standardness of inputs packed with legacy sigops
5863315e33 policy: make pathological transactions packed with legacy sigops non-standard.
5fa34951ea test: avoid unneeded block header hash -> integer conversions
2118301d77 test: rename CBlockHeader `.hash` -> `.hash_hex` for consistency
23be0ec2f0 test: rename CBlockHeader `.rehash()`/`.sha256` -> `.hash_int` for consistency
8b09cc350a test: remove bare CBlockHeader `.rehash()`/`.calc_sha256()` calls
0716382c20 test: remove header hash caching in CBlockHeader class
0f044e82bd test: avoid direct block header modification in feature_block.py
f3c791d2e3 test: refactor: dedup `CBlockHeader` serialization
fad040a578 ci: Use APT_LLVM_V in msan task
76fe0e59ec test: Migration of a wallet ending in `../`
f0bb3d50fe test: Migration of a wallet ending in `/`
41faef5f80 test: Migration fail recovery w/ `../` in path
63c6d36437 test: Migration of a wallet with `../` in path.
70f1c99c90 wallet: Fix migration of wallets with pathnames.
f6ee59b6e2 wallet: migration: Make backup in walletdir
e22c3599c6 test: wallet: Check direct file backup name.
060695c22a test: Failed load after migrate should restore backup
248b6a27c3 optimization: peel align-head and unroll body to 64 bytes
e7114fc6dc optimization: migrate fixed-size obfuscation from `std::vector<std::byte>` to `uint64_t`
478d40afc6 refactor: encapsulate `vector`/`array` keys into `Obfuscation`
377aab8e5a refactor: move `util::Xor` to `Obfuscation().Xor`
fa5d296e3b refactor: prepare mempool_persist for obfuscation key change
6bbf2d9311 refactor: prepare `DBWrapper` for obfuscation key change
0b8bec8aa6 scripted-diff: unify xor-vs-obfuscation nomenclature
972697976c bench: make ObfuscationBench more representative
618a30e326 test: compare util::Xor with randomized inputs against simple impl
a5141cd39e test: make sure dbwrapper obfuscation key is never obfuscated
54ab0bd64c refactor: commit to 8 byte obfuscation keys
7aa557a37b random: add fixed-size `std::array` generation
b6d4688f77 [doc] reword comments in test_mid_package_replacement
f3a613aa5b [cleanup] delete brittle test_mid_package_eviction
9f713b83dc Merge bitcoin/bitcoin#32837: depends: fix libevent `_WIN32_WINNT` usage
2dfeb6668c wallet: remove outdated `pszSkip` arg of database `Rewrite` func
faaaddaaf8 init: [gui] Avoid UB/crash in InitAndLoadChainstate
8a4cfddf23 wallet: Set migrated wallet name only on success
4f502baf8f doc: add alpine depends build instructions
d89c6fa4a7 wallet: Remove `upgradewallet` RPC
184159e4f3 Merge bitcoin/bitcoin#32922: test: use notarized v28.2 binaries and fix macOS detection
5d17e64a02 Merge bitcoin/bitcoin#32677: test: headers sync timeout
0087ba409b Merge bitcoin/bitcoin#32968: test: fix intermittent failure in rpc_invalidateblock.py
50024620b9 [bench] worst case LimitOrphans and EraseForBlock
45c7a4b56d [functional test] orphan resolution works in the presence of DoSy peers
835f5c77cd [prep/test] restart instead of bumpmocktime between p2p_orphan_handling subtests
b113877545 [fuzz] Add simulation fuzz test for TxOrphanage
03aaaedc6d [prep] Return the made-reconsiderable announcements in AddChildrenToWorkSet
ea29c4371e [p2p] bump DEFAULT_MAX_ORPHANAGE_LATENCY_SCORE to 3,000
24afee8d8f [fuzz] TxOrphanage protects peers that don't go over limit
a2878cfb4a [unit test] strengthen GetChildrenFromSamePeer tests: results are in recency order
7ce3b7ee57 [unit test] basic TxOrphanage eviction and protection
4d23d1d7e7 [cleanup] remove unused rng param from LimitOrphans
067365d2a8 [p2p] overhaul TxOrphanage with smarter limits
1a41e7962d [refactor] create aliases for TxOrphanage Count and Usage
b50bd72c42 [prep] change return type of EraseTx to bool
3da6d7f8f6 [prep/refactor] make TxOrphanage a virtual class implemented by TxOrphanageImpl
77ebe8f280 [prep/test] have TxOrphanage remember its own limits in LimitOrphans
d0af4239b7 [prep/refactor] move DEFAULT_MAX_ORPHAN_TRANSACTIONS to txorphanage.h
51365225b8 [prep/config] remove -maxorphantx
8dd24c29ae [prep/test] modify test to not access TxOrphanage internals
c3cd7fcb2c [doc] remove references to now-nonexistent Finalize() function
d8140f5f05 don't make a copy of m_non_base_coins
98ba2b1db2 [doc] MemPoolAccept coins views
ba02c30b8a [doc] always CleanupTemporaryCoins after a mempool trim
b53fab1467 Merge bitcoin/bitcoin#32948: refactor: cleanup index logging
62ed1f92ef txgraph: check that DoWork finds optimal if given high budget (tests)
f3c2fc867f txgraph: add work limit to DoWork(), try optimal (feature)
fa1fd07468 ci: Enable more shellcheck
e96b00d99e txgraph: make number of acceptable iterations configurable (feature)
cfe9958852 txgraph: track amount of work done in linearization (preparation)
6ba316eaa0 txgraph: 1-or-2-tx split-off clusters are optimal (optimization)
fad0eb091e txgraph: reset quality when merging clusters (bugfix)
61e800e75c test: headers sync timeout
28416f367a test: fix intermittent failure in rpc_invalidateblock.py
e72cb20c3f Merge bitcoin/bitcoin#32943: depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`
97fb46d0a0 Merge bitcoin/bitcoin#32880: ci: Avoid cd into build dir
69b9ad02da Merge bitcoin/bitcoin#32954: cmake: Drop no longer necessary "cmakeMinimumRequired" object
faa3171ff2 ci: Use optimized Debug build type in test-each-commit
fa21c3401e ci: [doc] reword debug log message
12a6959892 cmake: Drop no longer necessary "cmakeMinimumRequired" object
44f5327824 [fuzz] add SeedRandomStateForTest(SeedRand::ZEROS) to txorphan
15a4ec9069 [prep/rpc] remove entry and expiry time from getorphantxs
08e58fa911 [prep/refactor] move txorphanage to node namespace and directory
bb91d23fa9 [txorphanage] change type of usage to int64_t
c18bf0bd9b refactor: cleanup index logging
f5647c6c5a depends: fix libevent _WIN32_WINNT usage
44f3bae300 depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`
fad191ff48 ci: Avoid cd into build dir
4bb4c86599 test: document HOST for get_previous_releases.py
609203d507 test: stop signing previous releases >= v28.2
c6dc2c29f8 test: replace v28.0 with notarized v28.2
5bd73d96a3 test: fix macOS detection
d3b8a54a81 Refactor CFeeRate to use FeeFrac internally
f49840dd90 doc: Fix typo in files.md
f5cf0b1ccc bitcoin wrapper: improve help output
aac0b6dd79 test: test sendall and send do anti-fee-sniping
20802c7b65 wallet, rpc: add anti-fee-sniping to `send` and `sendall`
5fe7915c86 doc: Add musig() example
d576079ab4 tests: Test musig() parsing
a53924bee3 descriptor: Parse musig() key expressions
9473e9606c descriptors: Move DeriveType parsing into its own function
4af0dca096 descriptor: Add MuSigPubkeyProvider
c40dbbbf77 test: Move `script_assets_tests` into its own suite
d00d95437d Add MuSig2 Keyagg Cache helper functions
8ecea91bf2 sign: Add GetMuSig2ParticipantPubkeys to SigningProvider
fac0ee0bfc build: Enable secp256k1 musig module
1894f97503 descriptors: Add PubkeyProvider::IsBIP32()
12bc1d0b1e util/string: Allow Split to include the separator
8811312571 script/parsing: Allow Const to not skip the found constant
e017ef3c7e init: make `-blockmaxweight` startup option debug-only
6135e0553e wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC
c810b168b8 doc: Add description of installed files to files.md
94ffd01a02 doc: Add release notes describing libexec/ binaries
cd97905ebc cmake: Move internal binaries from bin/ to libexec/
ad132761fc [allocators] Apply manual ASan poisoning to PoolResource
5fe4c66462 XOnlyPubKey: Add GetCPubKeys
5332082d00 doc: add alpine build instructions
c6e2c31c55 rpc: unhide waitfor{block,newblock,blockheight}
0786b7509a rpc: add optional blockhash to waitfornewblock
5d82d92aff rpc: reserve space for `UniValue` variables  in `blockToJSON`
6a506d5c37 UniValue: add reserve member function
bd461195f4 bench: support benching all verbosity of `BlockToJson`
REVERT: 1ffc1c9d94 kernel: Fix bitcoin-chainstate for windows
REVERT: 686c4108cc kernel: Add Purpose section to header documentation
REVERT: 8d47a40731 kernel: Add pure kernel bitcoin-chainstate
REVERT: ba84650882 kernel: Add functions to get the block hash from a block
REVERT: a421727342 kernel: Add block index utility functions to C header
REVERT: aedbe73cf0 kernel: Add function to read block undo data from disk to C header
REVERT: 109dda0845 kernel: Add functions to read block from disk to C header
REVERT: 3e24c34ad4 kernel: Add function for copying block data to C header
REVERT: 9ab3d14c1d kernel: Add functions for the block validation state to C header
REVERT: 4408228f85 kernel: Add validation interface to C header
REVERT: 0c3054ef4b kernel: Add interrupt function to C header
REVERT: 45895c4ac7 kernel: Add import blocks function to C header
REVERT: 994c869ba2 kernel: Add chainstate load options for in-memory dbs in C header
REVERT: b4ad47e312 kernel: Add options for reindexing in C header
REVERT: 591b28d615 kernel: Add block validation to C header
REVERT: a1fe6b4264 kernel: Add chainstate loading when instantiating a ChainstateManager
REVERT: 0cf99f827e kernel: Add chainstate manager option for setting worker threads
REVERT: c18b35135c kernel: Add chainstate manager object to C header
REVERT: 1de2db7eac kernel: Add notifications context option to C header
REVERT: b1e6a28d17 kernel: Add chain params context option to C header
REVERT: 369cfd3f6c kernel: Add kernel library context object
REVERT: f9e13dbb1a kernel: Add logging to kernel library C header
REVERT: ce12888287 kernel: Introduce initial kernel C header API

git-subtree-dir: depend/bitcoin
git-subtree-split: 35fced5df783bc79720d8a74b89a5a0a62ebab72
fanquake added a commit that referenced this pull request Aug 21, 2025
0034dcf [doc] man pages for 29.1rc2 (glozow)
eb1574a [build] bump version to 29.1rc2 (glozow)
f9f1ca5 [doc] update release notes (glozow)
9dd7efc [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB (glozow)
bbdab3e [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit (glozow)
da30ca0 [prep/util] help MockMempoolMinFee handle more precise feerates (glozow)
a0ae3fc [prep/test] replace magic number 1000 with respective feerate vars (glozow)
1c1970f [miner] lower default -blockmintxfee to 1sat/kvB (glozow)
3a7e093 [doc] assert that default min relay feerate and incremental are the same (glozow)
567c3ee [test] explicitly check default -minrelaytxfee and -incrementalrelayfee (glozow)
6b5396c [test] RBF rule 4 for various incrementalrelayfee settings (glozow)
03da7af [test] check bypass of minrelay for various minrelaytxfee settings (glozow)
4e3cfa6 [test] check miner doesn't select 0fee transactions (glozow)

Pull request description:

  Backports #33106 and includes final changes for 29.1rc2. Based on current network conditions (in which nodes rejecting 0.1-1sat/vB are missing many transactions), it is recommended to change these policy settings.

  I did not include #32750 because it causes #33177 and I don't foresee any problems; it was just a nice to have.
  For reviewers: the backport is unclean but fairly straightforward. I just had to adapt a test that is no longer in master (#32973) and include `-datacarriersize` in order to pad transaction size (#32406).

ACKs for top commit:
  dergoegge:
    utACK 0034dcf
  marcofleon:
    ACK  0034dcf
  murchandamus:
    crACK 0034dcf
  brunoerg:
    crACK 0034dcf

Tree-SHA512: 1b7540ac3fec5b15cf36926dbf633054f14549d76aa445a2bf042b5667e8637db4f9c21c869af25a0c3f8c7cca6c585d17896d2f7e95a6264c1ff59817446694
@@ -45,8 +45,7 @@ FUZZ_TARGET(wallet_fees, .init = initialize_setup)
}
(void)GetDiscardRate(wallet);

const auto tx_bytes{fuzzed_data_provider.ConsumeIntegral<unsigned int>()};

const auto tx_bytes{fuzzed_data_provider.ConsumeIntegralInRange(0, std::numeric_limits<int32_t>::max())};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: any reason to exclude negative values? They are documented to behave as-if 0 was passed, so it seems fine to keep the fuzz coverage for it? (https://maflcko.github.io/b-c-cov/fuzz.coverage/src/policy/feerate.cpp.gcov.html)

(There is a unit test covering it, so just a nit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negatives values would make the code crash because of the Assume introduced in GetFee

Assume(virtual_bytes >= 0);

$ FUZZ=wallet_fees build_fuzz/bin/fuzz
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2952456953
INFO: Loaded 1 modules   (614477 inline 8-bit counters): 614477 [0x5832503883e0, 0x58325041e42d), 
INFO: Loaded 1 PC tables (614477 PCs): 614477 [0x58325041e430,0x583250d7e900), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
policy/feerate.cpp:22 GetFee: Assertion `virtual_bytes >= 0' failed.
==89225== ERROR: libFuzzer: deadly signal

The addition of that Assertion was recommended here #32750 (comment) and further discussed here #32750 (comment) and here #32750 (review)


Assume(virtual_bytes >= 0);
if (m_feerate.IsEmpty()) { return CAmount(0);}
CAmount nFee = CAmount(m_feerate.EvaluateFeeUp(virtual_bytes));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nano nit: The c-style cast isn't needed and the narrowing check can be enabled:

    CAmount nFee{m_feerate.EvaluateFeeUp(virtual_bytes)};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.