👍 hebasto approved a pull request: "bench: enable wallet creation benchmarks on all platforms"
(https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2064368273)
re-ACK 7c8abf3c2001152423da06d25f9f4906611685ea, I agree with changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2061694682).

💬 brunoerg commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2118365692)
Added https://github.com/bitcoin/bitcoin/pull/30134 to the list for review. It adds more coverage for `ScriptPubKeyMan`.

👍 theStack approved a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2064403732)
ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca

Convinced myself that this parser works reliably partly by looking at the original code, partly by extending the test framework's BDB parser to match this PR's logic (see #30125), which helped a lot in understanding the structure. Also tested with a few random signet legacy wallets that I still had around, though those were all quite small.

Btw, I couldn't manage to find or create a BDB wallet where the DELETE flag is set in a record header (wou
...

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1605585592)
typo nit
```suggestion
parser.add_argument("--swap-bdb-endian", action="store_true",help="When making Legacy BDB wallets, always make them byte swapped internally")
```

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1605553167)
nit: isn't thrown anywhere, can be removed
```suggestion
```

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1605552180)
nit: isn't thrown in the parser (commented out), hence can be removed here
```suggestion
```

💬 jonatack commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2118449438)
> If anything, I think the docs could be updated in some way to say that running the linters outside the container, or with versions that differ from the CI is generally unsupported.

Thanks for the feedback, done. Open to further suggestions (like whether to update the other dependencies).

⚠️ asp2alirezasusanichezeh opened an issue: "#asusani"
(https://github.com/bitcoin/bitcoin/issues/30135)

✅ willcl-ark closed an issue: "#asusani"
(https://github.com/bitcoin/bitcoin/issues/30135)

💬 ariard commented on pull request "policy: restrict all TRUC (v3) transactions to 10KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2118488640)
I think 10kvb is okay as a limit though pray for someone telling it more to LN folks.

There is very likely 2018 / 2019 channels open with `max_accepted_htlcs` at max 483 both-sides. With no dynamic upgrades deployed, no way to restraint, without force-close and re-open. Worst than pinning exposure is no propagating TRUC transaction due to “naive” upgrade.

In the (far) future, 10kvb could be dynamic at the transaction-relay level, though this would assume LN upgrading its channel policy set
...

:lock: fanquake locked an issue: "#asusani"
(https://github.com/bitcoin/bitcoin/issues/30135)

📝 rustyrussell opened a pull request: "doc: note that you can assume C++20."
(https://github.com/bitcoin/bitcoin/pull/30136)
We check this in configure.ac, line 99:

dnl Require C++20 compiler (no GNU extensions)

This was introduced in:

commit fa67f096bdea9db59dd20c470c9e32f3dac5be94
Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Sun Aug 27 10:45:39 2023 +0200

build: Require C++20 compiler

Which git says was before v27.0rc1:

$ git describe --contains fa67f096bdea
v27.0rc1~224^2~4

<!--
*** Please remove the following help text before submitting: ***

Pull requests without
...

💬 fanquake commented on pull request "util: avoid using thread_local variable that has a destructor":
(https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974)
If we are outlawing thread_local usage, then at the same time, this commit should remove other comments from our codebase suggesting further thread_local usage. i.e [ * If thread-safety is needed, the global could be made thread_local (given](https://github.com/bitcoin/bitcoin/blob/058af75874ffa2b4064e3d6d30cc50f0ec754ba8/src/test/util/random.h#L17)

💬 laanwj commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#discussion_r1605657935)
It's useful to document what version of the C++ standard can be used, but i don't think this needs to mention the version it was introduced (the document always applies to the branch it is in), nor is this really the place for documenting a compiler requirement.

Would word it as "Code needs to adhere to the C++20 standard.".

📝 fanquake opened a pull request: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137)
Based on #30095 and #30099.
Would close #29952.
Up for debate.

💬 fanquake commented on pull request "build: Enable `thread_local` for MinGW-w64 builds":
(https://github.com/bitcoin/bitcoin/pull/30099#issuecomment-2118613875)
> After this and https://github.com/bitcoin/bitcoin/pull/30095, is there any need to keep the thread_local option at all?

Maybe not. See #30137.

💬 laanwj commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#issuecomment-2118621410)
Concept ACK.

💬 laanwj commented on pull request "wallet, tests: Avoid stringop-overflow warning in PollutePubKey":
(https://github.com/bitcoin/bitcoin/pull/30131#discussion_r1605667276)
Maybe directly assert `pubkey.size() >= 1` to clarify.

⚠️ Geremia opened an issue: "LevelDB error: Corruption: block checksum mismatch didn't trigger reindex."
(https://github.com/bitcoin/bitcoin/issues/30138)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

For the past 29 hours, it's been stuck trying to sync:
>**Number of blocks left** Unknown. Syncing Headers (843771, 100.0%)…

and

>Connecting to peers…

I'm also pruning (`prune=550`).

### Expected behaviour

It should sync the full chain and reindex if needed, not try to sync what it can't (due to possibly corrupt chainstate DB).

### Steps to reproduce

Open up `bitcoin-qt`. It
...

💬 hebasto commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#issuecomment-2118669335)
Concept ACK.