Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: show stx transfers from contract call #1747

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

beguene
Copy link
Contributor

@beguene beguene commented Sep 27, 2021

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

Fixes #1713.
Shows all internal STX transfers triggered within contracts.
Those transfers txs are reconstructed from the contract events info and fetched from the api endpoint /transactions_with_transfers
and then merged with existing confirmed transactions. Those STX transfers are sorted to appear after the contract call (above the contract call row in the activity list)

stx-transfer-2

cc/ @aulneau @kyranjamie @fbwoolf

@changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2021

🦋 Changeset detected

Latest commit: 9732c97

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 Patch

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
Copy link

vercel bot commented Sep 27, 2021

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/4jxXmYENwEHUxxaHxSWT3uCueCc8
✅ Preview: https://stacks-wallet-web-git-feat-show-stx-transfer-e18ce5-blockstack.vercel.app

@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 4c23d34 to 4cde18f Compare September 27, 2021 13:01
@vercel vercel bot temporarily deployed to Preview September 27, 2021 13:01 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 4cde18f to 794af4d Compare September 27, 2021 13:02
@vercel vercel bot temporarily deployed to Preview September 27, 2021 13:02 Inactive
src/store/accounts/api.ts Outdated Show resolved Hide resolved
src/store/accounts/account.hooks.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Dates are now sorted backwards, too.

@kyranjamie
Copy link
Collaborator

Perhaps we can replace use of the /transaction endpoint altogether, as all the information we need is in the transaction_with_transfers one anyway.

This way, we could avoid adding a new query, and any logic to try and merge transactions together. Instead updating the store to save the different response type (which includes the whole tx in addition to the transfers).

@vercel vercel bot temporarily deployed to Preview September 30, 2021 13:38 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from b98afae to 1bca928 Compare September 30, 2021 13:39
@vercel vercel bot temporarily deployed to Preview September 30, 2021 13:39 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 1bca928 to c6b3533 Compare September 30, 2021 13:51
@vercel vercel bot temporarily deployed to Preview September 30, 2021 13:51 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from c6b3533 to be7886c Compare September 30, 2021 13:53
@vercel vercel bot temporarily deployed to Preview September 30, 2021 13:53 Inactive
Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a much better approach, thanks @beguene

I have many comments, but these are mostly around naming

src/common/date-utils.ts Outdated Show resolved Hide resolved
src/common/transactions/transaction-utils.ts Outdated Show resolved Hide resolved
src/components/popup/transaction-list.tsx Outdated Show resolved Hide resolved
src/components/popup/transaction-list.tsx Outdated Show resolved Hide resolved
src/components/popup/tx-item.tsx Outdated Show resolved Hide resolved
src/components/popup/tx-item.tsx Outdated Show resolved Hide resolved
src/components/popup/transaction-list.tsx Outdated Show resolved Hide resolved
src/common/transactions/transaction-utils.ts Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview October 1, 2021 09:04 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from bb5a09f to 30fec97 Compare October 1, 2021 11:56
@vercel vercel bot temporarily deployed to Preview October 1, 2021 11:56 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 30fec97 to d9a7430 Compare October 1, 2021 12:00
@vercel vercel bot temporarily deployed to Preview October 1, 2021 12:00 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from d9a7430 to f8a3a86 Compare October 1, 2021 12:07
@vercel vercel bot temporarily deployed to Preview October 1, 2021 12:07 Inactive
@vercel vercel bot temporarily deployed to Preview October 5, 2021 07:48 Inactive
@vercel vercel bot temporarily deployed to Preview October 5, 2021 11:53 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 3276f20 to 8cc6ceb Compare October 6, 2021 14:04
@vercel vercel bot temporarily deployed to Preview October 6, 2021 14:05 Inactive
@beguene
Copy link
Contributor Author

beguene commented Oct 7, 2021

@Eshwari007 @timstackblock This is ready for QA. It fixes issue #1432 and #1460 (for stx transfers) and #1713 for token transfers. A lot has changed in the activity list, so you can focus your test around that area.

@beguene beguene linked an issue Oct 7, 2021 that may be closed by this pull request
@Eshwari007
Copy link

@beguene Will test and keep you posted on my findings.

@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 8cc6ceb to 69bcfb2 Compare October 11, 2021 12:25
@vercel vercel bot temporarily deployed to Preview October 11, 2021 12:25 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 69bcfb2 to 7249c61 Compare October 11, 2021 12:31
@vercel vercel bot temporarily deployed to Preview October 11, 2021 12:31 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch from 7249c61 to 9732c97 Compare October 19, 2021 14:03
@vercel vercel bot temporarily deployed to Preview October 19, 2021 14:04 Inactive
@beguene beguene force-pushed the feat/show-stx-transfer-amount-from-contract/I1713 branch 4 times, most recently from e9a0083 to 934be6a Compare October 29, 2021 11:03
@kyranjamie kyranjamie changed the base branch from main to dev October 29, 2021 12:08
@beguene beguene merged commit 9bd7d10 into dev Nov 9, 2021
@beguene beguene deleted the feat/show-stx-transfer-amount-from-contract/I1713 branch November 9, 2021 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants