💬 EthanHeilman commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1494228089)
@hernanmarino I have libevent pinned to the version with the matching signature function. Let me know if you want anything else changed in this PR.

💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1494253155)
> * some CI regressions need to be fixed

The "tidy" and "TSan" CI tasks have been fixed.

💬 fanquake commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1494258971)
Note that even though the CI here is "green", [the tidy task](https://cirrus-ci.com/task/6643322689683456?logs=ci#L5204) has failed:
```bash
In file included from common/url.cpp:5:
In file included from ./common/url.h:8:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/string:40:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/char_traits.h:39:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/
...

👍 TheCharlatan approved a pull request: "validation: Move warningcache to ChainstateManager and rename to m_warningcache"
(https://github.com/bitcoin/bitcoin/pull/27357)
ACK 552684976b6df34ce563458f73812e6e494e3b0e

📝 MarcoFalke opened a pull request: "Use steady clock instead of system clock to measure durations"
(https://github.com/bitcoin/bitcoin/pull/27405)
`GetTimeMillis` has multiple issues:

* It doesn't denote the underlying clock type
* It isn't type-safe
* It is used incorrectly in places that should use a steady clock

Fix all issues here.

💬 dergoegge commented on pull request "Use steady clock instead of system clock to measure durations":
(https://github.com/bitcoin/bitcoin/pull/27405#issuecomment-1494263678)
Concept ACK

💬 hebasto commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1494267607)
> Ok, so it seems we agree there is no code quality benefit.

To clarify my point, this change _does_ improve code performance when the build is configured with `--enable-debug`.

I'm not saying that this fact justifies this PR sufficiently, but it should be considered, at the very least.

> Is the motivation = CI already failing or wanting to add --enable-debug to it?

I'd avoid it as it will change code paths being parsed now (for example, the `DEBUG_LOCKCONTENTION` macro). Maybe just
...

📝 fanquake opened a pull request: "depends: add `NO_HARDEN=` option"
(https://github.com/bitcoin/bitcoin/pull/27406)
Add an option that when passed, will disable hardening options, and pass `--disable-hardening` through to configure. Due to the way we link `libssp` for Windows builds, they now fail (after #27118), if building with depends, and configuring with `--disable-hardening` (Windows is the odd build out here). See: https://github.com/bitcoin/bitcoin/pull/27118#issuecomment-1492606272.

This change would add a depends option such that, if someone wants to build with depends, for Windows, without harde
...

💬 fanquake commented on pull request "depends: add `NO_HARDEN=` option":
(https://github.com/bitcoin/bitcoin/pull/27406#issuecomment-1494311710)
cc @hebasto @TheCharlatan

💬 fanquake commented on pull request "depends: harden libevent":
(https://github.com/bitcoin/bitcoin/pull/27118#issuecomment-1494313712)
> This PR introduced a regression when building with depends and disabled hardening:

Followup for discussion in 27406.

📝 dergoegge opened a pull request: "net, refactor: Privatise CNode send queue"
(https://github.com/bitcoin/bitcoin/pull/27407)
The send queue members on `CNode` should not be part of the public interface. This PR makes all of them private and creates a clear interface for the send queue.

The interface after this PR consists of:
* `CNode::PushMessage` for appending a message onto the send queue
* `CNode::SocketSendData` for pushing as many messages from the send queue as possible onto the wire
* `CNode::IsSendQueueEmpty` for checking if the send queue is empty
* (`CNode::TestOnlyClearSendQueue` a test-only utility
...

🚀 fanquake merged a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)

💬 fanquake commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1494353437)
Rebased for #27254.

💬 fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1494357963)
Rebased now that #27381 is resolved.

⚠️ glitteryskye99 opened an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)
null

✅ fanquake closed an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)

:lock: fanquake locked an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)

💬 furszy commented on issue "`listtransactions` fails to list self-send transactions (for imported descriptor wallet)":
(https://github.com/bitcoin/bitcoin/issues/26096#issuecomment-1494423948)
> For self-send detection to work we need to distinguish between receiving and change addresses. For descriptor wallets we do that by checking `internal` flag on the descriptor. But `internal` flag could only be set on an `active` descriptor. Both your imported descriptors are not `active`, therefore we can't determine which outputs are change and which are receiving.

@S3RK, #25979 aims to fix that.
note: It's currently up-to-date with master but I should revisit it after that many months.

💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1494455306)
Thanks everyone for the interest, finally reviving this. Rebased and modernized the code (use static casts and named parameters, make use of batched RPC calls after #26656).

@naumenkogs
> If we really want to check, we can measure how would fees differ by applying this change to historic blocks (?)

We'd also need historical snapshots of a mempool to do that. If someone has got some and is willing to test, i'd be very interested in seeing the results.

> Otherwise, it would help if @dar
...

💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1156064559)
Resolving this, i don't think we can get rid of this heuristic. https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1494455306 gives a counter example where it would lead to a false positive. Please let me know if you disagree.