📝 hebasto opened a pull request: "ci: Do not try to install for fuzz builds"
(https://github.com/bitcoin/bitcoin/pull/32014)
This PR is a follow-up to https://github.com/bitcoin/bitcoin/pull/31844 and extends the changes from fb0546b1c5ebb858605bef4c9fa001782e0ab213 to all fuzz builds in the CI.

Fixes https://github.com/bitcoin/bitcoin/issues/32001.

💬 hebasto commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705830610)
FWIW, `BITCOIN_GENBUILD_NO_GIT` was introduced in https://github.com/bitcoin/bitcoin/pull/7522.

💬 hebasto commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705837980)
> > in some build systems
>
> Which build systems? This should be documented with a specific example, especially since it seems like an uncommon usecase/workflow.

Gentoo, I assume: https://bugs.gentoo.org/476294.

💬 hebasto commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2705851491)
Should this issue still be tagged for v29.0? If so, what immediate actions are expected?

💬 hebasto commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705863417)
> You can make things work using `-DAPPEND_LDFLAGS`, but using this (non-standard workaround) shouldn't be necessary.

This can be achieved by introducing a dedicated build option, such as `ENABLE_STATIC_PIE`, if that's acceptable.

💬 Juma-creator commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705905947)
. **Replace Project Name and Version:**
- Change `project(MyProject VERSION 1.0)` to your project's name and version.
- Example:
```cmake
project(YourProjectName VERSION 2.0)
```

2. **Specify C++ Standard:**
- Adjust the C++ standard if necessary.
- Example:
```cmake
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
```

3. **Organize Project Structure:**
- Adjust the directory structure to match your project. Ensure your source fil
...

💬 fanquake commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705911816)
> Gentoo, I assume: https://bugs.gentoo.org/476294.

Reading that thread, it's not really clear. The last comment (7 years ago) suggests that there was no issue with 0.13.0, even though the fix from https://github.com/bitcoin/bitcoin/pull/7522 only went into 0.15.0? It'd be good if someone could demonstrate an issue today, with the current source code.

💬 fanquake commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705919305)
> introducing a dedicated build option, such as ENABLE_STATIC_PIE, if that's acceptable.

I don't think so. We shouldn't have to introduce our own options, to map to compiler options, to workaround CMake issues. This seems like something that needs to be solved in CMake itself.

💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#discussion_r1984719254)
Done

💬 dergoegge commented on pull request "ci: Do not try to install for fuzz builds":
(https://github.com/bitcoin/bitcoin/pull/32014#discussion_r1984723062)
Why does this need to change? The regular msan job is not broken afaik.

💬 fanquake commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-2705951177)
@reardencode It's no-longer possible to re-open:


You'll need to open a new PR.

💬 Sjors commented on issue "CI: Cmake warnings should be errors":
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2705965427)
This doesn't seem release blocking to me.

The only open PR that points to this issue is #31665 which isn't marked for v29.

💬 fanquake commented on issue "build: cmake --install fails after --target deploy":
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2705974429)
Note that this isn't Windows specific. The same happens for macOS build.

💬 hodlinator commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#discussion_r1984747128)
Thanks! Although you are still inserting the loop in the middle of the old comment. How about this? (Or putting the `nsat_return`-block with comment last, not sure what makes most sense).
```C++
// for nsat_return, it expects node.keys.size() number of ZEROs, thus adding this for loop
InputStack nsat_return;
for (size_t i = 0; i < node.keys.size(); ++i) {
nsat_return = std::move(nsat_return) + ZERO;
}
auto& nsat{nsat_return};
// The dissatisfaction consists of as many empty vectors a
...

💬 Sjors commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2705984429)
Should we just add a release note about this issue? IIUC it's a regression in our move to cmake, but I suspect that anyone who actually wants to use `-O3` can use master (once it's fixed) or will be happy to review a backport.

💬 Sjors commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705993383)
Same suggestion as https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2705984429 to document this as a known issue in release notes.

📝 vasild opened a pull request: "net: replace manual reference counting of CNode with shared_ptr"
(https://github.com/bitcoin/bitcoin/pull/32015)
Before this change the code used to count references to `CNode` objects manually via `CNode::nRefCount`. Unneeded `CNode`s were scheduled for deletion by putting them in `CConnman::m_nodes_disconnected` and were deleted after their reference count reached zero. Deleting consists of calling `PeerManager::FinalizeNode()` and destroying the `CNode` object.

Replace this scheme with `std::shared_ptr`. This simplifies the code and removes:
`CNode::nRefCount`
`CNode::GetRefCount()`
`CNode::AddRef
...

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2706012277)
cc @willcl-ark, @dergoegge, @theuni

💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2706014011)
If I understand the remaining criticism correctly, it seems best to merge this before branch-off and improve things later.

The number of users that will be confused if we delay this change until _after_ branch-off is potentially much larger than the number of "everyday developer" folks impacted by the silent breakage here. For people who build the release from source, it's better if we educate them about the cmake changes and the new binary locations at the same time.

> the silent failures
...

💬 michaelfolkson commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-2706016378)
> I encourage more general, conceptual discussion to happen on [Delving Bitcoin](https://delvingbitcoin.org/) and not on this pull request.

> @jamesob I'd be happy to help you keep this thread focused by moderating off-topic comments and pointing them to the appropriate forums.

To state the obvious if you push "conceptual discussion" (a critical part of the Bitcoin Core review process for a PR) to external forums (why not an accompanying issue in this repo?) the consideration of whether to
...