👍 theStack approved a pull request: "wallet, tests: Avoid stringop-overflow warning in PollutePubKey"
(https://github.com/bitcoin/bitcoin/pull/30131#pullrequestreview-2070750537)
utACK 2289d4524053ab71c0d9133987cb36412797c1a2

💬 maflcko commented on issue "test: rpc_bind.py --ipv4 fails when run on s390x or arm in debian":
(https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-2124403378)
Possibly unrelated, but `rpc_bind.py --ipv6 | ✖ Failed | 2 s`

With ASan, on GHA: https://github.com/m3dwards/bitcoin/actions/runs/9178831587/job/25239590853#step:5:5070

```
test 2024-05-21T18:27:24.024000Z TestFramework.node0 (DEBUG): RPC successfully started
test 2024-05-21T18:27:24.078000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "
...

💬 maflcko commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2124403920)
Nice. I think you can just temporarily disable rpc_bind, due to https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-2124403378 I presume?

👍 laanwj approved a pull request: "wallet, tests: Avoid stringop-overflow warning in PollutePubKey"
(https://github.com/bitcoin/bitcoin/pull/30131#pullrequestreview-2070783321)
ACK 2289d4524053ab71c0d9133987cb36412797c1a2

💬 maflcko commented on pull request "wallet, tests: Avoid stringop-overflow warning in PollutePubKey":
(https://github.com/bitcoin/bitcoin/pull/30131#issuecomment-2124451770)
ACK 2289d4524053ab71c0d9133987cb36412797c1a2 with g++ 14.1.1 🦄

<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 2289d45240
...

💬 maflcko commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1609716833)
Yeah, it could make sense to switch this from brittle raw pointers literals to type-safe `string_view` or `string` in a follow-up.

💬 maflcko commented on pull request "net: make the list of known message types a compile time constant":
(https://github.com/bitcoin/bitcoin/pull/29421#discussion_r1609718736)
Ah, IIRC `string_view` was tried in this pull and needed more patches, so it was left for a follow-up what to do here.

💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2124481593)
Updates:
- Addressed @luke-jr's [feedback](https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2013545536).

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124488325)
> Avoiding this complexity is another reason to add a NAT-PMP fallback, then we'd be right to keep the option named the same and only change the descriptions.

If you think such a fallback is easier than dealing with QT settings migration :-)

We can just sort them PCP, NAT-PMP, UPNP in the GUI. If we also add the word "recommended" to PCP, it should mitigate choice-paralysis. Then in the future we can add a little green dot to indicate that it actually works.

I can test on Windows, (Inte
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609746833)
```
curl -s localhost:8000/tweak-index/709632?dustLimit=992 | jq
{
"error": "Server does not allow dustLimits"
}
```

Do I need to add a setting or rebuild the index?

💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609748019)
done

💬 brunoerg commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#discussion_r1609748162)
done

💬 laanwj commented on pull request "depends: Fetch miniupnpc sources from GitHub":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124512367)
> Which one? http://miniupnp.free.fr/ without HTTPS?

If that does keep the sha256 hash the same then maybe. Mind that github-generated tar.gzs aren't guaranteed to have a stable hash.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609750350)
Restarting with `tweaks_full_with_dust_filter = 1`

💬 hebasto commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124526213)
> > Which one? http://miniupnp.free.fr/ without HTTPS?
>
> If that does keep the sha256 hash the same then maybe.

It does. Switched to that website.

💬 laanwj commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124547244)
> It does. Switched to that website.

Thanks. Agree it's really suboptimal to use http because of protocol-level mischief, though. i hope tuxfamily will be back.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124574440)
> If you think such a fallback is easier than dealing with QT settings migration :-)

Yes. It also avoids having to rename the command-line option. It would make the new code a drop-in replacement that also supports IPv6, no change for users.

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609801566)
Sorry, should have mentioned that before. Yes, that flag/setting is required.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124640155)
Restarting the node with the same port seems to trigger the `NO_RESOURCES` error with IPv6. I guess that's because the previous "lease" is still valid.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2124663276)
> During a clean shutdown, we should probably ask the gateway to delete the mapping :

i wasn't sure about this! Yes, it would be cleaner, but also complicates the code. In normal use it's unlikely for a user to restart a node in such a small timespan, and having a node unconnectable for a few minutes isn't a big deal. The mappings aren't too long (20 min) and expire automatically, and it will re-try periodically until it gets one. So it's a problem that solves itself.