💬 martinus commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1158326638)
The only thing guaranteed is the order; so `m_request_stop` will be set to `true` before `notify_all` is called. But there is no guarantee where the other threads currently are because there is now no lock anymore. E.g. a worker thread could be currently here:

```cpp
while (queue.empty() && !m_request_stop) {
if (fMaster && nTodo == 0) { // <-------- Worker thread waiting here due to context switch
nTotal--;
bool fRet = fAllOk;
// reset the status for new wo
...

💬 glozow commented on pull request "test: create random and coins utils, add amount helper, dedupe add_coin":
(https://github.com/bitcoin/bitcoin/pull/26940#issuecomment-1497266140)
What was the rationale for 81f5ade2a324167c03c5ce765a26bd42ed652723? test/util/random.h depends on test/util/setup_common because it's using `g_insecure_rand_ctx`, which means setup_common can't use any of the random functions without creating a circular dependency. Was this intentional?

🚀 fanquake merged a pull request: "compat: move (win) S_* defines into bdb"
(https://github.com/bitcoin/bitcoin/pull/26832)

💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1158337331)
Missed EOL?

💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1158336326)
Pinning to the latest available version looks suboptimal as it still forces users to upgrade their vcpkg installation.

OTOH, pinning to the previous one, for example, `2.1.12#7`, could be a better option for an additional reason: no need to change `build_msvc/bitcoin_config.h.in` and `build_msvc/common.init.vcxproj.in`.

🚀 fanquake merged a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)

💬 glozow commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1497274400)
rebased, there was a silent merge conflict

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1497279995)
Marking as draft for now, will rebase on top of #26933

📝 glozow converted_to_draft a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711)
The hope here is to end up with the incentive-compatible transactions in our mempool. Prior to this commit, if parents within the package relied on each other, we could end up (1) accepting a low-feerate child or (2) rejecting high-feerate parents. See the "interdependent parents" test case for a specific example.

This also includes some shortcuts to reduce the amount of validation work we are doing:
- Don't validate the last tx twice.
- If the last transaction is by itself, call `AcceptSin
...

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1497285538)
> Edit: Ah whatever, https://github.com/bitcoin/bitcoin/pull/27021 doesn’t do much without https://github.com/bitcoin/bitcoin/pull/26152 anyway

Seeing as this has missed the feature freeze date, I'm removing both from 25.0 milestone (sorry :cry: )

💬 fanquake commented on pull request "ci: use clang-16 in tidy task":
(https://github.com/bitcoin/bitcoin/pull/27404#issuecomment-1497286500)
> -I/usr/lib/llvm-16/lib/clang/16/include

Included this. I think we could merge this as-is, but should also figure out why it's needed. It feels like there is another issue here.

> Also https://github.com/bitcoin/bitcoin/pull/25466#discussion_r930891939

Removed the related suppressions.

💬 fanquake commented on pull request "ci: add unused-using-decls to clang-tidy":
(https://github.com/bitcoin/bitcoin/pull/25466#discussion_r1158349009)
Followed up with in #27404.

👋 fanquake's pull request is ready for review: "ci: use clang-16 in tidy task"
(https://github.com/bitcoin/bitcoin/pull/27404)

💬 dimitaracev commented on pull request "validation: Move warningcache to ChainstateManager and rename to m_warningcache":
(https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1497287457)
> ACK [5526849](https://github.com/bitcoin/bitcoin/commit/552684976b6df34ce563458f73812e6e494e3b0e)
>
> Looking at this, it occurs to me that we're actually being a bit wasteful scanning through ~780k historical headers 29 times (or 2.4M headers 29 times for testnet) every time we startup. Replacing `MinBIP9WarningHeight` with `MinBIP9WarningStartTime` (set it to say 2022-07-01 initially), and using it for `WarningBitsConditionChecker::BeginTime()`, and just bumping it at each release might b
...

🚀 fanquake merged a pull request: "log: Check that the timestamp string is non-empty to avoid undefined behavior"
(https://github.com/bitcoin/bitcoin/pull/27317)

👍 fanquake approved a pull request: "test: Remove windows workaround in authproxy"
(https://github.com/bitcoin/bitcoin/pull/27418)
ACK fa584b4d01ca055488a10b280f45dca323b474d4

👍 hebasto approved a pull request: "ci: use clang-16 in tidy task"
(https://github.com/bitcoin/bitcoin/pull/27404)
ACK a56c96507a9e943bbcd7e126bc827de9495f0ebd

💬 josibake commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1497308824)
I was able to reproduce this locally on master by running:

```
make distclean
FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh
```

I ssh'd into the container and found that the `git diff` command is failing due to `dubious permissions`, which I think `./ci/lint/docker-entrypoint.sh` is supposed to fix with `git config --global --add safe.directory /bitcoin`? Seems like maybe that fix isn't working everywhere but we didn't catch it before because of the whole two use
...

💬 MarcoFalke commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1497313723)
> `./ci/lint/` ...

The lint system is completely different from `./ci/test/`.

🚀 fanquake merged a pull request: "test: Remove windows workaround in authproxy"
(https://github.com/bitcoin/bitcoin/pull/27418)