💬 maflcko commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942562336)
Thx, happy to push, but I'll wait until you are done with your review until you reached an ack-state before pushing again. Otherwise, I'll just spend cycles on finding the right commit to fixup the unrelated typo and then pushing it for each typo/nit separately. It is also easier for reviewers to place a single ack and a single re-ack, instead of re-acks for each typo individually.

💬 laanwj commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1942568100)
> if pindex is an old block (not the actual tip) and we also have no good headers available. In that scenario, the function would return 1.0 due to a lack of visibility beyond pindex.

Maybe that's how it is used at the moment, but the documentation of the function implies that the estimate can be used on any block, not that pindex necessarily is the latest block.

> I think the only way this happens is if a node just reconnected after being offline or is somehow isolated from the network.
...

💬 l0rinc commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942588417)
I posted this again since I saw that the build was failing, thought you'll need to push soon anyway.

----

Will the [copyright changes](https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1941965691) remain part of this PR? I would prefer reviewing that separately, but if you insist, I can do it here as well.

💬 egruper commented on issue " Cannot figure out how to use std::atomic error for MacOS Sequoia 15.2":
(https://github.com/bitcoin/bitcoin/issues/31579#issuecomment-2636307637)
> I cannot reproduce the issue on my MacOS Sequoia 15.2 (24C101):
>
> ```
> % clang --version
> Apple clang version 16.0.0 (clang-1600.0.26.6)
> Target: arm64-apple-darwin24.2.0
> Thread model: posix
> InstalledDir: /Library/Developer/CommandLineTools/usr/bin
> % cmake -B build -DWITH_BDB=ON
> <snip>
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC
> -- Performing Test STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC - Success
> <snip>
> ```
>
> Can anyone else provide th
...

👍 laanwj approved a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2595254264)
re-ACK e3622a969293feea75cfadc8f7c6083edcd6d8de

💬 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-2636340086)
> master currently does not have this issue.

However, the `cs_main` object is duplicated:
```
$ git rev-parse HEAD
94ca99ac51dddbee79d6409ebcc43b1119b0aca9
$ objdump -t build/src/bitcoind | grep cs_main$
0000000000b9d220 g O .data 0000000000000028 cs_main
$ objdump -t build/src/kernel/libbitcoinkernel.so | grep cs_main$
00000000003dc0a0 g O .data 0000000000000028 cs_main
```

✅ fanquake closed an issue: "macOS 13.7 depends build can't find qt (symlink)"
(https://github.com/bitcoin/bitcoin/issues/31050)

🚀 fanquake merged a pull request: "depends: Avoid hardcoding `host_prefix` in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31358)

🚀 fanquake merged a pull request: "test: added additional coverage to waitforblock and waitforblockheight rpc's"
(https://github.com/bitcoin/bitcoin/pull/31784)

💬 maflcko commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#discussion_r1942665581)
Up to you, you can skip review of the commit (and say so, and ask me to drop it), or you can review it. I'll just wait for your ack and further comments, so that everything can be wrapped up in as few force pushes and re-reviews as possible.

The CI issue was on the CI machine, so I fixed it there and did a re-run.

🤔 vasild reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2595012158)
Approach ACK 99b7b857d3b8b9ce9bd7501e2480583369740c55

I think it would be best if all sources are build with the same flags and the same build mechanism is used by devs, CI and depends. I also do not see a harm of an extra option which is off by default, to use an external libmultiprocess, if that makes libmultiprocess' devs life easier.

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942468885)
pico nit: in the commit message of b75e13e6ecce1b2b406ef83099200df42acf37e0 `scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake`:

```diff
- a find_package is call is made
+ a find_package call is made
```

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942473099)
`s/git submodule/git subtree/`

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942477051)
The comment says "when ENABLE_IPC is enabled" but the code is `if(WITH_LIBMULTIPROCESS)`. Shouldn't that be `if(ENABLE_IPC AND WITH_LIBMULTIPROCESS)`?

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942473641)
I find it could be confusing to have the same option `WITH_LIBMULTIPROCESS` before and after this PR with different semantics. Maybe `WITH_EXTERNAL_LIBMULTIPROCESS` is a better name for the new one?

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942612485)
Commit 17329d17ef2f640c5f03e36351f1e19f43acf108 `lint: Add exclusions for libmultiprocess subtree` looks suboptimal because it adds exceptions for the entire `src/ipc/libmultiprocess/` subtree for a whole class of issues. Because there are a few specific issues currently. Even if they are ok, it means new issues will sneak in `src/ipc/libmultiprocess/` unnoticed.

It would be better to either resolve the issues and not exclude `src/ipc/libmultiprocess/` from linters or add exceptions for the p
...

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1942670069)
```suggestion
package sources, or for testing local changes that are not available to
```

💬 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-2636453499)
@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.

💬 maflcko commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2636466716)
> I guess you mean that message processing is single threaded - the `b-msghand` thread. I agree it could change in the future.

I left the comment, because if the work is done in a different thread, this feature will silently break. I guess that is fine if the work is minimal, or code could be added to do the additional accounting if the work is sustantial.

💬 maflcko commented on issue "bitcoind loglevel=info too noisy":
(https://github.com/bitcoin/bitcoin/issues/31796#issuecomment-2636476646)
If you don't want the logs to end up in the journal at all, you can configure it so. If you want to disable logs completely, you can do so as well. If you want to filter out info logs, you can do so as well.

You can also modify the log level locally of the log that you don't want to see (Bitcoin Core is open source).

However, I don't think modifying the log level in the source code here is the correct fix, because (as mentioned) this only happens during IBD and other people expect the log.

Us
...