From e9b46055233665c5a1d1164221033fb46e4b4b0a Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Wed, 5 Sep 2018 09:55:36 +0200 Subject: [PATCH 1/4] refactor(grpc): convert WalletUnlocker to class Convert the walletUnlocker object to a class with an interface that is consistent with the Lightning class. --- app/lib/lnd/lightning.js | 18 +-- app/lib/lnd/subscribe/channelgraph.js | 2 +- app/lib/lnd/subscribe/invoices.js | 2 +- app/lib/lnd/subscribe/transactions.js | 2 +- app/lib/lnd/walletUnlocker.js | 133 +++++++++++++++++---- app/lib/lnd/walletUnlockerMethods/index.js | 2 +- app/lib/zap/controller.js | 34 +++--- test/unit/lnd/lightning.spec.js | 2 +- 8 files changed, 139 insertions(+), 56 deletions(-) diff --git a/app/lib/lnd/lightning.js b/app/lib/lnd/lightning.js index 7c24defd..288ad9f1 100644 --- a/app/lib/lnd/lightning.js +++ b/app/lib/lnd/lightning.js @@ -25,7 +25,7 @@ type LightningSubscriptionsType = { */ class Lightning { mainWindow: BrowserWindow - lnd: any + service: any lndConfig: LndConfig subscriptions: LightningSubscriptionsType _fsm: StateMachine @@ -40,7 +40,7 @@ class Lightning { constructor(lndConfig: LndConfig) { this.mainWindow = null - this.lnd = null + this.service = null this.lndConfig = lndConfig this.subscriptions = { channelGraph: null, @@ -89,11 +89,11 @@ class Lightning { const credentials = grpc.credentials.combineChannelCredentials(sslCreds, macaroonCreds) // Create a new gRPC client instance. - this.lnd = new rpc.lnrpc.Lightning(host, credentials) + this.service = new rpc.lnrpc.Lightning(host, credentials) // Wait for the gRPC connection to be established. return new Promise((resolve, reject) => { - grpc.waitForClientReady(this.lnd, getDeadline(2), err => { + grpc.waitForClientReady(this.service, getDeadline(2), err => { if (err) { return reject(err) } @@ -109,8 +109,8 @@ class Lightning { onBeforeDisconnect() { mainLog.info('Disconnecting from Lightning gRPC service') this.unsubscribe() - if (this.lnd) { - this.lnd.close() + if (this.service) { + this.service.close() } } @@ -121,7 +121,7 @@ class Lightning { mainLog.info('Shutting down Lightning daemon') this.unsubscribe() return new Promise((resolve, reject) => { - this.lnd.stopDaemon({}, (err, data) => { + this.service.stopDaemon({}, (err, data) => { if (err) { return reject(err) } @@ -137,8 +137,8 @@ class Lightning { /** * Hook up lnd restful methods. */ - lndMethods(event: Event, msg: string, data: any) { - return methods(this.lnd, mainLog, event, msg, data) + registerMethods(event: Event, msg: string, data: any) { + return methods(this.service, mainLog, event, msg, data) } /** diff --git a/app/lib/lnd/subscribe/channelgraph.js b/app/lib/lnd/subscribe/channelgraph.js index 1ee5b38f..b7624032 100644 --- a/app/lib/lnd/subscribe/channelgraph.js +++ b/app/lib/lnd/subscribe/channelgraph.js @@ -2,7 +2,7 @@ import { status } from 'grpc' import { mainLog } from '../../utils/log' export default function subscribeToChannelGraph() { - const call = this.lnd.subscribeChannelGraph({}) + const call = this.service.subscribeChannelGraph({}) call.on('data', channelGraphData => { mainLog.info('CHANNELGRAPH:', channelGraphData) diff --git a/app/lib/lnd/subscribe/invoices.js b/app/lib/lnd/subscribe/invoices.js index 171471dc..7a7c0bf2 100644 --- a/app/lib/lnd/subscribe/invoices.js +++ b/app/lib/lnd/subscribe/invoices.js @@ -2,7 +2,7 @@ import { status } from 'grpc' import { mainLog } from '../../utils/log' export default function subscribeToInvoices() { - const call = this.lnd.subscribeInvoices({}) + const call = this.service.subscribeInvoices({}) call.on('data', invoice => { mainLog.info('INVOICE:', invoice) diff --git a/app/lib/lnd/subscribe/transactions.js b/app/lib/lnd/subscribe/transactions.js index c328abb2..93075686 100644 --- a/app/lib/lnd/subscribe/transactions.js +++ b/app/lib/lnd/subscribe/transactions.js @@ -2,7 +2,7 @@ import { status } from 'grpc' import { mainLog } from '../../utils/log' export default function subscribeToTransactions() { - const call = this.lnd.subscribeTransactions({}) + const call = this.service.subscribeTransactions({}) call.on('data', transaction => { mainLog.info('TRANSACTION:', transaction) diff --git a/app/lib/lnd/walletUnlocker.js b/app/lib/lnd/walletUnlocker.js index ae8b1979..318a2450 100644 --- a/app/lib/lnd/walletUnlocker.js +++ b/app/lib/lnd/walletUnlocker.js @@ -1,36 +1,117 @@ -import fs from 'fs' +// @flow + import grpc from 'grpc' import { loadSync } from '@grpc/proto-loader' -import walletUnlockerMethods from './walletUnlockerMethods' +import StateMachine from 'javascript-state-machine' +import LndConfig from './config' +import { getDeadline, validateHost, createSslCreds, createMacaroonCreds } from './util' +import methods from './walletUnlockerMethods' import { mainLog } from '../utils/log' -export const initWalletUnlocker = lndConfig => { - const walletUnlockerObj = walletUnlocker(lndConfig) - const walletUnlockerMethodsCallback = (event, msg, data) => - walletUnlockerMethods(lndConfig, walletUnlockerObj, mainLog, event, msg, data) +/** + * Creates an LND grpc client lightning service. + * @returns {WalletUnlocker} + */ +class WalletUnlocker { + service: any + lndConfig: LndConfig + _fsm: StateMachine - return walletUnlockerMethodsCallback -} + // Transitions provided by the state machine. + connect: any + disconnect: any + terminate: any + is: any + can: any + state: string + + constructor(lndConfig: LndConfig) { + this.service = null + this.lndConfig = lndConfig + + // Initialize the state machine. + this._fsm() + } + + // ------------------------------------ + // FSM Callbacks + // ------------------------------------ + + /** + * Connect to the gRPC interface and verify it is functional. + * @return {Promise} + */ + async onBeforeConnect() { + mainLog.info('Connecting to WalletUnlocker gRPC service') + const { rpcProtoPath, host, cert, macaroon } = this.lndConfig + + // Verify that the host is valid before creating a gRPC client that is connected to it. + return await validateHost(host).then(async () => { + // Load the gRPC proto file. + // The following options object closely approximates the existing behavior of grpc.load. + // See https://github.com/grpc/grpc-node/blob/master/packages/grpc-protobufjs/README.md + const options = { + keepCase: true, + longs: Number, + enums: String, + defaults: true, + oneofs: true + } + const packageDefinition = loadSync(rpcProtoPath, options) -export const walletUnlocker = lndConfig => { - const lndCert = fs.readFileSync(lndConfig.cert) - const credentials = grpc.credentials.createSsl(lndCert) - - // Load the gRPC proto file. - // The following options object closely approximates the existing behavior of grpc.load - // See https://github.com/grpc/grpc-node/blob/master/packages/grpc-protobufjs/README.md - const options = { - keepCase: true, - longs: Number, - enums: String, - defaults: true, - oneofs: true + // Load gRPC package definition as a gRPC object hierarchy. + const rpc = grpc.loadPackageDefinition(packageDefinition) + + // Create ssl and macaroon credentials to use with the gRPC client. + const [sslCreds, macaroonCreds] = await Promise.all([ + createSslCreds(cert), + createMacaroonCreds(macaroon) + ]) + const credentials = grpc.credentials.combineChannelCredentials(sslCreds, macaroonCreds) + + // Create a new gRPC client instance. + this.service = new rpc.lnrpc.WalletUnlocker(host, credentials) + + // Wait for the gRPC connection to be established. + return new Promise((resolve, reject) => { + grpc.waitForClientReady(this.service, getDeadline(2), err => { + if (err) { + return reject(err) + } + return resolve() + }) + }) + }) + } + + /** + * Discomnnect the gRPC service. + */ + onBeforeDisconnect() { + mainLog.info('Disconnecting from WalletUnlocker gRPC service') + if (this.service) { + this.service.close() + } } - const packageDefinition = loadSync(lndConfig.rpcProtoPath, options) - // Load gRPC package definition as a gRPC object hierarchy. - const rpc = grpc.loadPackageDefinition(packageDefinition) + // ------------------------------------ + // Helpers + // ------------------------------------ - // Instantiate a new connection to the WalletUnlocker interface. - return new rpc.lnrpc.WalletUnlocker(lndConfig.host, credentials) + /** + * Hook up lnd restful methods. + */ + registerMethods(event: Event, msg: string, data: any) { + return methods(this.service, mainLog, event, msg, data, this.lndConfig) + } } + +StateMachine.factory(WalletUnlocker, { + init: 'ready', + transitions: [ + { name: 'connect', from: 'ready', to: 'connected' }, + { name: 'disconnect', from: 'connected', to: 'ready' } + ] +}) + +export default WalletUnlocker diff --git a/app/lib/lnd/walletUnlockerMethods/index.js b/app/lib/lnd/walletUnlockerMethods/index.js index 094f79f8..bfd9f6a2 100644 --- a/app/lib/lnd/walletUnlockerMethods/index.js +++ b/app/lib/lnd/walletUnlockerMethods/index.js @@ -1,7 +1,7 @@ import { dirname } from 'path' import * as walletController from '../methods/walletController' -export default function(lndConfig, walletUnlocker, log, event, msg, data) { +export default function(walletUnlocker, log, event, msg, data, lndConfig) { const decorateError = error => { switch (error.code) { // wallet already exists diff --git a/app/lib/zap/controller.js b/app/lib/zap/controller.js index 6484e553..58171686 100644 --- a/app/lib/zap/controller.js +++ b/app/lib/zap/controller.js @@ -4,12 +4,13 @@ import { app, ipcMain, dialog, BrowserWindow } from 'electron' import pick from 'lodash.pick' import Store from 'electron-store' import StateMachine from 'javascript-state-machine' +import { mainLog } from '../utils/log' +import { isLndRunning } from '../lnd/util' + import LndConfig from '../lnd/config' import Lightning from '../lnd/lightning' import Neutrino from '../lnd/neutrino' -import { initWalletUnlocker } from '../lnd/walletUnlocker' -import { mainLog } from '../utils/log' -import { isLndRunning } from '../lnd/util' +import WalletUnlocker from '../lnd/walletUnlocker' type onboardingOptions = { type: 'local' | 'custom' | 'btcpayserver', @@ -53,6 +54,7 @@ class ZapController { mainWindow: BrowserWindow neutrino: Neutrino lightning: Lightning + walletUnlocker: WalletUnlocker splashScreenTime: number lndConfig: LndConfig _fsm: StateMachine @@ -244,24 +246,24 @@ class ZapController { /** * Start the wallet unlocker. */ - startWalletUnlocker() { + async startWalletUnlocker() { mainLog.info('Establishing connection to Wallet Unlocker gRPC interface...') + this.walletUnlocker = new WalletUnlocker(this.lndConfig) + + // Connect to the WalletUnlocker interface. try { - const walletUnlockerMethods = initWalletUnlocker(this.lndConfig) + await this.walletUnlocker.connect() - // Listen for all gRPC restful methods - ipcMain.on('walletUnlocker', (event, { msg, data }) => { - walletUnlockerMethods(event, msg, data) - }) + // Listen for all gRPC restful methods and pass to gRPC. + ipcMain.on('walletUnlocker', (event, { msg, data }) => + this.walletUnlocker.registerMethods(event, msg, data) + ) // Notify the renderer that the wallet unlocker is active. this.sendMessage('walletUnlockerGrpcActive') - } catch (error) { - dialog.showMessageBox({ - type: 'error', - message: `Unable to start lnd wallet unlocker. Please check your lnd node and try again: ${error}` - }) - app.quit() + } catch (err) { + mainLog.warn('Unable to connect to WalletUnlocker gRPC interface: %o', err) + throw err } } @@ -279,7 +281,7 @@ class ZapController { this.lightning.subscribe(this.mainWindow) // Listen for all gRPC restful methods and pass to gRPC. - ipcMain.on('lnd', (event, { msg, data }) => this.lightning.lndMethods(event, msg, data)) + ipcMain.on('lnd', (event, { msg, data }) => this.lightning.registerMethods(event, msg, data)) // Let the renderer know that we are connected. this.sendMessage('lightningGrpcActive') diff --git a/test/unit/lnd/lightning.spec.js b/test/unit/lnd/lightning.spec.js index 6a06e3ce..10ade8eb 100644 --- a/test/unit/lnd/lightning.spec.js +++ b/test/unit/lnd/lightning.spec.js @@ -15,7 +15,7 @@ describe('Lightning', function() { expect(this.lightning.mainWindow).toBeNull() }) it('should set the "lnd" property to null', () => { - expect(this.lightning.lnd).toBeNull() + expect(this.lightning.service).toBeNull() }) it('should initialise the "subscriptions" object with null values', () => { expect(this.lightning.subscriptions).toMatchObject({ From 6c3d43ead6b18b0797664c09413fa72ef806efb1 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Wed, 5 Sep 2018 10:18:32 +0200 Subject: [PATCH 2/4] fix(grpc): close connection after failed connect The gRPC Client waitForReady call waits for a specified time frame and either resolves or rejects depending on wether or not a successful connection was established within the given time frame. If the connection attempt failed gRPC will still continue to attempt to establish the connection endlessly. In this commit we explicitly close the client connection after the connect deadline has been exceeded to prevent gRPC from trying to connect. --- app/lib/lnd/lightning.js | 3 ++- app/lib/lnd/walletUnlocker.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/lib/lnd/lightning.js b/app/lib/lnd/lightning.js index 288ad9f1..7cdc0070 100644 --- a/app/lib/lnd/lightning.js +++ b/app/lib/lnd/lightning.js @@ -93,8 +93,9 @@ class Lightning { // Wait for the gRPC connection to be established. return new Promise((resolve, reject) => { - grpc.waitForClientReady(this.service, getDeadline(2), err => { + this.service.waitForReady(getDeadline(2), err => { if (err) { + this.service.close() return reject(err) } return resolve() diff --git a/app/lib/lnd/walletUnlocker.js b/app/lib/lnd/walletUnlocker.js index 318a2450..b712424c 100644 --- a/app/lib/lnd/walletUnlocker.js +++ b/app/lib/lnd/walletUnlocker.js @@ -74,8 +74,9 @@ class WalletUnlocker { // Wait for the gRPC connection to be established. return new Promise((resolve, reject) => { - grpc.waitForClientReady(this.service, getDeadline(2), err => { + this.service.waitForReady(getDeadline(2), err => { if (err) { + this.service.close() return reject(err) } return resolve() From 393215470a36fa6472ce5a8c70714ce9a15da977 Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Wed, 5 Sep 2018 11:10:27 +0200 Subject: [PATCH 3/4] refactor: remove unused method call --- app/lib/lnd/methods/index.js | 2 +- app/reducers/info.js | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/lib/lnd/methods/index.js b/app/lib/lnd/methods/index.js index 15254cb0..74be7be9 100644 --- a/app/lib/lnd/methods/index.js +++ b/app/lib/lnd/methods/index.js @@ -28,7 +28,7 @@ export default function(lnd, log, event, msg, data) { event.sender.send('receiveCryptocurrency', infoData.chains[0]) return infoData }) - .catch(() => event.sender.send('infoFailed')) + .catch(error => log.error('info:', error)) break case 'describeNetwork': networkController diff --git a/app/reducers/info.js b/app/reducers/info.js index a7304242..e14f875d 100644 --- a/app/reducers/info.js +++ b/app/reducers/info.js @@ -72,8 +72,6 @@ const networks = { unitPrefix: '' } } -// IPC info fetch failed -// export const infoFailed = (event, data) => dispatch => {} // ------------------------------------ // Action Handlers From 71ac6adeff39d6f8d9948b60e8f9f736c431871c Mon Sep 17 00:00:00 2001 From: Tom Kirkpatrick Date: Wed, 5 Sep 2018 11:11:09 +0200 Subject: [PATCH 4/4] fix(grpc): show error on connect to locked wallet When connecting to a remote wallet we assume that the wallet is already unlocked. In the case where it is not already unlocked, forward the error message to the UI. Fix #738 --- app/lib/lnd/lightning.js | 76 +++++++++++++++++++---------------- app/lib/lnd/walletUnlocker.js | 2 +- app/lib/zap/controller.js | 10 +++++ 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/app/lib/lnd/lightning.js b/app/lib/lnd/lightning.js index 7cdc0070..0f0e7600 100644 --- a/app/lib/lnd/lightning.js +++ b/app/lib/lnd/lightning.js @@ -11,6 +11,7 @@ import { mainLog } from '../utils/log' import subscribeToTransactions from './subscribe/transactions' import subscribeToInvoices from './subscribe/invoices' import subscribeToChannelGraph from './subscribe/channelgraph' +import { getInfo } from './methods/networkController' // Type definition for subscriptions property. type LightningSubscriptionsType = { @@ -65,43 +66,48 @@ class Lightning { const { rpcProtoPath, host, cert, macaroon } = this.lndConfig // Verify that the host is valid before creating a gRPC client that is connected to it. - return await validateHost(host).then(async () => { - // Load the gRPC proto file. - // The following options object closely approximates the existing behavior of grpc.load. - // See https://github.com/grpc/grpc-node/blob/master/packages/grpc-protobufjs/README.md - const options = { - keepCase: true, - longs: Number, - enums: String, - defaults: true, - oneofs: true - } - const packageDefinition = loadSync(rpcProtoPath, options) - - // Load gRPC package definition as a gRPC object hierarchy. - const rpc = grpc.loadPackageDefinition(packageDefinition) - - // Create ssl and macaroon credentials to use with the gRPC client. - const [sslCreds, macaroonCreds] = await Promise.all([ - createSslCreds(cert), - createMacaroonCreds(macaroon) - ]) - const credentials = grpc.credentials.combineChannelCredentials(sslCreds, macaroonCreds) - - // Create a new gRPC client instance. - this.service = new rpc.lnrpc.Lightning(host, credentials) - - // Wait for the gRPC connection to be established. - return new Promise((resolve, reject) => { - this.service.waitForReady(getDeadline(2), err => { - if (err) { - this.service.close() - return reject(err) - } - return resolve() + return validateHost(host) + .then(async () => { + // Load the gRPC proto file. + // The following options object closely approximates the existing behavior of grpc.load. + // See https://github.com/grpc/grpc-node/blob/master/packages/grpc-protobufjs/README.md + const options = { + keepCase: true, + longs: Number, + enums: String, + defaults: true, + oneofs: true + } + const packageDefinition = loadSync(rpcProtoPath, options) + + // Load gRPC package definition as a gRPC object hierarchy. + const rpc = grpc.loadPackageDefinition(packageDefinition) + + // Create ssl and macaroon credentials to use with the gRPC client. + const [sslCreds, macaroonCreds] = await Promise.all([ + createSslCreds(cert), + createMacaroonCreds(macaroon) + ]) + const credentials = grpc.credentials.combineChannelCredentials(sslCreds, macaroonCreds) + + // Create a new gRPC client instance. + this.service = new rpc.lnrpc.Lightning(host, credentials) + + // Wait for the gRPC connection to be established. + return new Promise((resolve, reject) => { + this.service.waitForReady(getDeadline(5), err => { + if (err) { + return reject(err) + } + return resolve() + }) }) }) - }) + .then(() => getInfo(this.service)) + .catch(err => { + this.service.close() + throw err + }) } /** diff --git a/app/lib/lnd/walletUnlocker.js b/app/lib/lnd/walletUnlocker.js index b712424c..d833a19d 100644 --- a/app/lib/lnd/walletUnlocker.js +++ b/app/lib/lnd/walletUnlocker.js @@ -74,7 +74,7 @@ class WalletUnlocker { // Wait for the gRPC connection to be established. return new Promise((resolve, reject) => { - this.service.waitForReady(getDeadline(2), err => { + this.service.waitForReady(getDeadline(5), err => { if (err) { this.service.close() return reject(err) diff --git a/app/lib/zap/controller.js b/app/lib/zap/controller.js index 58171686..b3592119 100644 --- a/app/lib/zap/controller.js +++ b/app/lib/zap/controller.js @@ -193,6 +193,16 @@ class ZapController { else if (e.code === 'LND_GRPC_MACAROON_ERROR') { errors.macaroon = e.message } + + // The `startLightningWallet` call attempts to call the `getInfo` method on the Lightning service in order to + // verify that it is accessible. If it is not, an error 12 is throw whcih is the gRPC code for `UNIMPLEMENTED` + // which indicates that the requested operation is not implemented or not supported/enabled in the service. + // See https://github.com/grpc/grpc-node/blob/master/packages/grpc-native-core/src/constants.js#L129 + if (e.code === 12) { + errors.host = + 'Unable to connect to host. Please ensure wallet is unlocked before connecting.' + } + // Other error codes such as UNAVAILABLE most likely indicate that there is a problem with the host. else { errors.host = `Unable to connect to host: ${e.details || e.message}`