Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/30829)
.
📝 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
...
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`