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

Feature Request: Ability to set base ports #533

Open
dangeross opened this issue Feb 10, 2022 · 7 comments · May be fixed by #878
Open

Feature Request: Ability to set base ports #533

dangeross opened this issue Feb 10, 2022 · 7 comments · May be fixed by #878
Labels
enhancement New feature or request up for grabs Anyone can work on this

Comments

@dangeross
Copy link

Is your feature request related to a problem? Please describe.
The base port for LND REST api for the first node is usually 8081. This port seems to conflict with other applications (e.g. local development), but in particular it conflicts with XCode build PhaseScriptExecution which runs on 8081.

PhaseScriptExecution Start\ Packager /Users/rosssavage/Library/Developer/Xcode/DerivedData/Satimoto-evtdshsaqaokluevsdbhyvsxutjv/Build/Intermediates.noindex/Satimoto.build/DebugTestnet-iphonesimulator/Satimoto.build/Script-FD10A7F022414F080027D42C.sh (in target 'Satimoto' from project 'Satimoto')
cd /Users/rosssavage/Source/Projects/react-mobile/ios
/bin/sh -c /Users/rosssavage/Library/Developer/Xcode/DerivedData/Satimoto-evtdshsaqaokluevsdbhyvsxutjv/Build/Intermediates.noindex/Satimoto.build/DebugTestnet-iphonesimulator/Satimoto.build/Script-FD10A7F022414F080027D42C.sh
Connection to localhost port 8081 [tcp/sunproxyadmin] succeeded!
Port 8081 already in use, packager is either not running or not running correctly
Command PhaseScriptExecution failed with a nonzero exit code

Describe the solution you'd like
To be able to configure the base port settings manually either in the settings.json file or via UI

@dangeross dangeross added the enhancement New feature or request label Feb 10, 2022
@jamaljsr
Copy link
Owner

Thanks for this feedback. It shouldn't be too difficult to add a screen to the UI to specify these base ports.

@limpbrains
Copy link

I'm having the same issue while working on React-native project. RN Metro bundler also uses 8081. Maybe you can just switch to some rare or random port?

@limpbrains
Copy link

Way to fix this:

  • add new LND to your network
  • remove first one
  • restart
    now 8081 will be free

@jamaljsr jamaljsr added the up for grabs Anyone can work on this label Dec 19, 2023
@Abdulkbk
Copy link

Hi @jamaljsr, I would like to take on this. I'll post updates once I begin.

@Abdulkbk
Copy link

Abdulkbk commented Mar 1, 2024

Hi @jamaljsr, I would like to take on this. I'll post updates once I begin.

I came across this code block:

Screenshot from 2024-03-01 23-17-30

I think it ensures that ports do not conflict by checking for open ports before assigning any to a node.

I also tried to make the ports conflict - I started a service on 8081 and the first LND node changed to 8082. Tried again by starting 2 different services on 8081 and 8082, and my first LND changed to 8083 this time.

@jamaljsr I believe you're aware of this, so do you think I should go ahead and implement the feature or what would you advise?

@jamaljsr
Copy link
Owner

jamaljsr commented Mar 5, 2024

@Abdulkbk yes, Polar will already detect and avoid using ports that are already in use. The situation that @dangeross is describing is that initially Polar assigns LND REST ports starting at 8081. So if you start a network in Polar, then open another app that needs ports 8081, it will conflict. So he wants to be able to configure Polar to assign ports starting at a number other than 8081.

The list of starting port numbers (BasePorts) that Polar uses are currently hard-coded in constants.ts. The functionality that needs to be implemented is to display UI somewhere that allows users to customize these BasePorts and have them stored in the Polar settings. So if you were to change the the LND REST base port to 8091, then Polar would assign ports 8091, 8092, 8093, etc. This would alleviate the conflict with the other app. The one thing that we have to be careful of is to not allow users to set these base port numbers to be too close to each other. If the user sets the LND REST port to be 8091 and the CLN REST port to be 8093, then the third LND node will overlap with the first CLN node and the docker network will not start. Right now we have the hard-coded values separated by 100 (LND 8081, CLN 8181, Eclair 8281) since it's unlikely for anyone to add 100 nodes.

I hope this all makes sense. Feel free to ask any clarifying questions.

@Abdulkbk
Copy link

Abdulkbk commented Mar 6, 2024

@Abdulkbk yes, Polar will already detect and avoid using ports that are already in use. The situation that @dangeross is describing is that initially Polar assigns LND REST ports starting at 8081. So if you start a network in Polar, then open another app that needs ports 8081, it will conflict. So he wants to be able to configure Polar to assign ports starting at a number other than 8081.

The list of starting port numbers (BasePorts) that Polar uses are currently hard-coded in constants.ts. The functionality that needs to be implemented is to display UI somewhere that allows users to customize these BasePorts and have them stored in the Polar settings. So if you were to change the the LND REST base port to 8091, then Polar would assign ports 8091, 8092, 8093, etc. This would alleviate the conflict with the other app. The one thing that we have to be careful of is to not allow users to set these base port numbers to be too close to each other. If the user sets the LND REST port to be 8091 and the CLN REST port to be 8093, then the third LND node will overlap with the first CLN node and the docker network will not start. Right now we have the hard-coded values separated by 100 (LND 8081, CLN 8181, Eclair 8281) since it's unlikely for anyone to add 100 nodes.

I hope this all makes sense. Feel free to ask any clarifying questions.

This makes perfect sense. Thanks for clarifying.

@Abdulkbk Abdulkbk linked a pull request Apr 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request up for grabs Anyone can work on this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants