💬 fanquake commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1792270323)
> so I guess someone can report it upstream?

https://github.com/capnproto/capnproto/issues/1833

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1792279588)
> No write will happen to the datadir cache after the first write, and no reader will have the reference before the first write, no?

That is not obvious to me. `m_cached_blocks_path` can be modified by repeated calls to `ArgsManager::GetBlocksDirPath()` which end up executing:

https://github.com/bitcoin/bitcoin/blob/9b68c9b85efebfa23daec6471b87e9cbb514a006/src/common/args.cpp#L293

`ArgsManager::GetBlocksDirPath()` is called by `init.cpp` and by the GUI, so it is not immediately obvious
...

👍 hebasto approved a pull request: "depends: drop -O1 workaround from arm64 apple Qt build"
(https://github.com/bitcoin/bitcoin/pull/28778#pullrequestreview-1712433808)
ACK 664c87354f9ec5df95346eab72a034296b83914d.

My Guix builds:
```
x86_64
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226 guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90 guix-build-664c87354f9e/output/arm64-apple-darwin
...

💬 TheCharlatan commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#discussion_r1381552026)
Should this be added to the non-native package too?

Nit: `--without-openssl`?

💬 TheCharlatan commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1792294779)
ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

👍 TheCharlatan approved a pull request: "depends: drop -O1 workaround from arm64 apple Qt build"
(https://github.com/bitcoin/bitcoin/pull/28778#pullrequestreview-1712533078)
ACK 664c87354f9ec5df95346eab72a034296b83914d

I get the same hashes on both platforms.

📝 maflcko opened a pull request: "test: Add missing sync on send_version in peer_connect"
(https://github.com/bitcoin/bitcoin/pull/28782)
Without the sync, the logic will be racy. For example, `p2p_sendtxrcncl.py` is failing locally (and on CI occasionally), because non-version messages will be sent before the version message:


```py
self.log.info('SENDTXRCNCL with version=0 triggers a disconnect')
sendtxrcncl_low_version = create_sendtxrcncl_msg()
sendtxrcncl_low_version.version = 0
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
wi
...

👍 TheCharlatan approved a pull request: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735#pullrequestreview-1712559143)
Re-ACK 3333f14efac815ba3c885398a6795c7e8ce68d08

💬 TheCharlatan commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1792371383)
Guix builds (x86_64)
```
4be7b005bd9d131dcdd9d0221250431061206f6ca5d882bc3825221d0c223741 guix-build-f7f8522ee3a0/output/dist-archive/bitcoin-f7f8522ee3a0.tar.gz
114bf9052b7a3e62d7c8a53526783eb3f09ba397e59ac9a40cd854ccecffc4e0 guix-build-f7f8522ee3a0/output/x86_64-w64-mingw32/SHA256SUMS.part
a7e9d709211a444fa09c30f1ec7a1bb68fdd7b1165696d3c537a8419ec4fd294 guix-build-f7f8522ee3a0/output/x86_64-w64-mingw32/bitcoin-f7f8522ee3a0-win64-debug.zip
8f1ea99f220643f7382882a142709533dd3b5189b6963af
...

💬 maflcko commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1792402672)
Concept ACK. I guess longer term it would be good to have the CI run in a path that has a space and some emojis in it.

👍 TheCharlatan approved a pull request: "libconsensus: adapt API header to be compliant to ANSI C"
(https://github.com/bitcoin/bitcoin/pull/28661#pullrequestreview-1712666600)
tACK 939918a9ab67a37294c6c63cc6ef4fe109bc75ad

💬 laanwj commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#discussion_r1381676999)
i would be surprised if this code path is ever reached, apparently [Pyramid OSx](https://en.wikipedia.org/wiki/DC/OSx) is a discontinued operating system from the 90's. Nothing that runs that will run bitcoin core even if we cared supporting it 😄

🚀 glozow merged a pull request: "Fuzz: Check individual and package transaction invariants"
(https://github.com/bitcoin/bitcoin/pull/28764)

👍 instagibbs approved a pull request: "refactors for subpackage evaluation"
(https://github.com/bitcoin/bitcoin/pull/28758#pullrequestreview-1712736256)
ACK b5a60abe8783852f5b31bc1e63b5836530410e65

💬 kevkevinpal commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1792517576)
reACK [d9cc99d](https://github.com/bitcoin/bitcoin/pull/28762/commits/d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9)

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1381772685)
I agree it's helpful, but I am not sure about having it for all occurances. Perhaps we could put in `test_framework`, e.g.:

```diff
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 220e51df38..aeb9a2691c 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -97,6 +97,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
"""Sets test
...

💬 furszy commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1381776328)
> I am not asking to add back the assert.

+1 on not re-adding the assert.

> However, I wonder what the point is of forcing the remote peer into a 10 minute delay+disconnect. Why not disconnect immediately? Also, if the remote has set a permission flag to avoid the disconnect (not sure if this exists), I wonder what the correct behavior would be.

If the remote peer has a permission flag to avoid the disconnection, maybe it could respond with a `NOTFOUND` message? Same as we do with tx g
...

💬 achow101 commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1792562211)
re-ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

💬 dergoegge commented on pull request "Improve peformance of CTransaction::HasWitness (28107 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/28766#discussion_r1381796168)
Personally, I don't think the extra bytes matter that much, so I would lean towards leaving this as is.

🚀 fanquake merged a pull request: "refactors for subpackage evaluation"
(https://github.com/bitcoin/bitcoin/pull/28758)