-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gracefully handle dropped UPnP support #843
Comments
Even ignoring the spurious setting would be better in the GUI case. No need to rub this in the users face. i didn't expect this consequence when i suggested adding a message 😅 Though it's still better than creashing with generic "-upnp: Unknown setting". i guess... But yes this needs to be informational level at most, it should just continue. Even worse: i guess it even happens if the settings json specifies upnp=0 to explicitly disable it? |
An easy first fix would be to downgrade |
Thanks Maflcko, i think it's a good short term solution to fix what i just broke. I'll open a PR shortly. That said, it does seem unknown entries in |
@darosior I could imagine a scenario in which we remove a Tor related setting and it would be unsafe to simply ignore it. But that's not the case here. |
Do we also need to explicitly remote the entry from settings.json as well, to make sure the message doesn't appear at every start after that. Or does this happen automatically in re-saving it (at shutdown) somehow? |
I don't think this was fixed fully for the gui |
bitcoin/bitcoin#31130 dropped UPnP support. It's now recommended that
users use PCP (with NATPMP fallback).
But this is not explained in a friendly manner:
A user would have to figure out that they need to manually edit settings.json or delete it and redo all their settings.
We should probably automatically delete it from settings.json. And then display a warning / info (instead of error) that tells users to consider the PCP setting in the GUI.
The text was updated successfully, but these errors were encountered: