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

🚀 fanquake merged a pull request: "depends: drop -O1 workaround from arm64 apple Qt build"
(https://github.com/bitcoin/bitcoin/pull/28778)

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1381803100)
You can specify different permissions for each connection direction.

🚀 achow101 merged a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762)

👍 fanquake approved a pull request: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735#pullrequestreview-1712898349)
ACK 3333f14efac815ba3c885398a6795c7e8ce68d08 - the response from upstream is that [if we submit a PR, they can take a look](https://github.com/capnproto/capnproto/issues/1833#issuecomment-1792582206), so if anyone would like this to work for Windows, I'd suggest sending a patch.

💬 vasild commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381834911)
This is still an issue in the latest version (8adbd44c9b). It seems unlikely that the fuzzer will generate a string that makes `LookupHost()` happy. I think it is best if this test keeps using `ConsumeNetAddr()` and `ConsumeSubNet()` but those functions should return valid/sensible values. Something like:

```cpp
inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept
{
const CNetAddr net_base = ConsumeNetAddr(fuzzed_data_provider, {NET_IPV4, NET_IPV6}, /*always_
...

📝 fanquake opened a pull request: "build: remove `-bind_at_load` usage"
(https://github.com/bitcoin/bitcoin/pull/28783)
This is deprecated on macOS:
```bash
ld: warning: -bind_at_load is deprecated on macOS
```
and likely redundant anyways, given the behaviour of dyld3.

Unfortunately libtool is still injecting a `-bind_at_load`, because it's version check is broken:
```bash
# Don't allow lazy linking, it breaks C++ global constructors
# But is supposedly fixed on 10.4 or later (yay!).
if test CXX = "$tagname"; then
case ${MACOSX_DEPLOYMENT_TARGET-10.0} in
10.[0123])
func_append com
...

💬 fanquake commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1792622714)
@theuni @TheCharlatan you might have some libtool ideas? Couldn't see too anything obvious to prune this out.

💬 kristapsk commented on pull request "depends: Allow PATH with spaces in directory names.":
(https://github.com/bitcoin/bitcoin/pull/28733#issuecomment-1792625547)
Concept ACK

💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381846491)
> but those functions should return valid/sensible values.

I think it is good to offer a path for the fuzz engine to produce an invalid value. Otherwise, it can miss bugs that only happen when an invalid value is passed.

📝 romanz opened a pull request: "rpc: keep .cookie if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28784)
Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock).