💬 hebasto commented on pull request "msvc: Provide `ObjectFileName` explicitly":
(https://github.com/bitcoin/bitcoin/pull/27687#issuecomment-1551446156)
> Can you explain what the issue currently is, and why this is the right fix

The issue is the file name conflict.

See https://api.cirrus-ci.com/v1/task/6646912535756800/logs/build.log:
```
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\MSBuild\Microsoft\VC\v170\Microsoft.CppBuild.targets(1098,5): warning MSB8027: Two or more files with the name of util.cpp will produce outputs to the same location. This can lead to an incorrect build result. The files involved are ..\..\
...

💬 pinheadmz commented on issue "Change handling of "h" versus "'" marker for hardened derivation in descriptors":
(https://github.com/bitcoin/bitcoin/issues/15740#issuecomment-1551450881)
Closing as completed by #26076

✅ pinheadmz closed an issue: "Change handling of "h" versus "'" marker for hardened derivation in descriptors"
(https://github.com/bitcoin/bitcoin/issues/15740)

📝 fanquake opened a pull request: "doc: remove mention of glibc 2.10+"
(https://github.com/bitcoin/bitcoin/pull/27689)
We already require glibc 2.27+, so mentioning a much older version here is redundant.

💬 pinheadmz commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1551471794)
<img width="1552" alt="Screen Shot 2023-05-17 at 10 06 51 AM" src="https://github.com/bitcoin/bitcoin/assets/2084648/43139c94-d32d-4469-952b-25980242e1f6">



I've been running this branch for 7 days on a VPS and noticed this morning the CPU on b-msghand is back up to 100%. It was about that high running v24 release but dropped to 30% or so when I first switched to this branch and restarted.

📝 MarcoFalke opened a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690)
After https://github.com/bitcoin/bitcoin/pull/27562 the task should finish in less than 10 minutes, so also using credits for it will be cheap and improve dev experience.

👋 hebasto's pull request is ready for review: "msvc: Provide `ObjectFileName` explicitly"
(https://github.com/bitcoin/bitcoin/pull/27687)

💬 hebasto commented on pull request "msvc: Provide `ObjectFileName` explicitly":
(https://github.com/bitcoin/bitcoin/pull/27687#issuecomment-1551496570)
MSVC build is [free](https://api.cirrus-ci.com/v1/task/5943704519704576/logs/build.log) from `MSB8027` now.

👍 fanquake approved a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690#pullrequestreview-1430835654)
ACK aaaa07bc845f0b8741270552e77b9694802fb0e3 - if green

👍 fanquake approved a pull request: "doc: Rework build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27685#pullrequestreview-1430856715)
ACK fa29651c3ff3e13a42d6505b14971265562f088a

💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1551531499)
> What is the status of this

I've analyzed the [recent](https://cirrus-ci.com/build/5086582458286080) `ccache` summaries at https://github.com/bitcoin/bitcoin/commit/a75c77ea903c100531e0fc5fde94bb9b52642145.

I don't think this PR can improve them.

✅ hebasto closed a pull request: "ci: A few fixes of `ccache` issues"
(https://github.com/bitcoin/bitcoin/pull/27084)

💬 fjahr commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1551540184)
> I guess a simpler way to phrase it is: what can go wrong if [c14ae13](https://github.com/bitcoin/bitcoin/commit/c14ae132c5a5204a9a755c84c6de05fb30459221) picks the incorrect chainstate for a block? What are all the different ways in which we can receive a block? What do we expect to happen in each of these cases? Tests would be a really nice way to illustrate that, but are apparently a pain to write for net_processing.

@Sjors I have tried to create an overview of the different cases and add
...

👍 ryanofsky approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1430775316)
Code review ACK 0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e. These changes all seem very clean and straightforward now. I left some more suggestions, but you can feel free to ignore them them

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196631303)
In commit "kernel: Add fatalError method to notifications" (0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e)

Would maybe make `notifications` the first parameter. It seems awkward for it to placed between the debug message and the user message.

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196623448)
In commit "refactor: Move system from util to common library" (0a7b3e5058b58c448bbc473ffb347910d781c3bd)

I think it makes sense to keep `util::insert` and `util::AnyPtr` functions in `util/`, not `common/` as these are header-only template functions with no dependencies, and there's no particular reason they shouldn't be used by kernel code even if they aren't used right now. In terms of functionality they are also more related to other helper functions like `util/overloaded.h` `util/strings.
...

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196568945)
In commit "kernel: Add notification interface" (c6f2fbef2c8ecc0ded3abb9ab2fdd5369e954d52)

It would be good to write this accessor as an inline method `kernel::Notifications& GetNotifications() const { return m_options.notifications; }` for discoverability and consistency with other accessors above. Could also drop "Get" prefix since the other accessors don't seem to have it, but whatever you prefer is fine.

💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196627541)
In commit "kernel: Add fatalError method to notifications" (0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e)

Would be good to change `bilingual_str` to `const bilingual_str&`. That should also avoid the need to remove the forward declaration and include `#include <util/translation.h>` above.

💬 MarcoFalke commented on pull request "ci: Use credits for ARM task":
(https://github.com/bitcoin/bitcoin/pull/27690#issuecomment-1551551092)
added a commit to improve tsan scheduling times as well

💬 fanquake commented on pull request "depends: Boost 1.82.0":
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1551562113)
I'll leave this as part of #24742 for now.