💬 luke-jr commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637357864)
Downloading something separately is not a real issue.

But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2637377337)
All jobs happy!

I opened #31802 as draft which takes over the original intention for this PR. It flips the depends default and includes 743ae9b4e2746f48d707d49fc2c8ba908995034d which I dropped here.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2637412067)
> But once it's a subtree, you can't avoid downloading it, and it might even be messy to remove it (to ensure it isn't used).

By design, it is not messy to remove. The subtree is referenced in exactly one [`add_subdirectory`](https://github.com/bitcoin/bitcoin/blob/99b7b857d3b8b9ce9bd7501e2480583369740c55/src/ipc/CMakeLists.txt#L8) call, and nowhere else in the cmake build. If the `-DWITH_LIBMULTIPROCESS=ON` is set the `add_subdirectory` call is not made and a `find_package` call is made inst
...

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2637415972)
Merge conflict is probably from #25832.

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637460278)
> @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.
>
> To clarify this discussion, which binary you are referring to—`bitcoind` or `bitcoin-chainstate`? I assume the latter, as the former uses the static kernel library.

What I'm building isn't really relevant... the combo of the inline var with hidden visibility when built as a shared lib is a potential proble
...

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-2637463025)
Using BIP155 network IDs is a good idea! Planning to address https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1943088145 in a followup.

📝 fjahr opened a pull request: "fuzz: Extend mini_miner fuzz coverage to max block weight"
(https://github.com/bitcoin/bitcoin/pull/31803)
This is follow-up to #31384 which should be merged shortly. Only the last commit is new.

It expands the fuzz test to cover the full range of allow block sizes in `BlockAssembler` by utilizing the `block_adjusted_max_weight` option if necessary.

See also the brief discussion here for context: https://github.com/bitcoin/bitcoin/pull/31384/files#r1942039833

💬 fjahr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1943316627)
Done in #31803

👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for spkm migration"
(https://github.com/bitcoin/bitcoin/pull/29694)

💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2637502854)
Just fixed CI, ready for review!

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2637520847)
> * verificationprogress = just blocks/headers

I think the problem doing this is that not all blocks "are worth the same". So when doing IBD we would go fast to high % and then slow down a lot because newer blocks have more transactions. If we want the % to be accurate by the time we should still measure it by txs.
E.g. The first 50k blocks would be around 5% using this approach, but it takes just 1-2min to sync them as they are empty. This could lead to a fake feeling of fast sync.
Also, w
...

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1943343041)
NoteToSelf: Check this before next push

💬 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-2637550455)
> What I'm building isn't really relevant... the combo of the inline var with hidden visibility when built as a shared lib is a potential problem.

I think it could be problematic when an inline variable with hidden visibility is part of a static library, which is then linked both to a binary and to a shared library. If the binary does not share dependencies with the shared library, it should be safe.

💬 sipa commented on pull request "feefrac: add support for evaluating at given size":
(https://github.com/bitcoin/bitcoin/pull/30535#issuecomment-2637554110)
Rebased because of datedness.

✅ hebasto closed a pull request: "ci: Replace `CMAKE_CXX_FLAGS` with `APPEND_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/31726)

📝 hebasto opened a pull request: "ci: Remove no longer needed '-Wno-error=documentation'"
(https://github.com/bitcoin/bitcoin/pull/31804)
Picked from https://github.com/bitcoin/bitcoin/pull/31726.

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637569405)
From the docs

> The module definition file will be passed to the linker causing all symbols to be exported from the .dll. **For global data symbols, __declspec(dllimport) must still be used when compiling against the code in the .dll.** All other function symbols will be automatically exported and imported by callers

See bold text. I suspect that's the real issue here, and that inlining is the opposite of what we want.

💬 hebasto commented on pull request "ci: Replace `CMAKE_CXX_FLAGS` with `APPEND_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/31726#discussion_r1943372430)
Sure! Please see https://github.com/bitcoin/bitcoin/pull/31804.

💬 theuni commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2637572312)
> If the binary does not share dependencies with the shared library, it should be safe.

The binary has to find `cs_main` somewhere though. And we don't want it adding its own.

I think I understand what's happening now and I'm testing an alternative.

💬 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-2637577272)
> > If the binary does not share dependencies with the shared library, it should be safe.
>
> The binary has to find `cs_main` somewhere though.

Right. `bitcoin-chainstate` finds it in `libbitcoinkernel.so`.