💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1792187238)
Updated 105a0f4db4ffdc25d3ad30300c949d46d5d8e647 -> 65839d6267af1f5e0e04aded72cbfa23b56a1237 ([simplifyMemPoolInteractions_4](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_4) -> [simplifyMemPoolInteractions_5](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_4..simplifyMemPoolInteractions_5))

* Addressed @ismaelsadeeq's [comment](https://github.com/bitc
...

💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1381461570)
According to https://developer.apple.com/support/xcode/, additionally, `OSX_MIN_VERSION=13.5` should be adjusted as well.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1792201473)
Rebased on master.

💬 Sun0fABeach commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1792205732)
Concept ACK

Node runners need a builtin option to ignore all modern forms of datacarrying so they don't have to resort to manually patching their nodes.

... and the 'censorship' framing is getting old by now and has been rebutted often enough. If you really hold these principles, then please remove all your email spam filters. Otherwise you're engaging in censorship of a permissionless protocol and thereby impairing an important source of revenue for network users.

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

nit: the commit message in da9aceba217bbded6909f06144eaa1e1a4ebcb69 still has the old names

👍 dergoegge approved a pull request: "Fuzz: Check individual and package transaction invariants"
(https://github.com/bitcoin/bitcoin/pull/28764#pullrequestreview-1712356208)
ACK fcb3069fa307942cf7f3edabcda1be96d615c91f

📝 fanquake opened a pull request: "depends: latest config.guess & config.sub"
(https://github.com/bitcoin/bitcoin/pull/28781)
Before we make any local modifications (i.e #28733) pull the latest files from upstream.

💬 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)