💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1608615606)
Hah

👍 theuni approved a pull request: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137#pullrequestreview-2069121155)
utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2123033320)
> Am I right that after this change (or alternatives which let BatchWrite just iterate dirty/fresh entries), it would be possible to (perhaps dramatically) reduce the periodic flush time, so that after IBD, we basically always have a mostly non-dirty cache? Such a change could be made today of course, but the cost of iterating the entire cache every time would add up quickly.

I believe you are right, but I'm unsure what the benefit is that you see. From my understanding, the periodic flushes
...

💬 achow101 commented on pull request "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB":
(https://github.com/bitcoin/bitcoin/pull/26596#issuecomment-2123038131)
Rebased following #26606. Now ready for review.

👋 achow101's pull request is ready for review: "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB"
(https://github.com/bitcoin/bitcoin/pull/26596)

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2123045950)
@andrewtoth I was talking about post-IBD. And one advantage to more frequent flushing is reducing the latency spikes you get when a flush is triggered.

👍 theStack approved a pull request: "refactor prep for package rbf"
(https://github.com/bitcoin/bitcoin/pull/30072#pullrequestreview-2068992752)
Code-review ACK 6fcd3cc016eaf07e47c5c448482ff844855a247b

Out of curiosity, I verified that the extended carve-out functional test in 868c3e77e01df8cfdc00a7599f16d0adf019aee7 (checking `testmempoolaccept` and `submitpackage` results) would also fail on master, but in a slightly different way, as the carve out leads to a failure later (in `CheckPackageLimits()` vs the earlier `PreChecks()` for `testmempoolaccept`; for `submitpackage` the first tx succeeds, and only the second fails with `"too-l
...

💬 theStack commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#discussion_r1608552045)
spelling nit:
```suggestion
/** When true, the transaction or package will not be submitted to the mempool. */
```

💬 achow101 commented on pull request "wallet, tests: Avoid stringop-overflow warning in PollutePubKey":
(https://github.com/bitcoin/bitcoin/pull/30131#discussion_r1608661403)
Done

📝 BenWestgate opened a pull request: "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs"
(https://github.com/bitcoin/bitcoin/pull/30147)
Closes #30145.

This PR solves two major issues in the `parse_version_string` function of verify-binaries:
1. `-aarch64` binaries cannot be specifically downloaded. The -platform string gets interpreted as a release candidate that doesn't exist due to containing sub-string "rc".
2. Specifying a platform with a "-" in the name causes the parser to ignore both "-platform" AND "-rcN" and download the potentially wrong (non-rc) version for every platform. This also prevented specifying just one
...

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#discussion_r1608681928)
Good idea, done.

💬 theStack commented on pull request "test: improve BDB parser (handle internal/overflow pages, support all page sizes)":
(https://github.com/bitcoin/bitcoin/pull/30125#issuecomment-2123091480)
Rebased on master (necessary since #26606 was merged and also touched the same functional test file).

💬 laanwj commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#issuecomment-2123110126)
Concept ACK

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1608563568)
IMO it was better to leave group/others as read-only, no write access

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1608634500)
`const` has no meaning here, keep it only in the actual function

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608704228)
Why?

💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#issuecomment-2123125755)
This PR originally contained a commit which purposely added a violation in init.cpp:
```c++
thread_local std::string bad_var;
```
From the tidy c-i task:

> init.cpp:723:5: error: Variable with non-trivial destructor cannot be thread_local. [bitcoin-nontrivial-threadlocal,-warnings-as-errors]
> 723 | thread_local std::string bad_var;

Since it was correctly detected, I'll now remove that commit.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608709082)
Why are we setting this, or why are we dropping it for Darwin?

We no-longer need it given we aren't building Darwin tools. It's been disabled for all other hosts since Guix was introduced. I can dig up the history and post it here.

💬 theuni commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1608712706)
Ah, sorry, I misread. Had it backwards. It looked like it was being applied only to darwin before, but it was applied to everything _but_ darwin before. Now darwin is no longer an exception.

Thanks.

💬 achow101 commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#issuecomment-2123141284)
ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef