💬 stickies-v commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2315263150)
> Wallet code originally did used ArgsManager directly until https://github.com/bitcoin/bitcoin/pull/22217, which changed it to use an interface class so wallet and node code could run in different processes and both could change settings without getting out of sync or corrupting the file.

Actually, doesn't the fact that the Rw methods on both `Node` and `Chain` interfaces directly use `ArgsManager` violate that principle too?

📝 l0rinc opened a pull request: "cmake: Improve cxx and linker flag caching"
(https://github.com/bitcoin/bitcoin/pull/30732)
Migration of https://github.com/hebasto/bitcoin/pull/340

`try_append_cxx_flags` and `try_append_linker_flag` are CMake functions used to test and conditionally apply compiler and linker flags to ensure compatibility with the build environment.

Currently the only case when the `SOURCE` parameter was overwritten was [bitcoin/bitcoin@8b6f1c4/CMakeLists.txt#L383-L389](https://github.com/bitcoin/bitcoin/blob/8b6f1c4353836bae6aa683cbc65251165bd031ba/CMakeLists.txt#L383-L389).
So currently the v
...

📝 l0rinc opened a pull request: "Remove unused src_dir param from run_tests"
(https://github.com/bitcoin/bitcoin/pull/30733)
Moved over from https://github.com/hebasto/bitcoin/pull/336

The `src_dir` usage was removed in https://github.com/bitcoin/bitcoin/commit/a8a2e364acf55bbe18404ab21f852d52257bcb6d#diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598, making the parameter unused.

📝 l0rinc opened a pull request: "Fix a few likely documentation typos"
(https://github.com/bitcoin/bitcoin/pull/30734)
Moved over from https://github.com/hebasto/bitcoin/pull/338

Found them during CMake migration - and ran a quick spellcheck for the rest to cover any remaining ones

💬 fanquake commented on pull request "cmake: fix a few likely documentation typos":
(https://github.com/bitcoin/bitcoin/pull/30734#issuecomment-2315293501)
You already have a PR open to fix typos.

💬 fanquake commented on pull request "doc: fix 3 simple CI codespell warnings":
(https://github.com/bitcoin/bitcoin/pull/30700#issuecomment-2315296321)
> The link in the pull description doesn't work, and can be removed.
> Also, you don't need to duplicate the full diff in the pull description.

> Can you update the .. commit to start with doc: instead of CI:?

Did you miss the review feedback here.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1734663166)
It contains the implementation for `TxDownloadManagerImpl` as well.

✅ maflcko closed a pull request: "cmake: improve cxx and linker flag caching"
(https://github.com/bitcoin/bitcoin/pull/30732)

📝 maflcko reopened a pull request: "cmake: improve cxx and linker flag caching"
(https://github.com/bitcoin/bitcoin/pull/30732)
Migration of https://github.com/hebasto/bitcoin/pull/340

`try_append_cxx_flags` and `try_append_linker_flag` are CMake functions used to test and conditionally apply compiler and linker flags to ensure compatibility with the build environment.

Currently the only case when the `SOURCE` parameter was overwritten was [bitcoin/bitcoin@8b6f1c4/CMakeLists.txt#L383-L389](https://github.com/bitcoin/bitcoin/blob/8b6f1c4353836bae6aa683cbc65251165bd031ba/CMakeLists.txt#L383-L389).
So currently the v
...

💬 maflcko commented on pull request "cmake: improve cxx and linker flag caching":
(https://github.com/bitcoin/bitcoin/pull/30732#issuecomment-2315302591)
(sorry, wrong button)

📝 maflcko opened a pull request: " cmake: Add option to use C++23 for testing "
(https://github.com/bitcoin/bitcoin/pull/30735)
There are no plans to switch to C++23 anytime soon in the next couple of years. The only place right now that is known to benefit is `src/compat/byteswap.h`.

However, it is still useful to test with the option, because deprecated, removed or changed language features, as well as compiler changes that are guarded by the language version will be tested and developers can learn about them upfront.

Also includes a minor doc fixup commit.

💬 maflcko commented on pull request "cmake: remove unused src_dir param from run_tests":
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2315327106)
You are modifying test files, not build files, so I don't think the cmake prefix makes sense. Also a two line trivial test-fix is easier to review in a single squashed commit, instead of being split up into several.

💬 maflcko commented on pull request "cmake: remove unused src_dir param from run_tests":
(https://github.com/bitcoin/bitcoin/pull/30733#issuecomment-2315330101)
Also, I think you can remove the mention where it is moved from. You left the comment in the thread where it was moved from and GitHub already registered the link. No need to put it into the pull description and merge commit forever.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1734686231)
Assuming this is on one of the earlier commits - the whole line is deleted in 0e6e135e303

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1734688133)
Woops, ran the script from HEAD~24 instead of HEAD~25. Fixed now.

⚠️ kandgioshima opened an issue: "Money"
(https://github.com/bitcoin/bitcoin/issues/30736)
10k to 100k

✅ fanquake closed an issue: "Money"
(https://github.com/bitcoin/bitcoin/issues/30736)

:lock: fanquake locked an issue: "Money"
(https://github.com/bitcoin/bitcoin/issues/30736)

💬 TheCharlatan commented on pull request "build: Add option to use C++23 for testing":
(https://github.com/bitcoin/bitcoin/pull/30735#issuecomment-2315345771)
Would it be better to add the required standard to the `core_interface` as a property and remove the global? Then the user can just pick their own standard with `-DCMAKE_CXX_STANDARD` on the command line.

💬 fanquake commented on pull request "build: Add option to use C++23 for testing":
(https://github.com/bitcoin/bitcoin/pull/30735#issuecomment-2315346031)
Kind of surprised we have to introduce a new option to make this work. I would have thought CMake would do something smarter where you could just override with `-DCMAKE_CXX_STANDARD=23` at configuration time.