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

RTL v0.15.2 update on LND #1156

Merged
merged 2 commits into from
Jun 28, 2024
Merged

Conversation

ShahanaFarooqui
Copy link
Contributor

No description provided.

@ShahanaFarooqui ShahanaFarooqui marked this pull request as draft June 12, 2024 21:07
@ShahanaFarooqui
Copy link
Contributor Author

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

  • RTL is now supporting block explorer with configurable BLOCK_EXPLORER_URL variable. It requires:
    1: mempool url and
    2: Bitcoin network value to append with block explorer url (eg. mempool.space/testnet).

    • 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?

@nmfretz
Copy link
Contributor

nmfretz commented Jun 17, 2024

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
Copy link
Contributor

nmfretz commented Jun 17, 2024

@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
Copy link
Contributor Author

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
Copy link
Contributor

nmfretz commented Jun 20, 2024

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

@ShahanaFarooqui
Copy link
Contributor Author

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

@ShahanaFarooqui ShahanaFarooqui changed the title RTL v0.15.1 update on LND RTL v0.15.2 update on LND Jun 26, 2024
@nmfretz
Copy link
Contributor

nmfretz commented Jun 26, 2024

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: 5c9cf75

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
Copy link
Contributor

nmfretz commented Jun 26, 2024

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:

EXPLORER_PORT: "${APP_MEMPOOL_PORT}"
EXPLORER_HIDDEN_SERVICE: "${APP_MEMPOOL_HIDDEN_SERVICE}"

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);
  },

@ShahanaFarooqui ShahanaFarooqui force-pushed the rtl-lnd-update branch 10 times, most recently from c359fb6 to 05286e2 Compare June 28, 2024 00:42
@nmfretz
Copy link
Contributor

nmfretz commented Jun 28, 2024

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

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 this pull request may close these issues.

2 participants