From 11c48c25beee9f7a916f265492541a0de93027bb Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 6 Jul 2018 18:25:29 -0700 Subject: [PATCH 1/9] Display tx fee on lightning pay confirm view --- src/action/payment.js | 19 +++++++++++++++++++ .../action/action-integration.spec.js | 3 ++- test/unit/action/payment.spec.js | 5 +++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/action/payment.js b/src/action/payment.js index 5cd27232a..aff0f48dd 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -68,8 +68,13 @@ class PaymentAction { const request = await this._grpc.sendCommand('decodePayReq', { pay_req: invoice.replace(PREFIX_URI, ''), }); + const fee = await this.estimateLightningFee({ + destination: request.destination, + satAmt: request.num_satoshis, + }); payment.amount = toAmount(parseSat(request.num_satoshis), settings.unit); payment.note = request.description; + payment.fee = toAmount(parseSat(fee), settings.unit); return true; } catch (err) { log.info(`Decoding payment request failed: ${err.message}`); @@ -114,6 +119,20 @@ class PaymentAction { } await this._transaction.update(); } + + async estimateLightningFee({ destination, satAmt }) { + try { + const { routes } = await this._grpc.sendCommand('queryRoutes', { + pub_key: destination, + amt: satAmt, + num_routes: 1, + }); + return routes[0].total_fees; + } catch (err) { + log.info(`Unable to retrieve fee estimate: ${err}`); + return 0; + } + } } export default PaymentAction; diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 29ebcd26f..0613b9a43 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -344,10 +344,11 @@ describe('Action Integration Tests', function() { expect(isValid, 'to be', false); }); - it('should decode invoice and return true', async () => { + it('should decode invoice, set fee and return true', async () => { const isValid = await payments1.decodeInvoice({ invoice: store2.invoice.uri, }); + expect(parseInt(store1.payment.fee), 'to be greater than or equal to', 0); expect(isValid, 'to be', true); }); diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index b94209c00..71b106019 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -139,11 +139,16 @@ describe('Action Payments Unit Tests', () => { grpc.sendCommand.withArgs('decodePayReq').resolves({ num_satoshis: '1700', description: 'foo', + destination: 'bar', + }); + grpc.sendCommand.withArgs('queryRoutes').resolves({ + routes: [{ total_fees: '100' }], }); const isValid = await payment.decodeInvoice({ invoice: 'some-invoice' }); expect(isValid, 'to be', true); expect(store.payment.amount, 'to match', /^0[,.]0{4}1{1}7{1}$/); expect(store.payment.note, 'to be', 'foo'); + expect(store.payment.fee, 'to match', /^0[,.]0{5}1{1}$/); }); it('should set response to null on error', async () => { From 860d6968a631e740f63eb6ffeea6516868f90265 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 16 Jul 2018 17:08:38 -0700 Subject: [PATCH 2/9] Properly log fee retrieval error --- src/action/payment.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/action/payment.js b/src/action/payment.js index aff0f48dd..69dd08dc2 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -129,7 +129,7 @@ class PaymentAction { }); return routes[0].total_fees; } catch (err) { - log.info(`Unable to retrieve fee estimate: ${err}`); + log.info(`Unable to retrieve fee estimate: ${JSON.stringify(err)}`); return 0; } } From 28c2a7c9b85227c1ae8fd327647b8f49b17d68e9 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 16 Jul 2018 22:56:34 -0700 Subject: [PATCH 3/9] Handle case where queryRoutes takes > 0.5s + fix integ test --- src/action/payment.js | 5 ++++- src/config.js | 1 + test/integration/action/action-integration.spec.js | 6 +++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 69dd08dc2..98eb2ae42 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -1,4 +1,4 @@ -import { PREFIX_URI } from '../config'; +import { PREFIX_URI, WAIT_DELAY } from '../config'; import { toSatoshis, toAmount, @@ -64,6 +64,7 @@ class PaymentAction { async decodeInvoice({ invoice }) { try { + const timer = setTimeout(() => this._nav.goWait(), WAIT_DELAY); const { payment, settings } = this._store; const request = await this._grpc.sendCommand('decodePayReq', { pay_req: invoice.replace(PREFIX_URI, ''), @@ -75,9 +76,11 @@ class PaymentAction { payment.amount = toAmount(parseSat(request.num_satoshis), settings.unit); payment.note = request.description; payment.fee = toAmount(parseSat(fee), settings.unit); + clearTimeout(timer); return true; } catch (err) { log.info(`Decoding payment request failed: ${err.message}`); + this._nav.goPay(); return false; } } diff --git a/src/config.js b/src/config.js index 41419ee31..1046e2524 100644 --- a/src/config.js +++ b/src/config.js @@ -1,6 +1,7 @@ module.exports.RETRY_DELAY = 3000; module.exports.LND_INIT_DELAY = 5000; module.exports.NOTIFICATION_DELAY = 5000; +module.exports.WAIT_DELAY = 500; module.exports.LND_PORT = 10009; module.exports.LND_PEER_PORT = 10019; diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 0613b9a43..a1af07132 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -348,7 +348,11 @@ describe('Action Integration Tests', function() { const isValid = await payments1.decodeInvoice({ invoice: store2.invoice.uri, }); - expect(parseInt(store1.payment.fee), 'to be greater than or equal to', 0); + expect( + parseFloat(store1.payment.fee), + 'to be greater than or equal to', + 0 + ); expect(isValid, 'to be', true); }); From 969c4b8260f85024c78e6fd912e829255f2250c1 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:02:50 +0200 Subject: [PATCH 4/9] Asynchronously estimate fee --- src/action/payment.js | 19 ++++++++----------- src/config.js | 1 - 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 98eb2ae42..81d469b91 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -1,4 +1,4 @@ -import { PREFIX_URI, WAIT_DELAY } from '../config'; +import { PREFIX_URI } from '../config'; import { toSatoshis, toAmount, @@ -64,23 +64,20 @@ class PaymentAction { async decodeInvoice({ invoice }) { try { - const timer = setTimeout(() => this._nav.goWait(), WAIT_DELAY); const { payment, settings } = this._store; const request = await this._grpc.sendCommand('decodePayReq', { pay_req: invoice.replace(PREFIX_URI, ''), }); - const fee = await this.estimateLightningFee({ + payment.amount = toAmount(parseSat(request.num_satoshis), settings.unit); + payment.note = request.description; + // asynchronously estimate fee to not block the UI + this.estimateLightningFee({ destination: request.destination, satAmt: request.num_satoshis, }); - payment.amount = toAmount(parseSat(request.num_satoshis), settings.unit); - payment.note = request.description; - payment.fee = toAmount(parseSat(fee), settings.unit); - clearTimeout(timer); return true; } catch (err) { log.info(`Decoding payment request failed: ${err.message}`); - this._nav.goPay(); return false; } } @@ -125,15 +122,15 @@ class PaymentAction { async estimateLightningFee({ destination, satAmt }) { try { + const { payment, settings } = this._store; const { routes } = await this._grpc.sendCommand('queryRoutes', { pub_key: destination, amt: satAmt, num_routes: 1, }); - return routes[0].total_fees; + payment.fee = toAmount(parseSat(routes[0].total_fees), settings.unit); } catch (err) { - log.info(`Unable to retrieve fee estimate: ${JSON.stringify(err)}`); - return 0; + log.error(`Estimating lightning fee failed!`, err); } } } diff --git a/src/config.js b/src/config.js index 1046e2524..41419ee31 100644 --- a/src/config.js +++ b/src/config.js @@ -1,7 +1,6 @@ module.exports.RETRY_DELAY = 3000; module.exports.LND_INIT_DELAY = 5000; module.exports.NOTIFICATION_DELAY = 5000; -module.exports.WAIT_DELAY = 500; module.exports.LND_PORT = 10009; module.exports.LND_PEER_PORT = 10019; From c5b78d2fb5b0da4227546e413123ad1979aef440 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:06:42 +0200 Subject: [PATCH 5/9] Fix integration test --- test/integration/action/action-integration.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index a1af07132..9cdf013d1 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -348,12 +348,13 @@ describe('Action Integration Tests', function() { const isValid = await payments1.decodeInvoice({ invoice: store2.invoice.uri, }); + expect(isValid, 'to be', true); + while (!store1.payment.fee) await nap(100); expect( parseFloat(store1.payment.fee), 'to be greater than or equal to', 0 ); - expect(isValid, 'to be', true); }); it('should send lightning payment from request', async () => { From e06a94610caad674ab38e94b7e570d81bf68a490 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:31:20 +0200 Subject: [PATCH 6/9] =?UTF-8?q?Don=E2=80=99t=20do=20call=20async=20since?= =?UTF-8?q?=20it=E2=80=99s=20pretty=20fast?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/action/payment.js | 3 +-- test/integration/action/action-integration.spec.js | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 81d469b91..52919f253 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -70,8 +70,7 @@ class PaymentAction { }); payment.amount = toAmount(parseSat(request.num_satoshis), settings.unit); payment.note = request.description; - // asynchronously estimate fee to not block the UI - this.estimateLightningFee({ + await this.estimateLightningFee({ destination: request.destination, satAmt: request.num_satoshis, }); diff --git a/test/integration/action/action-integration.spec.js b/test/integration/action/action-integration.spec.js index 9cdf013d1..5a04ad794 100644 --- a/test/integration/action/action-integration.spec.js +++ b/test/integration/action/action-integration.spec.js @@ -349,7 +349,6 @@ describe('Action Integration Tests', function() { invoice: store2.invoice.uri, }); expect(isValid, 'to be', true); - while (!store1.payment.fee) await nap(100); expect( parseFloat(store1.payment.fee), 'to be greater than or equal to', From 1904c61c07416df1eb402c68f3d8281f36613eb9 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:37:44 +0200 Subject: [PATCH 7/9] Move function up --- src/action/payment.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/action/payment.js b/src/action/payment.js index 52919f253..fd97ee8b2 100644 --- a/src/action/payment.js +++ b/src/action/payment.js @@ -81,6 +81,20 @@ class PaymentAction { } } + async estimateLightningFee({ destination, satAmt }) { + try { + const { payment, settings } = this._store; + const { routes } = await this._grpc.sendCommand('queryRoutes', { + pub_key: destination, + amt: satAmt, + num_routes: 1, + }); + payment.fee = toAmount(parseSat(routes[0].total_fees), settings.unit); + } catch (err) { + log.error(`Estimating lightning fee failed!`, err); + } + } + async payBitcoin() { try { const { payment, settings } = this._store; @@ -118,20 +132,6 @@ class PaymentAction { } await this._transaction.update(); } - - async estimateLightningFee({ destination, satAmt }) { - try { - const { payment, settings } = this._store; - const { routes } = await this._grpc.sendCommand('queryRoutes', { - pub_key: destination, - amt: satAmt, - num_routes: 1, - }); - payment.fee = toAmount(parseSat(routes[0].total_fees), settings.unit); - } catch (err) { - log.error(`Estimating lightning fee failed!`, err); - } - } } export default PaymentAction; From 61d68014ff359082eedf3f4d3a8c61303f89b764 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:43:06 +0200 Subject: [PATCH 8/9] Add unit test in case of fee estimate error --- test/unit/action/payment.spec.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index 71b106019..9798171ae 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -151,14 +151,30 @@ describe('Action Payments Unit Tests', () => { expect(store.payment.fee, 'to match', /^0[,.]0{5}1{1}$/); }); - it('should set response to null on error', async () => { + it('should set nothing on decode error', async () => { grpc.sendCommand.withArgs('decodePayReq').rejects(new Error('Boom!')); const isValid = await payment.decodeInvoice({ invoice: 'some-invoice' }); expect(isValid, 'to be', false); expect(store.payment.amount, 'to be', ''); expect(store.payment.note, 'to be', ''); + expect(store.payment.fee, 'to be', ''); expect(logger.info, 'was called once'); }); + + it('should set no fee on query route error', async () => { + grpc.sendCommand.withArgs('decodePayReq').resolves({ + num_satoshis: '1700', + description: 'foo', + destination: 'bar', + }); + grpc.sendCommand.withArgs('queryRoutes').rejects(new Error('Boom!')); + const isValid = await payment.decodeInvoice({ invoice: 'some-invoice' }); + expect(isValid, 'to be', true); + expect(store.payment.amount, 'to match', /^0[,.]0{4}1{1}7{1}$/); + expect(store.payment.note, 'to be', 'foo'); + expect(store.payment.fee, 'to be', ''); + expect(logger.error, 'was called once'); + }); }); describe('payBitcoin()', () => { From bc014dda2ae5191da9a58b4ebe68d03a8fbe1576 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Tue, 24 Jul 2018 12:45:39 +0200 Subject: [PATCH 9/9] Add arg pass check to unit tests --- test/unit/action/payment.spec.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/unit/action/payment.spec.js b/test/unit/action/payment.spec.js index 9798171ae..4e682d79d 100644 --- a/test/unit/action/payment.spec.js +++ b/test/unit/action/payment.spec.js @@ -141,9 +141,15 @@ describe('Action Payments Unit Tests', () => { description: 'foo', destination: 'bar', }); - grpc.sendCommand.withArgs('queryRoutes').resolves({ - routes: [{ total_fees: '100' }], - }); + grpc.sendCommand + .withArgs('queryRoutes', { + pub_key: 'bar', + amt: '1700', + num_routes: 1, + }) + .resolves({ + routes: [{ total_fees: '100' }], + }); const isValid = await payment.decodeInvoice({ invoice: 'some-invoice' }); expect(isValid, 'to be', true); expect(store.payment.amount, 'to match', /^0[,.]0{4}1{1}7{1}$/);