Skip to content

Parse transaction data correctly#3129

Merged
andrepimenta merged 2 commits intodevelopfrom
fix/parse-tx-data-correctly-v2
Sep 21, 2021
Merged

Parse transaction data correctly#3129
andrepimenta merged 2 commits intodevelopfrom
fix/parse-tx-data-correctly-v2

Conversation

@gantunesr
Copy link
Copy Markdown
Member

Description

This PR fixes the way we parse transaction data.

@ibrahimtaveras00
How to test:
Test token transfers (ERC20 and ERC721) requested from a Dapp. Would be nice to check the etherscan parameters against the same transaction made by the extension, especially:
image

Before:
Simulator Screen Shot - iPhone 11 Pro - 2021-08-25 at 17 09 01

After:
Simulator Screen Shot - iPhone 11 Pro - 2021-08-25 at 17 09 23

Checklist

  • There is a related GitHub issue
  • Any added code is fully documented

Issue

Resolves #2744
Resolves #3051

@gantunesr gantunesr requested a review from a team as a code owner September 15, 2021 22:57
@gantunesr gantunesr mentioned this pull request Sep 15, 2021
2 tasks
@cortisiko cortisiko added the Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing label Sep 15, 2021
Copy link
Copy Markdown
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

@andrepimenta the transaction data still appears scrambled. See here

2021-09-15_19-15-31

To reproduce:
Connect to this dappp: https://sorbet-finance-git-fix-metamask-mobile-bug-2-gelato.vercel.app/ (I used ropsten as my network)

Attempt to transfer some eth

Notice the transaction data appears wonky.

@cortisiko cortisiko added the QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed label Sep 15, 2021
@andrepimenta
Copy link
Copy Markdown
Member

@cortisiko Can you check against extension, if it's the same then it should be correct:

image

@cortisiko cortisiko added QA in Progress QA has started on the feature. and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Sep 16, 2021
Copy link
Copy Markdown
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

looks good 🌮 🌮 🌮 🌮

@cortisiko cortisiko added QA Passed QA testing has been completed and passed and removed QA in Progress QA has started on the feature. labels Sep 20, 2021
@andrepimenta andrepimenta merged commit 663620a into develop Sep 21, 2021
@andrepimenta andrepimenta deleted the fix/parse-tx-data-correctly-v2 branch September 21, 2021 12:45
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
Co-authored-by: andrepimenta <andrepimenta7@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

QA Passed QA testing has been completed and passed Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing

Projects

None yet

3 participants