Skip to content
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

Revert "net: do not advertise address where nobody is listening" #24648

Closed
wants to merge 1 commit into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Mar 23, 2022

This is a quick fix bitcoin-core/gui#567 which is assumed to be backported to 23.0rc3.

Effectively, this PR reverts #20769 which could be implemented later in a better way.

@fanquake
Copy link
Member

I'm very ~0 on last-minute reverting bug fixes to bitcoind, to fix problems in the GUI.

@vasild
Copy link
Contributor

vasild commented Mar 23, 2022

I am a bit ~-0.1 on this. However, the problem is quite serious - bitcoin-qt just will refuse to start. Lets see if there would be better solution...

@maflcko
Copy link
Member

maflcko commented Mar 23, 2022

I don't think there is anything wrong with reverting a hunk that was never released in a release

@hebasto
Copy link
Member Author

hebasto commented Mar 23, 2022

I'm very ~0 on last-minute reverting bug fixes to bitcoind, to fix problems in the GUI.

In comparison the "bug" that had been fixed in #20769 with the bitcoin-core/gui#567, the latter looks much more seriously.

And, considering #20657 (comment):

Should we automatically set listenonion to 0 if listen is set to 0?

we already do in

bitcoin/src/init.cpp

Lines 688 to 689 in d6f225f

if (args.SoftSetBoolArg("-listenonion", false))
LogPrintf("%s: parameter interaction: -listen=0 -> setting -listenonion=0\n", __func__);

@vasild
Copy link
Contributor

vasild commented Mar 23, 2022

What about this:

--- i/src/qt/optionsmodel.cpp
+++ w/src/qt/optionsmodel.cpp
@@ -148,14 +148,17 @@ void OptionsModel::Init(bool resetSettings)
     if (!gArgs.SoftSetBoolArg("-natpmp", settings.value("fUseNatpmp").toBool())) {
         addOverriddenOption("-natpmp");
     }
 
     if (!settings.contains("fListen"))
         settings.setValue("fListen", DEFAULT_LISTEN);
-    if (!gArgs.SoftSetBoolArg("-listen", settings.value("fListen").toBool()))
+    if (!gArgs.SoftSetBoolArg("-listen", settings.value("fListen").toBool())) {
         addOverriddenOption("-listen");
+    } else if (!settings.value("fListen").toBool()) {
+        gArgs.SoftSetBoolArg("-listenonion", false);
+    }
 
     if (!settings.contains("server")) {
         settings.setValue("server", false);
     }
     if (!gArgs.SoftSetBoolArg("-server", settings.value("server").toBool())) {
         addOverriddenOption("-server");

It fixes bitcoin-core/gui#567 without reintroducing #20657.

@hebasto
Copy link
Member Author

hebasto commented Mar 23, 2022

@vasild

What about this:

Mind submitting a PR in https://github.com/bitcoin-core/gui/pulls?

@vasild
Copy link
Contributor

vasild commented Mar 23, 2022

bitcoin-core/gui#568

@hebasto
Copy link
Member Author

hebasto commented Mar 23, 2022

I'm very ~0 on last-minute reverting bug fixes to bitcoind, to fix problems in the GUI.

I am a bit ~-0.1 on this. However, the problem is quite serious - bitcoin-qt just will refuse to start. Lets see if there would be better solution...

Okay, closing in favor of bitcoin-core/gui#568.

@hebasto hebasto closed this Mar 23, 2022
@hebasto hebasto deleted the 220323-listen branch March 23, 2022 16:22
@bitcoin bitcoin locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to open bitcoin-qt when incoming connections disabled in settings
4 participants