✅ maflcko closed an issue: "Failure to run Fuzz tests when running with corpus"
(https://github.com/bitcoin/bitcoin/issues/32089)

💬 maflcko commented on issue "Failure to run Fuzz tests when running with corpus":
(https://github.com/bitcoin/bitcoin/issues/32089#issuecomment-3563306804)
Closing for now. If someone finds this to be working at some point, they are welcome to add docs for it. In the meantime, please use libfuzzer-nosan, or other stuff, mentioned in https://github.com/bitcoin/bitcoin/pull/33921

🤔 fanquake requested changes to a pull request: "ci: Add Windows + UCRT jobs for cross-compiling and native testing"
(https://github.com/bitcoin/bitcoin/pull/33764#pullrequestreview-3493060075)
https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2549961718

💬 fanquake commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3563315913)
> In my view, dropping support for x86_64-w64-mingw32 should be considered separately and at a later stage, once the codebase becomes incompatible with MSVCRT or the toolchain landscape has matured.

Shouldn't it be removed once we remove it from the CI? which, according to the PR description in #33764, will be done once the Guix migration is done:
> MSVCRT-related CI jobs should be removed from the CI framework once the migration to UCRT is complete.

Just trying to get a clearer idea of w
...

🤔 l0rinc requested changes to a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493026362)
Concept ACK, but if we claim best-effort, we should cover the M series as well with latest AppleClang and latest LLVM.

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2549969970)
is this removal related to the mac cleanup?
it seems this was before the Mac section originally https://github.com/bitcoin/bitcoin/commit/33dd764984def9371f324d3add19ee894a0260bf#diff-7ff2e1d1ed59fa96766a663236d71bfdd00b4af3321fde39f7dbf022bd968a0aL87-L88

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2549966345)
can we specify in the beginning that we recommend linux and still provide the link to the mac section?

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2549976233)
since this is an alternative to the above linux one and since the build directory is changed now, should we add
```md
cmake --build build_fuzz_nosan -j$(nproc) && \
FUZZ=coins_view_db build_fuzz_nosan/bin/fuzz
```
after this?

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2549997346)
As described in https://github.com/bitcoin/bitcoin/pull/32084#issuecomment-3361478681 this doesn't seem to be enough for me, I'm getting a lot of failures locally on my non-intel-based Mac.
It needs a few more parameters to avoid being tangled with local `AppleClang`:
```
rm -rfd build_fuzz_nosan && cmake --preset=libfuzzer-nosan \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_OSX_SYSROOT="$(xcrun --show-sdk-pat
...

🤔 hodlinator reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3492740506)
Concept ACK e14650967dc95da5c10e0d6183b6eac3e8243fe5

#32541 contained some useful tricks that seem to be useful to shrink other index implementations. Thanks for letting it go in favor of this less invasive change though!

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2549751593)
No need to have `std::optional` for the offset, could just be sent in as `0` when reading whole blocks. Same for parameter types in rest.cpp.
```suggestion
bool BlockManager::ReadRawBlockPart(std::vector<std::byte>& data, const FlatFilePos& pos, size_t part_offset, std::optional<size_t> part_size) const
```

<details><summary>Full compiling diff (includes suggestion to skip noop seeks, see other comment).</summary>

```diff
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index
...

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2549741549)
nit: Could avoid syscall in common case
```suggestion
if (offset > 0) filein.seek(offset, SEEK_CUR);
```
Other PR avoiding similar syscall which was leading to slowdowns on Windows: #30884
Alternatively one could perform the noop-check inside of `AutoFile` itself.

💬 hodlinator commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2549772096)
nit: Could call `block_part.clear()` between each read.

💬 hebasto commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3563408869)
> > In my view, dropping support for x86_64-w64-mingw32 should be considered separately and at a later stage, once the codebase becomes incompatible with MSVCRT or the toolchain landscape has matured.
>
> Shouldn't it be removed once we remove it from the CI? which, according to the PR description in #33764, will be done once the Guix migration is done:

Agreed.

> > MSVCRT-related CI jobs should be removed from the CI framework once the migration to UCRT is complete.
>
> Just trying t
...

💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2550080275)
> Yeah, I guess there were some silent conflicts.
Indeed. Fixed.

💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3563434095)
> [#33764 (comment)](https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2549961718)

Thanks! Fixed.

💬 diegoviola commented on pull request "Fix bitcoin-qt visual glitches on Wayland":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3563438977)
> For example, on Fedora 43 with Gnome 49 and Wayland

Ah, I haven't tested this on GNOME. I suspect the use of `showNormal()` likely helps with the desired window behavior, which is why the revert works better.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2550098336)
Not a very strict reason, but to allow more flexibility and not to break the search for minimal whitespaces or indentation changes.

💬 hebasto commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#discussion_r2550139808)
> Concept ACK
>
> I am however unable to trigger the fatal error on `Ubuntu 24.04.3 LTS` with `Python 3.9.23`

I cannot reproduce the issue:
```
$ cat /etc/os-release | head -1
PRETTY_NAME="Ubuntu 24.04.3 LTS"
$ python3 --version
Python 3.9.23
$ cmake -B build
<snip>
-- Could NOT find Python3 (missing: Python3_EXECUTABLE Interpreter) (Required is at least version "3.10")
Reason given by package:
Interpreter: Wrong version for the interpreter "/home/hebasto/.pyenv/shim
...

📝 Sjors opened a pull request: "mining: add getMemoryLoad() and track template non-mempool memory footprint"
(https://github.com/bitcoin/bitcoin/pull/33922)
Implements the template memory footprint tracking discussed #33899, but does not yet impose a limit.