💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1790699672)
I didn't mean to suggest a change to the logic, just to make it more conditional on `-privatebroadcast`, so that in principle we wouldn't even need to initiate a `m_private_broadcast` object unless the user is running in `-privatebroadcast` mode. But I don't have a concrete suggestion right now (I might suggest on at a later point but need to look deeper into the grant logic for that) so it's fine to resolve for now.

👍 BrandonOdiwuor approved a pull request: "test: remove unused code from `script_tests`"
(https://github.com/bitcoin/bitcoin/pull/31051#pullrequestreview-2352705365)
ACK e0287bc4b2d83cefb7af48aef457153d0729bcd4

💬 fanquake commented on issue "win64-cross CI timeout after 2h ":
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2397658116)
Hitting the 20m `--timeout` https://cirrus-ci.com/task/5886749702881280:
```bash
[17:40:38.816] 129/136 Test #135: walletload_tests ..................... Passed 4.68 sec
[17:40:42.991] 130/136 Test #132: wallet_tests ......................... Passed 11.89 sec
[17:40:47.154] 131/136 Test #122: coinselector_tests ................... Passed 23.22 sec
[17:40:54.153] 132/136 Test #1: util_test_runner ..................... Passed 119.52 sec
[17:41:12.100] 133/136 Test #5: nov
...

💬 pablomartin4btc commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2397799341)
> > Ninja is required to build the `qt` package in depends. It is mentioned in `depends/README.md`. However, `doc/build-osx.md` does not cover building with depends, so I don't think it is necessary to mention Ninja there.
>
> Could you please explain why this is the case? Ninja seems like an odd requirement here.

It's described in the Qt 6/ 6.7 [dependencies](https://doc.qt.io/qt-6/integrity-installing-dependencies.html). I've tested it myself and can't build `depends` without `ninja-buil
...

💬 furszy commented on pull request "Decouple WalletModel from RPCExecutor":
(https://github.com/bitcoin-core/gui/pull/841#issuecomment-2397806974)
> Out of the scope of this PR, I noticed that the order of the wallets in the combo-boxes corresponds to the settings.json file and if the user changes the wallet in the main window, when the rpc console is open, the wallet in the console combo doesn't correspond with the one in the main window, which is ok if you want to compare data but I think when the console opens it should have the same. Also perhaps we should show the last used wallet (in ./config/Bitcoin-Qt-*.conf?) when qt starts (if st
...

💬 pablomartin4btc commented on pull request "Decouple WalletModel from RPCExecutor":
(https://github.com/bitcoin-core/gui/pull/841#issuecomment-2397822725)
> If the goal is still to move to a new GUI, I wouldn't start changing the views behavior now.

Yeah, neither do I, just mentioning it here since I just passed thru while testing.

🤔 pablomartin4btc reviewed a pull request: "build: Rename `PACKAGE_*` variables to `CLIENT_*`"
(https://github.com/bitcoin/bitcoin/pull/31042#pullrequestreview-2352879220)
ACK c87a9d2e4c0ad7f2653857f34d7e048585f00be2

🤔 mzumsande reviewed a pull request: "kernel: Move block tree db open to block manager"
(https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2352892509)
Conceptual question:

I think that just the goal of not loading the blocktreedb twice could have been achieved easier by simply moving the code from `CompleteChainstateInitialization` into `LoadChainstate()`.

You also list other motivations, but I don't completely get these:
It seems a little bit non-intuitive to me to have things like Cleanup of blk / rev files (`CleanupBlockRevFiles`) done in the constructor, instead of having a separate call for this.
It also doesn't feel consistent if
...

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2397918937)
> Also playing around with this and reading the [netdriver post](https://blog.swiecki.net/2018/01/fuzzing-tcp-servers.html) (i couldn't find any other docs on it) I'm not sure if this will ever even get past the version handshake? So I'm wondering how useful this tool really is for us.

Yes, I don't think it will pass the version handshake. I can't see what kind of bugs it would find that other fuzzers would not get.

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2397922175)
Re https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-2352892509

Thank you for the questions @mzumsande!

> So can you explain a bit more in detail how this helps kernel?

I will try to give a thought-dump of what I am attempting to achieve here. As said in the title description, the kernel-motivated goal of this PR is to allow users of the library to instantiate the block tree db in the block manager in the same way we would currently in the chainstate load routines without
...

📝 theuni opened a pull request: "RFC: build: support for pre-compiled headers."
(https://github.com/bitcoin/bitcoin/pull/31053)
This cuts off roughly a third of my total build time, either with `-j8` or `-j24`.

There are many different ways to go about adding pre-compiled header support with CMake. This is probably the simplest and most naive, but it has a substantial impact.

I used [ClangBuildAnalyzer](https://github.com/aras-p/ClangBuildAnalyzer) to measure the headers which took the most amount of time to parse while building the kernel library. I did this under the assumption that those are probably the most in
...

💬 theuni commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397943775)
Ping @hebasto, @fanquake

Also @willcl-ark, you might be interested in this as you've been benchmarking build times.

💬 TheCharlatan commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2397951718)
Concept ACK

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2397978598)
> Remaining (non nitty) things I think to be addressed:
>
> 1. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391768053)
>
> 2. [refactor: TxDownloadManager + fuzzing #30110 (comment)](https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1787010637)

Yes 👍 and thanks for looking through what happened with the nonstandard input txns @marcofleon @instagibbs! Sorry for the delay, I still need to sit down and
...

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1790920762)
Yeah, need to rework this, it's confusing

💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399035773)
Here's my depends/x86_64-apple-darwin22.6.0/toolchain.cmake

```
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

# This file is expected to be highly volatile and may still change substantially.

# If CMAKE_SYSTEM_NAME is set within a toolchain file, CMake will also
# set CMAKE_CROSSCOMPILING to TRUE, even if CMAKE_SYSTEM_NAME matches
# CMAKE_HOST_SYSTEM_
...

⚠️ Sjors opened an issue: "macOS 15.0.1 can't build Qt (Intel, without Xcode)"
(https://github.com/bitcoin/bitcoin/issues/31054)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

`make` in `depends` fails

This is very recent, will try to find an earlier commit where it works.

### Expected behaviour

To work.

### Steps to reproduce

```
cd depends
make NO_WALLET=1 NO_UPNP=1 NO_ZMQ=1
```

### Relevant log output

```
...
Configuring qt...
Creating qmake...
In file included from /Users/sjors/dev/bitcoin2/depends/work/build/x86_64-apple-darwin24.0.0/qt/5.1
...

💬 Sjors commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399057050)
It happens even on d71ac768424333b65a6d88c9752cc9c7fdb276f3, so it's not a recent change in the code. Will try some variations with Xcode, etc.

💬 Sjors commented on issue "macOS 15.0.1 can't build Qt (Intel, without Xcode)":
(https://github.com/bitcoin/bitcoin/issues/31054#issuecomment-2399165977)
When I install XCode 16 and build depends for master @ 62e4516722115c2d5aeb6c197abc73ca7c078b23 it's happy.

`brew install llvm` (currently at 19.1.1) does not help. I used:

```
export PATH="$(brew --prefix llvm)/bin:$PATH"
export LDFLAGS="-L$(brew --prefix llvm)/lib"
export CPPFLAGS="-I$(brew --prefix llvm)/include"
```

Doesn't work on d71ac768424333b65a6d88c9752cc9c7fdb276f3 either.

I went back as far as ef20add4c98674183720d9631ac780f9a248b487 and still getting this error.


...

💬 Sjors commented on issue "macOS 13.7 depends build can't find qt":
(https://github.com/bitcoin/bitcoin/issues/31050#issuecomment-2399173681)
Ditto on d71ac768424333b65a6d88c9752cc9c7fdb276f3.