Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 hebasto reviewed a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3554458851)
My Guix build:
```
aarch64
02de261cf6c90f15a69d42b43e63485a6c0cdb3d745b5916b6834278a1d63dbc guix-build-a72ff31b879f/output/aarch64-linux-gnu/SHA256SUMS.part
cf2d9615338be34f5082940d7d2b1a4c085226a410a75dceec764e42cd5df5ee guix-build-a72ff31b879f/output/aarch64-linux-gnu/bitcoin-a72ff31b879f-aarch64-linux-gnu-debug.tar.gz
e18f34ce40036367f611451ae4dfe39e7dedeb53e549f229cff118bf2c2836a8 guix-build-a72ff31b879f/output/aarch64-linux-gnu/bitcoin-a72ff31b879f-aarch64-linux-gnu.tar.gz
17cd0a5fad2a4e
...
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600468539)
True, changed to `ABCStep`. The point is that we want to sometimes stop here (and possibly call `CheckBlockIndex` etc.), because whenever we would release cs_main, the same would be possible from another thread.
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600470202)
makes sense, removed the condition
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600474557)
right, that changed recently with #29640
💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600486783)
looks good, I'll do some fuzzing with it / try to understand the non-determinism before I include it.
💬 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).
🤔 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.
💬 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`?
🚀 ryanofsky merged a pull request: "Add util::Expected (std::expected)"
(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
💬 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