💬 ryanofsky commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#discussion_r2055079547)
re: https://github.com/bitcoin/bitcoin/pull/32113

> Also, just because there may not be a performance difference today, doesn't mean there will be none in the future.

Am dubious of this. In general I'd think we'd want to have as few differences as possible between fuzzing and non-fuzzing code code paths and right now there is only 1 difference (outside of test code) in `CheckProofOfWork()`. Even if we were going to add more differences it's hard to imagine wanting to add them in hot code p
...

🚀 ryanofsky merged a pull request: "fuzz: enable running fuzz test cases in Debug mode"
(https://github.com/bitcoin/bitcoin/pull/32113)

💬 Asad1445 commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2822874857)
1FiHzRUVuA9rx3QJiCbnmoqtu7KouUoMPq

💬 Asad1445 commented on issue "utils: add support for `bitcoin-wallet` tool configuration file":
(https://github.com/bitcoin/bitcoin/issues/30421#issuecomment-2822890541)
1FiHzRUVuA9rx3QJiCbnmoqtu7KouUoMPq

💬 midnightmagic commented on pull request "correct wrong assumptions in the contrib linearize data script":
(https://github.com/bitcoin/bitcoin/pull/31888#issuecomment-2823063971)
The changes function to make linearize "less-perturb" on-disk data. Altogether, in the event of a block reorganization, the reorg'd blocks are sometimes shorter than prior blocks' data in individual output files when split on e.g. size. When the writer potentially moves on to the next blk data file therefore, the rest of the block data that pre-existed the reorganization might need to be truncated off the end of that file before moving on to the next one.

For the "wb" Python open mode, all pr
...

🤔 shahsb reviewed a pull request: "bitcoin-cli: Add -ipcconnect option"
(https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-2785939213)
Concept ACK

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2055412943)
Alright, I agree `OutboundConnectedToStr()` is a bit odd. If it is renamed to `OutboundConnectedToAddress()`, then what about `OutboundConnectedToService()`? Leave it like this? Then it would be confusing because we have the `CAddress` type and the `...Address()` suffix would imply such an argument. I wanted to avoid overload (two different methods with the same name) because that is not grep-friendly. Should I drop that and have an overload?

Maybe:
* `ConnectedToAddrPort(string)` and `Conne
...

💬 janb84 commented on pull request "depends: Fix cross-compiling on macOS":
(https://github.com/bitcoin/bitcoin/pull/32215#issuecomment-2823422336)
> @janb84
>
> > ... but I do get an different error back
>
> Please provide more details about your build environment (OS version, architecture, compiler version etc) and exact steps to reproduce the error.

Of course, here you go, ill will also try to figure it out also but please let me know if you need any other details.

Build env:
MacOS 15.4.1, M1
QT 6.9.0
Tried both Apple clang version 17.0.0 (clang-1700.0.13.3) & Homebrew clang version 20.1.3
SDK 15.4

Steps to reprodu
...

💬 scgbckbone commented on pull request "wallet: allow label for non-ranged external descriptor (if `internal=false`) & disallow label for ranged descriptors":
(https://github.com/bitcoin/bitcoin/pull/31514#issuecomment-2823457474)
PR description (and title) updated

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2823457894)
Rebased after #32293, updated the documentation it introduced, and added Cap'n Proto to `doc/dependencies.md`. Also took the LLM fix.

💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2041644069)
9c4aad2a: could remove `strprintf`

💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2041903792)
2acd12c: micro nit: s/trough/through.

💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2047059613)
655af59:
> If the malleated transaction wins then we will eventually stop broadcasting the original when it gets stale and gets removed from the "to broadcast" storage cause it is not acceptable in our mempool." doesn't happen.

looks like `Remove(stale_tx)` doesn't happen here (returns `nullopt` without removing the transaction from `m_by_txid`) and we keep re-attempting private broadcast of this malleated transaction scenario every 10-15 mins.

constructing a malleated transaction wasn't
...

💬 stratospher commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2041693437)
4e45e9c: micro nit: could we also mention "transaction" here and in `src/rpc/net.cpp`? people looking at help might be confused about what privacy-sensitive data is meant here.

💬 Sjors commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2055546342)
In ec5d2c7fca5e6c36aa59aeb7ce64b98739b6d094 "wallet: Disallow legacy wallet creation from the wallet tool": I'm getting a trailing `%` on macOS

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2055580572)
```suggestion
| [Cap'n Proto](../depends/packages/capnp.mk) | [link](https://capnproto.org) | [0.6.1](https://capnproto.org/install.html) | 0.5.3 | No |
```

💬 l0rinc commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2055590028)
Just sorted the imports for consistency - but I see that the corresponding header is sometimes separated, so I've reverted this move.

What do you think about the whole change in general, can you give me a conceptual review?

👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2786509505)
ACK 81c0b9edfe533afbb2f4dda56142afdedffdb347

> @vasild I think you got the wrong hash.

Yes, that was `HEAD~` at the time :space_invader:

💬 fanquake commented on pull request "test: Run all benchmarks in the sanity check":
(https://github.com/bitcoin/bitcoin/pull/32310#issuecomment-2823620332)
Locally the sanity bench (when running `ctest`) is taking ~19s, roughly the same time as the second worst secp256k1 test. Under the `DEBUG` build type it takes ~140s, the next longest running test is ~80s; so I imagine some devs will wonder why runing ctest is now taking > 2-3 minutes to complete. I checked that under Valgrind the runtime isn't so long that it'd cause the job to fail due to timeout (although there might be a different issue causing that).

I think ideally we could just delete
...

🚀 fanquake merged a pull request: "test: Run all benchmarks in the sanity check"
(https://github.com/bitcoin/bitcoin/pull/32310)