💬 fanquake commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1612637988)
> Yeah, tsan+bdb+aarch64 is impossible to run. You'll have to disable bdb.

Yea, this branch rebased on master + NO_BDB=1 works as expected.

🚀 fanquake merged a pull request: "test: Use TestNode datadir_path or chain_path where possible"
(https://github.com/bitcoin/bitcoin/pull/27884)

💬 glozow commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1612707077)
> Sparrow Wallet displays the fee rate histogram chart in the UI as another "tool in the toolbox". This allows a user to observe trends and adjust the fee rate to place a transaction in a particular fee rate band to suit their time preference.

> a fee rate histogram is a useful tool in the toolbox when estimating fees

I think it's worth noting that a feerate histogram built from individual transaction feerates can be very misleading for fee estimation. A CPFP could look like n low feerate
...

👍 fanquake approved a pull request: "Remove now-unnecessary poll, fcntl includes from net(base).cpp"
(https://github.com/bitcoin/bitcoin/pull/27530#pullrequestreview-1504921462)
ACK 8d9b90a61e3a2a451abfce25328d13aa1e8f749b

🚀 fanquake merged a pull request: "Remove now-unnecessary poll, fcntl includes from net(base).cpp"
(https://github.com/bitcoin/bitcoin/pull/27530)

📝 fanquake opened a pull request: "ci: filter all subtrees from tidy output"
(https://github.com/bitcoin/bitcoin/pull/27996)
We are currently dumping output for some. i.e:
```bash
diff --git a/src/minisketch/src/fields/clmul_1byte.cpp b/src/minisketch/src/fields/clmul_1byte.cpp
index 8826af9..7fd6f2a 100644
--- a/src/minisketch/src/fields/clmul_1byte.cpp
+++ b/src/minisketch/src/fields/clmul_1byte.cpp
@@ -4,21 +4,16 @@
* file LICENSE or http://www.opensource.org/licenses/mit-license.php.*
**********************************************************************/

-/* This file was substantially auto-generat
...

🚀 fanquake merged a pull request: "guix: Clean up manifest"
(https://github.com/bitcoin/bitcoin/pull/27811)

💬 Sjors commented on issue "Assertion failed: (data.size() > node.nSendOffset), function SocketSendData, file net.cpp, line 837":
(https://github.com/bitcoin/bitcoin/issues/27963#issuecomment-1612735230)
Maybe even more specifically an M2 bug? I've been using macOS for years on two Intel machines, leaving nodes running for weeks or months at a time, and never encountered this.

💬 MarcoFalke commented on pull request "ci: filter all subtrees from tidy output":
(https://github.com/bitcoin/bitcoin/pull/27996#issuecomment-1612774123)
lgtm ACK ce377a0a230d4cefdf03a6c183377fde52cf8f81

💬 MarcoFalke commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#discussion_r1246430456)
Just as another follow-up comment to the earlier question if this is too greedy: Yes, it is too greedy, but will actually print a nicer error message compared to using `enable_if` or concepts, because of the general serialization catch-all fallback to the member method.

For example, the following test diff would fail with the general error message for a non-greedy template:

```diff
diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
index b445ff8ffc..73b9e63011 10064
...

💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1246437951)
Thanks for checking the versions!

> I'm not sure if import asyncio should be added to this file.

`test/lint/lint-python.py` is happy and CI is green, so I guess no need to add it. If I do, then the linter is upset:

```
test/functional/test_framework/test_node.py:7:1: F401 'asyncio' imported but unused
```

👍 hebasto approved a pull request: "ci: filter all subtrees from tidy output"
(https://github.com/bitcoin/bitcoin/pull/27996#pullrequestreview-1505014381)
ACK ce377a0a230d4cefdf03a6c183377fde52cf8f81, I have reviewed the code and it looks OK.

💬 MarcoFalke commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1246447333)
Thanks, may do if I have to re-touch, to not invalidate the previous tests and comments.

💬 MarcoFalke commented on pull request "test: Use same timeout for all index sync":
(https://github.com/bitcoin/bitcoin/pull/27988#discussion_r1246447882)
Going to leave as-is for now, because the function pre-existing and is only moved, not created in this pull.

📝 darosior opened a pull request: "Miniscript: always treat unsatisfiables scripts as insane"
(https://github.com/bitcoin/bitcoin/pull/27997)
Due to how properties are tracked, some unsatisfiable scripts may pass the `IsSane()` checks. This is an issue as we would treat some unspendable scripts as safe. Fix this by explicitly checking a satisfaction exists when checking for sanity.

This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057).

💬 hebasto commented on pull request "ci: filter all subtrees from tidy output":
(https://github.com/bitcoin/bitcoin/pull/27996#discussion_r1246449744)
```suggestion
"src/crc32c",
"src/crypto/ctaes",
```

💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1612846638)
Looks like https://github.com/bitcoin/bitcoin/pull/27988 is a test-only fix for the initial issue reported here, and it increases consistency for now. I think it should be good to go, trivial to remove or rework in the future with a `std::future`-based approach.

💬 prusnak commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1612862819)
`zip foo.zip dist` should read `zip foo.zip dist/*` to drop the top-level `dist` folder

💬 MarcoFalke commented on issue "Discussion: Upgrading to C++20":
(https://github.com/bitcoin/bitcoin/issues/23363#issuecomment-1612868795)
Looks like gcc can be bumped from the now minimum required 9.1 to 10, to get roughly all of C++20. Given that all recent LTS releases of all supported operating systems ship with such a compiler, maybe this can be considered? Maybe not for 26.x, but 27.x should be fine?

⚠️ MarcoFalke reopened an issue: "Discussion: Upgrading to C++20"
(https://github.com/bitcoin/bitcoin/issues/23363)
(Previous discussion (C++17): https://github.com/bitcoin/bitcoin/issues/16684)

With package managers shipping newer versions of compilers and older releases of operating systems going EOL, it occurs that at some point in the future it will be almost uncontroversial to switch to C++20.

See the new features here: https://en.cppreference.com/w/cpp/compiler_support#cpp20

I think noteworthy are:

* <strike>Lambdas in unevaluated contexts: It will be possible to use `Assert(...)` (a lambda
...