📝 Wbaker7702 opened a pull request: "GUI Related"
(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
...
(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: "GUI Related"
(https://github.com/bitcoin/bitcoin/pull/30831)
(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
...
(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
...
(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)
(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
...
(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
...
(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
(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
(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.
(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.
(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!
(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.
(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.
(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)
(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.
(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
(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.
(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`
(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
...
(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
...