💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2094147733)
We're not always checking this, only when the index is available.
But after this change this check is for free, no need to recalculate the header's hash anymore.

📝 TheCharlatan opened a pull request: "kernel: Remove dependency on clientversion"
(https://github.com/bitcoin/bitcoin/pull/32543)
Including a Bitcoin-Core specific version does not make sense if used by external applications.

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888466453)
Force-pushed to fix a typo in commit description.

💬 katesalazar commented on issue "Option to use dark theme for Windows":
(https://github.com/bitcoin-core/gui/issues/378#issuecomment-2888477115)
There is too low contrast on the buttons in that pic

🤔 polespinasa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2848344096)
cACK

💬 polespinasa commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2094159634)
> In case we know there are unprocessed blocks in the chain (because we have their headers), we shouldn't say progress: 1

I agree we should not return 1 if we didn't verify the last block we know (we don't want Core to lie to users 😉)

What if we do something like:
```c++
if (pindex->nHeight == m_best_header->nHeight && pindex->nChainWork == m_best_header->nChainWork) {
const auto block_time{pindex->GetBlockTime() + Ticks<std::chrono::seconds>(2h)};
} else {
const auto block_
...

📝 theStack opened a pull request: "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls"
(https://github.com/bitcoin/bitcoin/pull/32544)
Descriptor wallets are already created by default [since v23.0](https://github.com/bitcoin/bitcoin/blob/7710a31f0cb69a04529f39840196826d0b9770ab/doc/release-notes/release-notes-23.0.md?plain=1#L171), but since the recent legacy wallet removal the `descriptors` parameter *must* be True for the `createwallet` RPC (see commit 9f04e02ffaee0fe64027dc56c7bea3885254321a), i.e. still passing it wouldn't contain any information for test readers anymore. So simply drop them in the functional tests in orde
...

💬 1440000bytes commented on pull request "scripted-diff: test: remove 'descriptors=True' argument for `createwallet` calls":
(https://github.com/bitcoin/bitcoin/pull/32544#issuecomment-2888488756)
Concept ACK

The argument `descriptors` could also be removed from `createwallet` RPC.

🤔 tapcrafter reviewed a pull request: "init: Configure reachable networks before we start the RPC server"
(https://github.com/bitcoin/bitcoin/pull/32539#pullrequestreview-2848348655)
tACK 12ff4be9c724c752fe67835419be3ff4d0e65990

<details>

<summary>Test protocol</summary>

## Current error message on master (7710a31f0cb69a04529f39840196826d0b9770ab):

```
$ bitcoind -regtest -rpcallowip=fc00:dead:beef::/64 -rpcuser=u -rpcpassword=p

2025-05-17T16:24:29Z Command-line arg: regtest=""
2025-05-17T16:24:29Z Command-line arg: rpcallowip="fc00:dead:beef::/64"
2025-05-17T16:24:29Z Command-line arg: rpcpassword=****
2025-05-17T16:24:29Z Command-line arg: rpcuser=****

...

💬 tapcrafter commented on pull request "init: Configure reachable networks before we start the RPC server":
(https://github.com/bitcoin/bitcoin/pull/32539#discussion_r2094164033)
I'm aware brevity is probably prioritized here. But I didn't recognize RFC4193 just from its number, so had to look it up. What about "RFC4193 style IPv6 ULA addresses are only allowed..."?

💬 yancyribbens commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2888498892)
cr ACK https://github.com/bitcoin/bitcoin/pull/32333/commits/135a0f0aa711b95c50aa4cbe0c38d82d647f1c8b

Nice, didn't know this rpc existed.

💬 yancyribbens commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888499988)
> Don't forget that fundrawtransaction accepts two different units, depending on snake case / camel case (!)

This seems like a bug that ought to be fixed separately?

💬 jonatack commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888506000)
> Don't forget that `fundrawtransaction` accepts two different units, depending on snake case / camel case (!)

See https://github.com/bitcoin/bitcoin/pull/20483.

💬 Ashkar776 commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888508710)
How to use

On Sat, 17 May, 2025, 9:38 pm Jon Atack, ***@***.***> wrote:

> *jonatack* left a comment (bitcoin/bitcoin#32093)
> <https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888506000>
>
> Don't forget that fundrawtransaction accepts two different units,
> depending on snake case / camel case (!)
>
> See #20483 <https://github.com/bitcoin/bitcoin/pull/20483>.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/32093#
...

💬 Ashkar776 commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888508833)
You have tutorial video step by step

On Sat, 17 May, 2025, 9:39 pm Ashkar Asku, ***@***.***>
wrote:

> How to use
>
> On Sat, 17 May, 2025, 9:38 pm Jon Atack, ***@***.***> wrote:
>
>> *jonatack* left a comment (bitcoin/bitcoin#32093)
>> <https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2888506000>
>>
>> Don't forget that fundrawtransaction accepts two different units,
>> depending on snake case / camel case (!)
>>
>> See #20483 <https://github.com/bitcoin/bitcoin/pull
...

💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2888528076)
Perhaps this should be backported to all current release branches?

💬 yancyribbens commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#issuecomment-2888533229)
Nice. I wonder about just adding a content-type like option so that the api endpoint is the same but you simply say if you want binary or json content returned? Maybe there's other endpoints that it could then be applied to as well..

🤔 rkrux reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2848375525)
I have just started reviewing this PR - have gone through the PR description & commit messages, going through the diff at the moment.

The intent of the PR is quite cool! Due to the nature of the change, I decided to run benchmarks on my system and have shared below the best results of the lot I had (the results seem to be consistent across multiple runs).

### Observations
1. I can clearly see noticeable performance improvements in the `WalletBalance` benchmarks.
2. However, both `WalletA
...

🤔 i-am-yuvi reviewed a pull request: "test: Move `script_assets_tests` into its own suite"
(https://github.com/bitcoin/bitcoin/pull/31576#pullrequestreview-2848380496)
tACK 0859fc22f0baf00259cf964f9d39d36fba6cd291

- making skipped tests explicitly appear as 'Skipped' rather than silently omitting

```
Start 89: script_tests
Start 85: script_p2sh_tests
Start 87: script_segwit_tests
Start 88: script_standard_tests
Start 86: script_parse_tests
Start 84: script_assets_tests
1/6 Test #86: script_parse_tests ............... Passed 0.16 sec
2/6 Test #84: script_assets_tests ..............***Skipped 0.16 sec
3/6 Test #88: s
...

💬 hebasto commented on pull request "build: Use clang-cl to build on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31507#discussion_r2094220240)
> > Also, any reason to not fix the issues in leveldb, rather than adding more suppressions?
>
> I'll consider this option.

Please see https://github.com/bitcoin-core/leveldb-subtree/pull/53.