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

Limit abuse of --useLocalhostForP2P option #4154

Merged

Conversation

freimair
Copy link
Member

@freimair freimair commented Apr 9, 2020

Fixes #4152, fixes #4123, fixes #3927

Until now, the --useLocalhostForP2P option could be used even for BTC_MAINNET. Even though the option has been clearly labelled to be for development purposes, it found its way into setup guides for eg. the Tails OS.

This PR only allows the option if the base currency network is set to something other than BTC_MAINNET.

That of course will most likely prevent users on Tails OS to use Bisq. Will look into the Tails stuff soon and followup on this one.

Until now, the --useLocalhostForP2P option could be used even for
BTC_MAINNET. That is bad and caused mediation cases.
@ripcurlx
Copy link
Contributor

This PR only allows the option if the base currency network is set to something other than BTC_MAINNET.

That of course will most likely prevent users on Tails OS to use Bisq. Will look into the Tails stuff soon and followup on this one.

Still better than having failed trades without the possibility of communication.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

Tested it on Regtest with different scenarios:

  • Not passing a BASE_CURRENCY_NETWORK and using USE_LOCALHOST_FOR_P2P fails with
    error: problem parsing option 'useLocalhostForP2P': Option(s) [useLocalhostForP2P] are unavailable given other options on the command line
  • Setting Mainnet as base currency network defaults USE_LOCALHOST_FOR_P2P to false.
  • Setting Regtest as base currency network still allows USE_LOCALHOST_FOR_P2P to get set.

@ripcurlx ripcurlx merged commit aeda234 into bisq-network:master Apr 10, 2020
@freimair
Copy link
Member Author

Tails is up again: https://bisq.wiki/Bisq_on_tails

thus, the issues #3927, #2278, #3791, #2841 can finally be closed properly (most of them already have been dropped already anyways)

@ripcurlx ripcurlx added this to the v1.3.2 milestone Apr 15, 2020
@ripcurlx ripcurlx added the is:priority PR or issue marked with this label is up for compensation label Apr 15, 2020
@freimair freimair deleted the prevent_misuse_of_dev_cmdline_option branch September 10, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:priority PR or issue marked with this label is up for compensation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

localhost error again Localhost.9999 bug Offer has localhost:9999 instead of Tor onion address
2 participants