From 6021c9857907f5418ae8718b60741633689c9b51 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Mon, 27 Aug 2018 18:39:40 +0200 Subject: [PATCH 01/15] Implement polling and retry helper functions --- src/helper.js | 40 ++++++++++++++++++++++++- test/unit/helper.spec.js | 64 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/src/helper.js b/src/helper.js index 1f831837c..f353f230a 100644 --- a/src/helper.js +++ b/src/helper.js @@ -2,7 +2,7 @@ * @fileOverview helper and utility functions that can be reused go here. */ -import { UNITS, LND_INIT_DELAY } from './config'; +import { UNITS, LND_INIT_DELAY, RETRY_DELAY } from './config'; /** * Format a number value in locale format with either . or , @@ -252,3 +252,41 @@ export const checkHttpStatus = response => { export const nap = (ms = LND_INIT_DELAY) => { return new Promise(resolve => setTimeout(resolve, ms)); }; + +/** + * A polling utility that can be used to poll apis. If the api returns + * a truthy value this utility will stop polling. Errors thrown by the + * api are just thrown up to the caller to handle. + * @param {Function} api The api wrapped in an asynchronous function + * @param {number} interval The time interval to wait between polls + * @param {number} retries The number of retries to poll the api + * @return {Promise} The return value of the api + */ +export const poll = async (api, interval = RETRY_DELAY, retries = Infinity) => { + while (retries--) { + const response = await api(); + if (response) return response; + await nap(interval); + } + throw new Error('Maximum retries for polling reached'); +}; + +/** + * A retry utility that can be used to try multiple requests to an api. This + * utility will resolve with the return value if the api resolves. Errors + * thrown by the api are swallowed by this utility and another retry is triggered. + * @param {Function} api The api wrapped in an asynchronous function + * @param {number} retries The number of retries to be sent to the api + * @return {Promise} The return value of the api + */ +export const retry = async (api, interval = 100, retries = 1000) => { + while (retries--) { + try { + return await api(); + } catch (err) { + if (!retries) throw err; + } + await nap(interval); + } + return null; +}; diff --git a/test/unit/helper.spec.js b/test/unit/helper.spec.js index ae2c7f972..d9864acfd 100644 --- a/test/unit/helper.spec.js +++ b/test/unit/helper.spec.js @@ -749,4 +749,68 @@ describe('Helpers Unit Tests', () => { expect(res, 'to equal', response); }); }); + + describe('poll()', () => { + it('should poll three times', async () => { + let counter = 0; + const stub = sinon.stub(); + const api = () => + new Promise(resolve => { + stub(); + if (++counter === 3) resolve(true); + else resolve(false); + }); + const response = await helpers.poll(api, 1, 10); + expect(response, 'to be true'); + expect(stub.callCount, 'to equal', 3); + }); + + it('should throw an error after ten retries', async () => { + let counter = 0; + const stub = sinon.stub(); + const api = () => + new Promise(resolve => { + stub(); + if (++counter === 11) resolve(true); + else resolve(false); + }); + await expect( + helpers.poll(api, 1, 10), + 'to be rejected with error satisfying', + /retries/ + ); + expect(stub.callCount, 'to equal', 10); + }); + }); + + describe('retry()', () => { + it('should retry 10 times', async () => { + const stub = sinon.stub(); + const api = () => + new Promise((resolve, reject) => { + stub(); + reject(new Error('Boom!')); + }); + await expect( + helpers.retry(api, 1, 10), + 'to be rejected with error satisfying', + /Boom!/ + ); + expect(stub.callCount, 'to equal', 10); + }); + + it('should try 3 times and then work', async () => { + let counter = 0; + const stub = sinon.stub(); + const api = () => + new Promise((resolve, reject) => { + stub(); + if (++counter === 3) resolve('foo'); + reject(new Error('Boom!')); + }); + const response = await helpers.retry(api, 1); + expect(response, 'to equal', 'foo'); + expect(stub.callCount, 'to equal', 3); + }); + }); }); From 3ec025f12976c580637fbacd807ff5f97cc6e619 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 11:34:52 +0200 Subject: [PATCH 02/15] Implement polling wallet balances in wallet action --- src/action/wallet.js | 21 ++++++++++++--------- test/unit/action/wallet.spec.js | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/action/wallet.js b/src/action/wallet.js index e2e023e44..993f68ff4 100644 --- a/src/action/wallet.js +++ b/src/action/wallet.js @@ -4,7 +4,7 @@ */ import { observe, when } from 'mobx'; -import { toBuffer, parseSat, checkHttpStatus, nap } from '../helper'; +import { toBuffer, parseSat, checkHttpStatus, nap, poll } from '../helper'; import { MIN_PASSWORD_LENGTH, NOTIFICATION_DELAY, RATE_DELAY } from '../config'; import * as log from './log'; @@ -105,17 +105,20 @@ class WalletAction { } /** - * Update the wallet on-chain balance, channel balance, wallet address - * and fiat/btc exchange rate. + * Update the wallet on-chain and channel balances. * @return {Promise} */ async update() { - await Promise.all([ - this.getBalance(), - this.getChannelBalance(), - this.getNewAddress(), - this.pollExchangeRate(), - ]); + await Promise.all([this.getBalance(), this.getChannelBalance()]); + } + + /** + * Poll the wallet balances in the background since there is no streaming + * grpc api yet + * @return {Promise} + */ + async pollBalances() { + await poll(() => this.update()); } /** diff --git a/test/unit/action/wallet.spec.js b/test/unit/action/wallet.spec.js index 3557e3774..0ef61ea08 100644 --- a/test/unit/action/wallet.spec.js +++ b/test/unit/action/wallet.spec.js @@ -110,11 +110,20 @@ describe('Action Wallet Unit Tests', () => { }); describe('update()', () => { - it('should refresh balances, exchange rate and address', async () => { + it('should refresh wallet balances', async () => { sandbox.stub(wallet, 'pollExchangeRate'); await wallet.update(); - expect(grpc.sendCommand, 'was called thrice'); - expect(wallet.pollExchangeRate, 'was called once'); + expect(grpc.sendCommand, 'was called twice'); + expect(wallet.pollExchangeRate, 'was not called'); + }); + }); + + describe('pollBalances()', () => { + it('should poll wallet balances', async () => { + sandbox.stub(wallet, 'update'); + wallet.update.onSecondCall().resolves(true); + await wallet.pollBalances(); + expect(wallet.update, 'was called twice'); }); }); From 05ec43daa068a4274420fdca325925797dc6c886 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 11:39:33 +0200 Subject: [PATCH 03/15] Use new poll util to get exchange rate --- src/action/wallet.js | 4 +--- test/unit/action/wallet.spec.js | 8 +++----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/action/wallet.js b/src/action/wallet.js index 993f68ff4..bbf25b767 100644 --- a/src/action/wallet.js +++ b/src/action/wallet.js @@ -309,9 +309,7 @@ class WalletAction { * @return {Promise} */ async pollExchangeRate() { - await this.getExchangeRate(); - clearTimeout(this.tPollRate); - this.tPollRate = setTimeout(() => this.pollExchangeRate(), RATE_DELAY); + await poll(() => this.getExchangeRate(), RATE_DELAY); } /** diff --git a/test/unit/action/wallet.spec.js b/test/unit/action/wallet.spec.js index 0ef61ea08..673606e56 100644 --- a/test/unit/action/wallet.spec.js +++ b/test/unit/action/wallet.spec.js @@ -5,7 +5,6 @@ import WalletAction from '../../../src/action/wallet'; import NavAction from '../../../src/action/nav'; import NotificationAction from '../../../src/action/notification'; import * as logger from '../../../src/action/log'; -import { nap } from '../../../src/helper'; import nock from 'nock'; import 'isomorphic-fetch'; @@ -33,7 +32,6 @@ describe('Action Wallet Unit Tests', () => { }); afterEach(() => { - clearTimeout(wallet.tPollRate); sandbox.restore(); }); @@ -357,11 +355,11 @@ describe('Action Wallet Unit Tests', () => { }); describe('pollExchangeRate()', () => { - it('should call getExchangeRate', async () => { + it('should poll getExchangeRate', async () => { sandbox.stub(wallet, 'getExchangeRate'); + wallet.getExchangeRate.onSecondCall().resolves(true); await wallet.pollExchangeRate(); - await nap(30); - expect(wallet.getExchangeRate.callCount, 'to be greater than', 1); + expect(wallet.getExchangeRate, 'was called twice'); }); }); From 6d5be8d571476b98748ef0e7c184163625af6f41 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 11:49:53 +0200 Subject: [PATCH 04/15] Use new polling util in info action --- src/action/info.js | 13 ++++++++++--- test/unit/action/info.spec.js | 29 +++++++++++++++++------------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/action/info.js b/src/action/info.js index 6dc8c69d3..60484cffe 100644 --- a/src/action/info.js +++ b/src/action/info.js @@ -4,8 +4,8 @@ * the current block height. */ -import { RETRY_DELAY } from '../config'; import { observe } from 'mobx'; +import { poll } from '../helper'; import * as log from './log'; class InfoAction { @@ -35,14 +35,21 @@ class InfoAction { this._notification.display({ msg: 'Syncing to chain', wait: true }); log.info(`Syncing to chain ... block height: ${response.block_height}`); this._store.percentSynced = this.calcPercentSynced(response); - clearTimeout(this.t3); - this.t3 = setTimeout(() => this.getInfo(), RETRY_DELAY); } + return response.synced_to_chain; } catch (err) { log.error('Getting node info failed', err); } } + /** + * Poll the getInfo api until synced_to_chain is true. + * @return {Promise} + */ + async pollInfo() { + await poll(() => this.getInfo()); + } + /** * A navigation helper called during the app onboarding process. The loader * screen indicating the syncing progress in displayed until syncing has diff --git a/test/unit/action/info.spec.js b/test/unit/action/info.spec.js index 651ed9b8f..bf93f044e 100644 --- a/test/unit/action/info.spec.js +++ b/test/unit/action/info.spec.js @@ -4,7 +4,6 @@ import NavAction from '../../../src/action/nav'; import NotificationAction from '../../../src/action/notification'; import InfoAction from '../../../src/action/info'; import * as logger from '../../../src/action/log'; -import { nap } from '../../../src/helper'; describe('Action Info Unit Tests', () => { let sandbox; @@ -27,7 +26,6 @@ describe('Action Info Unit Tests', () => { }); afterEach(() => { - clearTimeout(info.t3); sandbox.restore(); }); @@ -44,13 +42,12 @@ describe('Action Info Unit Tests', () => { expect(store.blockHeight, 'to equal', 'some-height'); }); - it('should only call once if chain is synced', async () => { + it('should return true if chain is synced', async () => { grpc.sendCommand.withArgs('getInfo').resolves({ synced_to_chain: true, }); - await info.getInfo(); - await nap(30); - expect(grpc.sendCommand.callCount, 'to equal', 1); + const synced = await info.getInfo(); + expect(synced, 'to be', true); }); it('should set percentSynced', async () => { @@ -63,22 +60,30 @@ describe('Action Info Unit Tests', () => { expect(store.percentSynced, 'to be within', 0, 1); }); - it('should retry if chain is not synced', async () => { + it('should return false if chain is not synced', async () => { grpc.sendCommand.withArgs('getInfo').resolves({ synced_to_chain: false, }); - await info.getInfo(); - await nap(30); - expect(grpc.sendCommand.callCount, 'to be greater than', 1); + const synced = await info.getInfo(); + expect(synced, 'to be', false); }); - it('should retry on failure', async () => { + it('should log error on failure', async () => { grpc.sendCommand.rejects(); await info.getInfo(); expect(logger.error, 'was called once'); }); }); + describe('pollInfo()', () => { + it('should poll wallet balances', async () => { + sandbox.stub(info, 'getInfo'); + info.getInfo.onSecondCall().resolves(true); + await info.pollInfo(); + expect(info.getInfo, 'was called twice'); + }); + }); + describe('initLoaderSyncing()', () => { it('should navigate straight to home if synced', async () => { grpc.sendCommand.withArgs('getInfo').resolves({ @@ -99,7 +104,7 @@ describe('Action Info Unit Tests', () => { grpc.sendCommand.withArgs('getInfo').resolves({ synced_to_chain: true, }); - await nap(10); + await info.getInfo(); expect(nav.goHome, 'was called once'); }); }); From cbd1782df32817f2d036291bdeb8969587d8ddd1 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:00:26 +0200 Subject: [PATCH 05/15] Remove wallet dependency from transaction action since wallet balances are now polled --- src/action/index.js | 2 +- src/action/transaction.js | 9 +++------ stories/screen-story.js | 2 +- test/integration/action/action-integration.spec.js | 4 ++-- test/unit/action/transaction.spec.js | 9 ++------- 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/action/index.js b/src/action/index.js index 6b7551a68..4342d2435 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -36,7 +36,7 @@ export const grpc = new GrpcAction(store, ipc); export const notify = new NotificationAction(store, nav); export const wallet = new WalletAction(store, grpc, db, nav, notify); export const info = new InfoAction(store, grpc, nav, notify); -export const transaction = new TransactionAction(store, grpc, wallet, nav); +export const transaction = new TransactionAction(store, grpc, nav); export const channel = new ChannelAction(store, grpc, transaction, nav, notify); export const invoice = new InvoiceAction( store, diff --git a/src/action/transaction.js b/src/action/transaction.js index 1cf0d19d6..12db6d8d1 100644 --- a/src/action/transaction.js +++ b/src/action/transaction.js @@ -7,10 +7,9 @@ import * as log from './log'; import { parseDate, parseSat, toHex } from '../helper'; class TransactionAction { - constructor(store, grpc, wallet, nav) { + constructor(store, grpc, nav) { this._store = store; this._grpc = grpc; - this._wallet = wallet; this._nav = nav; } @@ -37,8 +36,8 @@ class TransactionAction { } /** - * Update the on-chain transactions, invoice, lighting payments, and wallet - * balances in the app state by querying all required grpc apis. + * Update the on-chain transactions, invoice, and lighting payments in the + * app state by querying all required grpc apis. * @return {Promise} */ async update() { @@ -46,8 +45,6 @@ class TransactionAction { this.getTransactions(), this.getInvoices(), this.getPayments(), - this._wallet.getBalance(), - this._wallet.getChannelBalance(), ]); } diff --git a/stories/screen-story.js b/stories/screen-story.js index 32703bbef..5cb44f9bd 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -65,7 +65,7 @@ sinon.stub(wallet, 'checkSeed'); sinon.stub(wallet, 'checkNewPassword'); sinon.stub(wallet, 'checkPassword'); sinon.stub(wallet, 'getExchangeRate'); -const transaction = new TransactionAction(store, grpc, wallet, nav); +const transaction = new TransactionAction(store, grpc, nav); sinon.stub(transaction, 'update'); const invoice = new InvoiceAction( store, diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index fd71c277d..0abc9c828 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -149,7 +149,7 @@ describe('Action Integration Tests', function() { grpc1 = new GrpcAction(store1, ipc1); info1 = new InfoAction(store1, grpc1, nav1, notify1); wallet1 = new WalletAction(store1, grpc1, db1, nav1, notify1); - transactions1 = new TransactionAction(store1, grpc1, wallet1, nav1); + transactions1 = new TransactionAction(store1, grpc1, nav1); channels1 = new ChannelAction(store1, grpc1, transactions1, nav1, notify1); invoice1 = new InvoiceAction(store1, grpc1, transactions1, nav1, notify1); payments1 = new PaymentAction(store1, grpc1, transactions1, nav1, notify1); @@ -161,7 +161,7 @@ describe('Action Integration Tests', function() { grpc2 = new GrpcAction(store2, ipc2); info2 = new InfoAction(store2, grpc2, nav2, notify2); wallet2 = new WalletAction(store2, grpc2, db2, nav2, notify2); - transactions2 = new TransactionAction(store2, grpc2, wallet2, nav2); + transactions2 = new TransactionAction(store2, grpc2, nav2); channels2 = new ChannelAction(store2, grpc2, transactions2, nav2, notify2); invoice2 = new InvoiceAction(store2, grpc2, transactions2, nav2, notify2); payments2 = new PaymentAction(store2, grpc2, transactions2, nav2, notify2); diff --git a/test/unit/action/transaction.spec.js b/test/unit/action/transaction.spec.js index 91227a800..f794c92de 100644 --- a/test/unit/action/transaction.spec.js +++ b/test/unit/action/transaction.spec.js @@ -1,7 +1,6 @@ import { Store } from '../../../src/store'; import GrpcAction from '../../../src/action/grpc'; import TransactionAction from '../../../src/action/transaction'; -import WalletAction from '../../../src/action/wallet'; import NavAction from '../../../src/action/nav'; import * as logger from '../../../src/action/log'; @@ -10,7 +9,6 @@ describe('Action Transactions Unit Tests', () => { let sandbox; let grpc; let nav; - let wallet; let transaction; beforeEach(() => { @@ -20,8 +18,7 @@ describe('Action Transactions Unit Tests', () => { require('../../../src/config').RETRY_DELAY = 1; grpc = sinon.createStubInstance(GrpcAction); nav = sinon.createStubInstance(NavAction); - wallet = sinon.createStubInstance(WalletAction); - transaction = new TransactionAction(store, grpc, wallet, nav); + transaction = new TransactionAction(store, grpc, nav); }); afterEach(() => { @@ -48,11 +45,9 @@ describe('Action Transactions Unit Tests', () => { }); describe('update()', () => { - it('should refresh transactions and balances', async () => { + it('should refresh transactions', async () => { await transaction.update(); expect(grpc.sendCommand, 'was called thrice'); - expect(wallet.getBalance, 'was called once'); - expect(wallet.getChannelBalance, 'was called once'); }); }); From 44b2048c03a955fec22d2cffb9111ca09aa1fd63 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:07:04 +0200 Subject: [PATCH 06/15] Remove transaction dependency in invoice action since wallet balances are polled now and transactions are updated anyway when navigating to the transaction list --- src/action/index.js | 9 +-------- src/action/invoice.js | 4 +--- stories/screen-story.js | 9 +-------- test/integration/action/action-integration.spec.js | 4 ++-- test/unit/action/invoice.spec.js | 13 +------------ 5 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/action/index.js b/src/action/index.js index 4342d2435..f1270edaf 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -38,14 +38,7 @@ export const wallet = new WalletAction(store, grpc, db, nav, notify); export const info = new InfoAction(store, grpc, nav, notify); export const transaction = new TransactionAction(store, grpc, nav); export const channel = new ChannelAction(store, grpc, transaction, nav, notify); -export const invoice = new InvoiceAction( - store, - grpc, - transaction, - nav, - notify, - Clipboard -); +export const invoice = new InvoiceAction(store, grpc, nav, notify, Clipboard); export const payment = new PaymentAction(store, grpc, transaction, nav, notify); export const setting = new SettingAction(store, wallet, db, ipc); diff --git a/src/action/invoice.js b/src/action/invoice.js index 3a4cf0396..a6e908607 100644 --- a/src/action/invoice.js +++ b/src/action/invoice.js @@ -7,10 +7,9 @@ import { PREFIX_URI } from '../config'; import { toSatoshis } from '../helper'; class InvoiceAction { - constructor(store, grpc, transaction, nav, notification, clipboard) { + constructor(store, grpc, nav, notification, clipboard) { this._store = store; this._grpc = grpc; - this._transaction = transaction; this._nav = nav; this._notification = notification; this._clipboard = clipboard; @@ -70,7 +69,6 @@ class InvoiceAction { } catch (err) { this._notification.display({ msg: 'Creating invoice failed!', err }); } - await this._transaction.update(); } /** diff --git a/stories/screen-story.js b/stories/screen-story.js index 5cb44f9bd..b9a9f46b3 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -67,14 +67,7 @@ sinon.stub(wallet, 'checkPassword'); sinon.stub(wallet, 'getExchangeRate'); const transaction = new TransactionAction(store, grpc, nav); sinon.stub(transaction, 'update'); -const invoice = new InvoiceAction( - store, - grpc, - transaction, - nav, - notify, - Clipboard -); +const invoice = new InvoiceAction(store, grpc, nav, notify, Clipboard); sinon.stub(invoice, 'generateUri'); const payment = new PaymentAction(store, grpc, transaction, nav, notify); sinon.stub(payment, 'checkType'); diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 0abc9c828..8553db5ca 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -151,7 +151,7 @@ describe('Action Integration Tests', function() { wallet1 = new WalletAction(store1, grpc1, db1, nav1, notify1); transactions1 = new TransactionAction(store1, grpc1, nav1); channels1 = new ChannelAction(store1, grpc1, transactions1, nav1, notify1); - invoice1 = new InvoiceAction(store1, grpc1, transactions1, nav1, notify1); + invoice1 = new InvoiceAction(store1, grpc1, nav1, notify1); payments1 = new PaymentAction(store1, grpc1, transactions1, nav1, notify1); db2 = sinon.createStubInstance(AppStorage); @@ -163,7 +163,7 @@ describe('Action Integration Tests', function() { wallet2 = new WalletAction(store2, grpc2, db2, nav2, notify2); transactions2 = new TransactionAction(store2, grpc2, nav2); channels2 = new ChannelAction(store2, grpc2, transactions2, nav2, notify2); - invoice2 = new InvoiceAction(store2, grpc2, transactions2, nav2, notify2); + invoice2 = new InvoiceAction(store2, grpc2, nav2, notify2); payments2 = new PaymentAction(store2, grpc2, transactions2, nav2, notify2); }); diff --git a/test/unit/action/invoice.spec.js b/test/unit/action/invoice.spec.js index af9074159..f19018cc9 100644 --- a/test/unit/action/invoice.spec.js +++ b/test/unit/action/invoice.spec.js @@ -2,7 +2,6 @@ import { Store } from '../../../src/store'; import NavAction from '../../../src/action/nav'; import GrpcAction from '../../../src/action/grpc'; import InvoiceAction from '../../../src/action/invoice'; -import TransactionAction from '../../../src/action/transaction'; import NotificationAction from '../../../src/action/notification'; describe('Action Invoice Unit Tests', () => { @@ -10,7 +9,6 @@ describe('Action Invoice Unit Tests', () => { let nav; let grpc; let invoice; - let transaction; let notification; let clipboard; @@ -22,15 +20,7 @@ describe('Action Invoice Unit Tests', () => { grpc = sinon.createStubInstance(GrpcAction); notification = sinon.createStubInstance(NotificationAction); clipboard = { setString: sinon.stub() }; - transaction = sinon.createStubInstance(TransactionAction); - invoice = new InvoiceAction( - store, - grpc, - transaction, - nav, - notification, - clipboard - ); + invoice = new InvoiceAction(store, grpc, nav, notification, clipboard); }); describe('init()', () => { @@ -78,7 +68,6 @@ describe('Action Invoice Unit Tests', () => { expect(store.invoice.encoded, 'to equal', 'some-request'); expect(store.invoice.uri, 'to equal', 'lightning:some-request'); expect(nav.goInvoiceQR, 'was called once'); - expect(transaction.update, 'was called once'); }); it('should display notification on error', async () => { From 2d251f43944dea510617092f6a80bfcbc1775d2d Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:10:13 +0200 Subject: [PATCH 07/15] Remove transaction from the channel action since wallet balances are polled and transaction list is updated when navigating there --- src/action/channel.js | 4 +--- src/action/index.js | 2 +- stories/screen-story.js | 2 +- test/integration/action/action-integration.spec.js | 4 ++-- test/unit/action/channel.spec.js | 6 +----- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/action/channel.js b/src/action/channel.js index 24eb904d8..ed6949483 100644 --- a/src/action/channel.js +++ b/src/action/channel.js @@ -7,10 +7,9 @@ import { toSatoshis, parseSat } from '../helper'; import * as log from './log'; class ChannelAction { - constructor(store, grpc, transaction, nav, notification) { + constructor(store, grpc, nav, notification) { this._store = store; this._grpc = grpc; - this._transaction = transaction; this._nav = nav; this._notification = notification; } @@ -84,7 +83,6 @@ class ChannelAction { this.getPeers(), this.getChannels(), this.getPendingChannels(), - this._transaction.update(), ]); } diff --git a/src/action/index.js b/src/action/index.js index f1270edaf..e31415c6a 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -37,7 +37,7 @@ export const notify = new NotificationAction(store, nav); export const wallet = new WalletAction(store, grpc, db, nav, notify); export const info = new InfoAction(store, grpc, nav, notify); export const transaction = new TransactionAction(store, grpc, nav); -export const channel = new ChannelAction(store, grpc, transaction, nav, notify); +export const channel = new ChannelAction(store, grpc, nav, notify); export const invoice = new InvoiceAction(store, grpc, nav, notify, Clipboard); export const payment = new PaymentAction(store, grpc, transaction, nav, notify); export const setting = new SettingAction(store, wallet, db, ipc); diff --git a/stories/screen-story.js b/stories/screen-story.js index b9a9f46b3..c02b51646 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -73,7 +73,7 @@ const payment = new PaymentAction(store, grpc, transaction, nav, notify); sinon.stub(payment, 'checkType'); sinon.stub(payment, 'payBitcoin'); sinon.stub(payment, 'payLightning'); -const channel = new ChannelAction(store, grpc, transaction, nav, notify); +const channel = new ChannelAction(store, grpc, nav, notify); sinon.stub(channel, 'update'); sinon.stub(channel, 'connectAndOpen'); sinon.stub(channel, 'closeSelectedChannel'); diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 8553db5ca..27a3d95bf 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -150,7 +150,7 @@ describe('Action Integration Tests', function() { info1 = new InfoAction(store1, grpc1, nav1, notify1); wallet1 = new WalletAction(store1, grpc1, db1, nav1, notify1); transactions1 = new TransactionAction(store1, grpc1, nav1); - channels1 = new ChannelAction(store1, grpc1, transactions1, nav1, notify1); + channels1 = new ChannelAction(store1, grpc1, nav1, notify1); invoice1 = new InvoiceAction(store1, grpc1, nav1, notify1); payments1 = new PaymentAction(store1, grpc1, transactions1, nav1, notify1); @@ -162,7 +162,7 @@ describe('Action Integration Tests', function() { info2 = new InfoAction(store2, grpc2, nav2, notify2); wallet2 = new WalletAction(store2, grpc2, db2, nav2, notify2); transactions2 = new TransactionAction(store2, grpc2, nav2); - channels2 = new ChannelAction(store2, grpc2, transactions2, nav2, notify2); + channels2 = new ChannelAction(store2, grpc2, nav2, notify2); invoice2 = new InvoiceAction(store2, grpc2, nav2, notify2); payments2 = new PaymentAction(store2, grpc2, transactions2, nav2, notify2); }); diff --git a/test/unit/action/channel.spec.js b/test/unit/action/channel.spec.js index e29c40d6e..7f284d7dc 100644 --- a/test/unit/action/channel.spec.js +++ b/test/unit/action/channel.spec.js @@ -1,7 +1,6 @@ import { Store } from '../../../src/store'; import GrpcAction from '../../../src/action/grpc'; import ChannelAction from '../../../src/action/channel'; -import TransactionAction from '../../../src/action/transaction'; import NotificationAction from '../../../src/action/notification'; import NavAction from '../../../src/action/nav'; import * as logger from '../../../src/action/log'; @@ -16,7 +15,6 @@ describe('Action Channels Unit Tests', () => { let grpc; let channel; let nav; - let transaction; let notification; beforeEach(() => { @@ -26,10 +24,9 @@ describe('Action Channels Unit Tests', () => { store.settings.displayFiat = false; require('../../../src/config').RETRY_DELAY = 1; grpc = sinon.createStubInstance(GrpcAction); - transaction = sinon.createStubInstance(TransactionAction); notification = sinon.createStubInstance(NotificationAction); nav = sinon.createStubInstance(NavAction); - channel = new ChannelAction(store, grpc, transaction, nav, notification); + channel = new ChannelAction(store, grpc, nav, notification); }); afterEach(() => { @@ -85,7 +82,6 @@ describe('Action Channels Unit Tests', () => { it('should refresh channels and peers', async () => { await channel.update(); expect(grpc.sendCommand, 'was called thrice'); - expect(transaction.update, 'was called once'); }); }); From baa3797ef7ade6cf3221064c1e9cb5dd12873296 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:15:12 +0200 Subject: [PATCH 08/15] Remove transaction dependency from payment action --- src/action/index.js | 2 +- src/action/payment.js | 5 +---- stories/screen-story.js | 2 +- test/integration/action/action-integration.spec.js | 4 ++-- test/unit/action/payment.spec.js | 9 +-------- 5 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/action/index.js b/src/action/index.js index e31415c6a..da2e7a0b9 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -39,7 +39,7 @@ export const info = new InfoAction(store, grpc, nav, notify); export const transaction = new TransactionAction(store, grpc, nav); export const channel = new ChannelAction(store, grpc, nav, notify); export const invoice = new InvoiceAction(store, grpc, nav, notify, Clipboard); -export const payment = new PaymentAction(store, grpc, transaction, nav, notify); +export const payment = new PaymentAction(store, grpc, nav, notify); export const setting = new SettingAction(store, wallet, db, ipc); payment.listenForUrl(ipc); // enable incoming url handler diff --git a/src/action/payment.js b/src/action/payment.js index f5f2f79fc..0ac8b27bd 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -15,10 +15,9 @@ import { import * as log from './log'; class PaymentAction { - constructor(store, grpc, transaction, nav, notification) { + constructor(store, grpc, nav, notification) { this._store = store; this._grpc = grpc; - this._transaction = transaction; this._nav = nav; this._notification = notification; } @@ -161,7 +160,6 @@ class PaymentAction { } catch (err) { this._notification.display({ msg: 'Sending transaction failed!', err }); } - await this._transaction.update(); } /** @@ -192,7 +190,6 @@ class PaymentAction { this._nav.goPayLightningConfirm(); this._notification.display({ msg: 'Lightning payment failed!', err }); } - await this._transaction.update(); } } diff --git a/stories/screen-story.js b/stories/screen-story.js index c02b51646..6dfa0d1db 100644 --- a/stories/screen-story.js +++ b/stories/screen-story.js @@ -69,7 +69,7 @@ const transaction = new TransactionAction(store, grpc, nav); sinon.stub(transaction, 'update'); const invoice = new InvoiceAction(store, grpc, nav, notify, Clipboard); sinon.stub(invoice, 'generateUri'); -const payment = new PaymentAction(store, grpc, transaction, nav, notify); +const payment = new PaymentAction(store, grpc, nav, notify); sinon.stub(payment, 'checkType'); sinon.stub(payment, 'payBitcoin'); sinon.stub(payment, 'payLightning'); diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 27a3d95bf..9529e846d 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -152,7 +152,7 @@ describe('Action Integration Tests', function() { transactions1 = new TransactionAction(store1, grpc1, nav1); channels1 = new ChannelAction(store1, grpc1, nav1, notify1); invoice1 = new InvoiceAction(store1, grpc1, nav1, notify1); - payments1 = new PaymentAction(store1, grpc1, transactions1, nav1, notify1); + payments1 = new PaymentAction(store1, grpc1, nav1, notify1); db2 = sinon.createStubInstance(AppStorage); nav2 = sinon.createStubInstance(NavAction); @@ -164,7 +164,7 @@ describe('Action Integration Tests', function() { transactions2 = new TransactionAction(store2, grpc2, nav2); channels2 = new ChannelAction(store2, grpc2, nav2, notify2); invoice2 = new InvoiceAction(store2, grpc2, nav2, notify2); - payments2 = new PaymentAction(store2, grpc2, transactions2, nav2, notify2); + payments2 = new PaymentAction(store2, grpc2, nav2, notify2); }); after(async () => { diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index bdd81f4d6..3631b8fd1 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -3,7 +3,6 @@ import { Store } from '../../../src/store'; import IpcAction from '../../../src/action/ipc'; import GrpcAction from '../../../src/action/grpc'; import PaymentAction from '../../../src/action/payment'; -import TransactionAction from '../../../src/action/transaction'; import NotificationAction from '../../../src/action/notification'; import NavAction from '../../../src/action/nav'; import * as logger from '../../../src/action/log'; @@ -13,7 +12,6 @@ describe('Action Payments Unit Tests', () => { let store; let sandbox; let grpc; - let transaction; let payment; let nav; let notification; @@ -27,8 +25,7 @@ describe('Action Payments Unit Tests', () => { grpc = sinon.createStubInstance(GrpcAction); notification = sinon.createStubInstance(NotificationAction); nav = sinon.createStubInstance(NavAction); - transaction = sinon.createStubInstance(TransactionAction); - payment = new PaymentAction(store, grpc, transaction, nav, notification); + payment = new PaymentAction(store, grpc, nav, notification); }); afterEach(() => { @@ -199,14 +196,12 @@ describe('Action Payments Unit Tests', () => { }); expect(nav.goPayBitcoinDone, 'was called once'); expect(notification.display, 'was not called'); - expect(transaction.update, 'was called once'); }); it('should display notification on error', async () => { grpc.sendCommand.withArgs('sendCoins').rejects(); await payment.payBitcoin(); expect(notification.display, 'was called once'); - expect(transaction.update, 'was called once'); }); }); @@ -237,7 +232,6 @@ describe('Action Payments Unit Tests', () => { expect(nav.goWait, 'was called once'); expect(nav.goPayLightningDone, 'was called once'); expect(notification.display, 'was not called'); - expect(transaction.update, 'was called once'); }); it('should display notification on error', async () => { @@ -245,7 +239,6 @@ describe('Action Payments Unit Tests', () => { await payment.payLightning({ invoice: 'some-payment' }); expect(nav.goPayLightningConfirm, 'was called once'); expect(notification.display, 'was called once'); - expect(transaction.update, 'was called once'); }); }); }); From 2745f363d1ec369989f30bc8dedde14b814f0073 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:16:14 +0200 Subject: [PATCH 09/15] Use new polling util in integration tests --- .../action/action-integration.spec.js | 16 ++++++++-------- test/integration/action/test-util.js | 12 ------------ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 9529e846d..198d00ae7 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -1,4 +1,4 @@ -import { rmdir, poll, isPortOpen } from './test-util'; +import { rmdir, isPortOpen } from './test-util'; import { Store } from '../../../src/store'; import IpcAction from '../../../src/action/ipc'; import GrpcAction from '../../../src/action/grpc'; @@ -12,7 +12,7 @@ import ChannelAction from '../../../src/action/channel'; import TransactionAction from '../../../src/action/transaction'; import PaymentAction from '../../../src/action/payment'; import InvoiceAction from '../../../src/action/invoice'; -import { nap } from '../../../src/helper'; +import { nap, retry } from '../../../src/helper'; import { EventEmitter } from 'events'; const { @@ -108,7 +108,7 @@ describe('Action Integration Tests', function() { }; btcdProcess = await startBtcdProcess(btcdArgs); await nap(NAP_TIME); - await poll(() => isPortOpen(BTCD_PORT)); + await retry(() => isPortOpen(BTCD_PORT)); const lndProcess1Promise = startLndProcess({ isDev, lndSettingsDir: LND_SETTINGS_DIR_1, @@ -227,12 +227,12 @@ describe('Action Integration Tests', function() { btcdArgs.miningAddress = store1.walletAddress; btcdProcess = await startBtcdProcess(btcdArgs); await nap(NAP_TIME); - await poll(() => isPortOpen(BTCD_PORT)); + await retry(() => isPortOpen(BTCD_PORT)); await mineAndSync({ blocks: 400 }); }); it('should get public key node1', async () => { - await info1.getInfo(); + await info1.pollInfo(); expect(store1.pubKey, 'to be ok'); }); @@ -266,7 +266,7 @@ describe('Action Integration Tests', function() { }); it('should get public key node2', async () => { - await info2.getInfo(); + await info2.pollInfo(); expect(store2.pubKey, 'to be ok'); }); @@ -443,8 +443,8 @@ describe('Action Integration Tests', function() { const mineAndSync = async ({ blocks }) => { await mineBlocks({ blocks, logger }); - await info1.getInfo(); - await info2.getInfo(); + await info1.pollInfo(); + await info2.pollInfo(); }; const updateBalances = async () => { diff --git a/test/integration/action/test-util.js b/test/integration/action/test-util.js index e69eec1d9..94c84a6e4 100644 --- a/test/integration/action/test-util.js +++ b/test/integration/action/test-util.js @@ -1,6 +1,5 @@ import fs from 'fs'; import net from 'net'; -import { nap } from '../../../src/helper'; export const rmdir = path => { if (fs.existsSync(path)) { @@ -16,17 +15,6 @@ export const rmdir = path => { } }; -export const poll = async (api, interval = 100, retries = 1000) => { - while (retries--) { - try { - return await api(); - } catch (err) { - if (!retries) throw err; - } - await nap(interval); - } -}; - export const isPortOpen = async port => { await new Promise((resolve, reject) => { const client = new net.Socket(); From 86ebebd00c738323bf39619d6769c88e7d81ba25 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:20:11 +0200 Subject: [PATCH 10/15] Use new polling apis in action index --- src/action/index.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/action/index.js b/src/action/index.js index da2e7a0b9..861c30b9d 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -87,11 +87,13 @@ observe(store, 'walletUnlocked', async () => { * to and from lnd can be done. The display the current state of the * lnd node all balances, channels and transactions are fetched. */ -observe(store, 'lndReady', () => { - info.getInfo(); - wallet.update(); +observe(store, 'lndReady', async () => { channel.update(); transaction.update(); transaction.subscribeTransactions(); transaction.subscribeInvoices(); + wallet.pollBalances(); + wallet.pollExchangeRate(); + await info.pollInfo(); + wallet.getNewAddress(); // wait until neutrino is synced due to deadlock bug }); From b3a3263268cdccf11cc85f3ac72a6ab654178f86 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 12:21:18 +0200 Subject: [PATCH 11/15] Reduce default polling rate to 1 second --- src/config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.js b/src/config.js index 36a6e3409..f80868f5c 100644 --- a/src/config.js +++ b/src/config.js @@ -2,7 +2,7 @@ * @fileOverview this file is used to hardcode default settings for the app. */ -module.exports.RETRY_DELAY = 3000; +module.exports.RETRY_DELAY = 1000; module.exports.LND_INIT_DELAY = 5000; module.exports.NOTIFICATION_DELAY = 5000; module.exports.RATE_DELAY = 15 * 60 * 1000; From c27c37ec7ca86d1fda2a79ac611cce0f2ada32f6 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 13:38:45 +0200 Subject: [PATCH 12/15] Fix workaround to lnd deadlock bug --- src/action/index.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/action/index.js b/src/action/index.js index 861c30b9d..50179225c 100644 --- a/src/action/index.js +++ b/src/action/index.js @@ -88,12 +88,21 @@ observe(store, 'walletUnlocked', async () => { * lnd node all balances, channels and transactions are fetched. */ observe(store, 'lndReady', async () => { + // TODO: this is a workaround the deadlock bug in lnd that blocks + // calling NewAddress while netrino is syncing. + if (store.firstStart) { + // only fetch address before neutrino sync on first start + wallet.getNewAddress(); + } + wallet.pollBalances(); + wallet.pollExchangeRate(); channel.update(); transaction.update(); transaction.subscribeTransactions(); transaction.subscribeInvoices(); - wallet.pollBalances(); - wallet.pollExchangeRate(); await info.pollInfo(); - wallet.getNewAddress(); // wait until neutrino is synced due to deadlock bug + if (!store.firstStart) { + // wait until neutrino is synced on second start + wallet.getNewAddress(); + } }); From a924082a4340c4de8358d74fa9147bec98ddc6c1 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 13:39:03 +0200 Subject: [PATCH 13/15] Cleanup wallet action --- src/action/wallet.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/action/wallet.js b/src/action/wallet.js index bbf25b767..2a0d0d2f0 100644 --- a/src/action/wallet.js +++ b/src/action/wallet.js @@ -3,7 +3,7 @@ * call the corresponding GRPC apis for updating wallet balances. */ -import { observe, when } from 'mobx'; +import { observe } from 'mobx'; import { toBuffer, parseSat, checkHttpStatus, nap, poll } from '../helper'; import { MIN_PASSWORD_LENGTH, NOTIFICATION_DELAY, RATE_DELAY } from '../config'; import * as log from './log'; @@ -277,10 +277,7 @@ class WalletAction { this._nav.goNewAddress(); } else { this._nav.goWait(); - when( - () => typeof this._store.walletAddress === 'string', - () => this._nav.goNewAddress() - ); + observe(this._store, 'walletAddress', () => this._nav.goNewAddress()); } } From 228ad5689c11ea47b40abe609b07a157055951b9 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 28 Aug 2018 13:45:31 +0200 Subject: [PATCH 14/15] Fix helper docs --- src/helper.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/helper.js b/src/helper.js index f353f230a..ae55d098c 100644 --- a/src/helper.js +++ b/src/helper.js @@ -276,6 +276,7 @@ export const poll = async (api, interval = RETRY_DELAY, retries = Infinity) => { * utility will resolve with the return value if the api resolves. Errors * thrown by the api are swallowed by this utility and another retry is triggered. * @param {Function} api The api wrapped in an asynchronous function + * @param {number} interval The time interval to wait between retries * @param {number} retries The number of retries to be sent to the api * @return {Promise} The return value of the api */ From 5b04c6e8f471717a9e4cd94069758d15eb5a9533 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Wed, 29 Aug 2018 09:26:19 +0200 Subject: [PATCH 15/15] Revert "Cleanup wallet action" This reverts commit 9ba05bdabcc33dc768dad5caad23669364da3fb5. --- src/action/wallet.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/action/wallet.js b/src/action/wallet.js index 2a0d0d2f0..bbf25b767 100644 --- a/src/action/wallet.js +++ b/src/action/wallet.js @@ -3,7 +3,7 @@ * call the corresponding GRPC apis for updating wallet balances. */ -import { observe } from 'mobx'; +import { observe, when } from 'mobx'; import { toBuffer, parseSat, checkHttpStatus, nap, poll } from '../helper'; import { MIN_PASSWORD_LENGTH, NOTIFICATION_DELAY, RATE_DELAY } from '../config'; import * as log from './log'; @@ -277,7 +277,10 @@ class WalletAction { this._nav.goNewAddress(); } else { this._nav.goWait(); - observe(this._store, 'walletAddress', () => this._nav.goNewAddress()); + when( + () => typeof this._store.walletAddress === 'string', + () => this._nav.goNewAddress() + ); } }