📝 fanquake opened a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201)
Remove `FORCE_USE_SYSTEM_CLANG` in favour of always using the system Clang and lld for macOS cross-compilation; rather than downloading precompiled blobs.

For example, anyone using Ubuntu 24.04 should be able to `apt install clang llvm lld .. etc`, and then cross-compile for macOS using:
```bash
# clang --version
Ubuntu clang version 18.1.3 (1)
make -C depends HOST=arm64-apple-darwin FORCE_USE_SYSTEM_CLANG=1
./autogen.sh
CONFIG_SITE=/path/to/depends/arm64-apple-darwin/share/config.site
...

📝 vasild opened a pull request: "netbase: extend CreateSock() to support creating arbitrary sockets"
(https://github.com/bitcoin/bitcoin/pull/30202)
Allow the callers of `CreateSock()` to pass all 3 arguments to the `socket(2)` syscall. This makes it possible to create sockets of any domain/type/protocol.

The need for this came up during the discussion in https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618837102

💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1620744428)
Opened https://github.com/bitcoin/bitcoin/pull/30202 "netbase: extend CreateSock() to support creating arbitrary sockets"

💬 TheCharlatan commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2139598389)
Post-merge ACK e8c25e8a35e3
```
uname -a && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
Linux starfive 5.15.0-starfive #1 SMP Sun Mar 26 12:29:48 EDT 2023 riscv64 GNU/Linux
00becde2dd12878e3b9f50f27899a6a8b752343dade7c71781632715c3001473 guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/SHA256SUMS.part
a685b9cee54014e74639be1e8db2d55b7c008fdb3b31c1c708c364a49b56759a guix-build-e8c25e8a35e3/output/aarch64-linux-gnu/bi
...

💬 laanwj commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2139605484)
Concept and code review (but untested, will use this to make a testing harness for #30043) ACK 77e34ded543e19ba27cab8d0cfc464af27f04bbf

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1620771700)
That depends on `T`'s move constructor. Typically, this won't do anything, but it's not impossible for a `T::T(const T&&)` constructor to exist (and this may make sense if `T` has `mutable` member variables).

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1620771908)
Is it though? If the user has not specified that he wants no automatic connections (`-maxconnections != 0`) I'd argue he'd expect them to happen.

In any case, I think this is such an edge case that it is not worth blocking the PR on it. If you have a strong opinion on this, I'm happy to drop it and squash the rest of the commit to one of the previous ones

💬 sipa commented on pull request "util: add VecDeque":
(https://github.com/bitcoin/bitcoin/pull/30161#discussion_r1620773911)
Actually, no, you're right. Even if this does anything, it won't be the desired behavior. Will remove.

💬 maflcko commented on pull request "depends: remove `FORCE_USE_SYSTEM_CLANG`":
(https://github.com/bitcoin/bitcoin/pull/30201#discussion_r1620825787)
nit: Could make sense which version is "recommended" or "tested". I presume the version used in CI? If so, could refer to that.

🚀 fanquake merged a pull request: "clang-tidy: Add `bugprone-move-forwarding-reference` check"
(https://github.com/bitcoin/bitcoin/pull/30199)

📝 BrandonOdiwuor opened a pull request: "Enhance signet chain configuration in bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30203)
## !! Early draft for feedback on approach !!

Follow up to https://github.com/bitcoin/bitcoin/pull/29838 following https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2063151580 and https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-2070170967

This PR builds upon the changes introduced in https://github.com/bitcoin/bitcoin/pull/29838, which enabled the ability to run multiple Signet instances with distinct data directories. Now, it extends this functionality by allowing user
...

💬 ismaelsadeeq commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1620854919)
```suggestion
send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000))
# Ensure UTXO is enough to account for the required fee
assert_greater_than_or_equal(send_value, 0)
```

💬 ismaelsadeeq commented on pull request "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)":
(https://github.com/bitcoin/bitcoin/pull/30162#discussion_r1620853166)
Why are you incrementing by 3 twice
Please use `WITNESS_SCALE_FACTOR` const instead of just 4.

Magic numbers makes my head hurts 😃

Should be worth it to define 3 in wallet as `PADDING_OFFSET` ?

🤔 ismaelsadeeq reviewed a pull request: "test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)"
(https://github.com/bitcoin/bitcoin/pull/30162#pullrequestreview-2088475662)
Concept ACK

This change is really useful and needed.

I came across this issue while working on another PR and had to fix it in my PR #30157. You can see the relevant commit https://github.com/bitcoin/bitcoin/pull/30157/commits/93146927acf8ce7d929598b15224ef91ece6768c.

Third commit 93527b82e70c8e19d7317ce5287b0ee2a0020f1b looks good to me.

I will do a thorough review of the first two commits.

💬 instagibbs commented on pull request "policy: bump TX_MAX_STANDARD_VERSION to 3":
(https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1620865246)
few remaining spots that I can see:
```
src/test/fuzz/package_eval.cpp:176: tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
src/test/fuzz/tx_pool.cpp:229: tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION;
src/test/txvalidation_tests.cpp:289: mtx_many_sigops.nVersion = 3;
src/test/util/txmempool.cpp:121: if (tx_info.tx->nVersion == 3) {
src/test/util/txmempool.cpp:136:
...

🤔 furszy reviewed a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2088523996)
Code review ACK eeea0818c1

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1620894922)
Ended up taking this, since the limit will be kept in the end.

Thanks!

💬 stickies-v commented on pull request "Fix typos in 36 files | Almost only documentation":
(https://github.com/bitcoin/bitcoin/pull/30188#issuecomment-2139858597)
> I think this should be closed.

Agreed. Concept NACK.

👋 Sjors's pull request is ready for review: "Introduce Miner interface"
(https://github.com/bitcoin/bitcoin/pull/30200)

💬 Sjors commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2139890557)
This seems very useful. I'll try to use it for the (currently very brittle) `Sv2Transport` tests in #29432, and review it along the way.