umbrel-apps icon indicating copy to clipboard operation
umbrel-apps copied to clipboard

dynamic port allocation

Open Retropex opened this issue 9 months ago • 3 comments

irc there is no need to change the version number if we do not touch the images?

Retropex avatar Mar 31 '25 12:03 Retropex

If you want users to get an update, the version should be updated to something like 28.1-1 with the change mentioned in the release notes.

al-lac avatar Apr 01 '25 12:04 al-lac

Changes pushed, let me know if it's good for you.

Retropex avatar Apr 01 '25 14:04 Retropex

Looks good to me, lets let @nmfretz take a final look before we merge this.

al-lac avatar Apr 02 '25 14:04 al-lac

hey @al-lac @nmfretz, sorry for being insistent with this but this fix is important for knots to get inbound peers would it be possible to merge this?

Retropex avatar Apr 08 '25 10:04 Retropex

Tested on my side and it work!

The script was maybe faulty on your side since I still use umbrel v0.5.4 for my tests.

Retropex avatar Apr 09 '25 14:04 Retropex

Hey @Retropex. Thanks for pinging me, and really sorry for the delay on my end.

Nice work coming up with a workaround to get Knots to bind different ports on the host depending on whether or not Core is installed. Unfortunately, as @al-lac discovered this can lead to broken installs if say Knots is installed and then someone goes to also install Core. Core would attempt to bind to the same host ports and Docker would throw.

We could include similar logic in Core and then additional logic in both apps to detect existing installations, stop apps to avoid failed install on port clash and then bring up both with non-clashing ports. But this gets really hacky via these bash hooks and potentially unreliable, especially considering dependent apps would need to be re-triggered to restart in order to source the correct env vars and we also have the additional complexity of swappable apps for dependent apps like electrs/fulcrum/electrumx. Eventually we'll want umbreldOS to handle preferred ports and clashes so this can all be done very reliably.

In the meantime though, I think there's a simple solution to this:

hey @al-lac @nmfretz, sorry for being insistent with this but this fix is important for knots to get inbound peers would it be possible to merge this?

I think two things are happening:

  1. For incoming clearnet peers, users running behind nat MUST port forward from their router to get incoming connections to their umbrelOS device (regardless of running Knots vs Core). So in this case, users just port forward 8333 from their router to 9333 on their umbrel. And right now, 9333 is the P2P connection port shown to them when they open connection details in the app's UI. But other node's in the network don't need to know this in order to connect to you. They'll still use the default 8333 to connect.

  2. It looks like there's a misconfiguration in how tor is bound for incoming connections. In the umbrel-bitcoin.conf for Knots, the tor control port is set as:

bind=10.21.21.7:8334=onion

8334 being the default that Knots will set when the internal P2P port is 8333 (which it is).

But the $APP_BITCOIN_KNOTS_TOR_PORT in exports.sh that's used to set up the P2P hidden service is set to 9334. This is an internal bitcoind container port so it can just stay as 8334 and will not clash with any other app.

I've just tested this and get incoming connections:

$ bitcoin-cli -netinfo
Bitcoin Knots client v28.1.knots20250305 - server 70016/Satoshi:28.1.0/Knots:20250305/

         ipv4    ipv6   onion     i2p   total   block
in          7       0       1       0       8
out         5       0       3       1       9       2
total      12       0       4       1      17

Does that sound okay to you? Essentially, we change $APP_BITCOIN_KNOTS_TOR_PORT to 8334 to fix the misconfiguration.

nmfretz avatar Apr 11 '25 03:04 nmfretz

Hey thanks for the feedback,

I believe it’s a more practical approach to simply set the internal port to the same as the external. This way, the address manager of Bitcoin will advertise the correct address. Advanced users will be aware of how to forward the port 8333, while others may find it confusing.

Is the feature for port control already planed?

Retropex avatar Apr 11 '25 09:04 Retropex

@Retropex are you fine with me merging this now?

@nmfretz will probably get back to this once something like port control has been implemented. Not sure if it is planned yet.

al-lac avatar Apr 11 '25 09:04 al-lac

Sure you can merge!

Retropex avatar Apr 11 '25 09:04 Retropex

Hey @Retropex -

I believe it’s a more practical approach to simply set the internal port to the same as the external. This way, the address manager of Bitcoin will advertise the correct address.

It's totally fine to do it this way, but you would need further changes to get it working:

Set the bin arguments in exports.sh to:

BIN_ARGS+=( "-port=${APP_BITCOIN_KNOTS_INTERNAL_P2P_PORT}" )
BIN_ARGS+=( "-rpcport=${APP_BITCOIN_KNOTS_INTERNAL_RPC_PORT}" )

because right now they are hardcoded to:

BIN_ARGS+=( "-port=8333" )
BIN_ARGS+=( "-rpcport=8332" )

I believe the internal logic of the retropex/umbrel-bitcoin-knots container is writing out the conf file with: bind=0.0.0.0:8333 So that would need to change as well.

But I'm not sure it will make things easier in general, because:

  1. For P2P: users still need to port forward for incoming clearnet connections. They'd just forward 9333 on router to 9333 on their umbrel, instead of 9333 on router to 8333 on umbrel. Peers have no idea and don't care what port is exposed to the users host machine from a containerized bitcoind. Our main issue for incoming tor connections on the other hand was just a misconfiguration issue that is solved by changing that internal port to 8334.

  2. For RPC: From what I remember, the main reason we kept the internal rpc as the default 8332 was so that users who want to run cli commands inside the bitcoind container could do so without having to know the port is non standard. So right now someone can just run:

$ bitcoin-cli -netinfo

But with a non-default 9332 port they would need to run:

$ bitcoin-cli -rpcport=9332 -netinfo

What do you think is the best option?

Is the feature for port control already planed?

It's on the roadmap, but we don't have a planned date yet. Will be really nice to have this though.

nmfretz avatar Apr 11 '25 12:04 nmfretz

🎉   Linting finished with no errors or warnings   🎉

Thank you for your submission! This is an automated linter that checks for common issues in pull requests to the Umbrel App Store.

github-actions[bot] avatar Apr 12 '25 08:04 github-actions[bot]

Ok, knowing all of this I think it's better to just fix the tor port. I have also added a note on which ports need to be forwarded.

Retropex avatar Apr 12 '25 08:04 Retropex

Ok, knowing all of this I think it's better to just fix the tor port. I have also added a note on which ports need to be forwarded.

Sounds good, thanks again @Retropex. Going live.

nmfretz avatar Apr 21 '25 03:04 nmfretz