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

💬 dergoegge commented on issue "make cov fails with lcov-2":
(https://github.com/bitcoin/bitcoin/issues/28468#issuecomment-1790901853)
> Can't locate Capture/Tiny.pm in @INC (you may need to install the Capture::Tiny module)

Can be fixed by `apt install libcapture-tiny-perl libdatetime-perl` but it's still broken after that when trying to generate coverage.

e.g. from `make cov_fuzz`:
```
geninfo: ERROR: "/workdir/bitcoin/depends/x86_64-pc-linux-gnu/include/boost/date_time/gregorian/greg_month.hpp":40: function _ZN5boost9gregorian9bad_monthD0Ev found on line but no corresponding 'line' coverage data point. Cannot deri
...

💬 fanquake commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28775#issuecomment-1790947834)
> We rather are adjusting the current inappropriate values, no?

The default values shouldn't need adjusting in the qt source code, if our version setting works. So this would point to that not being the case?

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

In that case, maybe it would be easier to set something like `-DOS_ACTIVITY_OBJECT_API=1` for the Qt build, and not have to patch any source?

However that wou
...

📝 BrandonOdiwuor opened a pull request: "Wallet: Functions to enable adding used balance to GUI overview page"
(https://github.com/bitcoin/bitcoin/pull/28776)
First part to solving https://github.com/bitcoin-core/gui/issues/769

Functions to enable adding used balance to the overview page for wallets with the avoid_reuse flag