Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 l0rinc commented on pull request "Don't wipe coins cache when full and instead evict LRU clean entries":
(https://github.com/bitcoin/bitcoin/pull/31102#issuecomment-2567634389)
> I think https://github.com/bitcoin/bitcoin/pull/31132 is a better approach, because cache misses are fetched in parallel much faster so the misses are not as important.

I see the two changes as orthogonal - the main goal of this change as far as I understand is to avoid `ReallocateCache` calls (deleting unlikely entries to avoid filling the cache), while the https://github.com/bitcoin/bitcoin/pull/31132 is mostly about cache warming (adding likely ones to the cache in parallel).

But as y
...
⚠️ Prabhat1308 opened an issue: "Fuzz: Runtime errors when running fuzz tests on MacOs"
(https://github.com/bitcoin/bitcoin/issues/31591)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Whenever running any fuzz test on any fuzz target on MacOs according to the steps provided [here](https://github.com/bitcoin/bitcoin/issues/31049). There is a big error log being thrown before starting the fuzz tests. To be noted that the fuzz tests run fine afterwards.

### Expected behaviour

No error is thrown and fuzz tests run as expected.

### Steps to reproduce

Set up environment
...
💬 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