Commit Graph

45900 Commits

Author SHA1 Message Date
Eugene Siegel
5dda364c4b test: modify logging_filesize_rate_limit params
Change time_window from 20s to 1h so Reset is not accidentally called
if the test takes a while.

Change num_lines from 1024 to 10 since LogRateLimiter is parameterized
and does not require logging 1MiB of data.

Co-Authored-By: stickies-v <stickies-v@protonmail.com>
2025-08-19 10:25:30 -04:00
Hennadii Stepanov
d31dc8f818 Merge bitcoin/bitcoin#33200: cmake: Introduce translate.cmake script for translate target
05255d5d1e cmake: Drop dependency on sed for translate target (Daniel Pfeifer)
d5054beca5 cmake: Introduce translate.cmake script for translate target (Daniel Pfeifer)

Pull request description:

  Using `file(GLOB)` in the generates step is discouraged because the globbing result may be out of date when the target is built. Performing the globbing in a script that is executed as the build target means the result is always reproducable and the overhead of globbing is only paid when used.

  As a follow up, the dependency on `sed` may be removed by performing the replacement with cmake. Also, the logic from extract_strings_qt.py can be migrated to cmake.

ACKs for top commit:
  hebasto:
    ACK 05255d5d1e.

Tree-SHA512: ae55d9199e6294109b37e5e18f21f2b0e582c1f9903421cf22a237cfdbd215cc431706563b3caa03068cdba79f936b019526638fe3a1f83b4f01a72817e39be1
2025-08-18 11:54:21 +01:00
Daniel Pfeifer
05255d5d1e cmake: Drop dependency on sed for translate target 2025-08-18 09:03:34 +02:00
Daniel Pfeifer
d5054beca5 cmake: Introduce translate.cmake script for translate target
Using `file(GLOB)` in the generates step is discouraged because the
globbing result may be out of date when the target is built.
Performing the globbing in a script that is executed as the build
target means the result is always reproducable and the overhead
of globbing is only paid when used.

As a follow up, the dependency on `sed` may be removed by performing
the replacement with cmake. Also, the logic from extract_strings_qt.py
can be migrated to cmake.
2025-08-18 09:03:28 +02:00
Ava Chow
57e8f34fe2 Merge bitcoin/bitcoin#32977: wallet: Remove wallet version and several legacy related functions
60d1042b9a wallet: Remove unused `WalletFeature` enums (woltx)
66de58208a wallet: Remove `CWallet::nWalletVersion` and related functions (woltx)
7cda3d0f5b wallet: Remove `IsFeatureSupported()` and `CanSupportFeature()` (woltx)
ba01585229 wallet: `MigrateToDescriptor` no longer calls `CanSupportFeature` (woltx)
63acee2797 wallet: Remove `GetClosestWalletFeature()` (woltx)
e27da3150b wallet: Remove `GetVersion()` (woltx)

Pull request description:

  This PR incorporates the suggestion provided by PRabahy and pablomartin4btc in https://github.com/bitcoin/bitcoin/pull/32944 of removing `CWallet::nWalletVersion` and several related functions, such as `SetMinVersion()`, `GetVersion()`, `GetClosestWalletFeature()`, `IsFeatureSupported()`, `CanSupportFeature()`, etc ...

  This field is no longer used in the descriptor wallet and there is still a lot of code related to it, so the changes here provide a good cleanup in the wallet code.

  Built on top of https://github.com/bitcoin/bitcoin/pull/32944

ACKs for top commit:
  maflcko:
    review ACK 60d1042b9a 🐾
  achow101:
    ACK 60d1042b9a
  pablomartin4btc:
    ACK 60d1042b9a

Tree-SHA512: 1a7ad8e15d57df8f66545776e7d178a2cd5312c87769a29770588375e3de5f24247aab9919acf004ed3eca16d08ba595b5f1c7b2b3eef7752e89d9c295624583
2025-08-15 16:38:19 -07:00
Ava Chow
97593c1fd3 Merge bitcoin/bitcoin#32975: assumevalid: log every script validation state change
fab2980bdc assumevalid: log every script validation state change (Lőrinc)

Pull request description:

  The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download.
  Many new [users are surprised](https://github.com/bitcoin/bitcoin/issues/32832) when this suddenly slows their node to a halt.
  This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa).

  <details>
  <summary>Testing instructions</summary>

  The behavior can easily be tested by adding this before the new log:
  ```C++
      // TODO hack to enable/disable script checks based on block height for testing purposes
           if (pindex->nHeight < 100) fScriptChecks = false;
      else if (pindex->nHeight < 200) fScriptChecks = true;
      else if (pindex->nHeight < 300) fScriptChecks = false;
      else if (pindex->nHeight < 400) fScriptChecks = true;
  ```
  and exercise the new code with:
  ```bash
  cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation'
  ```
  showing something like:
  * Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048).
  * Enabling signature validations at block #100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a).
  * Disabling signature validations at block #200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320).
  * Enabling signature validations at block #300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609).

  </details>

ACKs for top commit:
  achow101:
    ACK fab2980bdc
  ajtowns:
    crACK fab2980bdc
  davidgumberg:
    untested crACK fab2980bdc

Tree-SHA512: e90b66f7423b639356daace476942ce83e65e70466544394cbe2f15738bdbf716163eaf590c64c5448f9b41aeeaafe3342c48c6a7a478678a70b0310ca94e11d
2025-08-15 13:54:09 -07:00
merge-script
7b4a1350df Merge bitcoin/bitcoin#33183: validation: rename block script verification error from "mandatory" to "block"
c0d91fc69c Add release note for #33050 and #33183 error string changes (Antoine Poinsot)
b3f781a0ef contrib: adapt max reject string size in tracing demo (Antoine Poinsot)
9a04635432 scripted-diff: validation: rename mandatory errors into block errors (Antoine Poinsot)

Pull request description:

  This is a followup to #33050 now that it's merged. Using "block"/"mempool" as the error reason is clearer to a user than "mandatory"/"non-mandatory". The "non-mandatory" errors got renamed to "mempool" in #33050 (see https://github.com/bitcoin/bitcoin/pull/33050#discussion_r2230103371). This takes care of the second part of the renaming.

ACKs for top commit:
  fjahr:
    utACK c0d91fc69c
  davidgumberg:
    lgtm ACK c0d91fc69c
  ajtowns:
    utACK c0d91fc69c
  Crypt-iQ:
    utACK c0d91fc69c
  janb84:
    utACK c0d91fc69c
  instagibbs:
    ACK c0d91fc69c

Tree-SHA512: b463e633c57dd1eae7c49d23239a59066a672f355142ec194982eddc927a7646bc5cde583dc8d6f45075bf5cbb96dbe73f7e339e728929b0eff356b674d1b68c
2025-08-15 12:13:38 +01:00
merge-script
c99f5c5e1b Merge bitcoin/bitcoin#33106: policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee
ba84a25dee [doc] update mempool-replacements.md for incremental relay feerate change (glozow)
18720bc5d5 [doc] release note for min feerate changes (glozow)
6da5de58ca [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB (glozow)
2e515d2897 [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit (glozow)
457cfb61b5 [prep/util] help MockMempoolMinFee handle more precise feerates (glozow)
3eab8b7240 [prep/test] replace magic number 1000 with respective feerate vars (glozow)
5f2df0ef78 [miner] lower default -blockmintxfee to 1sat/kvB (glozow)
d6213d6aa1 [doc] assert that default min relay feerate and incremental are the same (glozow)
1fbee5d7b6 [test] explicitly check default -minrelaytxfee and -incrementalrelayfee (glozow)
72dc18467d [test] RBF rule 4 for various incrementalrelayfee settings (glozow)
85f498893f [test] check bypass of minrelay for various minrelaytxfee settings (glozow)
e5f896bb1f [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.
  - https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3155627414
  - 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](https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3095260286) and [this comment](https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3142444090)
  - 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: https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3156213089

  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 ba84a25dee
  gmaxwell:
    ACK ba84a25dee
  jsarenik:
    Tested ACK ba84a25dee
  darosior:
    ACK ba84a25dee
  ajtowns:
    ACK ba84a25dee
  davidgumberg:
    crACK  ba84a25dee
  w0xlt:
    ACK ba84a25dee
  caesrcd:
    reACK ba84a25dee
  ismaelsadeeq:
    re-ACK ba84a25dee

Tree-SHA512: b4c35e8b506b1184db466551a7e2e48bb1e535972a8dbcaa145ce3a8bfdcc70a8807dc129460f129a9d31024174d34077154a387c32f1a3e6831f6fa5e9c399e
2025-08-15 10:39:16 +01:00
Ava Chow
578b512bdd Merge bitcoin/bitcoin#33011: log: rate limiting followups
5c74a0b397 config: add DEBUG_ONLY -logratelimit (Eugene Siegel)
9f3b017bcc test: logging_filesize_rate_limit improvements (stickies-v)
350193e5e2 test: don't leak log category mask across tests (stickies-v)
05d7c22479 test: add ReadDebugLogLines helper function (stickies-v)
3d630c2544 log: make m_limiter a shared_ptr (stickies-v)
e8f9c37a3b log: clean up LogPrintStr_ and Reset, prefix all logs with "[*]" when there are suppressions (Eugene Siegel)
3c7cae49b6 log: change LogLimitStats to struct LogRateLimiter::Stats (Eugene Siegel)
8319a13468 log: clarify RATELIMIT_MAX_BYTES comment, use RATELIMIT_WINDOW (Eugene Siegel)
5f70bc80df log: remove const qualifier from arguments in LogPrintFormatInternal (Eugene Siegel)
b8e92fb3d4 log: avoid double hashing in SourceLocationHasher (Eugene Siegel)
616bc22f13 test: remove noexcept(false) comment in ~DebugLogHelper (Eugene Siegel)

Pull request description:

  Followups to #32604.

  There are two behavior changes:
  - prefixing with `[*]` is done to all logs (regardless of `should_ratelimit`) per [this comment](https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2195710943).
  - a DEBUG_ONLY `-disableratelimitlogging` flag is added by default to functional tests so they don't encounter rate limiting.

ACKs for top commit:
  stickies-v:
    re-ACK 5c74a0b397
  achow101:
    ACK 5c74a0b397
  l0rinc:
    Code review ACK 5c74a0b397

Tree-SHA512: d32db5fcc28bb9b2a850f0048c8062200a3725b88f1cd9a0e137da065c0cf9a5d22e5d03cb16fe75ea7494801313ab34ffec7cf3e8577cd7527e636af53591c4
2025-08-14 15:15:25 -07:00
Ava Chow
8405fdb06e Merge bitcoin/bitcoin#33169: interfaces, chain, refactor: Remove unused getTipLocator and incaccurate getActiveChainLocator
2b00030af8 interfaces, chain, refactor: Remove inaccurate getActiveChainLocator (pablomartin4btc)
110a0f405c interfaces, chain, refactor: Remove unused getTipLocator (pablomartin4btc)

Pull request description:

  Remove `Chain::getTipLocator`, `Chain::GetLocator()`, and `Chain::getActiveChainLocator`:
  - `Chain::getTipLocator` is no longer used.
  - `Chain::GetLocator`, replaced its call by `GetLocator()`, which uses `LocatorEntries`, avoiding direct access to the chain itself (change suggested by l0rinc while reviewing this PR to maintain consistency with the overall refactoring).
  - `Chain::getActiveChainLocator`, whose name was misleading, has functionality redundant with Chain::findBlock.
    - Additionally, the comment for getActiveChainLocator became inaccurate following changes in commit ed470940cd (from PR #25717).

  This is a [follow-up](https://github.com/bitcoin/bitcoin/pull/29652#issuecomment-3151665095) to #29652.

ACKs for top commit:
  achow101:
    ACK 2b00030af8
  furszy:
    ACK 2b00030af8
  stickies-v:
    ACK 2b00030af8
  w0xlt:
    ACK 2b00030af8

Tree-SHA512: b12ba6a15feeaeec692d69204a6e155e3af43edfac25597dabf14cacca1e4a2152574816e58dc544f39043c5721f5e707acf544f4541d3b9c0f8c0c40069215e
2025-08-14 11:30:45 -07:00
Antoine Poinsot
c0d91fc69c Add release note for #33050 and #33183 error string changes 2025-08-14 14:11:04 -04:00
Ava Chow
e17b5da0d6 Merge bitcoin/bitcoin#33179: doc: update wallet build instruction
67e186deb0 doc: update wallet build instruction (Sjors Provoost)

Pull request description:

  Sqlite and the wallet are no longer optional, but they can still be opted out of. This PR updates the build instructions accordingly.

  Updating this text now reduces churn in #31802.

ACKs for top commit:
  jonatack:
    ACK 67e186deb0
  achow101:
    ACK 67e186deb0
  w0xlt:
    reACK 67e186deb0
  pablomartin4btc:
    ACK 67e186deb0
  janb84:
    ACK 67e186deb0

Tree-SHA512: ea87a83c9fcb884f9ca380a006a93ede3cd1af5eb2bb5ccb54fc8124faa532e9473ae2461d775439da59e86178fbd04983113b43ce3d4c382cabe27866acd027
2025-08-14 11:06:24 -07:00
merge-script
9b1a7c3e8d Merge bitcoin/bitcoin#33116: refactor: Convert uint256 to Txid
de0675f9de refactor: Move `transaction_identifier.h` to primitives (marcofleon)
6f068f65de Remove implicit uint256 conversion and comparison (marcofleon)
9c24cda72e refactor: Convert remaining instances from uint256 to Txid (marcofleon)
d2ecd6815d policy, refactor: Convert uint256 to Txid (marcofleon)
f6c0d1d231 mempool, refactor: Convert uint256 to Txid (marcofleon)
aeb0f78330 refactor: Convert `mini_miner` from uint256 to Txid (marcofleon)
326f244724 refactor: Convert RPCs and `merkleblock` from uint256 to Txid (marcofleon)
49b3d3a92a Clean up `FindTxForGetData` (marcofleon)

Pull request description:

  This is the final leg of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).

  All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.

ACKs for top commit:
  stickies-v:
    re-ACK de0675f9de, no changes since a20724d926 except address review nits.
  janb84:
    re ACK de0675f9de
  dergoegge:
    re-ACK de0675f9de
  theStack:
    Code-review ACK de0675f9de

Tree-SHA512: 2413160fca7ab146a8d79d18ce3afcf7384cacc73c513d41928904aa453b4dd7a350064cee71e9c5d015da5904c7c81ac17603e50a47441ebc5b0c653235dd08
2025-08-13 14:50:51 -04:00
Antoine Poinsot
b3f781a0ef contrib: adapt max reject string size in tracing demo
The Script errors were last touched in 2020. This value was calculated after that
in 2022 (commit 4b7aec2951). The previous commit
made the size of the largest reject reason string 4 characters smaller ("mandatory"
became "block"), so adapt the constant.
2025-08-13 11:06:01 -04:00
Antoine Poinsot
9a04635432 scripted-diff: validation: rename mandatory errors into block errors
Using "block" or "mempool" as the prefix in place of "mandatory" or "non-mandatory" is clearer
to a user. "non-mandatory" was renamed into "mempool" as part of #33050. This takes care of the
other half of this renaming as a scripted diff.

-BEGIN VERIFY SCRIPT-
sed -i 's/mandatory-script-verify/block-script-verify/g' $(git grep -l mandatory-script-verify)
-END VERIFY SCRIPT-
2025-08-13 11:05:54 -04:00
merge-script
dbf8b0980b Merge bitcoin/bitcoin#33171: ci: Update actions/checkout version
f83c01d882 ci: Update `actions/checkout` version (Hennadii Stepanov)

Pull request description:

  See: https://github.com/actions/checkout/releases/tag/v5.0.0.

ACKs for top commit:
  bc1cindy:
    ACK f83c01d8
  pablomartin4btc:
    ACK f83c01d882
  janb84:
    ACK f83c01d882

Tree-SHA512: 491652abcb99f6e5c9a7bc66867c5e5fc7032d35d091cdbef5cdf7ed125bd93e612c058a771373e4a67ddc320b3bb8e22a6294aeade73df0f24b5584df8e47ab
2025-08-13 10:51:46 +01:00
merge-script
d6887f0cec Merge bitcoin/bitcoin#33178: guix: increase maximum allowed (runtime) GCC to 7
776a163374 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 776a163374.

Tree-SHA512: 8e5a77c509eb6164314fdb644ea199916e151eb0c7f48703f3a2bdedf0dea29b7f402ceacb2aaf42ebffba59080cefbb84253b2721047d973a851090447ba3b5
2025-08-13 09:14:04 +01:00
pablomartin4btc
2b00030af8 interfaces, chain, refactor: Remove inaccurate getActiveChainLocator
The getActiveChainLocator method name was misleading, and its functionality
duplicated `Chain::findBlock`. This commit removes the method and replaces
all its usages with direct `Chain::findBlock` calls.

Additionally, the comment of getActiveChainLocator has been outdated since
commit ed47094 from #25717.

Finally, in CWallet::ScanForWalletTransactions, the findBlock calls are now
unified into a single call at the start of the function.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Matias Furszyfer <mfurszy@protonmail.com>
2025-08-13 00:21:17 -03:00
pablomartin4btc
110a0f405c interfaces, chain, refactor: Remove unused getTipLocator
Also removed CChain::GetLocator() and replaced its call
with GetLocator() which uses LocatorEntries instead.

Co-authored-by: ryanofsky <ryan@ofsky.org>
Co-authored-by: l0rinc <l0rinc@users.noreply.github.com>
2025-08-13 00:08:37 -03:00
Ava Chow
dadf15f88c Merge bitcoin/bitcoin#33050: net, validation: don't punish peers for consensus-invalid txs
876dbdfb47 tests: drop expect_disconnect behaviour for tx relay (Anthony Towns)
b29ae9efdf validation: only check input scripts once (Anthony Towns)
266dd0e10d net_processing: drop MaybePunishNodeForTx (Anthony Towns)

Pull request description:

  Because we do not discourage nodes for transactions we consider non-standard, we don't get any DoS protection from this check in adversarial scenarios, so remove the check entirely both to simplify the code and reduce the risk of splitting the network due to changes in tx relay policy.

  Then, because we no longer make use of the distinction between consensus and standardness failures during script validation, don't re-validate each script with only-consensus rules, reducing the cost to us of transactions that we won't relay.

ACKs for top commit:
  achow101:
    ACK 876dbdfb47
  darosior:
    re-ACK 876dbdfb47
  sipa:
    re-ACK 876dbdfb47
  glozow:
    ACK 876dbdfb47

Tree-SHA512: 8bb0395766dde54fc48f7077b80b88e35581aa6e3054d6d65735965147abefffa7348f0850bb3d46f6c2541fd384ecd40a00a57fa653adabff8a35582e2d1811
2025-08-12 14:35:18 -07:00
Ava Chow
73972d5617 Merge bitcoin/bitcoin#31296: wallet: Translate [default wallet] string in progress messages
db225cea56 wallet, refactor: Replace GetDisplayName() with LogName() (Ryan Ofsky)
01737883b3 wallet: Translate [default wallet] string in progress messages (Ryan Ofsky)

Pull request description:

  Noticed while reviewing https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843809721 that the [default wallet] part of progress messages remains untranslated while the rest of the string is translated.

  Fix this in all places where `CWallet::ShowProgress` (which has a cancel button) and `Chain::showProgress` (which doesn't have a cancel button) are called by making "default wallet" into a translated string.

ACKs for top commit:
  achow101:
    ACK db225cea56
  pablomartin4btc:
    ACK db225cea56
  furszy:
    utACK db225cea56

Tree-SHA512: 3e76e22ee692a7403d61c66615f56d0fa5f7883dd47553bcaec2f9ffd942daaa90ceb61830206bece50da53dcd737b6438c36bcb086030b2deb68c44172f3931
2025-08-12 11:33:42 -07:00
Sjors Provoost
67e186deb0 doc: update wallet build instruction
Sqlite is expected to be installed, but can still be opted out of.
2025-08-12 19:39:07 +02:00
Eugene Siegel
5c74a0b397 config: add DEBUG_ONLY -logratelimit
Use -nologratelimit by default in functional tests if the bitcoind
version supports it.

Co-Authored-By: stickies-v <stickies-v@protonmail.com>
2025-08-12 11:28:36 -04:00
stickies-v
9f3b017bcc test: logging_filesize_rate_limit improvements
- Add helper functions and structs to improve readability and
  reusability of test code
- Make tests more specific by comparing all produced log lines with
  expected log lines instead of relying on approximations or proxies.
2025-08-12 11:28:36 -04:00
stickies-v
350193e5e2 test: don't leak log category mask across tests
This ensures log tests behave consistently when other tests modify
the log category mask.
2025-08-12 11:28:36 -04:00
stickies-v
05d7c22479 test: add ReadDebugLogLines helper function
Deduplicates repeated usage of the same functionality.
2025-08-12 11:28:36 -04:00
stickies-v
3d630c2544 log: make m_limiter a shared_ptr
This allows us to safely and explicitly manage the dual dependency
on the limiter: one for the Logger, and one for the CScheduler.
2025-08-12 11:28:36 -04:00
merge-script
ec484bd5ce Merge bitcoin/bitcoin#31453: util: detect and warn when using exFAT on MacOS
db3228042b util: detect and warn when using exFAT on macOS (willcl-ark)

Pull request description:

  exFAT is known to cause intermittent corruption on MacOS.

  Therefore we should warn when using this fs format for either the blocks or data directories.

  See #28552 for more context.

ACKs for top commit:
  l0rinc:
    ACK db3228042b
  marcofleon:
    reACK db3228042b
  ismaelsadeeq:
    reACK db3228042b

Tree-SHA512: e4453a8e24b35c135e4eb0b4e47fe0c80f8b54700f458909c403aa37a0d2979ee165347bcd76e48e4d1ae5d3bae13f50e6afe714e33226a52f907b95df9d3b46
2025-08-12 10:23:13 -04:00
fanquake
776a163374 guix: increase maximum allowed (runtime) GCC to 7
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.
2025-08-12 14:45:03 +01:00
glozow
ba84a25dee [doc] update mempool-replacements.md for incremental relay feerate change 2025-08-12 09:22:53 -04:00
Ava Chow
273e600e65 Merge bitcoin/bitcoin#33021: test/refactor: revive test verifying that GetCoinsCacheSizeState switches from OK→LARGE→CRITICAL
554befd873 test: revive `getcoinscachesizestate` (Lőrinc)
64ed0fa6b7 refactor: modernize `LargeCoinsCacheThreshold` (Lőrinc)
1b40dc02a6 refactor: extract `LargeCoinsCacheThreshold` from `GetCoinsCacheSizeState` (Lőrinc)

Pull request description:

  After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` [always ended the test early](https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html#L65):

  | File                         | Line Rate | Line Total | Line Hit | Branch Rate | Branch Total | Branch Hit |
  |------------------------------|---------:|-----------:|---------:|------------:|-------------:|-----------:|
  | validation_flush_tests.cpp   | **31.5 %**   | 54         | 17       | 22.3 %      | 242          | 54         |

  The test revival was [extracted from a related PR](https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797) where it was [discovered](https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2044004503).

ACKs for top commit:
  achow101:
    ACK 554befd873
  LarryRuane:
    ACK 554befd873
  w0xlt:
    ACK 554befd873

Tree-SHA512: f5057254de8fb3fa627dd20fee6818cfadeb2e9f629f9972059ad7b32e01fcd7dc9922eff9da2d363b36a9f0954d9bc1c4131d47b2a9c6cc348d9864953b91be
2025-08-11 15:15:53 -07:00
glozow
18720bc5d5 [doc] release note for min feerate changes 2025-08-11 17:08:39 -04:00
glozow
6da5de58ca [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB
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 commercial services is 10c/GB, that's like 1-4c per kvB
of transaction data, so a 1000vB transaction should pay at least $0.04.

At a price of 120k USD/BTC, 100sat is about $0.12. This price allows us
to tolerate a large decrease in the conversion rate or increase in the
number of nodes.
2025-08-11 17:07:43 -04:00
glozow
2e515d2897 [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit 2025-08-11 16:58:26 -04:00
glozow
457cfb61b5 [prep/util] help MockMempoolMinFee handle more precise feerates
Use a virtual size of 1000 to keep precision when using a feerate
(which is rounded to the nearest satoshi per kvb) that isn't just an
integer.
2025-08-11 16:58:26 -04:00
glozow
3eab8b7240 [prep/test] replace magic number 1000 with respective feerate vars 2025-08-11 16:58:26 -04:00
glozow
5f2df0ef78 [miner] lower default -blockmintxfee to 1sat/kvB
Back when we implemented coin age priority as a miner policy, miners
mempools might admit transactions paying very low fees, but then want to
set a higher fee for block inclusion. However, since coin age priority
was removed in v0.15, the block assembly policy is solely based on fees,
so we do not need to apply minimum feerate rules in multiple places. In
fact, the block assembly policy ignoring transactions that are added to
the mempool is likely undesirable as we waste resources accepting and
storing this transaction.

Instead, rely on mempool policy to enforce a minimum entry feerate to
the mempool (minrelaytxfee). Set the minimum block feerate to the
minimum non-zero amount (1sat/kvB) so it collects everything it finds in
mempool into the block.
2025-08-11 16:58:26 -04:00
glozow
d6213d6aa1 [doc] assert that default min relay feerate and incremental are the same 2025-08-11 16:58:26 -04:00
glozow
1fbee5d7b6 [test] explicitly check default -minrelaytxfee and -incrementalrelayfee 2025-08-11 16:58:21 -04:00
glozow
72dc18467d [test] RBF rule 4 for various incrementalrelayfee settings 2025-08-11 16:48:56 -04:00
glozow
85f498893f [test] check bypass of minrelay for various minrelaytxfee settings 2025-08-11 16:46:22 -04:00
glozow
e5f896bb1f [test] check miner doesn't select 0fee transactions 2025-08-11 16:44:54 -04:00
marcofleon
de0675f9de refactor: Move transaction_identifier.h to primitives
Moves the file from `src/util` to `src/primitives`. Now that the
refactor is complete, Txid and Wtxid are fundamental types, so it
makes sense for them to reside in `src/primitives`.
2025-08-11 16:47:51 +01:00
marcofleon
6f068f65de Remove implicit uint256 conversion and comparison 2025-08-11 16:47:51 +01:00
marcofleon
9c24cda72e refactor: Convert remaining instances from uint256 to Txid
These remaining miscellaneous changes were identified by commenting out
the `operator const uint256&` conversion and the `Compare(const uint256&)`
method from `transaction_identifier.h`.
2025-08-11 16:47:43 +01:00
marcofleon
d2ecd6815d policy, refactor: Convert uint256 to Txid 2025-08-11 16:28:59 +01:00
marcofleon
f6c0d1d231 mempool, refactor: Convert uint256 to Txid 2025-08-11 16:26:35 +01:00
marcofleon
aeb0f78330 refactor: Convert mini_miner from uint256 to Txid 2025-08-11 16:12:42 +01:00
marcofleon
326f244724 refactor: Convert RPCs and merkleblock from uint256 to Txid 2025-08-11 15:53:34 +01:00
merge-script
41642d43b3 Merge bitcoin/bitcoin#33162: test: fix scripts in blockfilter_basic_test
ca64b71ed5 test: fix scripts in `blockfilter_basic_test` (UdjinM6)

Pull request description:

  `std::vector` fill ctor is like this:
  ```
  // Constructs a vector with `count` copies of elements with value `value`.
  explicit vector( size_type count, const T& value = T(), const Allocator& alloc = Allocator() ); // (until C++11)
  vector( size_type count, const T& value, const Allocator& alloc = Allocator() ); // (since C++11)(constexpr since C++20)
  ```
  https://en.cppreference.com/w/cpp/container/vector/vector.html

  i.e. `std::vector<unsigned char>(0, 65)` means a vector with `0` copies of `65` which feels wrong. I believe `count` and `value` were swapped in `blockfilter_basic_test` scripts.

ACKs for top commit:
  furszy:
    ACK ca64b71ed5
  pablomartin4btc:
    ACK ca64b71ed5
  janb84:
    ACK ca64b71ed5

Tree-SHA512: 2cfc7f09788b0a1afdffc9cd6663204c7f1775dabdbe1046cdcd42936c479658c348cb46e0d8835645e6c508e8b40a598cbe6534084b6780a6b60378bcbd0f96
2025-08-11 10:44:57 -04:00