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

bugfix/payment requests & deeplinks #1751

Merged
merged 12 commits into from
Aug 6, 2020
Merged

Conversation

estebanmino
Copy link
Contributor

@estebanmino estebanmino commented Aug 5, 2020

Description

We were using the parsed decimal number instead of the hex number to get the token amount from the data

https://trello.com/c/qhpAxjC1/192-walletconnect-not-working-on-android and https://trello.com/c/ubVPsajC/193-payment-request-deeplink-showing-wrong-amount

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #???

@estebanmino estebanmino requested a review from a team as a code owner August 5, 2020 19:37
@estebanmino estebanmino added the DO-NOT-MERGE Pull requests that should not be merged label Aug 5, 2020
@estebanmino estebanmino changed the title bugfix/payment requests bugfix/payment requests & deeplinks Aug 5, 2020
@estebanmino estebanmino added needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed DO-NOT-MERGE Pull requests that should not be merged labels Aug 5, 2020
!existingUser ? DeeplinkManager.setDeeplink(deeplink) : DeeplinkManager.parse(deeplink);
const { KeyringController } = Engine.context;
const isUnlocked = KeyringController.isUnlocked();
isUnlocked ? DeeplinkManager.parse(deeplink) : DeeplinkManager.setDeeplink(deeplink);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we're checking if keyring is unlocked to store a deeplink or parse it right away

@@ -292,7 +291,7 @@ class Send extends PureComponent {
amount: BNToHex(tokenAmount)
}),
value: '0x0',
readableValue: parameters.uint256 || '0'
readableValue: fromTokenMinimalUnit(parameters.uint256, selectedAsset.decimals) || '0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now we have to handle token "atomic" units from deeplinks

@@ -436,7 +436,7 @@ class Confirm extends PureComponent {
contractBalances[address] ? contractBalances[address] : '0',
decimals
)} ${symbol}`;
[transactionTo, , amount] = decodeTransferData('transfer', data);
[transactionTo, amount] = decodeTransferData('transfer', data);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were taking a parsed number from the hex format, now we take the hex number

@estebanmino estebanmino added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Aug 6, 2020
@estebanmino estebanmino requested a review from rickycodes August 6, 2020 00:51
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

QA Passed 👍

@ibrahimtaveras00 ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Aug 6, 2020
@estebanmino estebanmino merged commit 7eb1461 into develop Aug 6, 2020
@estebanmino estebanmino deleted the bugfix/payment-requestss branch August 6, 2020 15:30
rickycodes pushed a commit that referenced this pull request Jan 31, 2022
* use hex number

* password set

* publish-pre-release-android

* Revert "publish-pre-release-android"

This reverts commit 7a7941c.

* Revert "Revert "publish-pre-release-android""

This reverts commit 303bf2e.

* KeyringController.isUnlocked

* only isUnlocked

* fromTokenMinimalUnit

* Revert "Revert "Revert "publish-pre-release-android"""

This reverts commit 28ae877.

* || '0'

* changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants