💬 hebasto commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2357804575)
> Seems fine to do, but wasn't the idea to use qt6 once cmake was done? IIRC there was a tracking/meta issue, but I can't find it?

I'm working on migration to Qt 6. I'll post a few updates soon.

💬 maflcko commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2357810004)
review ACK 54227e681a4efa8961f1ad05d43366d88a9b686a

💬 maflcko commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#discussion_r1764614217)
Ok, fair enough. I just wanted to mention it. Seems fine either way.

💬 Zk2u commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2357864327)
Hi @starius, we've been testing this internally but can't seem to get our blocks below the 2m29s interval. Here's the signet challenge:

```
6a4c090105000000000000004c25512102efc19c310fd60351b84273392d5bc2c58746bd3e8deca043623c0a16065ec2ec51ae
```

Or:

```
6a
4c
09
01 0500000000000000 (signet params: 0x01, pow_target_spacing=5)
4c
25
512102efc19c310fd60351b84273392d5bc2c58746bd3e8deca043623c0a16065ec2ec51ae
```

Calibrated nbits via:
```
../../contrib/signet/miner --cli=".
...

💬 maflcko commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2357864794)
> So I think either the status quo or #30364 is better than this PR.

I tend to agree on a second thought. This pull just seems to increase the long-term confusion by treating the levels equivalent or similar, just because some parts of the code happen to confuse them (for various reasons).

Also, as pointed out in https://github.com/bitcoin/bitcoin/pull/30364#discussion_r1711345479 (and other comments) a good chunk of the log statements that this doc-only pull is trying to "fix" shouldn't b
...

💬 fanquake commented on issue "ci: ConnectionRefusedError: [WinError 10061] No connection could be made because the target machine actively refused it":
(https://github.com/bitcoin/bitcoin/issues/30390#issuecomment-2357907052)
https://github.com/bitcoin/bitcoin/actions/runs/10907936867/job/30272762595?pr=30918#step:12:226

💬 fanquake commented on pull request "cmake: Switch to crc32c upstream build system":
(https://github.com/bitcoin/bitcoin/pull/30905#issuecomment-2357931815)
Ok. Well the current changes would be unsafe to merge, and I don't think we should be adjusting our build system to work around the subtree, if anything, we should just change the subtree directly.

💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1764693774)
Makes sense. Don't feel strongly either, was asking for my own understanding. I meant the `RelayTransaction` comment as a separate topic, referring to transaction types (uint256 vs Txid/Wxid) in general, not specific to this line.

🚀 fanquake merged a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915)

🚀 fanquake merged a pull request: "doc: remove Eclipser fuzzing documentation"
(https://github.com/bitcoin/bitcoin/pull/30908)

💬 laanwj commented on pull request "ci: Use `ninja` to build in macOS native CI job":
(https://github.com/bitcoin/bitcoin/pull/30915#issuecomment-2357962161)
ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

Good to test this in the CI, i prefer building using ninja.

💬 pablomartin4btc commented on pull request "Fix both display issues for IPv6 proxy setup and reachable networks in Options Dialog (UI only, no functionality impact)":
(https://github.com/bitcoin-core/gui/pull/836#discussion_r1764753872)
I'll drop the changes on `src/qt/optionsdialog.cpp` as suggested by @vasild, since the approach is incorrect.

💬 maflcko commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1764760058)
> Definitely not a dumb question, this change results in an extra copy (only a move is happening on this line, but there is a copy inside the `getBlock()` function, but a copy is also removed later on due to adding std::move in `GenerateBlock`, so the net number of copies should be the same.

I don't understand this comment. This line is part of the `getblocktemplate` RPC, which is a real RPC (possibly used in production). Whereas the `GenerateBlock` function is a test-only function for the te
...

💬 rkrux commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2358014932)
@hodlinator Thanks for following the guide and reviewing it thoroughly. I agree with all your suggestions and have accepted them. The diagram is a nice touch. :)

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1764780151)
Marking as unresolved so we don't lose track of it.

💬 maflcko commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1764801712)
(I edited my comment, because it seems that the block is fully serialized into json anyway, which should be way more expensive than a copy, so probably fine, but I haven't benchmarked this)

💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2358175361)
```
operator(): Assertion `!txdownload_impl.RecentRejectsFilter().contains(package.back()->GetWitnessHash().ToUint256())' failed.
```

`ZT8AAAD/////ZaOjaf/y/////f39/f39odAq0KMAZWU+qOHh4eHdzMsrZTKgWuPMyytlcaBaZWVl
MaBaK8vczGUx/wwAUlrdzMsrZTGgWuPMyytlcaBa2WVl//////8xoFrczMsrZTEAUlrdzMsrZTGg
WuMrZTH/DABS/////w==`

[txdownload_impl_crash.txt](https://github.com/user-attachments/files/17042820/txdownload_impl_crash.txt)

Child is still in reject filter? Still (slowly and steadily) working t
...

👍 l0rinc approved a pull request: "log: Use ConstevalFormatString"
(https://github.com/bitcoin/bitcoin/pull/30889#pullrequestreview-2312360142)
ACK facbcd4cef8890ae18976fb53b67ea56b3c04454

💬 l0rinc commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1764863855)
Should this always be info?
Some `WalletLogPrintf` usages seem like [warnings](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/receive.cpp#L233-L234)), or [errors](https://github.com/bitcoin/bitcoin/blob/master/src/wallet/rpc/backup.cpp#L600).

💬 l0rinc commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1764855872)
nit: maybe we could dogfood this and use:
```suggestion
log_msg = tfm::format("Error \"%s\" while formatting log message: %s", fmterr.what(), fmt.fmt);
```