🤔 hebasto reviewed a pull request: "build: Remove manpages when making MacOS app"
(https://github.com/bitcoin/bitcoin/pull/32064#pullrequestreview-2685743124)
Post-merge ACK 80b5e7f2cb7fbfbd724e1f52b00c0e72b79a200b.

💬 maflcko commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#issuecomment-2724882650)
> I thought of removing the `consteval_ctor` hack as well, now that [consteval conversion is claimed to work in visual studio](https://developercommunity.visualstudio.com/t/consteval-conversion-function-fails/1579014#T-N10721450), but since https://godbolt.org/z/j837req5h still seems to fail for latest `x64 msvc v19`, I postponed doing that.

I think the upstream bug link may be wrong. https://developercommunity.visualstudio.com/t/Can-not-initialize-array-with-consteval/10828473 looks closer

💬 maflcko commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#discussion_r1995691590)
Not sure what the goal here is, or the improvement. Looks like extra code is added to do the same thing in two different ways?

🤔 maflcko reviewed a pull request: "RFC: Add `operator""_uint256` compile-time user-defined literal"
(https://github.com/bitcoin/bitcoin/pull/31991#pullrequestreview-2685765529)
tend toward NACK for now, as I don't see the goal or benefit

Would be happy to ack a doc fix of the upstream msvc bug report

💬 maflcko commented on pull request "test: Add support for mainnet addresses in address_to_scriptpubkey":
(https://github.com/bitcoin/bitcoin/pull/32060#issuecomment-2724895783)
Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

💬 Sjors commented on issue "depends: capnp build ignores config_opts":
(https://github.com/bitcoin/bitcoin/issues/32068#issuecomment-2724899476)
Both macs have GNU Make 3.81

Trying your modification. If it works, it's yet another reason to migrate depends to cmake...?

📝 furszy opened a pull request: "test: fix intermittent failure in wallet_reorgsrestore.py"
(https://github.com/bitcoin/bitcoin/pull/32069)
In response to #32066 intermittent failure.

Wait until the node's process has fully stopped before starting a new instance of it.

💬 furszy commented on issue "intermittent issue in wallet_reorgsrestore.py in test_reorg_handling_during_unclean_shutdown: FailedToStartError: [node 0] bitcoind exited with status 1 during initialization. Error: Cannot obtain a lock on directory":
(https://github.com/bitcoin/bitcoin/issues/32066#issuecomment-2724902606)
#32069 attempts to fix it.

✅ l0rinc closed a pull request: "RFC: Add `operator""_uint256` compile-time user-defined literal"
(https://github.com/bitcoin/bitcoin/pull/31991)

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1995695052)
Thanks for taking the time to elaborate.

"Initialization phase" was too terse for me - of course we're in the initialization phase... duh. :)

Changed to "Suppress these as they are expected during initialization."

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1995687071)
Correct, especially now after maflcko's change in fae3bf6b870eb0f9cddd1adac82ba72890806ae3 to not call `stop()` on test failure, I agree it makes sense to assert instead, good suggestion! Fixed.

> were there actually any cases where CannotSendRequest exceptions were caught and suppressed? If so what were those cases? Commit description should say what goals of this change are and if CannotSendRequest is an actual or theoretical change in behavior.

It was a common knock-on issue from attemp
...

💬 maflcko commented on pull request "build: enable libc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#issuecomment-2724927316)
Would be nice to have steps to test this with gcc and clang. Otherwise, it will be harder to test and the real benefits are harder to see and touch. Also the pull title could be adjusted to mention the Debug restriction.

💬 maflcko commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-2724938439)
introduced in 11f8ab140fe63857f6a93b81021efda8f90ceeda, so added 30.0 milestone

💬 maflcko commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-2724943277)
lgtm ACK 0f377b4ef959d5722c02d39d63916233f6e30c64

Maybe there could be a method on test_node, similar to stop_node to kill the node ?

💬 hodlinator commented on pull request "RFC: Add `operator""_uint256` compile-time user-defined literal":
(https://github.com/bitcoin/bitcoin/pull/31991#issuecomment-2724946307)
> Would be happy to ack a doc fix of the upstream msvc bug report

Yes, it seems like our issue was closer this "sub-issue" in the currently linked issue: https://developercommunity.visualstudio.com/t/consteval-conversion-function-fails/1579014#T-N10550785 and they apparently only fixed the primary issue. :\

💬 Sjors commented on issue "depends: capnp build ignores config_opts":
(https://github.com/bitcoin/bitcoin/issues/32068#issuecomment-2724958506)
That did the trick.

💬 furszy commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-2724965232)
> Maybe there could be a method on test_node, similar to stop_node to kill the node ?

Done as suggested.

💬 maflcko commented on pull request "build: align debugging flags to `-O0`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2724974512)
> So my question is.. what does depends built with `-O0` actually get us?

It can't hurt to have the option to have a stronger msan build for depends (see https://github.com/bitcoin/bitcoin/issues/22064#issuecomment-2607120689), but I don't know if it will be beneficial by default or in other scenarios.

💬 maflcko commented on pull request "test: fix intermittent failure in wallet_reorgsrestore.py":
(https://github.com/bitcoin/bitcoin/pull/32069#issuecomment-2724997907)
lgtm ACK 36b0713edc4655f6e0c291975d6d280fbc89cf2e

📝 Sjors opened a pull request: "build: use make < 3.82 syntax for define directive"
(https://github.com/bitcoin/bitcoin/pull/32070)
From the GNU 3.82 [release notes](https://www.cloudbees.com/blog/whats-new-gnu-make-382):

> The define directive now supports the same assignment operators that regular variable assignment alows: := , ?= and += , for simple, conditional and appending assignments.

macOS ships with 3.81. This caused the multiprocess config options to be ignored.

Fixes #32068