💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2054855051)
Done.

💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2054855288)
Leftover from a previous change. Fixed!

💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2822521897)
ACK a0becaaa9c7390bc80d5864f7fffb32e21502a50

Added assertion, cleaned up comment, and rebase on master

`git range-diff master d64bd31f9b6903f0a335a78b519d0b52e84cca1b a0becaaa9c7390bc80d5864f7fffb32e21502a50`

💬 ismaelsadeeq commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2054912979)
This explanation clarified the comment, thanks.

💬 hebasto commented on pull request "depends: Fix cross-compiling on macOS":
(https://github.com/bitcoin/bitcoin/pull/32215#issuecomment-2822566913)
@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.

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2054951460)
Thanks! Done.

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2054994737)
Yeah, that's right, fixed it.

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2054994837)
Fixed

💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2822656585)
Rebased and addressed @sipa 's feedback. Also made a few further improvements: Added some missing includes, dropped some docs that have been added in #32110 already (silent merge conflict), unified asmap version/checksum naming (I went with version because that's what we have been communicating to users already).

> Would it be possible to separate these into two commits (probably first switching the interpretation to be bit-of-std::byte based but leaving everything in std::vector<std::byte>)
...

💬 Jokacar10 commented on issue "fuzz: psbt_base64_decode fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/32135#issuecomment-2822672611)
assert(DecodeBase64PSBT(psbt, random_string, error) == error.empty());

💬 luke-jr commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2055030024)
nit: why moving this?

💬 achow101 commented on pull request "wallet: allow label for non-ranged external descriptor & disallow label for ranged descriptors":
(https://github.com/bitcoin/bitcoin/pull/31514#issuecomment-2822773252)
ACK 664657ed134365588914c2cf6a3975ce368a4f49

> passes as the bug does not manifest if "internal" is not provided by user

I see. Can you mention that in the PR description?

> re-based on current master

There's no need to rebase if there are no conflicts.

👍 ryanofsky approved a pull request: "fuzz: enable running fuzz test cases in Debug mode"
(https://github.com/bitcoin/bitcoin/pull/32113#pullrequestreview-2785668286)
Code review ACK 3669ecd4ccd8e7a1e2b1a9dcbe708c51c78e4d6c with just variable renamed and documentation added since last review

💬 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
...