💬 glozow commented on pull request "Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling":
(https://github.com/bitcoin/bitcoin/pull/28031#issuecomment-2510096748)
Rebased. I have split the first 12 commits (the majority of this PR) into #31397. The other 5 commits are for making the module internally thread-safe, and can be opened in another PR afterward.

💬 hebasto commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2510135641)
Rebased. The commit messages have been amended.

@theuni
> Wait, I take that back. It seems like we could still get 2 separate `cs_main`s here: one in the lib and the other in the binary. ACK retracted while I investigate.

That's an interesting observation. Could you elaborate on how you think the two separate `cs_main` instances might arise?

If there's a relevant section in the documentation or standard that you believe applies, pointing it out would be helpful for clarity.

👍 tdb3 approved a pull request: "build: Set shared linker flags in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31395#pullrequestreview-2471520613)
ACK a8e04704f930de168fdddbb1c040dd70fbbe8ff7

On master, was able to reproduce a build error cross compiling for `arm64-apple-darwin` and `x86_64-apple-darwin`:
```
$ make -C depends HOST=arm64-apple-darwin -j11
$ cmake -B build -DBUILD_KERNEL_LIB=ON --toolchain depends/arm64-apple-darwin/toolchain.cmake
$ cmake --build build -j11
...
[ 38%] Linking CXX shared library libbitcoinkernel.dylib
/usr/bin/ld: unrecognised emulation mode: llvm
Supported emulations: elf_x86_64 elf32_x86_64 elf
...

📝 Princeprince559 opened a pull request: "Create till update my friend"
(https://github.com/bitcoin/bitcoin/pull/31401)
testing repo

📝 Princeprince559 converted_to_draft a pull request: "Create till update my friend"
(https://github.com/bitcoin/bitcoin/pull/31401)
testing repo

👋 Princeprince559's pull request is ready for review: "Create till update my friend"
(https://github.com/bitcoin/bitcoin/pull/31401)

✅ achow101 closed a pull request: "Create till update my friend"
(https://github.com/bitcoin/bitcoin/pull/31401)

📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/31401)
.

💬 maflcko commented on pull request "util: Drop boost posix_time in ParseISO8601DateTime":
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2510762011)
Thanks for checking! The reason is that MSVC is using `int` for `std::chrono::minutes::rep`, which is actually allowed, according to https://en.cppreference.com/w/cpp/chrono/duration#Helper_types

I reduced it further in https://godbolt.org/z/d4TKo497e

I pushed a workaround. If that doesn't work, the unit test and fuzz test can be adjusted to exclude too "extreme" timepoints.

💬 hodlinator commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1865495151)
Don't get it, what does *src/test/fuzz/coinscache_sim.cpp* have to do with this? Something with clearing coins?
By "failure", do you mean the error half of `CoinOrError`?

💬 maflcko commented on issue "Crash upon RPC v1 connection in v28.0.0":
(https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2510979925)
Thanks for taking a detailed look. If you don't mind, you could run it again through KDE heaptrack to see if there are any obvious low-hanging fruits for improvement of the memory usage.

In any case, I think it is clear that this is not a regression and I've removed the 28.x milestone.

💬 maflcko commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2510982877)
cc @darosior ?

💬 maflcko commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1865513573)
Yes, and disallowing could be approximated with a single call to `git grep` in the linter, similar to https://github.com/bitcoin/bitcoin/blob/dbc8ba12f3b3548dd6955293c5d650320ca39c5b/test/lint/test_runner/src/main.rs#L270

💬 TheCharlatan commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2511028472)
>One alternative that I have considered before (for chainstate fuzzing) is to abstract and further modularize BlockManager, which would allow us to have an InMemoryBlockManager for tests (especially useful for fuzzing but also nice in unit tests).

Forgot to post this here at the time, but I was working on an abstract block store for a different context some months ago: https://github.com/TheCharlatan/bitcoin/commit/5f35e506554bae293012ac108d8336801da2204a. I'm not sure how useful this actuall
...

💬 maflcko commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-2511041671)
> since mounting a ramdisk just is very easy

It is indeed easy, but for some reason I found mixed results when doing it by default in the CI: https://github.com/bitcoin/bitcoin/pull/31182 . I couldn't find a real improvement there, except for the `utxo_total_supply` fuzz target. (See also https://github.com/bitcoin-core/qa-assets/issues/158#issuecomment-2118120742)

So forcing the blockstore into ram for this target (and possibly others) could still be useful.

🚀 fanquake merged a pull request: "ci, macos: Install `pkgconf` Homebrew's package"
(https://github.com/bitcoin/bitcoin/pull/31399)

🚀 fanquake merged a pull request: "Remove `src/config` directory"
(https://github.com/bitcoin/bitcoin/pull/31390)

💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2511123617)
> We already clamp the block size in the block assembler

True, but I think it's always better to clearly inform the user when they're trying to do something impossible. That said, not everyone might agree with that. If other reviewers don't like it you can always drop the check.

Will review shortly.

🚀 fanquake merged a pull request: "dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate"
(https://github.com/bitcoin/bitcoin/pull/30039)

📝 dergoegge opened a pull request: "doc: correct libfuzzer-nosan preset flag"
(https://github.com/bitcoin/bitcoin/pull/31402)
`--prefix` is not the correct option for using a preset (it's not a option at all).