💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1882967719)
Yeah, I guess so. The commit were:

* https://github.com/llvm/llvm-project/commit/d0eeb64be5848a7832d13db9d69904db281d02e8
* https://github.com/llvm/llvm-project/commit/6860b136b9e14e7c5771b710bea7370d5020a94b
* https://github.com/llvm/llvm-project/commit/e9b3f2573090a2fb9494975e4615f77b898e36a3
* https://github.com/llvm/llvm-project/commit/d8b6ae072d7734e2abadd54ecddfd6cb77b7e4c0
* https://github.com/llvm/llvm-project/commit/c8c176d999d2c2166dee8d67c32b4f8f42b4f1bd
* https://github.com/l
...

💬 stickies-v commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1882971897)
I support using `contains` going forward, but I'm not sure for this PR the benefits outweigh the costs?
1. I prefer the `contains` interface but imo using `count` is also pretty straightforward, so the benefits seem limited?
2. It conflicts with a large number of PRs
3. It's not a scripted diff

I think I'd prefer removing the clang-tidy check for now and only changing it in places where it doesn't conflict with other PRs, e.g. maybe in tests? And then organically have more `contains` adopt
...

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883016117)
> Should I change this pull to bump to 14 instead, and remove the m4 file?

Why change to 14, and not just add the m4 removal to this PR?

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883019819)
> Why change to 14, and not just add the m4 removal to this PR?

Oss-Fuzz is on clang-14, so a later version can not be used, for now. See https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1878712706

📝 maflcko opened a pull request: " build: Bump clang minimum supported version to 14 "
(https://github.com/bitcoin/bitcoin/pull/29208)

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883025587)
> Oss-Fuzz is on clang-14,

I was always under the impression it was 15: https://github.com/google/oss-fuzz/blob/6c19a0f229c244d41562ece92f9baac6b72426f9/infra/base-images/base-clang/checkout_build_install_llvm.sh#L53

> OUR_LLVM_REVISION=llvmorg-15-init-1464-gbf7f8d6f

💬 fanquake commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883028960)
Looking at a recent build of a project on oss-fuzz: https://github.com/google/oss-fuzz/actions/runs/7449538792/job/20266395883#step:7:283

> -- The C compiler identification is Clang 15.0.0

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1883029922)
That version number is bumped after the branch-off, so any commit prior to clang 15, but after clang 14, also (incorrectly) identifies as clang 15.

💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1883045460)
Concept ACK

💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#discussion_r1446088741)
Might have been a typo?

💬 fanquake commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1883055919)
I assume this be moved to the GUI repo? @hebasto

💬 fanquake commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#discussion_r1446092071)
@theuni If you want I can also just include a fix for this in here, while we are sorting out flags.

💬 furszy commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1883057125)
> I think the error message "Unable to parse settings file %s" could definitely be improved though. Would suggest maybe "Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, and can be fixed by removing the file, which will reset settings to default values." This would probably work OK for both command line and GUI (since the GUI already offers the option to reset the settings see [bitcoin-core/gui#379](https://github.com/bitcoin-core/gui/pull/379)
...

⚠️ hebasto opened an issue: "New crash in v26.0"
(https://github.com/bitcoin-core/gui/issues/785)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

bitcoin-qt crashed while loading wallets at startup.

I used to see occasional crashes on startup a few years ago, but it hasn't been happening at all in the last couple of major releases.

I've been running v26.0 for a week or two and haven't had any problem with it crashing until today.

Here's a backtrace. I run it in gdb habitually because I used to see a lot of crashes and
...

💬 hebasto commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1883062230)
Moved to https://github.com/bitcoin-core/gui/issues/785.

✅ hebasto closed an issue: "new crash in v26.0"
(https://github.com/bitcoin/bitcoin/issues/29153)

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446096447)
```suggestion
const std::string flag_name = fuzzed_data_provider.ConsumeRandomLengthString(100);
```

`ConsumeRandomLengthString`: "Designed to be more stable with respect to a fuzzer inserting characters than just picking a random length and then consuming that many bytes with |ConsumeBytes|."

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446090797)
nit: I think constants are not supposed to be prefixed with `g_`

```suggestion
const uint256 g_block_hash{uint256S("000000002c05cc2e78923c34df87fd108b22221ac6076c18f3ade378a4d915e9")};
const uint32_t g_bits{0x1d00ffff};
```

I think you could also just use the genesis block (`Params().GenesisBlock()`) for these two values.

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446101167)
Can we get rid of `g_setup` if we initialize a new `SignalInterrupt` here each interation?

🤔 furszy reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1811179954)
Cool notification. The CI should be happy now.