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: suggested routes are not calculated with preferred receiver networks #19668

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

briansztamfater
Copy link
Member

fixes #19666

Summary

This PR adds logic to force preferred networks from the receiver address when calculating suggested routes.

editpreferredreceiverchains.mp4

Platforms

  • Android
  • iOS

Areas that maybe impacted

Functional
  • wallet / transactions

Steps to test

  • Open Status
  • Login with an account with some funds, lets say on Optimism and Mainnet
  • Go to wallet
  • Select an account
  • Tap on Send button
  • Enter a multichain address for sending funds to, for example opt:0x000000000000000000000000000000000000dEaD
  • Select a token
  • Enter an amount
  • Wait for routes to be loaded, and verify that only Optimism network is taking into consideration (because the address has the opt: prefix)
  • Aditionally, Tap on the "+" button to edit receiver networks and add Mainnet and / or Arbitrum and apply changes
  • Verify routes are recalculated with the new edited preferred networks

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Apr 16, 2024

Jenkins Builds

Click to see older builds (12)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 878499e #1 2024-04-16 12:51:59 ~4 min tests 📄log
✔️ 878499e #1 2024-04-16 12:55:29 ~8 min android-e2e 🤖apk 📲
✔️ 878499e #1 2024-04-16 12:55:33 ~8 min android 🤖apk 📲
✔️ 878499e #1 2024-04-16 13:02:51 ~15 min ios 📱ipa 📲
✔️ 2eab949 #2 2024-04-16 13:20:25 ~4 min tests 📄log
✔️ 2eab949 #2 2024-04-16 13:23:15 ~7 min android-e2e 🤖apk 📲
✔️ 2eab949 #2 2024-04-16 13:23:42 ~7 min android 🤖apk 📲
✔️ 2eab949 #2 2024-04-16 13:27:24 ~11 min ios 📱ipa 📲
✔️ 57fb7dc #3 2024-04-18 13:40:22 ~3 min tests 📄log
✔️ 57fb7dc #3 2024-04-18 13:42:27 ~6 min android-e2e 🤖apk 📲
✔️ 57fb7dc #3 2024-04-18 13:42:33 ~6 min android 🤖apk 📲
✔️ 57fb7dc #3 2024-04-18 13:46:17 ~9 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 81ccacc #4 2024-04-19 16:02:39 ~4 min tests 📄log
✔️ 81ccacc #4 2024-04-19 16:05:06 ~6 min android 🤖apk 📲
✔️ 81ccacc #4 2024-04-19 16:05:14 ~6 min android-e2e 🤖apk 📲
✔️ 81ccacc #4 2024-04-19 16:06:56 ~8 min ios 📱ipa 📲
✔️ 4e55456 #5 2024-04-24 07:44:51 ~4 min tests 📄log
✔️ 4e55456 #5 2024-04-24 07:46:42 ~6 min android-e2e 🤖apk 📲
✔️ 4e55456 #5 2024-04-24 07:47:30 ~6 min android 🤖apk 📲
✔️ 4e55456 #5 2024-04-24 07:50:14 ~9 min ios 📱ipa 📲

@VolodLytvynenko
Copy link
Contributor

partially blocked by #19752

@VolodLytvynenko
Copy link
Contributor

Hi @briansztamfater thank you for PR. Take a look found issues:

PR_ISSUE 1: Route still shown as available despite Insufficient balance on current Network

Additional info

Unfortunately, the issue does not happen consistently, hope the logs might help

Steps:

  1. Go to send to page
  2. Enter addresses with 2-3 multichain prefixes (example: eth:opt:0x000000000000000000000000000000000000dEaD)
  3. Go to page of assets value input
  4. Enter value available only on 2 network
  5. Diselect the fetched route
  6. Enter value which is available on the deselected route

Actual result:

The route is still shown as available.

prefferable.mp4

Expected result:

'The routes are not found' message should be shown if Insufficient balance is entered

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

Logs

logs (4).zip

@VolodLytvynenko
Copy link
Contributor

The current issue can be considered as a follow-up

PR_ISSUE 2: Selected networks are not shown as preferred if a multichain wallet with preferred networks is scanned on the 'send to' page

Steps:

  1. User Asets up a multichain wallet QR with some preferred networks (e.g., 'arb1' only
    image
    )
  2. User B goes to 'send to' page and scans User A's QR
  3. User B Selects an asset for sending
  4. User B enters the available asset amount on the asset sending page.
  5. User B opens the 'edit receiver networks' drawer

Actual result:

All networks are fetched for the receiver as preferable. This happens because, after scanning the QR code in step 2, the address is fetched without an 'arb1' prefix.

android1.mp4

Expected result:

The scanned addresses on the 'send to' page, should be fetched with networks prefixes.

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

@VolodLytvynenko
Copy link
Contributor

VolodLytvynenko commented Apr 23, 2024

The current issue can be considered as a follow-up

PR_ISSUE 3: All networks are shown in the 'not preferable' section when a legacy wallet is scanned

Steps:

  1. Go to 'send to' page
  2. Paste the legacy address (without any prefixes)
  3. Select an asset for sending
  4. Open 'edit receiver netwroks' drawer

Actual result:

All networks are in the 'not preferred' section
image

Expected result:

Networks are shown in the preferred section

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

@VolodLytvynenko
Copy link
Contributor

The current issue can be considered as a follow-up

PR_ISSUE 4: Incorrect message is shown in case of changes within 'edit receiver networks' drawer

Steps:

  1. Go to 'send to' page
  2. Select an asset for sending
  3. Open 'edit receiver networks' drawer
  4. Check or uncheck any checkbox

Actual result:

image

Expected result:

image
https://www.figma.com/file/HKncH4wnDwZDAhc4AryK8Y/Wallet-for-Mobile?type=design&node-id=2388-503714&mode=design&t=bF6mMU3KcgYWlOBr-0

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

@VolodLytvynenko
Copy link
Contributor

VolodLytvynenko commented Apr 23, 2024

PR_ISSUE 5: "No routes found" Message not displayed when user lacks sufficient assets for receiver's network

Steps:

  1. Recover user with available assets on a particular network (example: mainnet only)
  2. Enter an address in 'sent to' with prefix of a network where you don't have any assets (example: arb1)
  3. Go to page of assets value input
  4. Enter value available only on sender network

Actual result:

The 'no routes are found' message is not shown

one_route.mp4

Expected result:

The 'no routes are found' message is shown

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

Logs

logs (6).zip

@VolodLytvynenko
Copy link
Contributor

The current issue can be considered as a follow-up

PR_ISSUE 6: The 'apply changes' button is enabled in all networks are disabling in 'edit receiver networks' drawer

Steps:

  1. Open the 'edit receiver networks' drawer
  2. Uncheck all checkboxes
  3. check the 'apply changes' button

Actual result:

'apply changes' button is enabled if all checkboxes are disabled
image

Expected result:

'apply changes' button is disabled if all checkboxes are disabled

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

Discussed with design team :) https://discord.com/channels/624634427930312714/852533718790570015/1232291186702549002
image

@VolodLytvynenko
Copy link
Contributor

Question 1:

Should the "Sending to unpreferred networks" driver should be included in the scope of this PR? Currently it doesn't
image

@VolodLytvynenko
Copy link
Contributor

@briansztamfater As far as I can see, the PR looks good. The happy path is working correctly, and all reported issues can be considered as follow-ups. Please take a look at them to see if it's better to fix them separately. Otherwise, the PR can be merged

@briansztamfater
Copy link
Member Author

briansztamfater commented Apr 24, 2024

Hey @VolodLytvynenko, thanks for testing and providing feedback! I'll proceed to merge so I can properly work on a refactor here #19768, then work on issues as follow ups.

ISSUE 1: That's a weird bug I think, we can work on it in a separate PR
ISSUE 2: Not strictly related to this changes, will work on it as a follow up
ISSUE 3: Not strictly related to this changes, will work on it as a follow up
ISSUE 4: Not strictly related to this changes, will work on it as a follow up
ISSUE 5: I think that is covered in #19682
ISSUE 6: Not strictly related to this changes, will work on it as a follow up
Question 1: We should add that alert, can work in a follow up issue too 👍

Signed-off-by: Brian Sztamfater <brian@status.im>
@briansztamfater briansztamfater merged commit 782d038 into develop Apr 24, 2024
6 checks passed
@briansztamfater briansztamfater deleted the fix/preferred-networks-routes branch April 24, 2024 07:50
@VolodLytvynenko
Copy link
Contributor

@briansztamfater . Thank you for merging. Here is one more issue

PR_ISSUE 7: Preferred routes not fetched when sending assets to additional account

Steps:

  1. Create an additional account -> Open -> edit -> open network preferences of current account -> Select particular network as preferences
  2. Go to send flow using your first main account
  3. Select your additional account in the 'accounts' tab to send assets

Actual result:

All routes are fetched

assets.mp4

Expected result:

Only the preferred routes selected in step 1 for the additional account in network preferences should be fetched

OS:

IOS, Android

Devices:

  • Pixel 7a, Android 13
  • iPhone 11 Pro Max, IOS 17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

Suggested routes are not calculated with preferred networks
4 participants