Bitcoin Core Github
44 subscribers
120K links
Download Telegram
BenWestgate closed a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147)
📝 BenWestgate reopened a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147)
Closes #30145.

This PR solves two major issues in the `parse_version_string` function of verify-binaries:
1. `-aarch64` binaries cannot be specifically downloaded. The -platform string gets interpreted as a release candidate that doesn't exist due to containing sub-string "rc".
2. Specifying a platform with a "-" in the name causes the parser to ignore both "-platform" AND "-rcN" and download the potentially wrong (non-rc) version for every platform. This also prevented specifying just one
...
💬 Sohanking1 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2126196798)
I want to get Bitcoin or usdt Flash
If you have so please contact me

https://t.me/Sohan20001
💬 BenWestgate commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2126211903)
> Merge commits in pull requests are discouraged in this repo, please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

Done.
👋 BenWestgate's pull request is ready for review: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147)
💬 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.
💬 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.