💬 maflcko commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2082271022)
> The failure can be reproduced locally by wrapping `callbacks.TransactionAddedToMempool` and `callbacks.TransactionRemovedFromMempool` (in `validationinterface.cpp`) with a lambda introducing an `UninterruptibleSleep(40ms)`, and calling `self.sync_mempools()` after `node_master.abandontransaction(tx3_id)` in `wallet_backwards_compatibility.py`

Can you produce a diff for this please, so that it can be applied locally by reviewers?

💬 brunoerg commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2082278225)
Up for grabs?

💬 laanwj commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#discussion_r1582783359)
Good to get rid of these libc patches, especially the rv64 one.

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2082291945)
Holding this off until #29987 is.merged.

💬 Sjors commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2082344035)
> I can try to split it up if it makes review easier.

Doesn't matter that much, but it's probably to put the `kernel/chainparams.h` change in its own commit so e.g. @TheCharlatan can do a drive-by review.

Here's some new snapshot torrents (not sure if I'm seeding them correctly...).

Torrent for testnet: `magnet:?xt=urn:btih:787f5917029876c83301c49dc97bb69224597285&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`

Torrent for signet: `magnet:?xt=urn:btih:
...

💬 Sjors commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1582744045)
6ed6bffcbedb6a28c3578721cc0a08086a15e417: this doesn't seem kernel-worthy. `chaintype.h` is a better home for it, next to `ChainTypeToString`.

💬 BrandonOdiwuor commented on pull request "Wallet: (Refactor) GetBalance to calculate used balance":
(https://github.com/bitcoin/bitcoin/pull/29062#discussion_r1582830424)
https://github.com/bitcoin/bitcoin/blob/a46065e36cf868265c909dc5edf29dc17be53c1f/src/wallet/rpc/coins.cpp#L480

My understanding from the above line is `min_depth` should be `zero` when calculating `full_balance` hence I didn't include the statement below in the above `if` since it uses `min_depth` provided by the caller

I didn't include the check in this `if` since `tx_depth` is always >= 0

💬 laanwj commented on pull request "rename bitcoin.conf in dist tarball":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2082382740)
Sure, if the outcome is "do nothing" that's optimal from our perspective. Enough serious issues...

💬 maflcko commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-2082391263)
ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑

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

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 30a6c999351041d6a1e8712a96
...

💬 maflcko commented on pull request "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime":
(https://github.com/bitcoin/bitcoin/pull/29872#discussion_r1582851553)
I think I'd also have to touch the header to address this, so I'll leave it as-is for now. Also, once could consider to remove this legacy interface function completely.

💬 maflcko commented on pull request "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime":
(https://github.com/bitcoin/bitcoin/pull/29872#issuecomment-2082402369)
Anything left to be done here?

💬 maflcko commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2082404311)
lgtm ACK 774359b4a96d2724dc70f900cb71e084a77164da

💬 josibake commented on pull request "rename bitcoin.conf in dist tarball":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2082429501)
> Sure, if the outcome is "do nothing" that's optimal from our perspective. Enough serious issues...

+1.

Per https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2063942430, it might make sense to revisit our tarball structure in the future and be in line with best practices for "installing by untarring into `/usr/local`", but I think that's a separate discussion so I'd suggest closing this for now.

🤔 hebasto reviewed a pull request: "depends: build zeromq with CMake"
(https://github.com/bitcoin/bitcoin/pull/29723#pullrequestreview-2028133161)
I've tested 796a271f0f4b57f61f357aac3e7a76072fed6f9a on Ubuntu 23.10 using a patch from #29960.

There are a few differences between Autotools and CMake builds:

1. In CMake, the resulted archive lacks object files from the following sources:
```
gssapi_client.cpp
gssapi_mechanism_base.cpp
gssapi_server.cpp
vmci_address.cpp
vmci_connecter.cpp
vmci_listener.cpp
vmci.cpp
```
2. CMake build is effectively compiled with `-O3` flag.

3. CMake adds `-DZMQ_CUSTOM_PLATFORM_HPP`.

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2082445522)
> For detailed information about the code coverage, see the [test coverage report](https://corecheck.dev/bitcoin/bitcoin/pulls/29428).

@aureleoules @adamjonas Looks like this is down again. ` LambdaTimeout - 2024-04-29T11:09:51.844Z 3957d9f6-7e67-4e79-bf45-9569b4491cd0 Task timed out after 10.01 seconds `

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#discussion_r1582885196)
Why is this removed? The block is in the most-work chain. I think this comment meant a block that is *not* in the most-work chain, no?

💬 hebasto commented on pull request "depends: Fix build of Qt for 32-bit platforms with recent glibc":
(https://github.com/bitcoin/bitcoin/pull/29985#issuecomment-2082485060)
My Guix builds:
```
x86_64
7bd0bf18cb3b17ee5829ca24e7488d0d022e280d0f56ff04c20f9199f787178a guix-build-2e266f33b5d2/output/aarch64-linux-gnu/SHA256SUMS.part
9c07a926fbdafff0f189c454652ccf9028e4ba9c44826b7874affc69bc5b5ae9 guix-build-2e266f33b5d2/output/aarch64-linux-gnu/bitcoin-2e266f33b5d2-aarch64-linux-gnu-debug.tar.gz
8d9ce23406bbcc33f2cc4147f06c715c44c5b0cc18af6be296862c3e531f3f97 guix-build-2e266f33b5d2/output/aarch64-linux-gnu/bitcoin-2e266f33b5d2-aarch64-linux-gnu.tar.gz
34a37952
...

💬 Sjors commented on pull request "[do not merge] validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-2082492618)
I switched to the halving block! Also dropped all prerequisites but #26045 from the description, but see conceptual discussion in #29616.

⚠️ Sjors opened an issue: "Pause IBD during AssumeUTXO snapshot load"
(https://github.com/bitcoin/bitcoin/issues/29993)
### Please describe the feature you'd like to see added.

The `loadtxoutset` RPC call should cause IBD to pause until the snapshot is loaded and the snapshot chain is activated.

### Is your feature related to a problem, if so please describe it.

When I start a fresh node and load a mainnet shapshot (#28553) any progress is completely buried in the log.

More importantly, I get the strong impression, though haven't properly tested this, that the IBD process slows down the snapshot load. This
...

💬 Sjors commented on issue "AssumeUTXO Mainnet Readiness Tracking":
(https://github.com/bitcoin/bitcoin/issues/29616#issuecomment-2082496290)
Definitely not a blocker: #29993