💬 fanquake commented on pull request "msvc: Provide `ObjectFileName` explicitly":
(https://github.com/bitcoin/bitcoin/pull/27687#issuecomment-1551378680)
Can you explain what the issue currently is, and why this is the right fix (as opposed to https://github.com/bitcoin-core/gui/pull/729)? Seems odd to have intermittent link failures.

💬 fanquake commented on pull request "doc: Rework build-unix.md":
(https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196508636)
Didn't drop `Security` ?

👍 fanquake approved a pull request: "build: Bump minimum supported Clang to clang-10"
(https://github.com/bitcoin/bitcoin/pull/27682#pullrequestreview-1430690452)
ACK fa199ee614a7ed99c6caf329093a3573ea5a664b

💬 MarcoFalke commented on pull request "doc: Rework build-unix.md":
(https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196517868)
Yeah, not sure if it fits in here, maybe open a separate pull for that?

💬 MarcoFalke commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551399068)
Looks like iwyu doesn't work with qt?

```
(qt/test/util.h has correct #includes/fwd-decls)

qt/test/util.cpp should add these lines:
#include <QtCore/qobjectdefs.h> // for FunctionPointer<>::IsPointerToMember...
#include <qapplication.h> // for QApplication
#include <qlist.h> // for QList
#include <qmessagebox.h> // for QMessageBox
#include <qobject.h> // for qobject_cast
#include <qpushbutton.h> // for QPushButton
#include <qstring.
...

💬 MarcoFalke commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1551402978)
> can't find logs

What happens if you start the program in a terminal app and pass `-printtoconsole=1` to it?

💬 fanquake commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551403254)
> Looks like iwyu doesn't work with qt?

We might have to provide a mapping file, for it to work sanely? i.e https://github.com/include-what-you-use/include-what-you-use/blob/master/qt5_11.imp.

💬 ryanofsky commented on pull request "init: verify blocks data existence only once for all the indexers":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1551406710)
Looking at current version of the PR 6dfec1b5fbb0f8e95a6134ea8b27e9d6c285d171 it seems to have changed a lot, and the other thing it is doing now is delaying startup of indexes until blocks are loaded, so there is no longer a conflict with `reindex-chainstate`. So it is basically reimplementing #25193 in a different way that doesn't require a `sleep_for(std::chrono::milliseconds(500))` waitloop. I think this approach is cleaner, but also that #25193 is a smaller more targeted change with a test,
...

💬 ddykeman1 commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1551416962)

💬 ryanofsky commented on pull request "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/25193#issuecomment-1551420724)
> Thanks! I'll rebase and address furszy's comments next week!

I think it'd be good to just rebase this and merge it and not try to do the "move the indexes threads start after the loading process" change here. This PR is pretty simple, has had a good amount of review and testing, and I think that change would make it more complicated. furszy also implemented that change separately in #27607, and it should simplify both PRs to base that change on top of this one.

💬 ddykeman1 commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1551423215)
This is all that I'm getting as of now the program then shuts down

👍 ryanofsky approved a pull request: "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/25193#pullrequestreview-1430738915)
Code review ACK 974140f9e721740f857b45d10d7dbab62fdbbe53. Confirmed this is just a clean rebase since my last review. This needs another rebase now, but after that I would like to merge it.

📝 fanquake opened a pull request: "doc: remove Security section from build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27688)
Our compile documentation isn't the right place for genric binary hardening notes, which are neither particularly Bitcoin-Core specific, or as relevant as they might have once been, i.e non-executable stacks are now just the norm.

Just remove the notes for now, if someone has something more interesting/Bitcoin Core specific, it could be added in separate documentation in the future (maybe into the devwiki or similar).

Split from https://github.com/bitcoin/bitcoin/pull/27685#discussion_r119
...

💬 fanquake commented on pull request "doc: Rework build-unix.md":
(https://github.com/bitcoin/bitcoin/pull/27685#discussion_r1196552617)
Ok. Done in #27688.

💬 furszy commented on pull request "init: verify blocks data existence only once for all the indexers":
(https://github.com/bitcoin/bitcoin/pull/27607#issuecomment-1551443566)
Was about to send the updates comment hehe. I was too tired last night to write it. Thanks for the review ryanofsky!

Updates list:

- Decoupled index `Init()` from `Start()`. So indexers can be initialized without spawning the sync thread.

- Simplified the pruning violation code by re-purposing the `GetFirstStoredBlock` function. Now called `IsBlockDataAvailable`.

- Fixed a small race, where we set the index `m_synced` flag (which enables `BlockConnected` events) before calling to th
...

💬 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.