💬 151henry151 commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#issuecomment-3417599834)
> Can you please fix the PR description and commit message? `CMAKE_SKIP_INSTALL_RPATH` is removed from the build system, and `CMAKE_SKIP_RPATH` (which does more than `CMAKE_SKIP_INSTALL_RPATH`) is added to the Guix build. Can you also remove the (what looks like) AI generated content from the PR description and commit message. i.e the bullet pointed list of changes, for a 2 line diff.

I think I've addressed the inaccuracies; thanks for the feedback. Regarding the bullet pointed list of change
...

💬 dipankarjana3657-bot commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-3417817836)
Hi

⚠️ dipankarjana3657-bot opened an issue: "Dj"
(https://github.com/bitcoin/bitcoin/issues/33648)
### Motivation

Hi

### Possible solution

_No response_

### Useful Skills

* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...


### Guidance for new contributors

Want to work on this issue?

For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.

💬 l0rinc commented on pull request "coins: use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#discussion_r2441590945)
I wanted to make it more realistic, but the tests do pass now without them, thanks for the feedback removed.

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-7.7x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2441593602)
Done, updated the commit messages and PR descriptions as well.

💬 purpleKarrot commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#issuecomment-3417857348)
ACK 4b41f99d57d822dfc258865d1dad03204fe0380f

It is a tiny step towards "variables that start with `CMAKE_` should not be set in `CMakeLists.txt`files".

💬 purpleKarrot commented on pull request "fuzz: don't pass (maybe) nullptr to memcpy()":
(https://github.com/bitcoin/bitcoin/pull/33644#issuecomment-3417898647)
> Given that the len argument is 0, memcpy() should not try to dereference the maybenull argument

According to the C standard (§7.26.1), the behaviour of `memcpy`is undefined if either `src`or `dst`are null. So it is not a bug.

✅ achow101 closed an issue: "Dj"
(https://github.com/bitcoin/bitcoin/issues/33648)

💬 fanquake commented on issue "[`v30.0`] `createNewBlock` never returns":
(https://github.com/bitcoin/bitcoin/issues/33647#issuecomment-3418147325)
cc @Sjors

💬 Sjors commented on issue "[`v30.0`] `createNewBlock` never returns":
(https://github.com/bitcoin/bitcoin/issues/33647#issuecomment-3418203160)
And it was not syncing new blocks during `createNewBlock()`? That's the only thing it's expect to wait for, but only once at startup:

```h
/**
* Construct a new block template.
*
* During node initialization, this will wait until the tip is connected.
*
* @param[in] options options for creating the block
* @retval BlockTemplate a block template.
* @retval std::nullptr if the node is shut down.
*/
virtual std::unique_ptr<BlockTemplate> createNewBloc
...

📝 famouswizard opened a pull request: "fix: invalid plist format and update values to meet macOS 1.0 standard"
(https://github.com/bitcoin/bitcoin/pull/33654)
corrected the outdated and invalid parts of the plist file to comply with the current macOS property list 1.0 specification.

**changes:**

* replaced the old `SYSTEM` declaration and `version="0.9"` with the proper `PUBLIC` declaration and `version="1.0"`.
* updated `LSMinimumSystemVersion` from `14` to `10.14.0` (using the proper `X.Y.Z` format).
* changed `<string>True</string>` to `<true/>` to use a valid boolean value instead of a string.

now validates correctly with Apple’s DTD an
...

⚠️ pinheadmz opened an issue: "importdescriptors: check for errors before rescanning"
(https://github.com/bitcoin/bitcoin/issues/33655)
I tried to import two descriptors with labels but one was an internal descriptor. I didn't realize that was not allowed until the entire rescan (for the first descriptor) completed and the return object included success false with that error message, for the second descriptor.

🤔 janb84 reviewed a pull request: "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script"
(https://github.com/bitcoin/bitcoin/pull/33470#pullrequestreview-3353589314)
re ACK 4b41f99d57d822dfc258865d1dad03204fe0380f

Changes since last ack:
- pr and commit message change

💬 ryanofsky commented on issue "[`v30.0`] `createNewBlock` never returns":
(https://github.com/bitcoin/bitcoin/issues/33647#issuecomment-3418442416)
Thanks for the clear steps to reproduce. I was able to see the rust client make the bitcoin node IPC hang very quickly (after around a minute) following them.

It seems like rust client is able to trigger a deadlock bug that was fixed in https://github.com/bitcoin-core/libmultiprocess/pull/201, and was backported in https://github.com/bitcoin/bitcoin/pull/33519 in the [30.x](https://github.com/bitcoin/bitcoin/commits/30.x) branch that was made after the [v30.0](https://github.com/bitcoin/bitcoin
...

💬 waketraindev commented on issue "importdescriptors: check for errors before rescanning":
(https://github.com/bitcoin/bitcoin/issues/33655#issuecomment-3418640920)
Reference: #32376, #31514, #33614

✅ l0rinc closed a pull request: "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions"
(https://github.com/bitcoin/bitcoin/pull/33569)

💬 l0rinc commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3418646843)
Thanks @hebasto, if we need to support earlier versions of the LLVM MinGW toolchain we can resurrect this PR.

💬 diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3418682105)
@pablomartin4btc can you please re-review?

👍 hodlinator approved a pull request: "net: make m_nodes_mutex non-recursive"
(https://github.com/bitcoin/bitcoin/pull/32394#pullrequestreview-3353773054)
ACK 4b3a2c23608e709a63b9d5bd69c3eb16cf08e462

Manually went through all functions using `CConnman::m_nodes_mutex` and only remaining issue I could find is now fixed (https://github.com/bitcoin/bitcoin/pull/32394/files#r2433600255).

Also checked out first commit and only cherrypicked mutex type change to re-confirm Clang spews warnings about missing annotations (and no warnings at HEAD of PR).

💬 hodlinator commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2442597625)
Agree that one cannot know unless one greps the entire codebase for insertions to `lNodesAnnouncingHeaderAndIDs`. Currently this is the only function.
But worst case, some parallel pull request could introduce another function that inserts more elements than 3. Using a dev-only `Assume` instead of `assert` as I suggested would make it less of a hazard.

Maybe it should be a precondition as well. Anyways, just a minor suggestion to attempt making existing tests against number 3 less magical.