Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 duduta opened a pull request: "contrib: Check build options for gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/32949)
This fixes #17506. Let me know what you think.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2200811745)
reworded to make it more clear
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2200811851)
added
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2200811973)
taken
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2200818206)
I've edited the number and the comment. I divide the max block weight by an approximate input weight (I chose 200) and make 1 huge transaction. The test has 20k inputs, but I think typical blocks have single digit thousands?
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2200818690)
reworded, took this as well
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3062452789)
> we should probably be calling LimitOrphans after/during AddAnnouncer()

Yes 👍 done.
💬 maflcko commented on pull request "contrib: Check build options for gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/32949#issuecomment-3062458206)
> This fixes #17506. Let me know what you think.

Have you read the issue? It mentions `HAVE_SYSTEM` and others, but they are not checked here.

Is this llm generated?
💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3062460743)
- Fixed functional test `test_no_args_passed()` to avoid CI failure (`multiprocess, i686, DEBUG`) adapting the code to when `CLI` option call is used (`--usecli`).
📝 duduta converted_to_draft a pull request: "contrib: Check build options for gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/32949)
This fixes #17506. Let me know what you think.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3062529488)
Rebased.

@maflcko

Thank you for the review! Your feedback has been addressed.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2200869238)
Sure. [Reworked](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3062529488).
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2200869809)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3062529488).
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2200870356)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3062529488).
💬 pablomartin4btc commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2200871871)
Maybe you can use `mocktime` here for consistency?

```diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -585,8 +585,10 @@ class WalletMigrationTest(BitcoinTestFramework):
)
assert (self.master_node.wallets_path / "plainfile").is_file()

+ mocked_time = int(time.time())
+ self.master_node.setmocktime(mocked_time)
migrate_
...
💬 fanquake commented on pull request "Enable `-Werror=dev` in CI & Guix":
(https://github.com/bitcoin/bitcoin/pull/32937#issuecomment-3062562004)
Guix build aarch64:
```bash
f73822833d8c3ff2b2dda2ddc5e16cb1794aa4eb299b1f468fa8dbadfd6b2310 guix-build-8f766f39df3e/output/aarch64-linux-gnu/SHA256SUMS.part
35783f79defa8b4032b78443a28e88a9c11314470fc177afa9e21d0e613bd937 guix-build-8f766f39df3e/output/aarch64-linux-gnu/bitcoin-8f766f39df3e-aarch64-linux-gnu-debug.tar.gz
b0345a28e82514d51cb363d798d950e44e630ba357a5e522537e1a7b4a37a151 guix-build-8f766f39df3e/output/aarch64-linux-gnu/bitcoin-8f766f39df3e-aarch64-linux-gnu.tar.gz
e476a91c
...
💬 dergoegge commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3062591459)
Drafted a branch for the `notfound` approach here: https://github.com/dergoegge/bitcoin/commits/2025-07-blk-notfound/. I think I'd slightly prefer it because:

* we wouldn't propagate invalid blocks (this does not really matter that much but why do it if we can avoid it)
* we wouldn't get rid of the timeout punishment

Happy to review if someone wants to run with it (haven't tested my branch and it's missing a functional test).
💬 duduta commented on pull request "contrib: Check build options for gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/32949#issuecomment-3062594803)
@maflcko Oh, sorry. I should have asked first.
Right now, if one builds, for example, with ZMQ, the bitcoind --help reflects that, which is reflected in the man pages that are generated for bitcoind. The same is true for HAVE_SYSTEM (-alertnotify does appear in the man pages generated for bitcoind).
As for USE_UPNP, I have grepped for it, couldn't even find it in the sources.
So I did a first step at least: to check config.ini for the list of binaries that should have been built.
I am ope
...
🤔 rkrux reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3010620105)
LGTM ACK d9ca23fb6f3ce6d4863583226f5ae967a7c49497

I have seen more usages of `avoid_reuse: false` than its `true` variant (which has only 1 usage) and every time I have to mentally translate it to "get all/full amount". Maybe a `bool cachable_amount_type` might have a place here with `ALL` and `AVOID_REUSE` values equalling to `false` and `true` respectively. But not suggesting this change because I notice that the term `avoid_reuse` is quite embedded in the wallet codebase already in terms o
...
📝 stratospher opened a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950)
Fixes https://github.com/bitcoin/bitcoin/issues/32173

even though we have a distinction between `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` in the codebase,
we don't use it for anything. Whenever we check for BlockStatus, we use `BLOCK_FAILED_MASK` which encompasses both of them.

Since there is no functional difference between `BLOCK_FAILED_VALID` and `BLOCK_FAILED_CHILD` and it's added
code complexity to correctly categorise them (ex: https://github.com/bitcoin/bitcoin/pull/31405#disc
...