🤔 theStack reviewed a pull request: "qa: clarify and document assumeutxo tests"
(https://github.com/bitcoin/bitcoin/pull/31907#pullrequestreview-2628319034)
Concept ACK, I agree that the snapshot modification tests are in a bit of a messy state currently.

💬 theStack commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1962588549)
Could even go one step further and eliminate the magic bytes by implementing VARINT serialization and amount compression routines in the test framework. If you want to pull in, feel free: https://github.com/theStack/bitcoin/commits/202502-test-introduce_varint_and_amount_compression_routines/

💬 yancyribbens commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1962700474)
nit `1 coin`?

📝 eval-exec opened a pull request: "random: Initialize variables in hardware RNG functions"
(https://github.com/bitcoin/bitcoin/pull/31912)
Re-create This PR since previous(https://github.com/bitcoin/bitcoin/pull/31902) failed to compile.

Maybe we could re-consider and re-review this after 29.0 branch-off:

1. https://github.com/bitcoin/bitcoin/pull/31902#issuecomment-2669021440
2. https://github.com/bitcoin/bitcoin/pull/31902#issuecomment-2669380644

📝 jirijakes converted_to_draft a pull request: "doc: Fix description of byte order of hashes in ZMQ documentation"
(https://github.com/bitcoin/bitcoin/pull/31862)
ZMQ produces transaction and block hashes in the same format as RPC, that is with bytes in a reversed order from the output of hashing function. Previously, the ZMQ documentation incorrectly informed that the two are in different formats, and this changes fixes this.

Additionally, as the terms 'Big Endian' and 'Little Endian' are misleading in terms of order of bytes in an arbitrary sequence of bytes, this change also removes this notion and uses the term 'reversed byte order' instead.

Fix
...

💬 jirijakes commented on pull request "doc: Fix description of byte order of hashes in ZMQ documentation":
(https://github.com/bitcoin/bitcoin/pull/31862#issuecomment-2670368371)
Switched to draft because I found another thing to fix in this PR. Will get to it within a day.

💬 maflcko commented on pull request "random: Initialize variables in hardware RNG functions":
(https://github.com/bitcoin/bitcoin/pull/31912#issuecomment-2670660195)
Has someone tried to report or fix this upstream in the linux kernel or elsewhere? See https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2669309907

💬 hugohn commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2670785420)
Just wanted to add that even though we discovered the issue on mobile devices (Samsung S25 and OnePlus 13), it looks like the same chip will be shipped in several laptop brands/models as well: https://www.google.com/search?q=laptop+snapdragon+x+elite

My colleague @giahuy98 did test the original fix (https://github.com/bitcoin/bitcoin/pull/31826) on a real S25 device (it works), and @laanwj tested as well. Unfortunately, after that, there were further code changes (code refactoring) that did
...

👍 maflcko approved a pull request: "wallet: Disable creating and loading legacy wallets"
(https://github.com/bitcoin/bitcoin/pull/31250#pullrequestreview-2629052742)
Clarified my nits a bit, nothing blocking.

I still haven't reviewed the last commit and the large commit. I wonder if it is possible to split the large commit into one that removes test files (or test cases) and one that does the test framework change, possibly making review easier.

re-ACK b8494154e174481bbc0350013f1960973f35f3fc 🔑

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -
...

💬 maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r1963072635)
> Will expand the commit message.

Thanks for explaining. I understand that the test framework calls `importdescriptors` on `importaddress` internally. However, if the checks are wrong, why does the test not fail on current master?

In fact there is a comment and workaround to address this specifically in this test:

```
# for descriptor wallets, the test framework maps the importaddress RPC to the
# importdescriptors RPC (with argument 'timestamp'='now'), which always r
...

💬 maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r1963117438)
Also, this is the only build-system commit in this pull request. I'd say it could make sense to split it up into it's own pull request, because:

* This allows the "build people" to review it, if they want, and allows them to skip over the test-only bulk changes in this pull.
* The change to imply with_sqlite on enable_wallet is fine and required anyway going forward. I don't see a use-case for a bdb-only build at this point, so a separate pull should be fine.

💬 maflcko commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r1963084258)
> I'm not sure what in the commit message needs fixing.

The commit message refers to the configure option, but the project is using cmake, but this is just a nit.

💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2670871958)
I only guix-built the macOS hosts, and those hashes match @achow101.

When downloading with Safari:
- the zip (GUI) archives work on both my ARM and Intel macs
- for the `tar.gz` archive both M4 and Intel macs still refused to start `bitcoind`

👍 vasild approved a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2629174152)
ACK 9725cd6a5c20302da3dcae8fc1156458537ce1df

👍 vasild approved a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2629197425)
ACK 828c440b0dea29ab41ffc32d88969176d1286655

Strong Concept ACK on https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1961881829 (enforce CMake style, unrelated to this PR).

💬 vasild commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1963170308)
Wow! I didn't know cmake-lint existed and didn't even imagine it could exist ;) It would be perfect to enforce this in CI.

💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2670949331)
@achow101 for 096525e92cc2f5a4318bae13cedd2cf36b928d5f did you only notarize the arm binaries and not x86? As a sanity check I tried that archive again, to see what happens I start binaries from the terminal. `bitcoind` works and the others refuse. Once I right-click open the other binaries, they start working. Since I right-click opened that version of `bitcoind` yesterday, that makes sense, and implies the notarization indeed helps.

💬 laanwj commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2670966778)
> I don't think they have a 32-bit mode, so we'd have to drop that:

They seem to have Aarch32 support! To verify this i spun up a `m7g` (Graviton 3) instance, running Ubuntu:
```
$ uname -a
Linux ip-172-31-13-55 6.8.0-1021-aws #23-Ubuntu SMP Mon Dec 9 23:51:16 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux
$ dpkg --add-architecture armhf
$ apt install libc6:armhf libstdc++6:armhf gcc-arm-linux-gnueabihf
$ arm-linux-gnueabihf-gcc test.c -o test
$ file test
test: ELF 32-bit LSB pie executa
...

💬 maflcko commented on pull request "ci: switch MSAN to use prebuilt Clang binaries":
(https://github.com/bitcoin/bitcoin/pull/31850#issuecomment-2670983217)
> > Needs a fixup for MSAN fuzz.
>
> What is the error?

Yeah, I don't think this works. This is probably https://github.com/bitcoin/bitcoin/issues/24289

💬 vasild commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r1963243344)
Ok, I see the point - maybe some day `try_exec()` could `fork()` and then `exec()` in which case it will return in case of success.