💬 fanquake commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2396262864)
@promag sed this commit in the switchover PR: https://github.com/bitcoin/bitcoin/commit/812f3e7727211a60989e613bd5edf0a4ec3f146f. Qt 6.7.0 or later will be required for Windows (although as far as I'm aware, Qt 5 won't be supported generally in any case).

💬 maflcko commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2396270745)
Looks good to me either way as well. Merge now and remove later, or change later. Not sure how hard the overall changes are, but "spreading-out" review could be useful. Though, no strong opinion, either way is fine.

💬 vasild commented on pull request "chainparams: Remove seed.bitcoinstats.com":
(https://github.com/bitcoin/bitcoin/pull/30720#issuecomment-2396293474)
> Great, I'm just testing it as we speak, and I'll open a new PR adding it, once its shown to work, if that's ok.

wen pr?

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1789788111)
Ugh, I should have drafted this PR once I opened #30968 , since I opened it after being fed up with the init logic in this change. The error handling introduced there forces you to do the right thing in this change now. Sorry for the churn!

💬 TheCharlatan commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#issuecomment-2396295598)
Rebased 63dfc6067bd7fe06e5a440c229cca030bba6d53a -> 63dfc6067bd7fe06e5a440c229cca030bba6d53a ([blockmanDB](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB) -> [blockmanDB](https://github.com/TheCharlatan/bitcoin/tree/blockmanDB), [compare](https://github.com/TheCharlatan/bitcoin/compare/blockmanDB..blockmanDB))

* Fixed conflict with #30968

👍 maflcko approved a pull request: "test: Fix copy-paste in wallet/test/db_tests ostream operator"
(https://github.com/bitcoin/bitcoin/pull/31038#pullrequestreview-2351318001)
lgtm ACK f50557f5d36f568b7fe65ff5e242303b16b9b258

💬 maflcko commented on pull request "test: Fix copy-paste in wallet/test/db_tests ostream operator":
(https://github.com/bitcoin/bitcoin/pull/31038#discussion_r1789799513)
unrelated nit: Could make this `string_view` to begin with, to avoid going the extra step over `Span`. This will also make my diff smaller to switch `Span` to `std::span`. But just a nit.

🚀 fanquake merged a pull request: "cmake: Avoid hardcoding Qt's major version in Find module / variable names"
(https://github.com/bitcoin/bitcoin/pull/31010)

💬 maflcko commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#discussion_r1789818210)
Did you also check the qemu builds? Valgrind and Tsan may be faster than system- or user- qemu, but I haven't checked. I wonder if `TEST_RUNNER_TIMEOUT_FACTOR * 30` can be re-used here, in case someone wants to run in qemu in addition to a sanitizer? (Either way is fine for the CI in this repo, but I don't know what people are doing outside this repo)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396353071)
> The PR description says you need Ninja, but `build-osx.md` does not mention this.

Ninja is required to build the `qt` package in depends. It is mentioned in `depends/README.md`. However, `doc/build-osx.md` does not cover building with depends, so I don't think it is necessary to mention Ninja there.

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2396394710)
I dropped 1ea75505d18eea8e5f00f08d1e9e5b348691c7c0 again and instead opened https://github.com/Sjors/bitcoin/pull/65 to figure out how to best enable multiprocess on non-depends CI. It seems rather non-trivial.

💬 hebasto commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2396400648)
After an offline discussion with @fanquake, I'm going to proceed with merging this PR.

🚀 hebasto merged a pull request: "qt6: Handle different signatures of `QANEF::nativeEventFilter`"
(https://github.com/bitcoin-core/gui/pull/840)

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2396412336)
Rebased.

💬 josibake commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1789888213)
Sorry, I don't think my last comment on this was very clear. Lines L109-L112 are moved up from below, and `cache_sizes.block_tree_db` is set. So I think L127 should be:

```suggestion
.block_tree_db_cache_size = cache_sizes.block_tree_db,
```

unless I'm missing something?

💬 fanquake commented on issue "assumeutxo: not syncing the snapshot chainstate":
(https://github.com/bitcoin/bitcoin/issues/30971#issuecomment-2396436910)
> But did loading of the snapshot actually complete eventually

It did. I haven't had time to (re-)test anything here further. I don't think I have the full logs to share any more, but I will try and recreate them.

💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1789897798)
semi-related: I wonder if Windows can be bumped to Windows 10 either way. If long EOL, and ancient versions of Windows are documented to be supported, people may use them and open up themselves to security issues. I think it could be useful to only list supported (receiving security updates) operating systems here.

Could be done here, or in a separate pull.

⚠️ fanquake opened an issue: "lint: markdown check linting build outputs"
(https://github.com/bitcoin/bitcoin/issues/31044)
Running the linter locally I see this output:
```bash
DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-linter
<snip>
One or more markdown links are broken.

Relative links are preferred (but not required) as jumping to file works natively within Emacs.

Markdown link errors found:
[Err ] ./depends/work/build/x86_64-apple-darwin/qt/6.7.2-b49980c3ba7/qtbase/tests/auto/corelib/serialization/qxmlstream/XML-Tes
...

💬 fanquake commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#discussion_r1789919608)
> I wonder if TEST_RUNNER_TIMEOUT_FACTOR * 30 can be re-used here,

Sure, updated to use this. I haven't run a s390x build for a while, but I'd assume it would fail here. Started one in any case.

💬 maflcko commented on pull request "ci: set a ctest test timeout of 1200 (20 minutes)":
(https://github.com/bitcoin/bitcoin/pull/31026#issuecomment-2396496103)
review ACK 56aad83307e46983a397236bd0959e634207f83e

No change to the new timeout value of 1200 in this force push, but making it configurable, in case someone needs that, and also in symmetry with the functional tests.