💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r2513196144)
In 7b71e6e056f5567152097d5c9a1a8e2f59c1b661 _Include Base58 encoded prefixes in chainparams_: this commit only adds base58 prefixes, silent payments will use bech32m.

💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r2513186197)
In 055dbaed69a2577b967fae3d9b7646ef3b01a254 _Added chaintype user-facing string display_:

This is used as follows in later commits:

> `Invalid Base58 %s address.`

In the original code it would not specify the network:

> `Invalid or unsupported Base58-encoded address.`

I don't think it's very useful to add the network name to address. Certainly not for mainnet, because most users have no idea (and no need to know) about test networks.

My suggestion would be drop this helper enti
...

💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r2513273103)
In 04d5d4acab4259505654a506c838fecb29fdb88d _Base58 decoding logic + bech32 decoding network awareness_: splitting bech32 and base58 handling in an earlier commit might also help to focus the current commit more on base58. I still a lot of lines touching bech32 code.

💬 Sjors commented on pull request "Enhanced error messages for invalid network prefix during address parsing.":
(https://github.com/bitcoin/bitcoin/pull/27260#discussion_r2513247821)
If this works out of the box it's worth taking, but otherwise I would keep the current approach. Address validation is simple enough that I'm not worried about the node being in a confused state.

💬 laanwj commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2513309951)
i remember that in the past we had tons of issues with thread-local variables. They've been a pain in the ass on some platforms, and decided to not use them except for one case. Not sure if this is still the case, but if not, this needs to be updated:
https://github.com/bitcoin/bitcoin/blob/master/src/util/threadnames.cpp#L39

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3515590666)
I restored to the previous commit id `29ef3c62de`:
`29ef3c62de...d419fbc42b`: remove the functional test
`d419fbc42b..29ef3c62de`: restore the functional test
the fuzz test wasn't there in the first place.

I think it needs some more polishing and is not ready for a non-draft PR.

💬 janb84 commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3515604722)
### My Guix Build Output

**Host architecture:** `aarch64`
**Commit:** `f06c6e189831`

```shell
05fc5cae41b19bbfdec5942e334e7b7d78ee0406ba69c221381b0b07b0a9e886 guix-build-f06c6e189831/output/aarch64-linux-gnu/SHA256SUMS.part
961613408c4d0b3b169488b43edc838135be0b712740b4015457f4f2808e103b guix-build-f06c6e189831/output/aarch64-linux-gnu/bitcoin-f06c6e189831-aarch64-linux-gnu-debug.tar.gz
906437c316d50e1f60fdfe2098fe72f917be109a68a2103e915183ed4fe01947 guix-build-f06c6e189831/outpu
...

🤔 janb84 reviewed a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3446920253)
Concept ACK f06c6e18983139dd63873b3537d2f87b8c6ec752

The build uses a lot more space 49,4 GIB ( 51 GiB (freespace before run - 1.6 GiB freespace after run) (will post proof later, have to run)

Good followup would be to adjust the guix build gate accordantly.
(will do some extra research on this to rule out something has changed on my guix vm)

💬 laanwj commented on pull request "Changing the rpcbind argument being ignored to a pop up warning, inst…":
(https://github.com/bitcoin/bitcoin/pull/33813#discussion_r2513364532)
Do we want to change this one to a warning as well? It even say "WARNING".

💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in ASan task":
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3515685725)
I was wondering if the std::span nullptr check was part of the C++26 library hardening, but it seems not (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3471r4.html#span-constructors), so I guess if we want it, we'd still have to write it ourselves.

💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2513459402)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:

```
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test

assert vout["ischange"]

~~~~^^^^^^^^^^^^

KeyError: 'ischange'

💬 fanquake commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2513507443)
There's also our `thread_local` clang-tidy plugin: https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools/bitcoin-tidy.

⚠️ fanquake opened an issue: "ci: failure in `wallet_basic.py` KeyError: 'ischange'"
(https://github.com/bitcoin/bitcoin/issues/33848)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:
```bash
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test

assert vout["ischange"]

~~~~^^^^^^^^^^^^

KeyError: 'ischange'
```

💬 fanquake commented on issue "ci: failure in `wallet_basic.py` KeyError: 'ischange'":
(https://github.com/bitcoin/bitcoin/issues/33848#issuecomment-3515909671)
cc @pinheadmz @achow101 #32517

💬 big14way commented on issue "Inconsistent CJDNS address handling in Local addresses and AddLocal logs":
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3515930062)
@willcl-ark thank you for clarifyingi will look through the CONTRIBUTING.md frist
i appreciate it

🚀 fanquake merged a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820)

💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3516135207)
Each commit should leave the code in a valid state, so it would make sense to move 000c685bcdd395d61a58ad9698563cab41e2d7d0 up.

💬 laanwj commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3516192532)
Concept ACK.

re "refactor: Use fixed size ints over (un)signed ints for serialized values"

i wonder if we can enforce this kind of thing? That only fixed-size values can be serialized, would make sense to avoid any kind of platform divergence. At least in theory, as we already make assumptions on these types it's not a pressing issue.

📝 AminMemariani opened a pull request: "Gpt/asmap tests"
(https://github.com/bitcoin/bitcoin/pull/33849)
## Motivation
Add direct Boost unit tests for the asmap interpreter and validator so regressions in ASN bucketing are caught earlier. Existing coverage relied on indirect behavior through addrman tests; these new cases exercise `Interpret` and `SanityCheckASMap` without large fixtures.

## Changes
- Introduce `src/test/asmap_tests.cpp` with helpers to synthesize compact asmap programs and five focused tests:
- verifies match vs default routing
- rejects tables missing a `RETURN`
- r
...

💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513732380)
A more thorough approach would be to introduce `typedef uint8_t HashType`, but it requires changes all over the codebase.

👍 TheCharlatan approved a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3447427213)
ACK fabf8e51083120883162399293c620f2afc0c5e2