💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2017677764)
> ... or drop the patch and re-add support for LTO later?

Agree on that.

Also, from https://bugreports.qt.io/browse/QTBUG-125034:
> LTCG is unfortunately known to be quite broken with MinGW toolchains.
> We don't support LTCG with MinGW but would accept patches if they're not too involved.

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2017677344)
Pushed new version of this commit - 51b2115a348f681e12aabf514cb156da4420da2e. Now links to the Python issue in the comment, and only mentions that it was encountered on Windows in the commit message.

🤔 hodlinator reviewed a pull request: "qa: Verify clean shutdown on startup failure"
(https://github.com/bitcoin/bitcoin/pull/30660#pullrequestreview-2723573980)
Pushed new version due to [recent discussion around socket timeout issue](https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2016198629).

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2017677498)
Fixed.

💬 willcl-ark commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2759695240)
I've found that I find that I can detect a gateway (with both tailscale `up` or `down`) by doing a specific routing query for a non-0.0.0.0 address. I am not totally sure what tailscale is doing behind the scenes to hijack netlink requests for a route to 0.0.0.0, but it's certainly interfering.

When I tried to debug what was returned by the original query for `0.0.0.0/0` it appears that I was getting about 70 results, all from table 52, which is a tailscale table.

I have got a patch which work
...

👍 hodlinator approved a pull request: "wallet: remove redundant `Assert` call when block is disconnected"
(https://github.com/bitcoin/bitcoin/pull/32153#pullrequestreview-2723786825)
Code Review ACK ae6b6ea296a228f342c3c635dc9e14c101e9534d

If something were so cheeky as to modify the incoming `const interfaces::BlockInfo& block::data` while we are using it, the approach on master would be broken too.

💬 hodlinator commented on pull request "wallet: remove redundant `Assert` call when block is disconnected":
(https://github.com/bitcoin/bitcoin/pull/32153#discussion_r2017695755)
If you re-touch, I think it would be more correct to remove the `#` in the commit message at the beginning of
`refs #https://github.com/bitcoin/bitcoin/pull/31757#discussion_r1995416821`
since it is a link and not a PR/Issue number.

💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#issuecomment-2759755909)
Alright, after spending some more time with this I have convinced myself that the "fill up with small transactions" approach is somewhat reasonable and intuitive. An alternative approach would have been to continue looping after the last large transaction didn't fit and try to halven the last input and output numbers until we encounter a transaction that fits. This would save us a few loops probably but I think it would be less intuitive.

What I wasn't happy with was that we were doing only f
...

💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r2017708099)
Went ahead an used your suggestion in [be71af3](https://github.com/bitcoin/bitcoin/pull/29500/commits/be71af3cc0b0bcb7d917cc6f2e5fda119f1b1bd6)

it now looks like this
```
2025-03-27T23:14:10.629000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/mnt/shared_drive/DEVDIR/bitcoin/test/functional/test_framework/test_framework.py", line 182, in main
self.run_test()
File "/mnt/shared_drive/DEVDIR/bitcoin/./build_dir/test/functional/p2p_blockfilters.p
...

🚀 fanquake merged a pull request: "test: get rid of redundant TODO tag"
(https://github.com/bitcoin/bitcoin/pull/32058)

🚀 fanquake merged a pull request: "Follow-ups for txgraph #31363"
(https://github.com/bitcoin/bitcoin/pull/32151)

💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2017831104)
> 1. Restrict search paths to the depends.

Looking at CMake output on my Fedora box, it seems to be searching in a mix of the depends and system libraries:
```bash
-- Found XCB_XFIXES: /root/bitcoin/depends/aarch64-unknown-linux-gnu/lib/libxcb-xfixes.so (found version "1.14")
-- Found XCB_ATOM: /root/bitcoin/depends/aarch64-unknown-linux-gnu/lib/libxcb-util.a (found version "0.4.0")
-- Found XCB_AUX: /root/bitcoin/depends/aarch64-unknown-linux-gnu/lib/libxcb-util.a (found version "0.4.0")
...

🤔 Prabhat1308 reviewed a pull request: "wallet: remove redundant `Assert` call when block is disconnected"
(https://github.com/bitcoin/bitcoin/pull/32153#pullrequestreview-2724166661)
Code Review ACK [`ae6b6ea`](https://github.com/bitcoin/bitcoin/pull/32153/commits/ae6b6ea296a228f342c3c635dc9e14c101e9534d)

The check to make sure `block.data` is non-null inside the loop is redundant. The check is already made at the start and the check inside the loop can be removed safely/.

🤔 Prabhat1308 reviewed a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134#pullrequestreview-2724223481)
crACK [`78327f9`](https://github.com/bitcoin/bitcoin/pull/32134/commits/78327f9a8d2e43c48f6b8f8fbdd7ef4848537483)

💬 Prabhat1308 commented on pull request "descriptors: Multipath/PR 22838 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32134#discussion_r2017891457)
I don't like the name `substitutes`. It implies that it is an alternative to a one major path which I don't think is the case ?

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

Thank you for your review!

Your feedback has been addressed:

1. Removed `utc_from_string_no_optimize.patch`. See https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2016764944.

2. Removed `macos_skip_version_checks.patch`. See https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2016781416.

3. Removed `windows_lto.patch`. See https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2016791355.

We have to re-add LTO support for Windows builds in a follow-u
...

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2018005753)
Thanks! It works for me too, so I've dropped the patch. It was possibly a side effect of https://github.com/bitcoin/bitcoin/pull/31048.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2018006874)
Thanks for checking it out! I’ve [removed](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2760340414) the patch.

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2018007605)
The patch has been [dropped](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2760340414).

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r2018011199)
I've [added](https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2760340414) a new patch that addresses you comment for trivial cases. We can address other cases, where tools are more deeply integrated in the Qt's build system, in follow-ups.