🚀 fanquake merged a pull request: "ci: Drop no longer needed `macos_sdk_cache`"
(https://github.com/bitcoin/bitcoin/pull/28269)

👍 fanquake approved a pull request: "ci: Use hard-coded root path for CI containers (bugfix)"
(https://github.com/bitcoin/bitcoin/pull/28185#pullrequestreview-1578304050)
ACK https://github.com/bitcoin/bitcoin/pull/28185/commits/fafa17c00b87c685200198979ed13018e55e474a - somewhat tested. MSAN changes are the same as what we did for tidy.

🚀 fanquake merged a pull request: "ci: Use hard-coded root path for CI containers (bugfix)"
(https://github.com/bitcoin/bitcoin/pull/28185)

💬 Retropex commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1678723521)
> A personal thought: A BIP obsoleting change should have a larger and more diverse set of community discussion. Possibly even running it through the mailing list first. As it stands the ACKs/NACKs ratio doesn't even reach 2/3, currently at 64.5% ACK rate

Although there was a debate in particular with the BIP47, we must not forget that some people like Mike are there only to protect their business with the src-20 and stamps. They absolutely do not care about the decentralization of Bitcoin an
...

💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-1678730811)
Rebased.

💬 fanquake commented on pull request "refactor: Remove unused boost signals2 from torcontrol":
(https://github.com/bitcoin/bitcoin/pull/28240#discussion_r1294438016)
Is this (another) a bug in IWYU? Seems a bit weird to remove the `<event2/event.h>` include, which, as far as I can tell, is correct. It's where `event_base` is defined, and what is [expected when using libevent](https://github.com/libevent/libevent/blob/c9ec6aafb6a025f03636ae2ae7c18a2d4b8a7ed8/include/event2/event.h#L55):
> section usage Standard usage
> Every program that uses Libevent must include the <event2/event.h>
header, and pass the -levent flag to the linker.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1294444800)
I've reverted back to using `ConsumeDeserializable`. I had the same intuition as @brunoerg but from my testing it's not clear that the custom way is more efficient (in terms of coverage per unit of time). Absent this, it makes sense to not introduce more code and simply use the existing utilities.

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1294456468)
Good question. Yes, it should be like this to preserve the behavior from `master` wrt which networks are reachable if `-onlynet` is used. The relevant snippet from this PR:

```diff
if (args.IsArgSet("-onlynet")) {
- std::set<enum Network> nets;
+ g_reachable_nets.RemoveAll();
for (const std::string& snet : args.GetArgs("-onlynet")) {
enum Network net = ParseNetwork(snet);
if (net == NET_UNROUTABLE)
return InitError(
...

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1294456676)
Done, thanks for the suggestion!

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1678810238)
`8b495486e2...be7ae7b1ec`: rebase due to conflicts and address suggestions

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1294501734)
There is no built in construct in the language to do that and I do not like `NET_MAX` which I perceive as a hack.

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1294503814)
Since https://github.com/bitcoin/bitcoin/pull/27116 is stalled I have added `AssertLockNotHeld()`, e.g.:

```cpp
AssertLockNotHeld(m_mutex);
LOCK(m_mutex);
m_reachable.insert(net);
```

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1678816243)
@mzumsande, what's your take on this now that it does not touch the GUI?

💬 fanquake commented on pull request "[WIP] guix: use `-muse-unaligned-vector-move` for Windows builds":
(https://github.com/bitcoin/bitcoin/pull/28151#issuecomment-1678870156)
> Edit: Ok, right, this is avx-only. Ignore most of the above.

Yea. It seems hard to (generally) produce working Windows binaries outside of Guix, because I assume most Windows toolchains are broken with the same bug (unless they are all being patched similar to Debian/Ubuntu).

So the best we can do is retain our GCC patching, so the entire toolchain/libs/bins are built correctly, and then in configure, we could pass the assembler flags as a best-effort, to _try_ and produce working binari
...

⚠️ Crypt-iQ opened an issue: "compact block fingerprinting"
(https://github.com/bitcoin/bitcoin/issues/28272)
I haven't written a test for this, but it seems that a peer can fingerprint a chunk of our mempool by announcing a new compact block with a valid header, but inserting many `shorttxids` that don't belong to the block but are suspected to be in our mempool. Assuming no short id collisions, we'll then request each tx that we didn't find in either `vExtraTxnForCompact` or the mempool.

I'm not sure how to fix this since compact block relay relies on this behavior. One possible way of fixing this
...

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1294434738)
Forgot `CI_FAILFAST_TEST_LEAVE_DANGLING` ? Otherwise, it would be good to see a functional test failure log

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1294603992)
Also, `CCACHE_NOHASHDIR`? Or maybe that'd be better moved to `./ci/test/00_setup_env.sh`

💬 achow101 commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1294605712)
Requesting the already confirmed transaction is surprising to me. Is this expected to change in future work (if so a comment mentioning that would be useful) or is it intended behavior?

🤔 achow101 reviewed a pull request: "test: tx orphan handling"
(https://github.com/bitcoin/bitcoin/pull/28199#pullrequestreview-1578574354)
ACK 9eac5a0529f869075f0331e40d322c34fc8fc2af

💬 achow101 commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1294606522)
Could check that `parent_low_fee_nonsegwit` is not in the mempool as done in the other test cases.