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

Missing host string after creating new wallet connection with Node Launcher #1997

Closed
mrfelton opened this Issue Apr 10, 2019 · 3 comments

Comments

2 participants
@mrfelton
Copy link
Member

mrfelton commented Apr 10, 2019

Description

Missing host string after creating new wallet connection with Node Launcher

Expected Behavior

Host should be set properly in the wallet config.

Actual Behavior

Host value is empty.

Steps to Reproduce

  1. Launch the node launcher bitcoin and lnd nodes
  2. Click the Configure Zap Desktop button to launch Zap
  3. Confirm host as 127.0.0.1
  4. Log out of the wallet
  5. Notice that the Host is blank.
  6. Attempt to enter a host value and save
  7. App crashes with Uncaught TypeError: Cannot read property 'split' of undefined at split (webpack:/renderer/components/Util/WalletName.js:11)

image

Context

lndconnect

Your Environment

  • Zap version: 0.4.0
  • Operating System and version: Mac / Windows

@mrfelton mrfelton added this to the v0.4.1-beta milestone Apr 10, 2019

@mrfelton mrfelton self-assigned this Apr 10, 2019

@mrfelton

This comment has been minimized.

Copy link
Member Author

mrfelton commented Apr 10, 2019

This looks to be because Node Launcher are using old style lndconnect links, eg:

lndconnect:?cert=~/.lnd/tls.cert&macaroon=~/.lnd/admin.macaroon&host=example.com:10009

New style links work correctly:

lndconnect://example.com:10009?cert=~/.lnd/tls.cert&macaroon=~/.lnd/admin.macaroon
@JimmyMow

This comment has been minimized.

Copy link
Member

JimmyMow commented Apr 10, 2019

@mrfelton let's create an issue within the Node Launcher repo and close this?

mrfelton added a commit to mrfelton/zap-desktop that referenced this issue Apr 10, 2019

fix(lndconnect): support old style lndconnect links
Convert old style lndconnect links to the new format before storing as
part of a wallet config.

fix LN-Zap#1997
@mrfelton

This comment has been minimized.

Copy link
Member Author

mrfelton commented Apr 10, 2019

@JimmyMow I put a fix in place for this: #2000

Also created issue for Node Launcher to update their link format: lightning-power-users/node-launcher#272

Once other thing we should probably do is a migration script to delete any wallet configs with missing host info. I'll do another PR for that.

mrfelton added a commit to mrfelton/zap-desktop that referenced this issue Apr 11, 2019

fix(lndconnect): support old style lndconnect links
Convert old style lndconnect links to the new format before storing as
part of a wallet config.

fix LN-Zap#1997
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.