Merge pull request #3145 from mrfelton/fix/payments-sending

Ensure failed payments marked as failed

renderer/components/Form/util.js

@@ -1,8 +1,8 @@
 /**
- * isFieldValid - Check wether a field is valid or not.
+ * isFieldValid - Check whether a field is valid or not.
  *
  * @param  {{value, error, asyncError, touched}} fieldState Informed field state.
- * @returns {boolean} Boolean indicating wether field is valie.
+ * @returns {boolean} Boolean indicating whether field is valie.
  */
 export const isFieldValid = ({ value, error, asyncError, touched }) => {
   return value && !error && !asyncError && touched

renderer/components/Pay/PaySummaryLightning.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import { Box, Flex } from 'rebass/styled-components'
 import { FormattedMessage } from 'react-intl'
 import { convert } from '@zap/utils/btc'
-import { decodePayReq, getNodeAlias } from '@zap/utils/crypto'
+import { decodePayReq, getNodeAlias, getTag } from '@zap/utils/crypto'
 import BigArrowRight from 'components/Icon/BigArrowRight'
 import { Bar, DataRow, Spinner, Text } from 'components/UI'
 import { CryptoSelector, CryptoValue, FiatValue } from 'containers/UI'
@@ -63,8 +63,7 @@ class PaySummaryLightning extends React.Component {
     }
 
     const { satoshis, millisatoshis, payeeNodeKey } = invoice
-    const descriptionTag = invoice.tags.find(tag => tag.tagName === 'description') || {}
-    const memo = descriptionTag.data
+    const memo = getTag(invoice, 'description')
     const amountInSatoshis = satoshis || convert('msats', 'sats', millisatoshis) || amount
 
     const nodeAlias = getNodeAlias(payeeNodeKey, nodes)

renderer/components/Request/RequestSummary.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import { Box, Flex } from 'rebass/styled-components'
 import { FormattedMessage, FormattedTime, injectIntl } from 'react-intl'
 import copy from 'copy-to-clipboard'
-import { decodePayReq } from '@zap/utils/crypto'
+import { decodePayReq, getTag } from '@zap/utils/crypto'
 import getUnixTime from '@zap/utils/time'
 import { Bar, DataRow, Button, QRCode, Text, Countdown } from 'components/UI'
 import { CryptoSelector, CryptoValue, FiatSelector, FiatValue } from 'containers/UI'
@@ -31,13 +31,12 @@ const RequestSummary = ({ invoice = {}, payReq, intl, showNotification, ...rest
     showNotification(notifBody)
   }
 
-  const { satoshis: invoiceAmount, tags, timestampString } = decodedInvoice
+  const { satoshis: invoiceAmount, timestampString } = decodedInvoice
   const satoshis = invoice.finalAmount || invoiceAmount || 0
-  const descriptionTag = tags.find(tag => tag.tagName === 'description') || {}
-  const memo = descriptionTag.data
+  const memo = getTag(decodedInvoice, 'description')
 
-  const fallbackTag = tags.find(tag => tag.tagName === 'fallback_address')
-  const fallback = fallbackTag && fallbackTag.data.address
+  const fallbackTag = getTag(decodedInvoice, 'fallback_address')
+  const fallback = fallbackTag && fallbackTag.address
 
   const getStatusColor = () => {
     if (invoice.settled) {

renderer/reducers/activity.js

@@ -84,17 +84,6 @@ const propMatches = function propMatches(prop) {
   return item[prop] && item[prop].toLowerCase().includes(searchTextSelector.toLowerCase())
 }
 
-/**
- * invoiceExpired - Check whether an invoice is expired.
- *
- * @param  {object}  invoice Invoice
- * @returns {boolean} Boolean indicating if the invoice has expired
- */
-const invoiceExpired = invoice => {
-  const expiresAt = parseInt(invoice.creation_date, 10) + parseInt(invoice.expiry, 10)
-  return expiresAt < Math.round(new Date() / 1000)
-}
-
 /**
  * returnTimestamp - Returns invoice, payment or transaction timestamp.
  *
@@ -568,7 +557,7 @@ const pendingActivityRaw = createSelector(
       ...paymentsSending,
       ...transactionsSending,
       ...transactions.filter(transaction => transaction.isPending),
-      ...invoices.filter(invoice => !invoice.settled && !invoiceExpired(invoice)),
+      ...invoices.filter(invoice => !invoice.settled && !invoice.isExpired),
     ].map(addDate)
   }
 )
@@ -577,7 +566,7 @@ const pendingActivityRaw = createSelector(
 const expiredActivityRaw = createSelector(
   invoicesSelector,
   invoices => {
-    return invoices.filter(invoice => !invoice.settled && invoiceExpired(invoice)).map(addDate)
+    return invoices.filter(invoice => !invoice.settled && invoice.isExpired).map(addDate)
   }
 )
 

renderer/reducers/autopay.js

@@ -2,6 +2,7 @@ import { createSelector } from 'reselect'
 import { isAutopayEnabled } from '@zap/utils/featureFlag'
 import { showSystemNotification } from '@zap/utils/notifications'
 import { getIntl } from '@zap/i18n'
+import { getTag } from '@zap/utils/crypto'
 import truncateNodePubkey from '@zap/utils/truncateNodePubkey'
 import { contactFormSelectors } from './contactsform'
 import { tickerSelectors } from './ticker'
@@ -232,7 +233,7 @@ function setAutopayListFromArray(state, { list }) {
 export const showAutopayNotification = invoice => async (dispatch, getState) => {
   const nodes = networkSelectors.nodes(getState())
 
-  const descriptionTag = invoice.tags.find(tag => tag.tagName === 'description') || {}
+  const memo = getTag(invoice, 'description')
   const node = nodes.find(n => n.pub_key === invoice.payeeNodeKey)
   const nodeName = node ? getNodeDisplayName(node) : truncateNodePubkey(invoice.payeeNodeKey)
 
@@ -247,9 +248,9 @@ export const showAutopayNotification = invoice => async (dispatch, getState) =>
   })
 
   let body = message
-  if (descriptionTag.data) {
+  if (memo) {
     const detail = intl.formatMessage(messages.autopay_notification_detail, {
-      reason: descriptionTag.data,
+      reason: memo,
     })
     body += ` ${detail}`
   }

renderer/reducers/invoice.js

@@ -65,6 +65,10 @@ const decorateInvoice = invoice => {
   // Add a `finalAmount` prop which shows the amount paid if set, or the invoice value if not.
   decoration.finalAmount = invoice.amt_paid_sat ? invoice.amt_paid_sat : invoice.value
 
+  // Add an `isExpired` prop which shows whether the invoice is expired or not.
+  const expiresAt = parseInt(invoice.creation_date, 10) + parseInt(invoice.expiry, 10)
+  decoration.isExpired = expiresAt < Math.round(new Date() / 1000)
+
   return {
     ...invoice,
     ...decoration,

renderer/reducers/payment.js

@@ -1,11 +1,13 @@
 import config from 'config'
 import { createSelector } from 'reselect'
 import uniqBy from 'lodash/uniqBy'
+import find from 'lodash/find'
 import errorToUserFriendly from '@zap/utils/userFriendlyErrors'
 import { getIntl } from '@zap/i18n'
-import { decodePayReq, getNodeAlias } from '@zap/utils/crypto'
+import { decodePayReq, getNodeAlias, getTag } from '@zap/utils/crypto'
 import { convert } from '@zap/utils/btc'
 import delay from '@zap/utils/delay'
+import genId from '@zap/utils/genId'
 import { grpc } from 'workers'
 import createReducer from './utils/createReducer'
 import { fetchBalance } from './balance'
@@ -91,18 +93,6 @@ const decoratePayment = (payment, nodes = []) => {
   }
 }
 
-/**
- * getLastSendingEntry - Find the latest temporary paymentsSending entry for the payment.
- *
- * @param  {object} state Redux state
- * @param  {string} paymentRequest Payment request
- * @returns {object} sendingPayments entry
- */
-const getLastSendingEntry = (state, paymentRequest) =>
-  [...state.payment.paymentsSending]
-    .sort((a, b) => b.creation_date - a.creation_date)
-    .find(p => p.paymentRequest === paymentRequest)
-
 // ------------------------------------
 // Actions
 // ------------------------------------
@@ -126,9 +116,9 @@ export function getPayments() {
  */
 export const sendPayment = data => dispatch => {
   const invoice = decodePayReq(data.paymentRequest)
-  const paymentHashTag = invoice.tags ? invoice.tags.find(t => t.tagName === 'payment_hash') : null
+  const paymentHash = getTag(invoice, 'payment_hash')
 
-  if (!paymentHashTag || !paymentHashTag.data) {
+  if (!paymentHash) {
     dispatch(showError(getIntl().formatMessage(messages.payment_send_error)))
     return
   }
@@ -138,7 +128,7 @@ export const sendPayment = data => dispatch => {
     status: PAYMENT_STATUS_SENDING,
     isSending: true,
     creation_date: Math.round(new Date() / 1000),
-    payment_hash: paymentHashTag.data,
+    payment_hash: paymentHash,
   }
 
   dispatch({
@@ -171,12 +161,12 @@ export const receivePayments = payments => dispatch => {
 /**
  * decPaymentRetry - Decrement payment request retry count.
  *
- * @param {object} paymentRequest Lightning payment request.
- * @returns {Function} Thunk
+ * @param {string} paymentId Internal id of payment whose retry count to decrease
+ * @returns {object} Action
  */
-const decPaymentRetry = paymentRequest => ({
+const decPaymentRetry = paymentId => ({
   type: DECREASE_PAYMENT_RETRIES,
-  paymentRequest,
+  paymentId,
 })
 
 /**
@@ -187,25 +177,32 @@ const decPaymentRetry = paymentRequest => ({
  * @param {string} options.payReq Payment request
  * @param {number} options.amt Payment amount (in sats)
  * @param {number} options.feeLimit The max fee to apply
- * @param {boolean} options.isRetry Boolean indicating whether this is a retry attempt
  * @param {number} options.retries Number of remaining retries
+ * @param {string} options.originalPaymentId Id of the original payment if (required if this is a payment retry)
  * @returns {Function} Thunk
  */
 export const payInvoice = ({
   payReq,
   amt,
   feeLimit,
-  isRetry = false,
   retries = 0,
+  originalPaymentId,
 }) => async dispatch => {
-  // if it's a retry - only decrease number of retries left
-  if (isRetry) {
-    dispatch(decPaymentRetry(payReq))
+  let paymentId = originalPaymentId
+
+  // If we already have an id then this is a retry. Decrease the retry count.
+  if (originalPaymentId) {
+    dispatch(decPaymentRetry(originalPaymentId))
   }
-  // Otherwise, add to sendingPayments in the state.
+
+  // Otherwise, add to paymentsSending in the state.
   else {
+    // Generate a unique id for the payment that we will use to track the payment across retry attempts.
+    paymentId = genId()
+
     dispatch(
       sendPayment({
+        paymentId,
         paymentRequest: payReq,
         feeLimit,
         value: amt,
@@ -218,18 +215,15 @@ export const payInvoice = ({
   // Submit the payment to LND.
   try {
     const data = await grpc.services.Lightning.sendPayment({
+      paymentId,
       payment_request: payReq,
       amt,
       fee_limit: { fixed: feeLimit },
     })
     dispatch(paymentSuccessful(data))
   } catch (e) {
-    dispatch(
-      paymentFailed({
-        error: e.message,
-        payment_request: e.payload.payment_request,
-      })
-    )
+    const { details: data, message: error } = e
+    dispatch(paymentFailed(error, data))
   }
 }
 
@@ -253,9 +247,9 @@ export const updatePayment = paymentRequest => async dispatch => {
  * @param {{payment_request}} payment_request Payment request
  * @returns {Function} Thunk
  */
-export const paymentSuccessful = ({ payment_request }) => async (dispatch, getState) => {
-  const state = getState()
-  const paymentSending = getLastSendingEntry(state, payment_request)
+export const paymentSuccessful = ({ paymentId }) => async (dispatch, getState) => {
+  const paymentSending = find(paymentsSendingSelector(getState()), { paymentId })
+
   // If we found a related entry in paymentsSending, gracefully remove it and handle as success case.
   if (paymentSending) {
     const { creation_date, paymentRequest } = paymentSending
@@ -264,7 +258,7 @@ export const paymentSuccessful = ({ payment_request }) => async (dispatch, getSt
     await delay(2000 - (Date.now() - creation_date * 1000))
 
     // Mark the payment as successful.
-    dispatch({ type: PAYMENT_SUCCESSFUL, paymentRequest })
+    dispatch({ type: PAYMENT_SUCCESSFUL, paymentId })
 
     // Wait for another second.
     await delay(1500)
@@ -282,14 +276,14 @@ export const paymentSuccessful = ({ payment_request }) => async (dispatch, getSt
 /**
  * paymentFailed - Error handler for payInvoice.
  *
- * @param {object} details Details
- * @param {string} details.payment_request Payment request
- * @param {string} details.error Error message
+ * @param {Error} error Error
+ * @param {object} details Failed payment details
+ *
  * @returns {Function} Thunk
  */
-export const paymentFailed = ({ payment_request, error }) => async (dispatch, getState) => {
-  const state = getState()
-  const paymentSending = getLastSendingEntry(state, payment_request)
+export const paymentFailed = (error, { paymentId }) => async (dispatch, getState) => {
+  const paymentSending = find(paymentsSendingSelector(getState()), { paymentId })
+
   // errors that trigger retry mechanism
   const RETRIABLE_ERRORS = [
     'payment attempt not completed before timeout', // ErrPaymentAttemptTimeout
@@ -305,7 +299,7 @@ export const paymentFailed = ({ payment_request, error }) => async (dispatch, ge
       const data = {
         ...paymentSending,
         payReq: paymentRequest,
-        isRetry: true,
+        originalPaymentId: paymentId,
       }
       const retryIndex = maxRetries - remainingRetries + 1
       // add increasing delay
@@ -316,7 +310,7 @@ export const paymentFailed = ({ payment_request, error }) => async (dispatch, ge
       await delay(2000 - (Date.now() - creation_date * 1000))
 
       // Mark the payment as failed.
-      dispatch({ type: PAYMENT_FAILED, paymentRequest, error: errorToUserFriendly(error) })
+      dispatch({ type: PAYMENT_FAILED, paymentId, error: errorToUserFriendly(error) })
     }
   }
 }
@@ -340,28 +334,26 @@ const ACTION_HANDLERS = {
     state.paymentsSending.push(payment)
   },
 
-  [DECREASE_PAYMENT_RETRIES]: (state, { paymentRequest }) => {
+  [DECREASE_PAYMENT_RETRIES]: (state, { paymentId }) => {
     const { paymentsSending } = state
-    const item = paymentsSending.find(
-      i => i.paymentRequest === paymentRequest && i.status === PAYMENT_STATUS_SENDING
-    )
+    const item = find(paymentsSending, { paymentId })
     if (item) {
-      item.remainingRetries -= 1
+      item.remainingRetries = Math.max(item.remainingRetries - 1, 0)
       if (item.feeLimit) {
         item.feeLimit = Math.ceil(item.feeLimit * config.invoices.feeIncrementExponent)
       }
     }
   },
-  [PAYMENT_SUCCESSFUL]: (state, { paymentRequest }) => {
+  [PAYMENT_SUCCESSFUL]: (state, { paymentId }) => {
     const { paymentsSending } = state
-    const item = paymentsSending.find(i => i.paymentRequest === paymentRequest)
+    const item = find(paymentsSending, { paymentId })
     if (item) {
       item.status = PAYMENT_STATUS_SUCCESSFUL
     }
   },
-  [PAYMENT_FAILED]: (state, { paymentRequest, error }) => {
+  [PAYMENT_FAILED]: (state, { paymentId, error }) => {
     const { paymentsSending } = state
-    const item = paymentsSending.find(i => i.paymentRequest === paymentRequest)
+    const item = find(paymentsSending, { paymentId })
     if (item) {
       item.status = PAYMENT_STATUS_FAILED
       item.error = error