💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1790719912)
Some test commits have been split off into #28764.

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1380105208)
Since `fanout` is already initialized `true`, couldn't we simplify it?

```diff
@@ -5878,19 +5878,17 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
if (reconciles_txs) {
auto txiter = m_mempool.GetIter(txinfo.tx->GetHash());
if (txiter) {
- if ((*txiter)->GetCountWithDescendants() > 1) {
- // If a transaction has in-mempool children, alwa
...

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1380107969)
> > In other words, if the peer's time deviates slightly from the node time
>
> I don't think `GetAdjustedTime` offers this precision even. Also, it is not a per-peer offset, but a global one.

Yeah I know. I just thought it was better to do that than nothing.
Still, I'm quite sure we cannot trigger the edge case I mentioned above only using core vanilla alone. The pruning process isn't that aggressive for the time being. It might arise across different implementations but we can revisit i
...

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1790729762)
Rebased 4052c2d5b9f84289752093981f475ea2ca5b64e7 -> 1a589335617944b755235597aafc254e28e4acf9 ([kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_3) -> [kernelInternalLib_3](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_3..kernelInternalLib_4))
* Fixed merge conflict.

💬 BrandonOdiwuor commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1790730225)
@jrakibi I'm currently working on https://github.com/bitcoin-core/gui/issues/769, you can work on it

💬 instagibbs commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1380140616)
realigned

🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1710304120)
Updated per feedback. Thanks everyone.

* Improved test coverage.
* Moved `GetAdjustedTime` usage to `GetTime`.
* Renamed `in_ibd` variable to `past_net_limited`.
* Updated comments and code to clearly state the responsibility of setting the 'initial sync finished' flag backwards is inside the 'CheckForStaleTip' process. Which performs a more exhaustive stale tip verification.

📝 hebasto opened a pull request: "build: Patch Qt to handle minimum macOS version properly"
(https://github.com/bitcoin/bitcoin/pull/28775)
This PR is:
- required to [switch](https://github.com/bitcoin/bitcoin/pull/28622) to macOS 14 SDK (Xcode 15).
- an alternative to https://github.com/bitcoin/bitcoin/pull/28732.

💬 hebasto commented on pull request "build: Bump `native_clang` up to 17.0.2":
(https://github.com/bitcoin/bitcoin/pull/28732#issuecomment-1790766663)
Closing in favour of https://github.com/bitcoin/bitcoin/pull/28775.

✅ hebasto closed a pull request: "build: Bump `native_clang` up to 17.0.2"
(https://github.com/bitcoin/bitcoin/pull/28732)

💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790775227)
Can you explain the patch? What is the actual problem, and how does this fix it?
Seems a bit odd to have to patch version detection checks inside Qt, can the correct values not be based in from the outside?

💬 fanquake commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1790785177)
https://cirrus-ci.com/task/6687992417353728:
```bash
clang-tidy-17 -p=/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/common/args.cpp
common/args.cpp:281:1: error: return type 'const fs::path' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type,-warnings-as-errors]
281 | const
...

💬 hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790792236)
> Can you explain the patch? What is the actual problem, and how does this fix it? Seems a bit odd to have to patch version detection checks inside Qt, can the correct values not be passed in from the outside?

In macOS SDK, the problematic `os/activity.h` header relies on the `OS_LOG_TARGET_HAS_10_12_FEATURES` macro:
```c++
#if OS_LOG_TARGET_HAS_10_12_FEATURES
#ifndef OS_ACTIVITY_OBJECT_API
#define OS_ACTIVITY_OBJECT_API 1
#endif // OS_ACTIVITY_OBJECT_API
#else
#if OS_ACTIVITY_OBJECT_A
...

💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790817038)
> the problematic os/activity.h header

What is the problem with the header?

The (availability.h) macros are set via the `-mmacosx-version-min` flag (which we set):
> For these macros to function properly, a program must specify the OS version range
> it is targeting. The min OS version is specified as an option to the compiler:
> -mmacosx-version-min=10.x when building for Mac OS X,

Why do we need to hardcode them to the values that they should already be being set too?

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1790819342)
`e13401084f...93cd65566a`: fix https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1790785177, thanks!

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1380216381)
In 983f8c6e305fd9707c109c2a92637825262b9b09: Correct me if I'm wrong, but we're going use `inbounds_nonrcncl_tx_relay` and `outbounds_nonrcncl_tx_relay` only whether `reconciles_txs` is true, couldn't we only fill them if so?:
```suggestion
if (reconciles_txs) {
```

👍 MarnixCroes approved a pull request: "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog"
(https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-1710440253)
tack 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27

💬 hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790840873)
> > the problematic os/activity.h header
>
> What is the problem with the header?

The `OS_ACTIVITY_OBJECT_API` is set to `0`, which makes a bunch of code unavailable, which in turn causes compile errors.

> The (availability.h) macros are set via the `-mmacosx-version-min` flag (which we set):
>
> > For these macros to function properly, a program must specify the OS version range
> > it is targeting. The min OS version is specified as an option to the compiler:
> > -mmacosx-versio
...

💬 maflcko commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1790859091)
I don't think a mutex is required for read-only access to a const blob of memory?

💬 hebasto commented on pull request "Replace RecursiveMutex m_cs_banned with Mutex, and rename it":
(https://github.com/bitcoin/bitcoin/pull/24097#issuecomment-1790875301)
@fanquake @achow101

Is there anything else to do here?