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

[Bug?] APP_LIGHTNING_NODE_IP and APP_LIGHTNING_NODE_REST_PORT no longer available #2041

Closed
knorrium opened this issue Jan 9, 2025 · 3 comments · Fixed by #2042
Closed

[Bug?] APP_LIGHTNING_NODE_IP and APP_LIGHTNING_NODE_REST_PORT no longer available #2041

knorrium opened this issue Jan 9, 2025 · 3 comments · Fixed by #2042

Comments

@knorrium
Copy link
Contributor

knorrium commented Jan 9, 2025

Did something change with how these variables are made available to the app containers?

On the mempool frontend, we check for the LND port to enable the Lightning tab

https://github.com/getumbrel/umbrel-apps/blob/master/mempool/docker-compose.yml#L21

On the backend, we construct the LND REST url like other apps do:

https://github.com/getumbrel/umbrel-apps/blob/master/mempool/docker-compose.yml#L59

However, after upgrading to umbrelOS 1.3, these are now empty:

frontend:

/patch $ export
export BACKEND_MAINNET_HTTP_HOST='10.21.21.27'
export FRONTEND_HTTP_PORT='3006'
export HOME='/'
export HOSTNAME='8382c95fbfd4'
export LIGHTNING_DETECTED_PORT=''

backend:

  "LND": {
    "TLS_CERT_PATH": "/lnd/tls.cert",
    "MACAROON_PATH": "/lnd/data/chain/bitcoin/mainnet/readonly.macaroon",
    "REST_API_URL": "https://:",
    "TIMEOUT": 120000
  },

@nmfretz @mayankchhabra can you please take a look?

@nmfretz
Copy link
Contributor

nmfretz commented Jan 10, 2025

Ah darn it, yes there was a small change to ensure that apps are only sourcing environment variables from apps that they list in the required field of their umbrel-app.yml manifest. So this change has broken mempool's LN functionality because the Lightning Node app isn't actually required for the mempool app... we've kind of cheated a bit and made it "optional" even though there is no way to show that in our current apps framwork. This will be changing in the future when we rewrite the apps framework to be able to handle optional dependencies.

I can issue an app update for mempool right now that will fix this. Unless there is an imminent mempool update that you'd rather I wait for?

@nmfretz
Copy link
Contributor

nmfretz commented Jan 10, 2025

@knorrium - I can merge this fix if there is no other mempool version update to sneak in #2042

@knorrium
Copy link
Contributor Author

@nmfretz thanks for the quick fix, I'll follow up there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants