💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#issuecomment-3234922944)
a425bff75203edf855ac4d8a2fea3a0a60978147 added snapshot height

fd504bb351 modify the way verification progress is calculated for background validation -> context: https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2306215222

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308553353)
nit: should also error on `recv_result == 0`, e.g. from libnetlink:

```c
static int __rtnl_recvmsg(int fd, struct msghdr *msg, int flags)
{
int len;

do {
len = recvmsg(fd, msg, flags);
} while (len < 0 && (errno == EINTR || errno == EAGAIN));

if (len < 0) {
fprintf(stderr, "netlink receive error %s (%d)\n",
strerror(errno), errno);
return -errno;
}

if (len == 0) {
fprintf(stderr, "EOF on netlink\n");
return -ENODATA;
}

return len;
}
```
https:/
...

💬 polespinasa commented on pull request "rpc, logging: add backgroundvalidation to getblockchaininfo":
(https://github.com/bitcoin/bitcoin/pull/33259#discussion_r2308578286)
do we want to test that in `feature_assumeutxo.py` or in `rpc_blockchain.py`

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308638969)
For context on this, IPv6 allows "scoped addresses", e.g. I can send a packet to a non-globally unique address, like a link-local address that has two destinations on two different links, so in order to disambiguate scoped addresses [RFC4007#section-6](https://datatracker.ietf.org/doc/html/rfc4007#section-6) recommends the use of unique zone indexes:

> Because the same non-global address may be in use in more than one zone of the same scope (e.g., the use of link-local address fe80::1 in tw
...

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308655496)
nit:

`processed_one`, `multi_part` and the check at the bottom of the outer for loop could be dropped, with the following check:

```suggestion
if (!(hdr->nlmsg_flags & NLM_F_MULTI)) {
done = true;
}
```

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308665037)
nit:

I think we should return early on `total_bytes_read == 0` as well.

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3235124766)
untested crACK 4c531782569b42fc474

https://github.com/bitcoin/bitcoin/pull/32159/commits/57ce645f05d18d8ad10711c347a5989076f1f788 corrects a bug where `QueryDefaultGatewayImpl` would return *not* default gateways.

https://github.com/bitcoin/bitcoin/pull/32159/commits/42e99ad77396e4e9b02d67daf46349e215e99a0f adds a belt and suspenders check that we have received a sensible reply to our `GETROUTE` message.

https://github.com/bitcoin/bitcoin/pull/32159/commits/4c531782569b42fc47478a468f4a7
...

📝 achow101 opened a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268)
One of the ways that the wallet would determine if a transaction belonged to the wallet was by checking if the total amount being spent by a transaction from outputs known to the wallet was greater than 0. This has worked fine until recently since there was no reason for 0-value outputs to be created. However, with ephemeral dust and P2A, it is possible to create standard 0-value outputs, and the wallet was not correctly identifying the spends of such outputs. This PR updates `IsFromMe` to only
...

💬 davidgumberg commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2308685773)
Feel free to mark resolved, this suggestion is actually incorrect, since when `recv_result == 0` the `NLMSG_OK` check [below](https://github.com/bitcoin/bitcoin/blob/4c531782569b42fc47478a468f4a79e0e2d93946/src/common/netif.cpp#L110) would fail:


```c
#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
(nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
(nlh)->nlmsg_len <= (len))
```
https://github.com/torvalds/linux/blob/39f90c1967215375f7d87b81d14b0f3ed6b40c
...

🚀 achow101 merged a pull request: "Revert compact block cache inefficiencies"
(https://github.com/bitcoin/bitcoin/pull/33253)

💬 achow101 commented on pull request "rpc: require integer verbosity; remove boolean 'verbose'":
(https://github.com/bitcoin/bitcoin/pull/33214#issuecomment-3235256712)
The way to deprecate these properly would be to first require `-deprecatedrpc=boolverbose` (or similar) which gates the old behavior. Then the following release can remove them.

💬 davidgumberg commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308783181)
In commit https://github.com/bitcoin/bitcoin/pull/30469/commits/e7b111a63067cf6c66140228c28ab1fa96f7ff18 (_refactor, index: Remove member variables in coinstatsindex_):

for other reviewers: `read_out.second.total_subidy` is updated below:

https://github.com/bitcoin/bitcoin/blob/e7b111a63067cf6c66140228c28ab1fa96f7ff18/src/index/coinstatsindex.cpp#L200

💬 davidgumberg commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308806966)
Isn't incrementing `new_value.second.block_unspendables_scripts` undefined behavior since it never gets initialized? (and the other `block_*` members of DBVal)

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3235300290)
post-merge ACK

💬 nervana21 commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2308941532)
Will this entry have an effect? In the positional map `members`, the key `("dumptxoutset", 2)` is first set to `false`. My understanding is that subsequent encounters of the same key are ignored by `std::map::emplace`. Is this the desired behavior, or should the subsequent value overwrite the first?

```cpp
{ "dumptxoutset", 2, "options", /*also_string=*/false }, // inserted
{ "dumptxoutset", 2, "rollback", /*also_string=*/true }, // ignored or used to overwrite?
```

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308974859)
Shouldn't this be an error even if you can find the previous block header? You'll calculate incorrect values for `total_amount` otherwise?

🤔 ajtowns reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3167118313)
Sorry for not looking at this earlier. Approach ACK.

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308952120)
Keeping the `block_unspendables_*` as cumulative totals would be more flexible, and would be nice to be able to directly query. (dropping `total_unspendable_amount` would be possible in that case, considering it's just the sum of these totals)

Likewise keeping `total_subsidy`.

With those values, `total_subsidy = total_amount + total_unspendable_amount` should be an accounting identity, with `total_amount` also matching a manual sum of values in the utxo set, and `total_subsidy` being preci
...

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308983931)
I also have this question. Adding `{0}` default initializers in the `struct DBVal` declaration seems sensible?

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2308987847)
This could be a reference rather than a copy: `const Coin& coin{tx_undox.vprevout[j]};`