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: Account Unification errors #1003

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

impelcrypto
Copy link
Member

@impelcrypto impelcrypto commented Nov 1, 2023

Pull Request Summary

  • fix: displaying account icon
  • fix: use unified account
    • map to proper SS58 address for EVM dApp staking
    • map to proper H160 address for sending assets from EVM to Native(unified)

Check list

  • contains breaking changes
  • adds new feature
  • modifies existing feature (bug fix or improvements)
  • relies on other tasks
  • documentation changes
  • tested on mobile devices

This pull request makes the following changes:

Fixes

  • fix: displaying account icon

=Before=

  • Not unified account but isUnifiedAccount was true
    image

  • This is unified account but AuIcon component was not imported
    Screenshot 2023-11-01 at 3 10 05 PM

=After=
image

Changes

  • removed getSS58Address function from useAccount hooks and use getMappedNativeAddress from AccountUnificationService

Copy link

github-actions bot commented Nov 1, 2023

Visit the preview URL for this PR (updated for commit a7d692b):

https://astar-apps--pr1003-fix-send-evm-assets-7uncl4ai.web.app

(expires Thu, 09 Nov 2023 04:59:54 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db

@impelcrypto impelcrypto marked this pull request as ready for review November 1, 2023 09:12
@gluneau
Copy link
Contributor

gluneau commented Nov 1, 2023

Well done.

image

@Kahonnohak
Copy link
Contributor

Checked if account is not unified, portal doesn't allow transactions to happen
Screenshot 2023-11-01 at 15 22 52

Checked if account is unified
Sent ERC20 to native address and received the token in unified account.

Copy link
Contributor

@Kahonnohak Kahonnohak left a comment

Choose a reason for hiding this comment

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

LGTM

@impelcrypto impelcrypto merged commit bab1445 into main Nov 2, 2023
6 checks passed
@impelcrypto impelcrypto deleted the fix/send-evm-assets-to-unified-account branch November 2, 2023 07:31
@impelcrypto impelcrypto mentioned this pull request Nov 7, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants