💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3629461809)
[7927473](https://github.com/bitcoin/bitcoin/commit/7927473c97b688d0c7162e4de294d137b8779568) to [62edcea](https://github.com/bitcoin/bitcoin/commit/62edceaf6d331cf7c06e277338b37831c910b1f7):
Addressed feedback (didn't include `getblockfrompeer` behavior of re-downloading prune blocks yet, see above).
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3629461809)
[7927473](https://github.com/bitcoin/bitcoin/commit/7927473c97b688d0c7162e4de294d137b8779568) to [62edcea](https://github.com/bitcoin/bitcoin/commit/62edceaf6d331cf7c06e277338b37831c910b1f7):
Addressed feedback (didn't include `getblockfrompeer` behavior of re-downloading prune blocks yet, see above).
🤔 hebasto reviewed a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3554616850)
Approach ACK a72ff31b879f164ca7875c0121953af93d799aee.
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3554616850)
Approach ACK a72ff31b879f164ca7875c0121953af93d799aee.
💬 hebasto commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2600556460)
Why not using the same minor version for both packages `gcc-14` and `gcc-toolchain-14`?
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2600556460)
Why not using the same minor version for both packages `gcc-14` and `gcc-toolchain-14`?
🚀 ryanofsky merged a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006)
(https://github.com/bitcoin/bitcoin/pull/34006)
💬 ryanofsky commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3629757028)
I merged this since it seems like a clear improvement that other changes can build on. Also took "whichever happens earlier" (https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546) as a hint to mean the author is happy to see this merged as is. Followup suggestion are listed in: https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3552578391
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3629757028)
I merged this since it seems like a clear improvement that other changes can build on. Also took "whichever happens earlier" (https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546) as a hint to mean the author is happy to see this merged as is. Followup suggestion are listed in: https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3552578391
💬 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.
(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.
(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
...
(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.
(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
(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.
(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
(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.
(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
(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
...
(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
(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
(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
(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
(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
(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