Qt: Add network port input box to GUI settings #7107

pull hsjoberg wants to merge 8 commits into bitcoin:master from hsjoberg:qtnetworkport changing 10 files +182 −7
  1. hsjoberg commented at 10:43 PM on November 26, 2015: contributor

    Related issue #7039

  2. tulip0 commented at 2:23 AM on November 27, 2015: none

    It's counterintuitive to most people that Bitcoin Core will refuse to use non-standard ports as anything but a last resort, does the option label need a GUI label or a confirmation popup to reflect this? I suspect a lot of people will be mystified otherwise that they don't get any incoming connections after changing a fairly innocuous looking option.

  3. in src/qt/optionsdialog.cpp:None in 8e63144068 outdated 
      44 | @@ -45,6 +45,8 @@ OptionsDialog::OptionsDialog(QWidget *parent, bool enableWallet) :
  45 |      ui->threadsScriptVerif->setMinimum(-GetNumCores());
  46 |      ui->threadsScriptVerif->setMaximum(MAX_SCRIPTCHECK_THREADS);
  47 |  
  48 | +    ui->networkPort->setValidator(new QIntValidator(1, 65535, this));
    jonasschnelli commented at 7:52 AM on November 27, 2015:

    We need a better protection to not run into a startup termination if the user did enter a invalid port. IIRC the QIntValidator still allows one to enter 0 as port number which will very likely result in a locked Bitcoin-QT (startup will stop because of a invalid port). Haven't tested although. Maybe check: #7025

    tulip0 commented at 10:46 AM on November 27, 2015:

    This should probably see 1-1023 as invalid as well.

    luke-jr commented at 8:57 PM on November 27, 2015:

    @tulip0 That is not universally true. I think Windows allows non-admin to bind them, and on Linux there are capabilities to allow binding them.

  4. jonasschnelli added the label GUI on Nov 27, 2015
  5. jonasschnelli commented at 7:57 AM on November 27, 2015: contributor

    Concept ACK.

    We just need to make sure, people can only enter reasonable/valid ports. Because if someone enters port 80, bitcoin-qt is very likely "bricked" (unless he start with root permissions!) until he manages to startup Bitcoin-Qt with a override port setting (through a shell with `-port=8333' or by changing bitcoin.conf).

    Best would be to ask the user – during startup – if he likes to switch to the standard port if the given port can't be used (permissions or already used through another process).

  6. laanwj added the label Feature on Nov 27, 2015
  7. jonasschnelli commented at 8:46 AM on November 28, 2015: contributor

    https://bitcoin.jonasschnelli.ch/pulls/7107/

  8. hsjoberg commented at 11:31 AM on November 28, 2015: contributor

    Thanks for the feedback @jonasschnelli, I'll work on the things you pointed out.
    I'll be available more and more in #bitcoin #bitcoin-dev etc as cocoBTC.

  9. fanquake commented at 10:55 AM on December 24, 2015: member

    @Cocosoft Are you still working on this?

  10. hsjoberg commented at 12:44 AM on December 26, 2015: contributor

    @fanquake Yes, I do intend to continue working on this.

  11. tulip0 commented at 2:17 AM on December 26, 2015: none

    Again, this needs a warning that users will get essentially no incoming connections if they change the port. It's not initiative behaviour and will cause significant confusion, I'm not convinced that this option should even be in a configuration window like this to begin with.

     // do not allow non-default ports, unless after 50 invalid addresses selected already
if (addr.GetPort() != Params().GetDefaultPort() && nTries < 50)
    continue;

    The address rumouring system really can't be used in conjunction with anything but a single static port.

  12. laanwj commented at 11:57 AM on January 28, 2016: member

    Concept ACK.

    Agree that adding a warning (e.g. as mouseover) would make sense, and that ports should be restricted to valid ports (>1024 at least).

  13. hsjoberg commented at 12:05 PM on January 28, 2016: contributor

    Just FYI, I am still committed to this issue. I am still alive. @laanwj, agreed.

  14. luke-jr commented at 8:48 PM on February 12, 2016: member

    @Cocosoft I've addressed the port number concern, and modified NetworkStyle to store regtest settings separately from testnet.

    git pull https://github.com/luke-jr/bitcoin.git qtnetworkport
  15. hsjoberg commented at 9:56 PM on February 18, 2016: contributor

    @luke-jr, great, thank you for your help!

  16. in src/qt/optionsmodel.cpp:None in 3d696024d1 outdated 
     253 | @@ -246,6 +254,18 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
 254 |              fMinimizeToTray = value.toBool();
 255 |              settings.setValue("fMinimizeToTray", fMinimizeToTray);
 256 |              break;
 257 | +        case NetworkPort:
 258 | +            if (settings.value("nNetworkPort") != value) {
 259 | +                // If the port input box is empty, set to default port
 260 | +                if (value.toString().isEmpty()) {
 261 | +                    settings.setValue("nNetworkPort", (quint16)Params().GetDefaultPort());
    hsjoberg commented at 10:25 PM on February 18, 2016:

    Maybe I'm mistaken, but won't this code never be reached because of acc8f4e?

    luke-jr commented at 7:35 AM on April 5, 2016:

    You mean because the empty field is invalid input? I suppose so.

  17. jonasschnelli commented at 4:03 PM on February 19, 2016: contributor

    Needs rebase. I could continue this. I think it needs some additional "focus-lost" checks (also see how proxy port settings do work) and I could imaging a problem when setting the port to a port-number that is already in use (could prevent from starting Bitcoin-Qt).

  18. jonasschnelli commented at 7:27 AM on April 5, 2016: contributor

    @ping Cocosoft. Are you interested to finish this?

  19. hsjoberg commented at 9:08 AM on April 5, 2016: contributor

    @jonasschnelli, I am interested in finishing this. I do understand however if you or someone else wants to take over because it has been idling for so long.

    I'll try to be on the IRC channel this week.

  20. laanwj commented at 2:10 PM on April 25, 2016: member

    I could imaging a problem when setting the port to a port-number that is already in use (could prevent from starting Bitcoin-Qt).

    Possibly in the case of the GUI it could be non-fatal if binding a port at startup fails. After warning the user there's no harm in continuing with the port closed.

    This would make this change a lot less dangerous.

  21. fanquake commented at 10:47 AM on June 22, 2016: member

    @Cocosoft Are you still planning on finishing this? This needs a rebase at least.

  22. hsjoberg commented at 12:55 PM on June 22, 2016: contributor

    @fanquake Yes, I haven't forgotten about this and I still plan to continue, I just need to find time. I fully understand if someone wants to finish it up though, as it has been delayed for so long.

  23. hsjoberg force-pushed on Jun 26, 2016
  24. luke-jr commented at 9:29 PM on June 27, 2016: member

    Why did you add an enableOkButton and disableOkButton methods that are unused?

  25. luke-jr referenced this in commit 35ecd12658 on Jun 27, 2016
  26. luke-jr referenced this in commit e7e62bb4b4 on Jun 27, 2016
  27. hsjoberg commented at 9:56 PM on June 27, 2016: contributor

    I did not, actually you made them according to Git:

    $ git blame -L 227,227 src/qt/optionsdialog.cpp
578064a6 (Luke Dashjr 2016-02-12 20:08:00 +0000 227) void OptionsDialog::enableOkButton()

    I'll rebase again and fix that.

    Or did you merge the qtnetworkport branch to the master branch of Bitcoinknots? If that's the case, I maybe could just make a new commit removed the unused code.

  28. luke-jr commented at 9:57 PM on June 27, 2016: member

    That code wasn't part of my commit; somehow it got added in when you rebased. Perhaps an accident during a merge?

  29. hsjoberg commented at 10:02 PM on June 27, 2016: contributor

    You're right, sorry. It seems like Philip Kaufman wrote the functions back in 2013:

    $ git blame -L 227,227 src/qt/optionsdialog.cpp
7e195e84 (Philip Kaufmann 2013-12-03 09:10:10 +0100 227)     setOkButtonState(false);
  30. hsjoberg commented at 10:19 PM on June 27, 2016: contributor

    I'm not sure where the functions come from, I can confirm (by a backup) that they existed before the rebase I did.

    I'll remove them.

    Edit: Right, I had a merge conflict regarding the functions during the rebase. My bad!

  31. luke-jr referenced this in commit f9669fcc93 on Jun 28, 2016
  32. fanquake commented at 12:55 PM on August 31, 2016: member

    This needs a rebase.

  33. Qt/Options: Adding network port to GUI settings, fixes #7039
    Adds Network port input box to the Network tab in the Options dialog.
-port takes priority over the GUI setting.
If left blank, it will default to the default port
    8b23b2c1b8
  34. Qt/Options: Avoid changing names of unrelated widgets fb9083d974
  35. Qt/Options: Don't allow setting a port below 1024 4cd0d4638f
  36. Qt/Options: Actually check validator acceptability for network port b438458c2d
  37. Qt/Options: Allow an "invalid" network port if it is the current value already ccdb3b7506
  38. Qt/NetworkStyle: Use different QSettings for regtest vs testnet 4396a3ac99
  39. hsjoberg force-pushed on Sep 3, 2016
  40. Adding functions SetArg and SetBoolArg to util.cpp
    Lets you override an argument even if it has already been set,
in contrast to SoftSetArg/SoftSetBoolArg.
    f5267bfde5
  41. Qt: Ask user to use standard port on startup if specified port is in use
    If the port (specified by -port or GUI setting) is already in use when
starting the GUI client, and it's not the standard port, ask the user if
they want to listen via the standard network port instead.
    1f37c87d8f
  42. hsjoberg force-pushed on Sep 5, 2016
  43. hsjoberg commented at 10:13 PM on September 5, 2016: contributor

    As per @jonasschnelli suggestion, I added a Question box on startup to ask if the user wants to use the standard network port if the specified port (whether it's from -port or in GUI) is already in use.

  44. jonasschnelli commented at 1:37 PM on September 20, 2016: contributor

    Needs rebase and nit fixes.

  45. jonasschnelli commented at 7:35 PM on October 18, 2016: contributor

    Closing due to inactivity. Happy to reopen once this has made progress.

  46. jonasschnelli closed this on Oct 18, 2016
  47. luke-jr referenced this in commit 74ab1e1d76 on Oct 20, 2016
  48. luke-jr referenced this in commit b55737cddb on Oct 20, 2016
  49. luke-jr referenced this in commit 1f83e7c3da on Feb 18, 2017
  50. luke-jr referenced this in commit d59f1d20eb on Feb 18, 2017
  51. hsjoberg commented at 12:15 AM on February 23, 2017: contributor

    Hi @jonasschnelli, I've rebased the branch upon master on cocosoft/bitcoin branch qtnetworkport. I'm not sure what nits are still missing, but I would be happy to fix them.

    I added a warning prompt on startup if the user tries to use a non-standard port that is already in use, and gives the option to use the standard port.

    Best Hampus

  52. luke-jr referenced this in commit 23e457985a on Aug 27, 2017
  53. luke-jr referenced this in commit 539694d50d on Aug 29, 2017
  54. luke-jr referenced this in commit 90ae1dd7c1 on Mar 13, 2018
  55. luke-jr referenced this in commit 5542d56114 on Dec 24, 2018
  56. luke-jr referenced this in commit 25a398680f on Dec 29, 2018
  57. luke-jr referenced this in commit e8bacec5cf on Apr 22, 2019
  58. luke-jr referenced this in commit 77319d7696 on Jan 5, 2020
  59. luke-jr referenced this in commit f4bea6b376 on Jun 9, 2020
  60. luke-jr referenced this in commit 0d5447342d on Dec 15, 2020
  61. DrahtBot locked this on Sep 8, 2021
Contributors
hsjobergtulip0jonasschnelliluke-jrfanquakelaanwj
Labels
FeatureGUI
Linked (view graph)
#7039 Set port from the GUI settings
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-19 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me