✅ achow101 closed a pull request: "GUI Related"
(https://github.com/bitcoin/bitcoin/pull/30831)

📝 Wbaker7702 opened a pull request: "Wbaker7702/patch 753"
(https://github.com/bitcoin/bitcoin/pull/30832)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30831)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ achow101 closed a pull request: "Wbaker7702/patch 753"
(https://github.com/bitcoin/bitcoin/pull/30832)

📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30832)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

🤔 ryanofsky reviewed a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2284579707)
Rebased af24810eeed397c2fe5f61ef9769e1b7ee0ecdf9 -> e225f7cbc3e6842f8e7f1c482c2aacd810e99c1b ([`pr/ipc-bind.5`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.5) -> [`pr/ipc-bind.6`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-bind.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-bind.5-rebase..pr/ipc-bind.6)) implementing suggestions and updating to use cmake

re: https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2263113816

> It would be ni
...

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1746474025)
re: https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1733464897

> I think this should be added to the `rpc_help.py` tests.

Is there mistake in this suggestion? This shouldn't be related to RPC

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1746473636)
re: https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1732716844

> Nit: There is a problem with the indentation here.

Thanks! Fixed

💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1746474704)
re: https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1733490705

> I think it would be good to sanity-check that invalid addresses make `bind` and `connect` throw:

Thanks added suggested tests.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746523131)
> By counting the excess we could give more fine-grained errors, e.g:

I don't think this is useful, because the error message will say that there are "too few/many format specifiers". However, the inverse might be true: "Too many/few args".

So I'll leave this as-is for now. I'd say the important thing is the error, not the exact wording of the error message.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746523781)
> If you still insist that this is better, please resolve the comment.

Yes, for now I'll resolve this. Thanks for the review!

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746525983)
Compilers can only walk so far in a consteval/constexpr context, so that it doesn't matter if there are 31 bits to count, or 64.

I'll leave this as-is for now.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2333261303)
> throw "Invalid format specifier!";


In tinyformat there is no such thing, so I'll leave this as-is for now.

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1746533300)
Thanks, switched to lambdas! (Kept the string_view for now)

💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2333269373)
Replied to all review comments and force pushed a small style-only change in the test.

👍 TheCharlatan approved a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2285020731)
Re-ACK e225f7cbc3e6842f8e7f1c482c2aacd810e99c1b

💬 TheCharlatan commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1746579005)
Huh, no idea what I was thinking here, but `rpc_help.py` is obviously wrong. Maybe I copy-pasted a wrong value? I think where it could go is the `system.cpp` fuzz tests.

💬 Sjors commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746620425)
Oops, should be `OP_TRUE`

💬 maflcko commented on pull request "build: work around issue with Boost <= 1.80 and Clang >= 18":
(https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2333439434)
Can you explain this change a bit better?

The warning will be turned into a hard error in a follow-up release to clang 18.

Testing with clang-20 shows a new warning, according to the release notes https://github.com/llvm/llvm-project/pull/102364/files#diff-ec770381d76c859f5f572db789175fe44410a72608f58ad5dbb14335ba56eb97R138

```
In file included from /b-c/src/rest.cpp:22:
In file included from /b-c/src/rpc/blockchain.h:13:
In file included from /b-c/src/validation.h:28:
/b-c/src/txme
...

💬 maflcko commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#discussion_r1746652701)
Not sure if this is the right fix. It may avoid an error when using specific versions of Clang in combination with specific versions of boost. However, it may also result in a new warning with specific versions of Clang: https://github.com/bitcoin/bitcoin/pull/30821#issuecomment-2333439434