π¬ 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_
...
(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
...
(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).
(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
...
(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
...
(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
...
(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
...
π rkrux approved a pull request: "refactor: use options struct for signing and PSBT operations"
(https://github.com/bitcoin/bitcoin/pull/32876#pullrequestreview-3010658272)
LGTM ACK 70925416317683b0a62a9c3b45df6385e9c5b292
Reviewed range-diff this time, thanks for incorporating minor points:
```
git range-diff d94c02e...7092541
```
(https://github.com/bitcoin/bitcoin/pull/32876#pullrequestreview-3010658272)
LGTM ACK 70925416317683b0a62a9c3b45df6385e9c5b292
Reviewed range-diff this time, thanks for incorporating minor points:
```
git range-diff d94c02e...7092541
```
π¬ rkrux commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2200957447)
> introduce two different names for the same thing.
Yeah, this is what I had in mind & would prefer this over breaking change with the RPC. But fine to leave it as-is to avoid more churn.
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2200957447)
> introduce two different names for the same thing.
Yeah, this is what I had in mind & would prefer this over breaking change with the RPC. But fine to leave it as-is to avoid more churn.
π¬ 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3062734539)
@jonasschnelli
The copyright notice and MIT license were removed from the SVG file because "Bitboy," the original designer of the Bitcoin logo, released the work into the public domain. This is further confirmed by Wikimedia Commons, where the Bitcoin logo is explicitly marked as public domainβa fact that has been verified multiple times.
The goal of this PR is not just to correct the licensing information but also to provide optimized logo files for all operating systems, ensuring they use
...
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3062734539)
@jonasschnelli
The copyright notice and MIT license were removed from the SVG file because "Bitboy," the original designer of the Bitcoin logo, released the work into the public domain. This is further confirmed by Wikimedia Commons, where the Bitcoin logo is explicitly marked as public domainβa fact that has been verified multiple times.
The goal of this PR is not just to correct the licensing information but also to provide optimized logo files for all operating systems, ensuring they use
...
π¬ TheCharlatan commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#issuecomment-3062757072)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32950#issuecomment-3062757072)
Concept ACK
π¬ hebasto commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3062792405)
> The goal of this PR is not just to correct the licensing information ...
Please update the PR description accordingly.
> Given that the current files are 12β13 years old, itβs time for an update.
Thatβs not a justification for the changes, neither from a technical perspective nor a design one.
NACK from me.
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3062792405)
> The goal of this PR is not just to correct the licensing information ...
Please update the PR description accordingly.
> Given that the current files are 12β13 years old, itβs time for an update.
Thatβs not a justification for the changes, neither from a technical perspective nor a design one.
NACK from me.
π¬ ryanofsky commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2201222699)
Just an idea, and not necessarily a suggestion for this PR since it could be a followup, but I was thinking since `httpserver.h` and `httpserver.cpp` files are basically rewritten here it could also be good to rename them to `rpc/http/server.h` and `rpc/http/server.cpp` and put them in a matching `rpc::http` namespace. Reasons for suggesting this:
- To take some files out of the top level directory.
- To choose a namespace name `rpc::http` that matches the directory path `rpc/http`.
- To ma
...
(https://github.com/bitcoin/bitcoin/pull/32061#discussion_r2201222699)
Just an idea, and not necessarily a suggestion for this PR since it could be a followup, but I was thinking since `httpserver.h` and `httpserver.cpp` files are basically rewritten here it could also be good to rename them to `rpc/http/server.h` and `rpc/http/server.cpp` and put them in a matching `rpc::http` namespace. Reasons for suggesting this:
- To take some files out of the top level directory.
- To choose a namespace name `rpc::http` that matches the directory path `rpc/http`.
- To ma
...
π€ hebasto reviewed a pull request: "Enable `-Werror=dev` in CI & Guix"
(https://github.com/bitcoin/bitcoin/pull/32937#pullrequestreview-3011083019)
My Guix build:
```
aarch64
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
...
(https://github.com/bitcoin/bitcoin/pull/32937#pullrequestreview-3011083019)
My Guix build:
```
aarch64
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
...
π theStack approved a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-3011294284)
Code-review ACK a60f863d3e276534444571282f432b913d3967db
(https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-3011294284)
Code-review ACK a60f863d3e276534444571282f432b913d3967db
π¬ instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3063139475)
@dergoegge a node can still avoid punishment by announcing the sending a `notfound` after 9m59s, but this may be better behaved average behavior. Might be worth bringing up on an irc meeting to get some more eyes on this problem and potential solution?
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3063139475)
@dergoegge a node can still avoid punishment by announcing the sending a `notfound` after 9m59s, but this may be better behaved average behavior. Might be worth bringing up on an irc meeting to get some more eyes on this problem and potential solution?
π¬ 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3063162709)
@hebasto Long live the Russian Federation! π·πΊ I hope they kill all Ukranians who are nationalist scumbags like yourself. I see you added the Ukranian flag to your profile. Shane on you. I know you love the nazi Stepan Bandera. Better you stop thinking you running Bitcoin Core and you can decide what can be merged or not. I need other reviewers who doesn't come from Ukraine. By the way, why you'rr not on the front lines? All Ukranian man required to join the military.
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3063162709)
@hebasto Long live the Russian Federation! π·πΊ I hope they kill all Ukranians who are nationalist scumbags like yourself. I see you added the Ukranian flag to your profile. Shane on you. I know you love the nazi Stepan Bandera. Better you stop thinking you running Bitcoin Core and you can decide what can be merged or not. I need other reviewers who doesn't come from Ukraine. By the way, why you'rr not on the front lines? All Ukranian man required to join the military.
π¬ KATHI160 commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3063165275)
> Maybe this is [#32524](https://github.com/bitcoin/bitcoin/issues/32524), but with N=1 and no way to reproduce, this seems difficult to debug.
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3063165275)
> Maybe this is [#32524](https://github.com/bitcoin/bitcoin/issues/32524), but with N=1 and no way to reproduce, this seems difficult to debug.
π glozow merged a pull request: "refactor: Convert GenTxid to `std::variant`"
(https://github.com/bitcoin/bitcoin/pull/32631)
(https://github.com/bitcoin/bitcoin/pull/32631)
π¬ fanquake commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3063205687)
@1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw that kind of commentary is unacceptable.
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3063205687)
@1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw that kind of commentary is unacceptable.
β
fanquake closed a pull request: "New SVG, Icons, PNGs and X PixMaps"
(https://github.com/bitcoin-core/gui/pull/879)
(https://github.com/bitcoin-core/gui/pull/879)