getumbrel / umbrel-apps

The official app repository of the Umbrel App Store. Submit apps and updates here. Learn how → https://github.com/getumbrel/umbrel-apps#readme
https://apps.umbrel.com
480 stars 357 forks source link

RTL v0.15.2 update on LND #1156

Closed ShahanaFarooqui closed 2 days ago

ShahanaFarooqui commented 2 weeks ago
nmfretz commented 2 weeks ago

Hi @ShahanaFarooqui thanks very much for the RTL updates!

The latest RTL release requires nodejs >=v18.19.0. How can I confirm the nodejs version on Umbrel?

Since the RTL image is built with this Dockerfile it will include the correct nodejs version and we don't need to worry about node on umbrelOS. As of this writing, the Dockerfile is using node:18-alpine which right now is node v18.20.3.

$ echo -n "RTL v0.15.1 --> node " && sudo docker exec -it ride-the-lightning_web_1 node --version && echo -n "umbrelOS 1.x --> node " && node --version
RTL v0.15.1 --> node v18.20.3
umbrelOS 1.x --> node v18.19.0
  • Is there a way to configure the explorer url with local mempool url (if it is already installed on Umbrel) otherwise fallback to mempool.space url?
  • How can I capture the Bitcoin Network value?

Great addition with mempool! Yes, we can pass in the correct BLOCK_EXPLORER_URL based on whether or not a local mempool instance is installed and also based on what Bitcoin network the user is currently running on.

I'll create an exports.sh file for you with the logic and we can test.

nmfretz commented 2 weeks ago

@ShahanaFarooqui I just tested the update before working on BLOCK_EXPLORER_URL and I am hitting the same error on both a fresh install and update:

When I open the UI I am met with:

Screenshot 2024-06-17 at 9 59 43 AM

The upstream error (logged in the RTL container) is:

file:///RTL/backend/utils/common.js:43
            delete node.authentication.macaroonPath;
                        ^

TypeError: Cannot convert undefined or null to object
    at CommonService.removeAuthSecureData (file:///RTL/backend/utils/common.js:43:25)
    at file:///RTL/backend/utils/common.js:56:46
    at Array.map (<anonymous>)
    at CommonService.removeSecureData (file:///RTL/backend/utils/common.js:56:27)
    at file:///RTL/backend/controllers/shared/RTLConf.js:107:40
    at FSReqCallback.readFileAfterClose [as oncomplete] (node:internal/fs/read_file_context:68:3)

https://github.com/Ride-The-Lightning/RTL/blob/b6dbd23ae7404ed5e8ad0425e0ba9d4b6b5daf31/backend/utils/common.js#L42-L48

https://github.com/Ride-The-Lightning/RTL/blob/b6dbd23ae7404ed5e8ad0425e0ba9d4b6b5daf31/backend/controllers/shared/RTLConf.js#L107

I have confirmed that in both RTL 0.15.1 and previous 0.14.1 I do not in fact have an authentication key in my RTL-Config.json, which is what RTL is erroring out when trying to access.

Here's my RTL-Config.json:

 $ cat ~/umbrel/app-data/ride-the-lightning/rtl/RTL-Config.json
{
  "multiPass": <redacted>,
  "defaultNodeIndex": 1,
  "SSO": {
    "rtlSSO": 0,
    "rtlCookiePath": "",
    "logoutRedirectLink": ""
  },
  "nodes": [
    {
      "index": 1,
      "lnNode": "Umbrel",
      "settings": {
        "userPersona": "MERCHANT",
        "themeMode": "DAY",
        "themeColor": "PURPLE",
        "fiatConversion": true,
        "logLevel": "INFO"
      }
    }
  ]
}
ShahanaFarooqui commented 1 week ago

As you already figured, missing authentication key is the issue. I added removeAuthSecureData logic in 0.15.1 release and forgot to consider that it could be missing (special setup for Umbrel only).

I will update the PR after release RTL v0.15.2 with the fix.

nmfretz commented 1 week ago

Sounds good @ShahanaFarooqui. I'll push mempool/bitcoin-network url changes after testing them with RTL v0.15.2

ShahanaFarooqui commented 5 days ago

@nmfretz Hey, just pushed a commit with v0.15.2 update.

nmfretz commented 4 days ago

Thanks @ShahanaFarooqui!

Bitcoin network value to append with block explorer url (eg. mempool.space/testnet).

I have added an exports.sh file with logic that appends a user's current Bitcoin network value to the block explorer url: https://github.com/getumbrel/umbrel-apps/pull/1156/commits/5c9cf7547c10625730b81d9fc10eb73cf09acac6

Is there a way to configure the explorer url with local mempool url (if it is already installed on Umbrel) otherwise fallback to mempool.space url?

Looking into this properly I've realized that to robustly link to a user's local Mempool instance, RTL would need to include some additional logic.

This is because we can't know the exact URL that a user is accessing umbrelOS from. For example, are they connected to their local network and http://umbrel.local is available, or are they using a custom local domain like http://umbrel-pi.local, or are they accessing remotely over tailscale at something like http://umbrel, etc. So there is no one URL we can assign to BLOCK_EXPLORER_URL that will work for everyone.

One idea is for us to pass in APP_MEMPOOL_PORT (which is 3006) as an environment variable to the rtl container. Then RTL could check if this variable was set and, if so, construct a URL on the frontend based on the user's current URL. Something like:

localExplorerUrl = `${window.location.protocol}//${window.location.hostname}:${LOCAL_MEMPOOL_PORT}`;

Shall we send out this update without local Mempool functionality (once arm64 hiccup is solved), and target next release for local Mempool?

nmfretz commented 4 days ago

Here's what we do for the Lightning Node app for your reference:

We pass in both the local Mempool port and also the hidden service for Mempool for users who access remotely over Tor (may be overkill for you guys since this is quite umbrelOS specific). Both of these environment variables are available to RTL: https://github.com/getumbrel/umbrel-apps/blob/f61b89bc276455ed982d80b18794c97e6abcf3d1/lightning/docker-compose.yml#L37-L38

The frontend of the Lightning Node app then constructs the URL: https://github.com/getumbrel/umbrel-lightning/blob/525d37c8f856ca746f1bea9471fc0aedada43a4f/apps/frontend/src/store/modules/system.js#L143-L156

async getLocalExplorerUrl({ commit }) {
    const {port, hiddenService} = await API.get(
      `${process.env.VUE_APP_API_BASE_URL}/v1/system/explorer`
    );

    let localExplorerUrl = false;

    if (window.location.origin.endsWith(".onion") && hiddenService) {
      localExplorerUrl = `http://${hiddenService}`;
    } else if (port) {
      localExplorerUrl = `${window.location.protocol}//${window.location.hostname}:${port}`;
    }
    commit("setLocalExplorerUrl", localExplorerUrl);
  },
nmfretz commented 2 days ago

Thanks for the arm64 fix @ShahanaFarooqui. Tested on arm64 and x86. Going live.