💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193914009)
Yeah, lgtm. Maybe use `g_` prefix for the static globals?

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1193919304)
done. liked the idea.

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#discussion_r1193920890)
yes! done now.

💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193923732)
> Side note: The 50k max inv size also seems to overflow, which is probably another source for false negatives, depending on the use case.

You lost me there. Do you mean this may be a possible source of false negatives for the current path, or for both?

In this patch, I can see that potentially being an issue (that's what the original concern from AJ's comes from), but I don't see how that is an issue for master, given nothing is added to `m_recently_announced_invs` when building this huge
...

💬 mzumsande commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1547987039)
> @mzumsande do you want to propose some changes to remove its exposure?

no, I don't want to spend time on discussions on whether some change breaks user space or not. I'd be happy to have an alternative way to query the actual, unfiltered numbers per network, which are much more relevant to _my_ use cases as a developer.

> I don't see it as unfortunate; as a frequent user of getnodeaddresses and -addrinfo it seems useful that the data doesn't include terrible/non-recent (and banned/discou
...

💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193944810)
> You lost me there.

My understanding is that we'd never even send the `inv` at all for anything that exceeds 50k entries, but I might be wrong? If I am not wrong, I wonder if long-term we have no choice other than rate- and size-limit the reply?

💬 furszy commented on issue ""Create Unsigned" should not show the message: "The amount exceeds you balance" without suggesting alternatives":
(https://github.com/bitcoin/bitcoin/issues/27659#issuecomment-1548009498)
Well, I also coded a test for this there https://github.com/bitcoin/bitcoin/pull/26699/commits/68eed5df8656bed1be6526b014e58d3123102b03.

It seems that the problematic presented here is not that one. Based on the information provided @GregTonoski:

> getwalletinfo
{
"walletname": "test-watch_only",
"walletversion": 169900,
"format": "sqlite",
"balance": 0.00000000,
"unconfirmed_balance": 0.00000000,
"immature_balance": 0.00000000,
"txcount": 0,
"keypoolsize": 0,
"keypoolsize_hd_int
...

💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193951396)
> You lost me there.

Apologies, I typed something incorrect, again.

💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193954353)
Yea, gonna do it!

💬 MarcoFalke commented on pull request "p2p: Stop relaying non-mempool txs":
(https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1548022004)
> I think the last reason to hang on to mapRelay is the reason I gave above

My understanding is that `REPLACED` would also need to be provided to peers: https://github.com/bitcoin/bitcoin/pull/27625#issuecomment-1544171255. Otherwise there may be another round trip if the miner mines a previous version of a recently replaced transaction, which is in `mapRelay` and `vExtraTxnForCompact`, but not the mempool?

However, that may be solved, but just replacing the `mapRelay` lookup by a `vExtra
...

💬 brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1548028401)
Force-pushed

- Rebased
- Moved static globals into `initialize_setup` - https://github.com/bitcoin/bitcoin/pull/27647#discussion_r1193099637

Thanks @MarcoFalke for the review

💬 furszy commented on issue ""Create Unsigned" should not show the message: "The amount exceeds you balance" without suggesting alternatives":
(https://github.com/bitcoin/bitcoin/issues/27659#issuecomment-1548034031)
Still, this is not the case but have to add that a psbt creation on a wallet only with unconfirmed balance isn't possible (coins received from an external source are treated as "untrusted"). In the same way as it is not enabled for regular spending neither in the GUI.

💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1548035439)
Thanks, when updating the keys I found that `keys.txt` no longer exists in this repo, so I solved it by doing:

```
KEY_URL=https://raw.githubusercontent.com/bitcoin-core/guix.sigs/main/builder-keys

curl -sSL ${KEY_URL}/CoinForensics.gpg | gpg --import - \
&& curl -sSL ${KEY_URL}/Emzy.gpg | gpg --import - \
&& curl -sSL ${KEY_URL}/Sjors.gpg | gpg --import - \
&& curl -sSL ${KEY_URL}/TheCharlatan.gpg | gpg --import - \
&& curl -sSL ${KEY_URL}/achow101.gpg | gpg --import - \
...

💬 MarcoFalke commented on pull request "ci: Run iwyu on all src files":
(https://github.com/bitcoin/bitcoin/pull/27571#issuecomment-1548038156)
Should be ready for review

💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1193975226)
All good, I was just wondering if I may have been missing something.

💬 MarcoFalke commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1548044235)
> So why is there not a single file that bundles all the current keys so that I dont have to download them all individually?

No one can know all the current keys, because they can change for every release. However, it is possible to find them for a given release by looking into the folder. For example, you can look into https://github.com/bitcoin-core/guix.sigs/tree/main/25.0rc2 to find all current ones for that tag.

💬 MarcoFalke commented on pull request "build: Drop support for g++-8":
(https://github.com/bitcoin/bitcoin/pull/27662#issuecomment-1548047843)
Looks like the only affected code is `constexpr` functions calling `assert(...)`.

Failure can be tested with:

```
git show HEAD~|git apply --reverse # First checkout this pull, then revert the centos 9 bump commit
podman kill ci_i686_centos && MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_i686_centos.sh" ./ci/test_run_all.sh # Run the CI task
```

```
...
qt/addresstablemodel.cpp: In function ‘constexpr AddressTableEntry::Type translateTransactionType(wallet::AddressPurpo
...

💬 GregTonoski commented on issue ""Create Unsigned" should not show the message: "The amount exceeds you balance" without suggesting alternatives":
(https://github.com/bitcoin/bitcoin/issues/27659#issuecomment-1548052614)
> So, probably the `importdescriptors` RPC command call was done with `timestamp=now` and no rescan was performed.

I confirm that there was the `importdescriptors` RPC command call done with `timestamp=now` and no rescan was performed.

💬 kroese commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1548055212)
For a script it will be hard to parse the directory names for a Github folder reliably with only `curl` calls.

It was so much easier if there was a text-file containing a list of names, so that I could just download that and loop over that line by line. In any case I made an issue in the `guix.sigs` repo about this, because this is a bit off-topic here. Thanks.

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1548057302)
I was getting some sybil-stalls at <=10 limit, so for now uncapped it.

I also added in logic to ensure that at least one of the 3 slots is taken by an outbound peer. Added more extensive tests.