💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2447569710)
Added the doc commit, ready for further review

💬 jb55 commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch on Linux ext4 SATA SSDs":
(https://github.com/bitcoin/bitcoin/issues/30692#issuecomment-2447587199)
> In my main machine, is ryzen with 32gb ddr4 with 2 sticks of memory. The other machine is Xeon, with 32gb memory, in quad channel ddr3. So i dont think its memory, because the 2 computers are stable. I can run prime95 in both for 2 hours with no problem.

it's true, I just ran disk diagnostics, no issues. going to run a memory diagnostic... but my system has been stable so I would be very surprised if it was that. I'm also running on ZFS. I keep hitting this, even after a full redownload...
...

💬 maflcko commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#discussion_r1822897369)
Hex parsing is used in mining via DecodeHexBlk, no?

💬 dergoegge commented on issue "Support validating a PoW-free block over over RPC":
(https://github.com/bitcoin/bitcoin/issues/31136#issuecomment-2447653274)
At a glance, [this](https://github.com/bitcoin/bitcoin/blob/97b790e844abd2f92c928239a7dc786d37fad18b/src/rpc/mining.cpp#L683-L710) looks like what you're looking for? i.e. `getblocktemplate`'s proposal mode: https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki#block-proposal

💬 dergoegge commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#discussion_r1822955914)
Does that meaningfully impact block submission? if so, seems like `DecodeHexBlk` should be the target for the benchmark.

💬 sr-gi commented on issue "`Wunused-member-function` in test each commit":
(https://github.com/bitcoin/bitcoin/issues/31180#issuecomment-2447895711)
> It may be trivial to suppress either way by using https://en.cppreference.com/w/cpp/language/attributes/maybe_unused in the commit?
>
> It should also be trivial to remove the error completely from the CI task by specifying `Wno-error=unused-member-function`?

Agreed on both. The purpose of the issue was mostly to agree on what the expectations are for methods that may be used within the same PR but not within the commit they were defined in.

Will open a PR adding `Wno-error=unused-mem
...

📝 hebasto opened a pull request: "msvc: Update vcpkg manifest"
(https://github.com/bitcoin/bitcoin/pull/31186)
This PR updates the vcpkg manifest baseline from the [2023.08.09 Release ](https://github.com/microsoft/vcpkg/releases/tag/2023.08.09) to the [2024.09.30 Release](https://github.com/microsoft/vcpkg/releases/tag/2024.09.30), with the following package changes:
- `boost`: 1.82.0#2 --> 1.85.0#1,2
- `qt5`: 5.15.10#5 -> 5.15.15
- `sqlite3`: 3.42.0#1 --> 3.46.1
- `zeromq`: 2023-06-20#1 --> 4.3.5#2

The previous update was made in https://github.com/bitcoin/bitcoin/pull/28938.

For additional m
...

📝 sr-gi opened a pull request: "ci: Do not error on unused-member-function in test each commit"
(https://github.com/bitcoin/bitcoin/pull/31187)
After https://github.com/bitcoin/bitcoin/pull/31045, an unused method in a commit will trigger a compilation error, even if that method is later used in a following commit within the same PR.

Do not enforce unused-member-function in test each commit.

Close #31180

💬 davidgumberg commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2447990044)
Concept ACK, Approach NACK

I think there should be a burden on PR's that claim to "improve performance" to demonstrate a user-impacting scenario where the improvements are relevant, and agree that having benchmarks that test workloads that will never impact users leads to PR's being opened and review being spent on things that don't deserve attention.

But, I think a similar burden exists for this PR to demonstrate or at least describe a rationale for why these removed benchmarks don't test
...

💬 achow101 commented on pull request "key: clear out secret data in `DecodeExtKey`":
(https://github.com/bitcoin/bitcoin/pull/31166#issuecomment-2447994081)
ACK 559a8dd9c0aafcecf00f9ccd9aabe5720bcebe8c

💬 edilmedeiros commented on pull request "doc: note that you can assume C++20.":
(https://github.com/bitcoin/bitcoin/pull/30136#issuecomment-2448004215)
Looks like this is not relevant anyway after merging the cmake build.
Lack of this info in the docs doesn't seem to be an issue.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1823207236)
You're right, should have been added to the previous commit. Will fix

💬 Sjors commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2448143516)
The GUI needs to handle this more gracefully:

<img width="496" alt="Scherm­afbeelding 2024-10-30 om 12 13 44" src="https://github.com/user-attachments/assets/f55a9e3a-12e3-4f5c-be19-123128f5f825">

A user would have to figure out that they need to manually edit `settings.json` or delete it and redo all their settings.

We should probably automatically delete it from `settings.json`. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GU
...

💬 jarolrod commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2448149505)

> A user would have to figure out that they need to manually edit `settings.json` or delete it and redo all their settings.
>
> We should probably automatically delete it from `settings.json`. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.

I posted about this on irc but no one commented on it :(

GUI should have option to remove the setting.

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1823263917)
Do you mean squashing these changes instead of having a "move" commit?

⚠️ Urpferd opened an issue: "Bitcoin Core 28.0 (Flatpak Linux Mint) pointing or saving blockchain to external drives does not work"
(https://github.com/bitcoin/bitcoin/issues/31188)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

- bitcoin core 28.0 (flatpak) won't save data (blockchain nor chainstate) to external drive
- dialogue box for creating path to save blockchain to save elsewhere will never appear
- changing config file doesn't change anything but continue to save to home directory

### Expected behaviour

saving or continuing blockchain on external harddrive

### Steps to reproduce

1. Fresh install of
...

💬 TheCharlatan commented on pull request "ci: Place datadirs for tests under tmpfs ":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2448207892)
> (see comment below for results).

What is this referring to?

💬 laanwj commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2448208204)
> We should probably automatically delete it from settings.json. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.

> I posted about this on irc but no one commented on it :(

Yes. Please create a new issue for this

👍 TheCharlatan approved a pull request: "ci: Do not error on unused-member-function in test each commit"
(https://github.com/bitcoin/bitcoin/pull/31187#pullrequestreview-2406138489)
lgtm ACK 54d07dd37d5919c4e3bc535ae498d623282741d1

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1823307100)
> Ok. This is interesting, because `(@available(macOS` is used in more places in the Qt codebase (and I assume this will only increase in future) than what is patched out here, and, was also present in Qt5, but we didn't have to patch it out there?

Qt 5 codebase uses `(@available(macOS` in code that we do not use, specifically in the RHI implementation for the Metal backend (`src/gui/rhi/qrhimetal.mm`) and in iOS plugins (`plugins/platforms/ios/...`).

> So maybe there is some other build-r
...