💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2126357008)
> Success! My node is reachable publicly, without configuration.

Nice!!! Thanks for testing again.

💬 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#discussion_r1611207868)
I'll check, thanks.

💬 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#discussion_r1611216066)
Since I moved `enableHistoryAction` within `setWalletActionsEnabled` I thought there was a evaluation of the `wallet_view` privacy there... taking that back down.

💬 maflcko commented on pull request "test: Assumeutxo: snapshots with less work should not be loaded":
(https://github.com/bitcoin/bitcoin/pull/29428#issuecomment-2126512922)
utACK df6dc2aaaeffc664006b86ee8c8797dc484ec40e

💬 maflcko commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1611238787)
I think you'll have to tell maintainers if you want to address this nit, or leave it for a follow-up, otherwise they won't know whether it is fine to merge this from your side or not.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611240354)
Agree, i think i would prefer
`QueryDefaultGatewayNetlink`
`QueryDefaultGatewaySysctl`
`QueryDefaultGatewayWin32` -- only the WIN32 one is truly OS specific, the other ones are POSIX-ish

💬 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#discussion_r1611244824)
It doesn't work without it...

💬 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-2126541648)
> ```c++
> historyAction->setEnabled(enabled && !isPrivacyModeActivated());
> ```

Just replacing the operator with `||` (e.g. when the user closes all wallets, all tabs will be disabled except for `Transactions`).

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611259233)
Unlike the linux code, here it does matter to call `NetworkErrorString`, because it calls `Win32ErrorString`, which calls `FormatMessage(W)` as the docs recommend: https://learn.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getbestinterfaceex

My understanding is that using `strerror_s` (called by SysErrorString) here would be wrong: https://stackoverflow.com/a/20057368

I wonder if we can prevent doing that by accident.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611261160)
That said, this Windows code is a lot simpler than the Linux stuff above (ducks...).

💬 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-2126551473)
Updates:

- Addressed @luke-jr's [feedback](https://github.com/bitcoin-core/gui/pull/815#pullrequestreview-2072577992).

🚀 fanquake merged a pull request: "depends: Fetch miniupnpc sources from an alternative website"
(https://github.com/bitcoin/bitcoin/pull/30151)

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611265859)
OHH, my thinking was that it's a WIN32 API function, not a network function. So i thought `SysErrorString` would be correct. But i think you're right. "SysError" is more like "posix errno emulation error". Which is not what is needed here.

👍 t-bast approved a pull request: "policy: restrict all TRUC (v3) transactions to 10kvB"
(https://github.com/bitcoin/bitcoin/pull/29873#pullrequestreview-2073292210)
ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11

💬 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#discussion_r1611272235)
done.

💬 fanquake commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2126564542)
Backported to 27.x in #30092.

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1611280293)
> That said, this Windows code is a lot simpler than the Linux stuff above (ducks...).

You mean that centrally-controlled proprietary OSes sometimes have well-documented API's that consider use-cases, while FOSS often uses haphazard grabbag API's that were grown in accordance with one tool (where everyone is expected to parse text output of-)... or some custom library, with equally confusing interface. you can just say that out loud here you know... 😓
Standards like RFCs, and POSIX (ignori
...

💬 virtu commented on pull request "net: add ASMap info in `getrawaddrman` RPC":
(https://github.com/bitcoin/bitcoin/pull/30062#issuecomment-2126596267)
ACK [1e54d61](https://github.com/bitcoin/bitcoin/commit/1e54d61c4698debf3329d1960e06078ccbf8063c)

Successfully re-ran all previously mentioned tests.

💬 maflcko commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2126604835)
An alternative might be to try to set `--privileged` on the GHA docker, but I am not sure if this is possible.

💬 fanquake commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2126616244)
> CI should be bumped as well, if this is taken out of draft.

Added a (WIP) commit to bump a couple CIs to 13.2.0. Also bumped the time-machine further, now that we've upstreamed GCC 13.3.0: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=750148ce1ea6c65a7c14424546db0078161f7e17.