👍 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.

💬 GBKS commented on pull request "wallet: Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-2124671596)
Good points. I think it could work well to combine the two warnings to a single dialog. It could be placed between the initial `Create wallet` dialog, and the password input.

1. **Create wallet dialog:** The user checks `Encrypt wallet` and clicks `Continue`.
2. **Password warning dialog:** The application informs the user about the risks of using a password and offers `Back` and `Continue` options. The user considers, decides to move forward with the password, and clicks `Continue`.
3. **P
...

⚠️ epiccurious opened an issue: "Improve the bitcoin.conf instructions in init.md doc"
(https://github.com/bitcoin/bitcoin/issues/30153)
Update the `init.md` instructions about generating an example `bitcoin.conf` file.

Additionally, the `share/examples/bitcoin.conf` placeholder file says to follow the instructions in `contrib/devtools/README.md`, which was confusing to me since that README talks about many scripts. It would be helpful to point out to the user which section to refer to.

This issue will include an accompanying PR (to be assigned).

## Background

- Two years ago, `share/examples/bitcoin.conf` was replac
...

📝 epiccurious opened a pull request: "doc: Update mentions of generating bitcoin.conf"
(https://github.com/bitcoin/bitcoin/pull/30154)
Closes #30153.

This PR includes two changes as separate commits:

1. Update `doc/init.md` to mention generating an example bitcoin.conf instead of referencing the placeholder `share/examples/bitcoin.conf`. Update the code-formatted text with a markdown link.

2. Also update `share/examples/bitcoin.conf` with a mention of the appropriate section of `contrib/devtools/README.md`.

## Background

- Two years ago, `share/examples/bitcoin.conf` was replaced with [a placeholder file](https:/
...