Feat/track txs locally #1726

aulneau · 2021-09-19T21:43:24Z

Try out this version of the Hiro Wallet - download extension builds.

With the issues being reported via discord and all the extensive feedback in #1723, I wanted to create a proof of concept of how we could better track local transactions from this #1712 topic.

I've tested this and I feel as though it's a good combination between relying on what currently exists, with adding the minimal to be able to leverage locally persisted transactions.

Here is a video to show how it works. Very open to better suggestions around how to display them, what to label them (if we need any).

Kapture.2021-09-19.at.16.42.26.mp4

cc/ @aulneau @kyranjamie @fbwoolf

changeset-bot · 2021-09-19T21:43:26Z

🦋 Changeset detected

Latest commit: 54c2577

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@stacks/wallet-web	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2021-09-19T21:43:27Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blockstack/stacks-wallet-web/3VQrqWSHhanbv1vP6rCnZwegK6wk
✅ Preview: https://stacks-wallet-web-git-feat-track-txs-locally-blockstack.vercel.app

fbwoolf · 2021-09-20T00:41:06Z

I like the feedback this provides when it moves into pending quickly, but I'm curious how users will respond if they have backed up Submitted transactions for 24+ hrs ...this would hopefully prevent that by keeping track of the nonce locally? Also, just questioning what we do with available balance? We will have to implement this in the explorer too in some way ...the Pending list would have a Submitted state at top?

fbwoolf · 2021-09-20T01:40:31Z

We will have to implement this in the explorer too in some way ...the Pending list would have a Submitted state at top?

Just realized that makes absolutely no sense bc the explorer won't have any awareness of this ...so we just do nothing in the explorer?

aulneau · 2021-09-20T03:14:19Z

I like the feedback this provides when it moves into pending quickly, but I'm curious how users will respond if they have backed up Submitted transactions for 24+ hrs ...this would hopefully prevent that by keeping track of the nonce locally?

Yes -- part of the reason that some transactions are pending for so long is due to not having a 100% accurate nonce all the time. This hopefully will help that somewhat. Additionally, this is more about fixing an issue where sometimes transactions don't even show up, let alone show up for 24 hours. So i'd ask which is better: no transaction, or a long pending transaction (which currently happens anyways).

We will have to implement this in the explorer too in some way ...the Pending list would have a Submitted state at top?

Just realized that makes absolutely no sense bc the explorer won't have any awareness of this ...so we just do nothing in the explorer?

Yeah this is completely local and there would be no way to do this. This is meant for the local state of a users wallet, nothing more.

Also, just questioning what we do with available balance?

This is an open question and has its own set of complexities, unrelated to this feature.

beguene · 2021-09-20T12:10:17Z

src/store/accounts/nonce.ts

+    if (latestLocallySumbmittedNonce) {
+      // if we have a locally submitted nonce, and it's higher than the api nonce, use that
+      if (latestLocallySumbmittedNonce.toNumber() > apiNonce)
+        return latestLocallySumbmittedNonce.toNumber();


@aulneau If this is the last locally submitted nonce, shouldn't this be latestLocallySubmittedNonce + 1 ?

beguene · 2021-09-20T12:10:31Z

src/store/accounts/nonce.ts

-  // We try to use the api nonce first since it will be the most accurate value
-  if (typeof apiNonce === 'number') return apiNonce;
+  if (typeof apiNonce === 'number') {
+    if (latestLocallySumbmittedNonce) {


Note that the api might still return a value but an incorrect value (because the prod api has not picked up the latest tx). So it won't use the local nonce calculation. This won't solve this issue https://github.com/hirosystems/devops/issues/797 but it will solve 'api down' issues and the wallet can correctly build new tx. 👍

kyranjamie

We should definitely include some logic to remove submitted txs when they appear in a block, otherwise it'll grow indefinitely (with all the caching of contracts going on, we may even hit the 5MB limit)

kyranjamie

Had a more thorough look. Works alright. Appreciate it being kept simple for now (no Suspense, leaving TxList compnent as is etc).

Think we should have a separate visual style (grey) with a tool tip maybe explaining what submitted means.

Local txs should probably have its own folder, rather than being in account activity.

I guess we'd need to think through how we swap between nonce calculation logic, purely the presence of local txs?

kyranjamie · 2021-09-20T13:46:33Z

src/components/popup/balances-and-activity.tsx

@@ -27,16 +32,44 @@ function EmptyActivity() {
  );
 }

+const LocalTxListItem = ({ txid }: { txid: string }) => {


Move to its own component.

aulneau · 2021-09-20T13:52:56Z

Had a more thorough look. Works alright. Appreciate it being kept simple for now (no Suspense, leaving TxList compnent as is etc).

Yep it was my goal to have it be as minimal as possible, while adding the new feature.

Think we should have a separate visual style (grey) with a tool tip maybe explaining what submitted means.

Yeah, thinking about it more, "queued" seems to make more sense to me vs "submitted"

I guess we'd need to think through how we swap between nonce calculation logic, purely the presence of local txs?

Yeah, see this line https://github.com/blockstack/stacks-wallet-web/pull/1726/files#diff-2ac8b8a230a77c0902125ef70aa464b65f2868d7d0e9a75c63aaf62e45ea018eR40, this will be the last tx, and only if the tx does not exist in the mempool response. the logic is simple, if the last local tx nonce is greater than the api nonce, use the local tx nonce. Might need some work or adjustment.

markmhendrickson · 2021-09-20T13:53:09Z

Looking good to me from a UX perspective, though perhaps it'd be more consistent to show it under "Today" but instead of "Pending" label it "Submitted" with a different color, perhaps a (mid-tone) grey as @kyranjamie suggests?

src/components/popup/balances-and-activity.tsx

src/components/popup/tx-item.tsx

aulneau · 2021-09-22T13:42:12Z

Okay, to summarize my changes per comments given:

I've modified the nonce calculation to include the key isMissing to allow us to default to that, rather than a missing nonce. see this commit
I've changed the language to "Queued" vs "submitted"
I've updated the colors of the status so it does not look like a pending transaction
I've moved all the new components around so they are in their own files
I've added the queued=true query param to the explorer link
I've added a tooltip to explain what "queued" means

To test this yourself, it's best to use the throttle functionality in the chrome network dev tools panel

See the video:

Kapture.2021-09-22.at.08.32.08.mp4

Eshwari007 · 2021-09-28T17:24:01Z

@aulneau Looking good in first round of testing.Will test around few other tokens and between the networks

Eshwari007 · 2021-09-28T21:43:06Z

Test passed.Moving the ticket to QA Done
cc @aulneau

aulneau requested review from kyranjamie, beguene and fbwoolf and removed request for kyranjamie September 19, 2021 21:43

vercel bot temporarily deployed to Preview September 19, 2021 21:45 Inactive

beguene reviewed Sep 20, 2021

View reviewed changes

kyranjamie reviewed Sep 20, 2021

View reviewed changes

markmhendrickson requested a review from jasperjansz September 20, 2021 13:53

markmhendrickson linked an issue Sep 20, 2021 that may be closed by this pull request

Show new transactions instantly in activity #1715

Closed

fbwoolf reviewed Sep 20, 2021

View reviewed changes

src/components/popup/balances-and-activity.tsx Outdated Show resolved Hide resolved

fbwoolf reviewed Sep 20, 2021

View reviewed changes

src/components/popup/tx-item.tsx Outdated Show resolved Hide resolved

andresgalante mentioned this pull request Sep 20, 2021

Track user activity to better calculate nonce #1516

Closed

markmhendrickson linked an issue Sep 21, 2021 that may be closed by this pull request

Transaction delay causes nonce issue and 'locked' wallet for many hours #1723

Closed

vercel bot temporarily deployed to Preview September 22, 2021 13:33 Inactive

vercel bot temporarily deployed to Preview September 22, 2021 13:36 Inactive

vercel bot temporarily deployed to Preview September 22, 2021 13:38 Inactive

vercel bot temporarily deployed to Preview September 22, 2021 13:41 Inactive

aulneau marked this pull request as ready for review September 22, 2021 13:42

aulneau requested a review from beguene September 22, 2021 13:42

aulneau requested review from kyranjamie, beguene and fbwoolf September 28, 2021 15:51

Eshwari007 self-assigned this Sep 28, 2021

fbwoolf approved these changes Sep 28, 2021

View reviewed changes

aulneau added 14 commits September 28, 2021 16:46

feat: local txs atoms

e9f3e90

feat: support local StacksTransactions

120bc1b

fix: nonce should be number

0f286ee

fix: isMissing for api nonce

d679362

fix: submitted -> queued

d57f155

refactor: move things around

226d1c4

chore: allow utils

1f77ea6

chore: add queued query param

7a17adc

refactor: move balances-activity to feature

ce957d0

chore: deps

e029f6d

fix: typo, increment nonce

5596cec

fix: default empty string

1bbbfa5

feat: implement cleanup logic

1b6358e

chore: changeset

54c2577

aulneau force-pushed the feat/track-txs-locally branch from 4c74900 to 54c2577 Compare September 28, 2021 21:47

vercel bot temporarily deployed to Preview September 28, 2021 21:48 Inactive

aulneau merged commit 9fe5b22 into main Sep 28, 2021

aulneau deleted the feat/track-txs-locally branch September 28, 2021 21:54

blockstack-devops mentioned this pull request Sep 28, 2021

[RELEASE] Version extension #1751

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/track txs locally #1726

Feat/track txs locally #1726

aulneau commented Sep 19, 2021 •

edited by github-actions bot

Loading

changeset-bot bot commented Sep 19, 2021 •

edited

Loading

vercel bot commented Sep 19, 2021 •

edited

Loading

fbwoolf commented Sep 20, 2021

fbwoolf commented Sep 20, 2021

aulneau commented Sep 20, 2021

beguene Sep 20, 2021

beguene Sep 20, 2021

beguene Sep 20, 2021

kyranjamie left a comment

kyranjamie left a comment

kyranjamie Sep 20, 2021

aulneau commented Sep 20, 2021

markmhendrickson commented Sep 20, 2021

aulneau commented Sep 22, 2021 •

edited

Loading

Eshwari007 commented Sep 28, 2021

Eshwari007 commented Sep 28, 2021

Feat/track txs locally #1726

Feat/track txs locally #1726

Conversation

aulneau commented Sep 19, 2021 • edited by github-actions bot Loading

changeset-bot bot commented Sep 19, 2021 • edited Loading

🦋 Changeset detected

vercel bot commented Sep 19, 2021 • edited Loading

fbwoolf commented Sep 20, 2021

fbwoolf commented Sep 20, 2021

aulneau commented Sep 20, 2021

beguene Sep 20, 2021

Choose a reason for hiding this comment

beguene Sep 20, 2021

Choose a reason for hiding this comment

beguene Sep 20, 2021

Choose a reason for hiding this comment

kyranjamie left a comment

Choose a reason for hiding this comment

kyranjamie left a comment

Choose a reason for hiding this comment

kyranjamie Sep 20, 2021

Choose a reason for hiding this comment

aulneau commented Sep 20, 2021

markmhendrickson commented Sep 20, 2021

aulneau commented Sep 22, 2021 • edited Loading

Eshwari007 commented Sep 28, 2021

Eshwari007 commented Sep 28, 2021

aulneau commented Sep 19, 2021 •

edited by github-actions bot

Loading

changeset-bot bot commented Sep 19, 2021 •

edited

Loading

vercel bot commented Sep 19, 2021 •

edited

Loading

aulneau commented Sep 22, 2021 •

edited

Loading