📝 maflcko opened a pull request: "test: Avoid duplicate curl call in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/30703)
Seems odd having to translate `404` to "Binary tag was not found". Also, it seems odd to write a for-loop over a list with one item.

Fix both issues by just using a single call to `curl --fail ...`.

Can be tested with: `test/get_previous_releases.py -b v99.99.99`

Before:

```
Releases directory: releases
Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
...

🤔 dergoegge reviewed a pull request: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661#pullrequestreview-2257087050)
Code review ACK 4e87dcc205ba2bb7996dc077ad46b1b1e309dd8e

[Coverage report](http://bitcoind-fuzz.dergoegge.de:8000/bitcoin-pr30661/harnesses/p2p_headers_sync/coverage_report/coverage/workdir/bitcoin/index.html).

💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1728873486)
I think the use of `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` is a good approach. `CheckProofOfWorkImpl` will always be a blocker in fuzz tests, so getting it out of the way for all harnesses that aren't testing the check itself makes sense.

💬 dergoegge commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1728880703)
Any reason not to let the fuzzer decide the order?

If the order is interesting (i.e. a specific order might trigger a bug) then I think it'd make more sense to let the fuzzer choose the order.

💬 stickies-v commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728869229)
nit: requires `#include <util/check.h>`

👍 stickies-v approved a pull request: "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings"
(https://github.com/bitcoin/bitcoin/pull/30697#pullrequestreview-2257080546)
ACK 2cba0bed5be8a25e1635b7cde0a98285f64b5469, left non-blocking nits but happy to quickly re-ACK.

Would suggest updating the commit message to describe the change a bit better, add `bugfix` to the title, and remove the double `[]` and `:` notation:

> wallet: bugfix: lock wallet context before adding/removing wallet settings
>
> When performing certain wallet operations simultaneously, a race condition can be
> triggered. For example, simultaneously loading two wallets can lead to only
...

💬 stickies-v commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728873933)
```suggestion
// concurrently, ensuring no race conditions occur during either process.
```

💬 stickies-v commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728871804)
nit: while touching adjacent line, could help a neighbour out
```suggestion
#include <cstdint>
#include <future>
#include <memory>
#include <thread>
```

💬 fanquake commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-2306986439)
Rebased on master and dropped a commit, also bumped the glibc 2.33 branch to the latest commit. Still based on #30433, but the main blocker here remains the glibc bump.

💬 dergoegge commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1728887570)
To avoid the circular dependency, you could get rid of this file and move its contents to `txdownloadman_impl.cpp`.

💬 fanquake commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix)":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-2306991503)
For now, the Guix built bins could be inspected with:
```bash
# bitcoin/guix-build-30af1c56da93/output/aarch64-linux-gnu/bitcoin-30af1c56da93/bin# readelf -n * | grep "AArch64"
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
Properties: AArch64 feature: BTI, PAC
```

💬 ismaelsadeeq commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728897817)
Added

💬 ismaelsadeeq commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728898085)
Done.

💬 ismaelsadeeq commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1728898329)
Taken

💬 ismaelsadeeq commented on pull request "Wallet, Bugfix: Lock Wallet Context mutex Before Adding/Removing Settings":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2307003695)
> Would suggest updating the commit message to describe the change a bit better, add `bugfix` to the title, and remove the double `[]` and `:` notation:

Done, thank you.

💬 fanquake commented on pull request "guix: GCC 12 consolidation":
(https://github.com/bitcoin/bitcoin/pull/30511#discussion_r1728900761)
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=9aa38f98cbd41644abe2c43bb4f44f74df642df2 - landed, so we can follow up here.

🤔 stickies-v reviewed a pull request: "node: reduce unsafe uint256S usage"
(https://github.com/bitcoin/bitcoin/pull/30569#pullrequestreview-2257111871)
Force-pushed to address all review comments, limited to nits and tests so should be an easy re-review:
- [grammar nit](https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1726880388) for `RANDOM_CTX_SEED`
- [added](https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1727507122) `from_user_hex` test case to cover more > 64 char cases and cleaned up the test a bit by replacing `valid_hex_65` with `valid_hex_64` which should be less confusing
- [re-added mixed case testing](https://g
...

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1728922692)
yeah, I could've sworn a previous version introduced this newline and this was me reverting it, but I suppose it must be a rebase artifact, thanks for spotting and fixed now.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1728905343)
Resolved by [replacing](https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1727507122) `valid_hex_65` with `valid_hex_64` which I think addresses the confusion, too?

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1728920638)
Hmm, fair point. I think I dropped it because of #30560 dropping mixed case support for the consteval ctor, but that's unrelated to this pull of course so keeping the test is the right approach. I've added mixed case into 4 existing checks, but I'd still like to drop the `Ffa` tests since we can't use the `uint8_t` ctor which would make an equality test quite verbose for I think nothing that's not already tested in the other lines?

```cpp
BOOST_CHECK_EQUAL(uint256::FromUserHex("0xFf").va
...