💬 ryanofsky commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2600677706)
re: https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546

FWIW, I think an explicit `(void)connection_client.release()` statement would make it more obvious that `connection_client` is null than the other other alternatives of calling `release()` inline or it assigning to a `_` variable. Similarly, I don't think would be very useful to declare a `bootstrap_cap` variable that is immediately moved from and unusable. But any of the approaches seems ok.

💬 hebasto commented on pull request "guix: Build for macOS using Clang only":
(https://github.com/bitcoin/bitcoin/pull/32764#issuecomment-3629761373)
> Looks like this still using the GNU binutils for building native tools.

Should be fixed now.

🤔 ryanofsky reviewed a pull request: "test: interface_ipc.py minor fixes and cleanup"
(https://github.com/bitcoin/bitcoin/pull/34003#pullrequestreview-3555280211)
Thanks for the reviews!

<!-- begin push-3 -->
Updated 6ef092c0e4343fc421c323ff09d3c791fb4bc33a -> c358910b8cd6d1db51c10becb0cbcb58709b738f ([`pr/pyipc.2`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.2) -> [`pr/pyipc.3`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/pyipc.2..pr/pyipc.3))<!-- end --> applying suggestions and adding a new commit to test actions during waitNext calls instead of before them.

I tried to
...

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600997265)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209

Added a new commit to make this check work as originally intended and call generate during the wait instead of before.

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2601000748)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2592842483

Make sense, applied the suggestion

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600998615)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386

Added a new commit to make this check work as originally intended and send the transaction during the wait instead of before.

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600996312)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598816528

Nice suggestion. Applied with a few tweaks

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600995593)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2592834249

Thanks, fixed message. I forgot to update it when the commit was split up.

💬 frankomosh commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#issuecomment-3630520154)
re-ACK 67740df

📝 maflcko opened a pull request: "util: Add some more Unexpected and Expected methods"
(https://github.com/bitcoin/bitcoin/pull/34032)
Reviewers requested more member functions In https://github.com/bitcoin/bitcoin/pull/34006.

They are currently unused, but bring the port closer to the original `std::expected` implementation:

* Make `Expected::value()` throw when no value exists
* Add `Unexpected::error()` methods
* Add `Expected<void, E>` specialization
* Add `Expected::value()&&` and `Expected::error()&&` methods

Also, include a tiny tidy fixup:

* tidy: Set `AllowCastToVoid` in the `bugprone-unused-return-value
...

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601567460)
thx, added `void std::expected<void, B>::value()` in https://github.com/bitcoin/bitcoin/pull/34032/commits, fa4794dfeb9eddf703c9ea5c5a29433063038639

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601569108)
thx, pushed in https://github.com/bitcoin/bitcoin/pull/34032/commits , faf8b247ba9b23e7c4300019de972b947bfbc698

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601570571)
thx, pushed in https://github.com/bitcoin/bitcoin/pull/34032/commits, fa7ceec5b07fbc99c4cc247803f78fb8b019c138

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601573527)
> Well, Rust practically encourages shadowing variables. 🤮

I love it :heart_eyes:



> (void)

Thx, pushed https://github.com/bitcoin/bitcoin/pull/34032/commits, fa5811c2e80c48af598c646c5c917472623c55fb

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601578414)
thx, fixed as a side-effect of https://github.com/bitcoin/bitcoin/pull/34032/commits , fa4794dfeb9eddf703c9ea5c5a29433063038639

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601580170)
Sure, pushed https://github.com/bitcoin/bitcoin/pull/34032/commits, fa20a50f46307b45a82a6e4283a945ef2f9f92a2

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601583952)
thx, this is part of https://github.com/bitcoin/bitcoin/pull/34032/commits, fa4794dfeb9eddf703c9ea5c5a29433063038639

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2601585428)
left as-is for now

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3630985370)
I pushed https://github.com/bitcoin/bitcoin/pull/34032 to add all of the unused member functions requested by reviewers, and resolved the threads here. let me know if I have forgotten anything.

💬 maflcko commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#issuecomment-3631049559)
> I think this would benefit from a brief release note. People might be confused why they're no longer seeing inbound connections in their logs?

I don't think there is a behavior change here. Previously, inbound would only be logged in when net-debug logging, now, it is the same. The only behavior change is the added `[net]` in `[net] New inbound peer ...`, no?

💬 maflcko commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2601641954)
The if-else has both branches, so the inversion (`!`) could also be dropped and the branches re-ordered