💬 maflcko commented on issue "Add generate method to debug console":
(https://github.com/bitcoin-core/gui/issues/55#issuecomment-1790608649)
Closing for now, due to lack of progress on this test-only feature request. Discussion can continue on the pull request.

💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1790613552)
Re ACK 105a0f4db4ffdc25d3ad30300c949d46d5d8e647

💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1380032491)
Changes in this commit is identical to the one in https://github.com/bitcoin/bitcoin/pull/28762/commits/d525998b07be066c97a36536b58f0b98d816be39 , but here the `txmempool.h` dependency from `mini_miner.h`, and the imports are not updated?

💬 maflcko commented on pull request "depends: Bump to capnproto-c++-1.0.1":
(https://github.com/bitcoin/bitcoin/pull/28735#issuecomment-1790674841)
Disabled openssl, should be easy to re-ACK

📝 vasild opened a pull request: "Avoid returning references to mutex guarded members"
(https://github.com/bitcoin/bitcoin/pull/28774)
Avoid this unsafe pattern from `ArgsManager` and `CWallet`:

```cpp
class A
{
Mutex mutex;
Foo member GUARDED_BY(mutex);
const Foo& Get()
{
LOCK(mutex);
return member;
} // callers of `Get()` will have access to `member` without owning the mutex.
```

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#issuecomment-1790706698)
Concept ACK

💬 jrakibi commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1790709630)
Is anyone working on this? I'd be happy to take a look

🤔 glozow reviewed a pull request: "Fuzz: Check individual and package transaction invariants"
(https://github.com/bitcoin/bitcoin/pull/28764#pullrequestreview-1710219708)
ACK b847d48f70b5a67263c362f9d28ee6d092068e27, thanks for shaving!

💬 glozow commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#discussion_r1380088102)
c1319395f74c0016b367e860050c0ff5a99dea85 nit: the whitespace here is weird, should probably align with parentheses

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