-
Notifications
You must be signed in to change notification settings - Fork 37.7k
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32750. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. LLM Linter (✨ experimental)Possible typos and grammar issues:
drahtbot_id_4_m |
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
There was a problem hiding this 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)
There was a problem hiding this 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.
fdc3d20
to
417bd07
Compare
417bd07
to
ccb53ed
Compare
There was a problem hiding this 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
ccb53ed
to
13efb13
Compare
Concept ACK |
13efb13
to
990010f
Compare
db63d5b removes an unused include and some nit on arg names. |
cc894b5
to
bf63317
Compare
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>
fe2cc17
to
d3b8a54
Compare
d3b8a54 |
There was a problem hiding this 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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK d3b8a54 📦
There was a problem hiding this 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
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); } | ||
SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.m_feerate.fee, obj.m_feerate.size); } |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#22962 ;)
ACK d3b8a54 |
…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
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.
…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
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.
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
…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
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
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())}; |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
bitcoin/src/policy/feerate.cpp
Line 22 in 73220fc
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)); |
There was a problem hiding this comment.
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)};
The
FeeFrac
type represents a fraction, intended to be used forsats/vbyte
orsats/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 encapsulatingFeeFrac
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