-
Notifications
You must be signed in to change notification settings - Fork 984
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
Conversation
Jenkins BuildsClick to see older builds (12)
|
878499e
to
2eab949
Compare
2eab949
to
57fb7dc
Compare
57fb7dc
to
81ccacc
Compare
partially blocked by #19752 |
Hi @briansztamfater thank you for PR. Take a look found issues: PR_ISSUE 1: Route still shown as available despite Insufficient balance on current NetworkAdditional infoUnfortunately, the issue does not happen consistently, hope the logs might help Steps:
Actual result:The route is still shown as available. prefferable.mp4Expected result:'The routes are not found' message should be shown if Insufficient balance is entered OS:IOS, Android Devices:
Logs |
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' drawerSteps:
Actual result:Expected result:
OS:IOS, Android Devices:
|
PR_ISSUE 5: "No routes found" Message not displayed when user lacks sufficient assets for receiver's networkSteps:
Actual result:The 'no routes are found' message is not shown one_route.mp4Expected result:The 'no routes are found' message is shown OS:IOS, Android Devices:
Logs |
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' drawerSteps:
Actual result:'apply changes' button is enabled if all checkboxes are disabled Expected result:'apply changes' button is disabled if all checkboxes are disabled OS:IOS, Android Devices:
Discussed with design team :) https://discord.com/channels/624634427930312714/852533718790570015/1232291186702549002 |
@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 |
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 |
Signed-off-by: Brian Sztamfater <brian@status.im>
81ccacc
to
4e55456
Compare
@briansztamfater . Thank you for merging. Here is one more issue PR_ISSUE 7: Preferred routes not fetched when sending assets to additional accountSteps:
Actual result:All routes are fetched assets.mp4Expected result:Only the preferred routes selected in step 1 for the additional account in network preferences should be fetched OS:IOS, Android Devices:
|
fixes #19666
Summary
This PR adds logic to force preferred networks from the receiver address when calculating suggested routes.
editpreferredreceiverchains.mp4
Platforms
Areas that maybe impacted
Functional
Steps to test
opt:0x000000000000000000000000000000000000dEaD
opt:
prefix)status: ready