💬 laanwj commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1864872630)
An alternative idea would be to add a `btcStringToSat` function, instead of overloading `btcToSat` on type, as this makes type checking harder.

💬 laanwj commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1864872826)
Yes, passing sats as a decimal should never be necessary.

💬 purpleKarrot commented on pull request "cmake: add optional source files to bitcoin_crypto directly":
(https://github.com/bitcoin/bitcoin/pull/31268#issuecomment-2509746149)
Yeah, they might also not realize that compiler options specified at the target level might take precedence over those specified globally...

📝 suriyaa opened a pull request: "doc: Update license year range to 2025"
(https://github.com/bitcoin/bitcoin/pull/31400)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
**Just a trivial change by updating the year.**

*Comparable to older PRs such as https://github.com/bitcoin/bitcoin/pull/29222.*

(This PR can be merged by a Bitcoin Core maintainer at the end of year 2024 or at
...

💬 l0rinc commented on pull request "doc: Update license year range to 2025":
(https://github.com/bitcoin/bitcoin/pull/31400#issuecomment-2509755566)
Nack - we don't usually update copyright headers one-by-one.

💬 hebasto commented on pull request "util: Drop boost posix_time in ParseISO8601DateTime":
(https://github.com/bitcoin/bitcoin/pull/31391#issuecomment-2509829044)
> Looks like MSVC is broken, according to the GHA run:
>
> ```
> D:/a/bitcoin/bitcoin/src/test/util_tests.cpp(334): error: in "util_tests/util_ParseISO8601DateTime": check ParseISO8601DateTime("9999-12-31T23:59:59Z").value() == 253402300799 has failed [-4295736961 != 253402300799]
> ```
>
> It would be good if a someone using Windows reproduced this locally and then reported the bug to MSVC. Godbolt draft: https://godbolt.org/z/4WYn6E61W

See https://developercommunity.visualstudio.com
...

👍 TheCharlatan approved a pull request: "ci, macos: Install `pkgconf` Homebrew's package"
(https://github.com/bitcoin/bitcoin/pull/31399#pullrequestreview-2471453504)
Lgtm ACK e2f2698395c2535eeada8d91f270643c77aa7099

💬 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