📝 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]};`

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2309010798)
Both `prevout_spent_amount` and `new_outputs_ex_coinbase_amount` can overflow an int64_t in extreme cases -- filling a block with 15000 65-byte transactions (975kvB) repeatedly spending the same 7M BTC will hit 105 billion BTC, which is 1.05e19 sats which is about 13% more than 2**63-1 which is the max value a CAmount can store, and this is undefined behaviour in C++. (You'd need to cycle spends of ~12M BTC or more to overflow a uint64_t, though that is not undefined behaviour)

Even if we don
...

💬 ajtowns commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3235597424)
When running this index over signet, I end up with ~700 55kB ldb files, and a single 8.8MB ldb file. That seems like something is probably suboptimal?

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3235780122)
There was a TODO still in the src/CMakeLists.txt file; I cleaned it up and squashed the commits just now and think that it is now all correct.

📝 maflcko opened a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269)
The assumption was removed in commit
3eab8b724044dc321f70e5eed66b149713158a04.

💬 maflcko commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2309281175)
It is less code this way:

```diff
diff --git a/src/test/fuzz/difference_formatter.cpp b/src/test/fuzz/difference_formatter.cpp
index 49e859ffdc..38b54352c1 100644
--- a/src/test/fuzz/difference_formatter.cpp
+++ b/src/test/fuzz/difference_formatter.cpp
@@ -4,25 +4,13 @@

#include <blockencodings.h>
#include <streams.h>
-#include <test/fuzz/FuzzedDataProvider.h>
+#include <random.h>
#include <test/fuzz/fuzz.h>

#include <vector>

namespace {
-// Test struct for VectorFo
...

💬 maflcko commented on pull request "test: p2p block malleability":
(https://github.com/bitcoin/bitcoin/pull/33172#issuecomment-3235915404)
lgtm ACK d0e1bbad016cc4949094daea2934712f92dfeecd

💬 frankomosh commented on pull request "p2p: add `DifferenceFormatter` fuzz target and invariant check":
(https://github.com/bitcoin/bitcoin/pull/33252#discussion_r2309383866)
I think this is more cleaner and simpler to follow. thanks