Fabo/better handling of sequence in ActionModal

changes/fabo_fix-sequence-issue

@@ -0,0 +1,2 @@
+[Fixed] [#3364](https://github.com/cosmos/lunie/pull/3364) Improve handling of sequence querying in ActionModal @faboweb
+[Added] [#3364](https://github.com/cosmos/lunie/pull/3364) Track failing transactions clientside in Sentry @faboweb

package.json

@@ -60,6 +60,7 @@
     "moment": "^2.24.0",
     "no-scroll": "^2.1.1",
     "register-service-worker": "^1.6.2",
+    "serialize-javascript": ">=2.1.1",
     "subscriptions-transport-ws": "0.9.16",
     "timezone-mock": "^1.0.8",
     "vue": "^2.6.10",
@@ -130,4 +131,4 @@
     "handlebars": ">=4.5.3",
     "serialize-javascript": ">=2.1.1"
   }
-}
+}

src/ActionModal/components/ActionModal.vue

@@ -17,9 +17,9 @@
         <i class="material-icons">close</i>
       </div>
       <div class="action-modal-header">
-        <span class="action-modal-title">{{
-          requiresSignIn ? `Sign in required` : title
-        }}</span>
+        <span class="action-modal-title">
+          {{ requiresSignIn ? `Sign in required` : title }}
+        </span>
         <Steps
           v-if="
             [defaultStep, feeStep, signStep].includes(step) &&
@@ -63,9 +63,9 @@
             field-id="gasPrice"
             field-label="Gas Price"
           >
-            <span class="input-suffix">{{
-              network.stakingDenom | viewDenom
-            }}</span>
+            <span class="input-suffix">
+              {{ network.stakingDenom | viewDenom }}
+            </span>
             <TmField
               id="gas-price"
               v-model="gasPrice"
@@ -106,8 +106,12 @@
           />
         </div>
         <div v-else-if="step === signStep" class="action-modal-form">
+          <!-- fallback to prevent the user to submit a tx before the sequence was loaded -->
+          <TmDataLoading
+            v-if="!context.account || context.account.sequence === undefined"
+          />
           <TmFormGroup
-            v-if="signMethods.length > 1"
+            v-else-if="signMethods.length > 1"
             class="action-modal-form-group"
             field-id="sign-method"
             field-label="Signing Method"
@@ -120,7 +124,7 @@
             />
           </TmFormGroup>
           <HardwareState
-            v-if="selectedSignMethod === SIGN_METHODS.LEDGER"
+            v-else-if="selectedSignMethod === SIGN_METHODS.LEDGER"
             :icon="session.browserWithLedgerSupport ? 'usb' : 'info'"
             :loading="!!sending"
           >
@@ -138,7 +142,7 @@
             </div>
           </HardwareState>
           <HardwareState
-            v-if="selectedSignMethod === SIGN_METHODS.EXTENSION"
+            v-else-if="selectedSignMethod === SIGN_METHODS.EXTENSION"
             :icon="session.browserWithLedgerSupport ? 'laptop' : 'info'"
             :loading="!!sending"
           >
@@ -187,9 +191,7 @@
         </div>
         <div v-else-if="step === inclusionStep" class="action-modal-form">
           <TmDataMsg icon="hourglass_empty" :spin="true">
-            <div slot="title">
-              Sent and confirming
-            </div>
+            <div slot="title">Sent and confirming</div>
             <div slot="subtitle">
               Waiting for confirmation from {{ networkId }}.
             </div>
@@ -200,9 +202,7 @@
           class="action-modal-form success-step"
         >
           <TmDataMsg icon="check" :success="true">
-            <div slot="title">
-              {{ notifyMessage.title }}
-            </div>
+            <div slot="title">{{ notifyMessage.title }}</div>
             <div slot="subtitle">
               {{ notifyMessage.body }}
               <br />
@@ -284,6 +284,7 @@ import TmBtn from "src/components/common/TmBtn"
 import TmField from "src/components/common/TmField"
 import TmFormGroup from "src/components/common/TmFormGroup"
 import TmFormMsg from "src/components/common/TmFormMsg"
+import TmDataLoading from "src/components/common/TmDataLoading"
 import FeatureNotAvailable from "src/components/common/FeatureNotAvailable"
 import TmDataMsg from "common/TmDataMsg"
 import TableInvoice from "./TableInvoice"
@@ -294,6 +295,7 @@ import { between, requiredIf } from "vuelidate/lib/validators"
 import { track } from "scripts/google-analytics"
 import { UserTransactionAdded } from "src/gql"
 import config from "src/../config"
+import * as Sentry from "@sentry/browser"
 
 import ActionManager from "../utils/ActionManager"
 
@@ -340,6 +342,7 @@ export default {
     TmFormGroup,
     TmFormMsg,
     TmDataMsg,
+    TmDataLoading,
     TableInvoice,
     Steps,
     FeatureNotAvailable
@@ -469,6 +472,22 @@ export default {
     },
     prettyIncludedHeight() {
       return prettyInt(this.includedHeight)
+    },
+    // TODO lets slice this monstrocity
+    context() {
+      return {
+        url: this.network.api_url,
+        networkId: this.network.id,
+        chainId: this.network.chain_id,
+        connected: this.connected,
+        userAddress: this.session.address,
+        rewards: this.rewards,
+        totalRewards: this.overview.totalRewards,
+        delegations: this.delegations,
+        bondDenom: this.network.stakingDenom,
+        isExtensionAccount: this.isExtensionAccount,
+        account: this.overview.accountInformation
+      }
     }
   },
   watch: {
@@ -480,15 +499,18 @@ export default {
           this.selectedSignMethod = signMethods[0].value
         }
       }
+    },
+    context: {
+      immediate: true,
+      handler(context) {
+        this.actionManager.setContext(context)
+      }
     }
   },
   created() {
     this.$apollo.skipAll = true
   },
   updated: function() {
-    // TODO do we need to set the context on every update of the component?
-    const context = this.createContext()
-    this.actionManager.setContext(context)
     if (
       (this.title === "Withdraw" || this.step === "fees") &&
       this.$refs.next
@@ -497,21 +519,6 @@ export default {
     }
   },
   methods: {
-    createContext() {
-      return {
-        url: this.network.api_url, // state.connection.externals.node.url,
-        networkId: this.network.id, // state.connection.lastHeader.chain_id,
-        chainId: this.network.chain_id, // state.connection.lastHeader.chain_id,
-        connected: this.connected,
-        userAddress: this.session.address,
-        rewards: this.rewards, // state.distribution.rewards,
-        totalRewards: this.overview.totalRewards, // getters.totalRewards,
-        delegations: this.delegations, // state.delegates.delegates,
-        bondDenom: this.network.stakingDenom, // getters.bondDenom,
-        isExtensionAccount: this.isExtensionAccount,
-        account: this.overview.accountInformation
-      }
-    },
     confirmModalOpen() {
       let confirmResult = false
       if (this.session.currrentModalOpen) {
@@ -618,7 +625,7 @@ export default {
           this.gasEstimate = await this.actionManager.simulate(memo)
         } else {
           this.gasEstimate = await this.actionManager.simulateTxAPI(
-            this.createContext(),
+            this.context,
             type,
             properties,
             memo
@@ -656,8 +663,9 @@ export default {
         if (!this.useTxService) {
           hashResult = await this.actionManager.send(memo, feeProperties)
         } else {
+          await this.$apollo.queries.overview.refetch()
           hashResult = await this.actionManager.sendTxAPI(
-            this.createContext(),
+            this.context,
             type,
             memo,
             properties,
@@ -668,8 +676,8 @@ export default {
         const { hash } = hashResult
         this.txHash = hash
         this.step = inclusionStep
-      } catch ({ message }) {
-        this.onSendingFailed(message)
+      } catch (error) {
+        this.onSendingFailed(error)
       }
     },
     onTxIncluded() {
@@ -683,10 +691,17 @@ export default {
       this.$emit(`txIncluded`)
       this.$apollo.queries.overview.refetch()
     },
-    onSendingFailed(message) {
+    onSendingFailed(error) {
+      Sentry.withScope(scope => {
+        scope.setExtra("signMethod", this.selectedSignMethod)
+        scope.setExtra("transactionData", this.transactionData)
+        scope.setExtra("gasEstimate", this.gasEstimate)
+        scope.setExtra("gasPrice", this.gasPrice)
+        Sentry.captureException(error)
+      })
       this.step = signStep
-      this.submissionError = `${this.submissionErrorPrefix}: ${message}.`
-      this.trackEvent(`event`, `failed-submit`, this.title, message)
+      this.submissionError = `${this.submissionErrorPrefix}: ${error.message}.`
+      this.trackEvent(`event`, `failed-submit`, this.title, error.message)
       this.$apollo.queries.overview.refetch()
     },
     async connectLedger() {
@@ -842,7 +857,7 @@ export default {
               this.onTxIncluded()
             } else {
               /* istanbul ignore next */
-              this.onSendingFailed(log)
+              this.onSendingFailed(new Error(log))
             }
           }
           this.txHash = null

src/ActionModal/utils/ActionManager.js

@@ -200,7 +200,7 @@ export default class ActionManager {
     if (result.success) {
       return { hash: result.hash }
     } else {
-      throw Error("Broadcast was not successfull: " + result.error)
+      throw Error("Broadcast was not successful: " + result.error)
     }
   }
 

tests/unit/specs/components/ActionModal/components/ActionModal.spec.js

@@ -15,6 +15,11 @@ let mockSend = jest.fn(() => ({
 }))
 let mockSetContext = jest.fn()
 
+jest.mock("src/../config.js", () => ({
+  default_gas_price: 2.5e-8,
+  graphqlHost: "http://localhost:4000"
+}))
+
 jest.mock(`src/ActionModal/utils/ActionManager.js`, () => {
   return jest.fn(() => {
     return {
@@ -28,6 +33,11 @@ jest.mock(`src/ActionModal/utils/ActionManager.js`, () => {
   })
 })
 
+// TODO move into global mock to not duplicate everywhere
+jest.mock("@sentry/browser", () => ({
+  withScope: () => {}
+}))
+
 describe(`ActionModal`, () => {
   let wrapper, $store, $apollo
 
@@ -132,7 +142,12 @@ describe(`ActionModal`, () => {
       },
       stubs: ["router-link"]
     })
-    wrapper.setData({ network, overview })
+    const context = {
+      account: {
+        sequence: 0
+      }
+    }
+    wrapper.setData({ network, overview, context })
     wrapper.vm.open()
   })
 
@@ -167,10 +182,10 @@ describe(`ActionModal`, () => {
     }
     await ActionModal.methods.submit.call(self)
     expect(self.onSendingFailed).toHaveBeenCalledWith(
-      "some kind of error message"
+      new Error("some kind of error message")
     )
 
-    ActionModal.methods.onSendingFailed.call(self, "some kind of error message")
+    ActionModal.methods.onSendingFailed.call(self, new Error("some kind of error message"))
     expect(self.submissionError).toEqual(`PREFIX: some kind of error message.`)
   })
 
@@ -618,7 +633,12 @@ describe(`ActionModal`, () => {
         isValidInput: jest.fn(() => true),
         selectedSignMethod: `local`,
         step: `details`,
-        validateChangeStep: jest.fn(() => {})
+        validateChangeStep: jest.fn(() => {}),
+        context: {
+          account: {
+            sequence: 0
+          }
+        }
       }
     })
 
@@ -703,6 +723,15 @@ describe(`ActionModal`, () => {
         ).toBe(true)
         expect(wrapper.element).toMatchSnapshot()
       })
+
+      it("should display loading if the sequence is not available yet", () => {
+        wrapper.setData({
+          context: { account: { sequence: undefined } },
+          step: "sign"
+        })
+        expect(wrapper.find("tmdataloading-stub").exists()).toBe(true)
+        expect(wrapper.element).toMatchSnapshot()
+      })
     })
 
     describe(`invalid step`, () => {