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

Parse transaction data correctly #3129

Merged
merged 2 commits into from
Sep 21, 2021
Merged

Conversation

gantunesr
Copy link
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
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
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
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 A successful QA run through has been done 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 A successful QA run through has been done 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