💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1879866925)
> Could mark as draft while CI is red?

Sorry for the extra-late reply, missed this message and the CI fail. Rebased on master and resolved the silent merge conflict (caused by the module move `test_framework.muhash` -> `test_framework.crypto.muhash` in #28374). Also fixed the Windows CI issue by closing the sqlite connections properly with explicit `con.close()` calls (see e.g. https://github.com/bitcoin/bitcoin/pull/28204). CI is green now.

>> What is the rationale for encoding as text ra
...

💬 hebasto commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#issuecomment-1879869514)
The CI has been fixed and this PR is ready for review now.

Friendly ping @dergoegge :)

🤔 theStack reviewed a pull request: "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py"
(https://github.com/bitcoin/bitcoin/pull/29067#pullrequestreview-1807648209)
Concept ACK

I agree with the outlined drawbacks of `struct.{un,}pack` (especially the "format string consists of characters that may be confusing and may need to be looked up in the documentation" part) and that using `int.{from,to}_bytes` should be used instead.

Small side-note unrelated to this PR:
For (de)serializing single bytes I usually prefer `bytes([val])` over `val.to_bytes(1, "little")` (and `bs[0]` over `int.from_bytes(bs, "little")`), as it feels a bit weird having to specify
...

🤔 pablomartin4btc reviewed a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114#pullrequestreview-1807655447)
> addressed [#27114 (comment)](https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1365833858) and [#27114 (comment)](https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1440760716)

re ACK 6175a2ee09663f7cf20a048b33e537442dfeb81d

(nit: if you have to touch 2nd and/ or 4th commits, naming could be shorter)

💬 niftynei commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1879925212)
> Assuming you mean the sequence emitted from Transaction{AddedTo,RemovedFrom}Mempool, note that this is not actually the case (they're usually off by 1 or more due to when the sequence increments happen).

Can you say more about this divergence or point me to a doc that contains more info?

I should add that one problem with the existing `getrawmempool` RPC semantics is that currently requesting the mempool_sequence *and* asking for verbose results is specifically disallowed.

I attempte
...

💬 niftynei commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1879925440)
> Also, [missing](https://github.com/bitcoin/bitcoin/pull/29016/checks?check_run_id=19389652409) test coverage for the new command:

Yes, will add test coverage when this approach gets a concept ack.

Instead, working usage of this proposed code change has been included in the original PR

💬 hebasto commented on issue "Broken `--enable-suppress-external-warning` for Apple Clang 15 on `x86_64`":
(https://github.com/bitcoin/bitcoin/issues/29174#issuecomment-1880029036)
According to [Wikipedia](https://en.wikipedia.org/wiki/Xcode#Xcode_15.0_-_(since_visionOS_support)_2), the Apple clang version 15.0.0 (clang-1500.0.40.1) corresponds to LLVM clang 16.0.0.

On Ubuntu 23.10:
```
$ clang++-16 -Xclang -internal-isystem/usr/local/include test.cpp
error: unknown argument: '-internal-isystem/usr/local/include'
```

However,
```
$ clang++-15 -Xclang -internal-isystem/usr/local/include test.cpp # Works.
```

📝 hebasto opened a pull request: "ci: Switch native macOS CI job to Xcode 15.0"
(https://github.com/bitcoin/bitcoin/pull/29195)
The same Xcode version is used for the release binaries.

This PR:
- addresses https://github.com/bitcoin/bitcoin/pull/29165#discussion_r1439433156
- fixes https://github.com/bitcoin/bitcoin/issues/29174

💬 hebasto commented on pull request "ci: Switch native macOS CI job to Xcode 15.0":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1880035579)
https://github.com/bitcoin/bitcoin/actions/runs/7437962301/job/20236177232:
```
checking whether C++ preprocessor accepts -Xclang -internal-isystem -Xclang /usr/local/include/... yes
```

💬 hebasto commented on issue "Broken `--enable-suppress-external-warning` for Apple Clang 15 on `x86_64`":
(https://github.com/bitcoin/bitcoin/issues/29174#issuecomment-1880038733)
Fixed in #29195.

💬 hebasto commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1880038943)
> If someone created a separate pull request to bump xcode in the macOS CI, that'd be great.

Please see #29195.

📝 hebasto opened a pull request: "ci: Do not set inane value for `LD_LIBRARY_PATH`"
(https://github.com/bitcoin/bitcoin/pull/29196)
For example, in the native macOS CI job, the `LD_LIBRARY_PATH=/Users/runner/work/bitcoin/bitcoin/depends/x86_64-apple-darwin/lib` makes no sense.

💬 sipa commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880059347)
utACK fb5bfed26a564014b83ccfc96ff00b630930fc61

💬 theStack commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1880061924)
Nice! Concept ACK

👍 theStack approved a pull request: "RPC/Blockchain: scanblocks: Accept named param for filter_false_positives"
(https://github.com/bitcoin/bitcoin/pull/29184#pullrequestreview-1807818372)
ACK 5779010ed7be1cbe9b98a91c7487d3d14b7cf24d

💬 kristapsk commented on pull request "RPC: add new `listmempooltransactions`":
(https://github.com/bitcoin/bitcoin/pull/29016#issuecomment-1880094062)
Concept ACK

⚠️ zzzi2p opened an issue: "I2P: Change encryption type"
(https://github.com/bitcoin/bitcoin/issues/29197)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Similar to signature types, I2P supports multiple encryption types.
SAM defaults to the oldest type for both, for backward compatibility.
Unfortunately I forgot about this for encryption types.

The qbittorrent / libtorrent projects just discovered encryption types
in this issue:
https://github.com/qbittorrent/qBittorrent/issues/19625

The encryption type is a property of the sessi
...

💬 maflcko commented on pull request "build: remove `--enable-lto`":
(https://github.com/bitcoin/bitcoin/pull/29185#issuecomment-1880098822)
unrelated: I wonder if one of the sanitizer CI tasks should have LTO enabled. Otherwise it is just oss-fuzz, which enables it, so errors and warnings are easier to miss.

💬 jonatack commented on issue "I2P: Change encryption type":
(https://github.com/bitcoin/bitcoin/issues/29197#issuecomment-1880122065)
Thank you, @zzzi2p. Am looking now at reproducing and fixing this.

📝 reardencode opened a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198)
This pull request contains a the same implementation of OP_CHECKTEMPLATEVERIFY (BIP119) as @jamesob's [Covenant Tools](https://github.com/bitcoin/bitcoin/pull/28550), an implementation of [OP_CHECKSIGFROMSTACK(VERIFY)](https://github.com/bitcoin/bips/pull/1535) and of [OP_INTERNALKEY](https://github.com/bitcoin/bips/pull/1534).

There are no testnet or mainnet activation parameters proposed in this pull request. I am deeply uninterested in the details of activation semantics.

This combinati
...