🤔 virtu reviewed a pull request: "tracing: network connection tracepoints"
(https://github.com/bitcoin/bitcoin/pull/25832#pullrequestreview-2027891049)
ACK [996029b](https://github.com/bitcoin/bitcoin/pull/25832/commits/996029b1fdc131b5a65c34b4898a515d523268d3) (modulo comment and [this](https://github.com/bitcoin/bitcoin/pull/25832/files/996029b1fdc131b5a65c34b4898a515d523268d3#diff-adf830d8b5ab19af24c3f9be2cb2c8a14fa49455d22613c50adfd5e3cd583013))

Successfully ran `log_p2p_connections.bt` over the weekend on both x86_64/clang and aarch64/gcc.

Extended functional test (`interface_usdt_net.py`) equally passed on both machines.

💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1582727992)
```suggestion
OUTBOUND conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, total_out=1
INBOUND conn from 127.0.0.1:45324: id=1, type=inbound, network=0, total_in=1
MISBEHAVING conn id=1, score_before=0, score_increase=20, message='getdata message size = 50001', threshold_exceeded=false
CLOSED conn to 127.0.0.1:15287: id=0, type=block-relay-only, network=0, established=1231006505
EVICTED conn to 127.0.0.1:45324: id=1, type=inbound, network=0, established=1612312312
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1582768678)
I learned that we can RBF a transaction that gets into the mempool using carveout. I wanted to test that you wouldn't RBF that transaction using a package.

Even if the arrangement attempts to RBF or not, I think the error in the suggestion is how it would fail. We would set `m_allow_carveout` to false while evaluating the first parent transaction and fail its evaluation due to a `TX_MEMPOOL_POLICY` violation of `too-long-mempool-chain`. hence, all the descendants would fail due to missing
...

💬 willcl-ark commented on pull request "rename bitcoin.conf in dist tarball":
(https://github.com/bitcoin/bitcoin/pull/29903#issuecomment-2082267922)
@josibake @laanwj I'm happy to close this (and the issue) based on discussion above and in agreement with @pinheadmz [comment](https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-2068038387) on the issue, if you both agree?

💬 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?