💬 Sjors commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#discussion_r2013942951)
Fixed in 4281e3603a2eadefc8535b863128a05cf3a5a75f

💬 laanwj commented on pull request "test: Add functional test for bitcoin-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32145#discussion_r2013983690)
FWIW #32057 went with `-prune=550` to avoid this condition, i don't know if that's possible here but would prefer to be consistent.

💬 laanwj commented on pull request "test: Add functional test for bitcoin-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32145#discussion_r2014016833)
It might make sense to factor out the add of `"\n"` to `add_block`?

💬 hebasto commented on issue "Source code mapping for debugger has changed since cmake":
(https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2754215884)
https://github.com/bitcoin/bitcoin/issues/31204#issuecomment-2741816753:

> This seems to work for me on Linux without any `.lldbinit` file:

I tested v28.1 on Ubuntu 24.04 with GCC 13.3:
```
$ ./configure --enable-ccache
$ make -j $(nproc) src/bitcoind
$ lldb
(lldb) version
lldb version 18.1.3
(lldb) target create src/bitcoind
Current executable set to '/home/hebasto/dev/bitcoin/src/bitcoind' (x86_64).
(lldb) settings show target.source-map
target.source-map (path-map) =
(lldb) image lookup -vn
...

💬 fjahr commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#issuecomment-2754230919)
re-ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f

Just removed the change of zmq ports.

💬 laanwj commented on pull request "fuzz: avoid returning non-conforming results from FuzzedSock::GetSockName()":
(https://github.com/bitcoin/bitcoin/pull/32109#issuecomment-2754298403)
> Code that, when reading the socket addr buffer, disregards said buffer size and blindly relies on the socket family type is brittle and should be fixed.

Yes. Though to be fair, it's something hard to get right due to lack of guarantees with regard to the socket name structure on different platforms. For example, even reading `.sa_family` could ostensibly reach outside the buffer. It's common that these are the first two bytes, but not guaranteed. So to be 100% sure you'd have to check *firs
...

💬 cculianu commented on pull request "Fix 11-year-old mis-categorized error code in OP_IF evaluation":
(https://github.com/bitcoin/bitcoin/pull/32143#issuecomment-2754363159)
Ok, squashed.

💬 maflcko commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#issuecomment-2754396141)
lgtm ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f 🔊

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK aa7a898c236eb519
...

💬 laanwj commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754401102)
Concept ~0. i understand the reasoning but not sure whether this is worth it. This isn't a lot of code, and it's very well-defined code that isn't likely to change over time. Making the script depend on the test framework means that the test framework needs to maintain an interface for external use, something we don't want to encourage in general as it limits flexibility to change. Also this use itself isn't tested. If the functions move, the script breaks.

> with the advantage of having prop
...

💬 instagibbs commented on pull request "doc: Update comments for AreInputsStandard to match code":
(https://github.com/bitcoin/bitcoin/pull/32129#issuecomment-2754427367)
ACK 52ede28a8adb2c2d44d7f800bbfbef8aed86070e

👍 darosior approved a pull request: "doc: Update comments for AreInputsStandard to match code"
(https://github.com/bitcoin/bitcoin/pull/32129#pullrequestreview-2717220554)
ACK 52ede28a8adb2c2d44d7f800bbfbef8aed86070e

💬 laanwj commented on pull request "Accept unordered tracepoints in interface_usdt_utxocache.py":
(https://github.com/bitcoin/bitcoin/pull/32101#discussion_r2014175596)
Maybe add a comment about the events possible arriving out of order.

💬 hebasto commented on issue "callgrind_annotate broken after cmake migration?":
(https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-2754463597)
Are there any requirements for debug info when using the `valgrind --tool=callgrind` + `callgrind_annotate` toolset? Are there any incompatible or unsupported compiler flags?

💬 hebasto commented on issue "callgrind_annotate broken after cmake migration?":
(https://github.com/bitcoin/bitcoin/issues/31957#issuecomment-2754481118)
I've tested the master branch @ c0b7159de47592c95c6db061de682b4af4058102 with the diff from c0b7159de47592c95c6db061de682b4af4058102 on Ubuntu 24.04:
```
$ callgrind_annotate ./cg-c-72381.out
<snip>
...
--------------------------------------------------------------------------------
The following files chosen for auto-annotation could not be found:
--------------------------------------------------------------------------------
./elf/../sysdeps/generic/dl-new-hash.h
./elf/../sysdeps/x86_64/d
...

💬 hodlinator commented on pull request "Move some tests and documentation from testnet3 to testnet4":
(https://github.com/bitcoin/bitcoin/pull/32096#issuecomment-2754488161)
re-ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f

💬 rkrux commented on pull request "contrib: refactor: dedup deserialization routines in utxo-to-sqlite script":
(https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754510282)
https://github.com/bitcoin/bitcoin/pull/32116#issuecomment-2754401102 contains good points to consider.

> This isn't a lot of code, and it's very well-defined code that isn't likely to change over time. Making the script depend on the test framework means that the test framework needs to maintain an interface for external use, something we don't want to encourage in general as it limits flexibility to change.

Agree, it's not a lot of code and is unlikely to change over time. Also, agree th
...

💬 laanwj commented on pull request "contrib: document asmap-tool commands more thoroughly":
(https://github.com/bitcoin/bitcoin/pull/32110#discussion_r2014225805)
nit: 30MB

👍 ryanofsky approved a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2717272610)
Code review ACK cac218dcbdd2b5682a7ac8659aac0306e68a65f5. Just changed exception to assert and assert to exception since last review. Left a minor suggestion but it's not important so feel free to ignore. Thanks for being so responsive with previous feedback.

I think this PR should be pretty helpful to developers writing python tests since it cleans up logging, returns more complete error information, provides better feedback if stop_node is used incorrectly, and adds test coverage for the te
...

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2014198466)
re: https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2003409216

In commit "qa: Workaround Windows issue with socket timeouts" (c92a8fcd79d76b23c366a07be1c8778db7bea891)

FWIW, I think the best way to write this would be simply:

```python
# Work around issue observed on Windows, Python v3.13.1 where socket timeouts don't have errno set.
if error_num is None and isinstance(e, TimeoutError):
error_num = errno.ETIMEDOUT

if error_num not in [ ... ]:
raise
```

Raisin
...

💬 ryanofsky commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2014196948)
>