💬 maflcko commented on issue "Fuzz: Runtime errors when running fuzz tests on MacOs":
(https://github.com/bitcoin/bitcoin/issues/31591#issuecomment-2567660283)
Does it work with the `libfuzzer-nosan` preset?

💬 maflcko commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2567675519)
> version: 16.2

Thanks. Though, I can't reproduce in GHA (run: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/12582079644/job/35067062801#step:3:14 ). Though, I also get a different output for the clang installed dir:

```
sudo xcode-select --switch /Applications/Xcode_16.2.app
clang --version

Apple clang version 16.0.0 (clang-1600.0.26.6)
Target: arm64-apple-darwin24.2.0
Thread model: posix
InstalledDir: /Applications/Xcode_16.2.app/Contents/Developer/Toolchains/X
...

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822246)
thx, may do if I have to re-touch, or in a follow-up

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822308)
No, I don't want to affect performance in one way or another on some compiler (optimization level)s

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822388)
I am not touching this line of code, and don't want to, so maybe a separate issue or pull request seems more appropriate?

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822444)
No, signed chars are explicitly disallowed. See also: `git grep delete ./src/serialize.h`

```
src/serialize.h:template <typename Stream, CharNotInt8 V> void Serialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t
src/serialize.h:template <typename Stream, CharNotInt8 V> void Unserialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822481)
No, std::memcpy is not `constexpr`, so adding it serves no purpose.

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900822539)
I am not touching this line of code, and don't want to, so maybe a separate issue or pull request seems more appropriate?

💬 glozow commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#discussion_r1900830981)
Should this be 2025?

🤔 glozow reviewed a pull request: "[28.x] Finalize 28.1"
(https://github.com/bitcoin/bitcoin/pull/31582#pullrequestreview-2527491597)
ACK 898db487f3f4b4ec131e46cb66ab01b35de2c4c7 unless copyright year should be changed

✅ maflcko closed a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692)

💬 maflcko commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-2567705751)
Closing for now, due to inactivity for more than a year.

💬 maflcko commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#discussion_r1900835187)
I think no code from 2025 was backported, so this seems fine to keep as-is for now.

💬 maflcko commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2567708618)
lgtm ACK 898db487f3f4b4ec131e46cb66ab01b35de2c4c7

💬 fanquake commented on pull request "[28.x] Finalize 28.1":
(https://github.com/bitcoin/bitcoin/pull/31582#issuecomment-2567714958)
There are still things tagged for backport for 28.x that haven't been backported.

💬 l0rinc commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900842605)
sure, I don't have a strong preference either way (while this reflects the method-name-to-size connection better, I admit it's more verbose), please resolve the comment

💬 l0rinc commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900843215)
That was my understanding as well, what I suggested was just to remove the redundant `inline`

💬 l0rinc commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900844439)
Yes, I saw, but is this the level where it should be disallowed (since this would technically work with signed char, disallowing it seems like a different abstraction level)? But if it is, please resolve the comment

📝 maflcko opened a pull request: "ci: Run functional tests in msan task"
(https://github.com/bitcoin/bitcoin/pull/31592)
Now that the CI machines have a bit more CPU, it seems good to run the functional tests as well under msan. (Also, bump the llvm minor version)

💬 maflcko commented on pull request "refactor: Allow std::byte in Read(LE/BE)":
(https://github.com/bitcoin/bitcoin/pull/31524#discussion_r1900848028)
If there is a use-case for this, it can be added, but I think limiting to "unsigned" bytes seems fine and preferable for now.