Test status go PR 6271 and 6304 #21963
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (12)
|
79% of end-end tests have passedFailed tests (10)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestFallbackMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (44)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestFallbackMultipleDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestWalletOneDevice:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
|
|
@saledjenic thank you for the PR. Please, take a look at the issue below ISSUE 1 'Not available for chain id` error during building route on ArbitrumReproducible for send/bridge/swap flows Steps:
Actual result: 'Not available for chain id` error Status-debug-logs - 2025-01-22T175604.362.zip telegram-cloud-document-2-5226705501055643384.mp4 |
|
@pavloburykh could you update the statusgo and try again now? |
briansztamfater
force-pushed
the
test-go-6271
branch
from
28c66b8 to
a068131
Compare
89% of end-end tests have passedFailed tests (4)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestFallbackMultipleDevice:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Passed tests (50)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestFallbackMultipleDevice:
Class TestWalletMultipleDevice:
Class TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
|
|
Hi @saledjenic. Thank you for fixes. Take a look one more issue ISSUE 2: Swap and approval transactions fail for ERC-20 assetsSteps:
Actual result:Approval and swap transactions fail for ERC-20 assets approve.mp4approval tx https://basescan.org/tx/0x9a315ebe37f8ac2e280ce2b6f17b785242fb660bbce4117222f1f6b25ca4ce11 OS:IOS, Android Devices:
Logshttps://drive.google.com/file/d/1YM35k49lNyJF3VtojQ6OtulZTknSwMwn/view?usp=drive_link |
|
@VolodLytvynenko does this happen only in this branch or in the master branch for the mobile app as well? |
|
hi @saledjenic Could you take a look at one question/potential issue? The fee calculation seems fine in the sense that the user doesn't pay more than the estimated max fee. However, in the current PR, the fee has increased significantly, and I can’t determine if this is expected behavior. Additionally, the max fee calculation doesn’t align with the formula described here: How max fees are calculated. I tried to summarize it in the table below:
Am I making a mistake in my calculations, or is this an issue with how the max fee is being calculated? https://docs.google.com/spreadsheets/d/1gBtwdiO-2j6Hts8pRzL_6Q8wdAok7Qi9Kuq-fTUR24Q/edit?usp=sharing |
@saledjenic this happens only in the current PR. ERC-20 approval and swap works ok in the latest master |
|
@VolodLytvynenko thanks for the table view. Yes, they are significantly higher, and that was the reason why we disabled L1 fees a few months ago and also that's the reason why I asked you in statusgo PR to check it. Firstly, the formula you shared So the correct current formula, including L1 fees should be:
Could you compare where are we in comparison to MetaMask in terms of fees? After that, we should determine:
|
Thank you for clarification @saledjenic |
|
@saledjenic I noticed some new commits were added into status go PR. status-im/status-go#6271. just let you know, the issue 2 still present. Can't perform swap approval with ERC-20 |
96% of end-end tests have passedFailed tests (1)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (54)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestFallbackMultipleDevice:
Class TestDeepLinksOneDevice:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
|
hi, @saledjenic could you take a look at one additional issue? I can't bridge USDC on any network, however, this is not reproducible on the latest develop PR_ISSUE 3: Bridge fails for USDC assetsPreconditions:My user has enough ETH to cover the fee:
Actual result:Bridge tx with USDC fails with insufficient gas fee error: unsifigas.mp4
Additional info:I tried to bridge ETH or DAI and it works well, looks like current issue happens on USDC only IOS, Android Devices:
Logshttps://drive.google.com/file/d/1FGsMTZyNjGeEyo4WC0GR6KpRSBCUKyoT/view?usp=drive_link |
|
@saledjenic issue 2 still reproducible swappp.mp4Logs:https://drive.google.com/file/d/18snVlIiV1LHvi_m7iGZm01eMRtjL81hq/view?usp=drive_link |
@VolodLytvynenko that's really odd, cause I've tested the desktop app for both, swap and bridge and it worked fine. Sorry, but I have to ask you, are you sure that you've used the latest statusgo changes in the app you've tested? |
@saledjenic, I am using the latest builds from the current PR and have rechecked the issue again. It is still reproducible for me. To avoid conflicts, I always remove the previous build before installing the new one. Additionally, I've checked that the current PR correctly references the appropriate Status Go branch: #21963. Perhaps some adjustments are needed on the UI side. I will ask someone from the mobile team to check it. |
|
We need someone from the mobile team to review this PR. Below is a summary of what needs to be checked:
https://docs.google.com/spreadsheets/d/1gBtwdiO-2j6Hts8pRzL_6Q8wdAok7Qi9Kuq-fTUR24Q/edit?gid=0#gid=0 |
@VolodLytvynenko mobile PR has not been updated yet, so you have been testing the old commit. @saledjenic , please, ignore Volodymyr's comment above. We will retest after mobile PR will be updated. |
briansztamfater
force-pushed
the
test-go-6271
branch
from
a068131 to
85547a8
Compare
79% of end-end tests have passedNot executed tests (2)Failed tests (10)Click to expandClass TestFallbackMultipleDevice:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (2)Click to expandClass TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Passed tests (44)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestWalletOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestDeepLinksOneDevice:
|
|
@saledjenic That was my fault. Apologies. Issue 2 is resolved after updating the PR. Thanks for your help, @pavloburykh @briansztamfater! However, the fees have not been updated and are still displayed as higher than in metamask. This may still require someone from the mobile team to review and adjust our client as suggested here. The goal is to align the fees more closely with those shown in MetaMask, as discussed here. Currently, the fees remain the same as described in the table below.
https://docs.google.com/spreadsheets/d/1gBtwdiO-2j6Hts8pRzL_6Q8wdAok7Qi9Kuq-fTUR24Q/edit?gid=0#gid=0 |
|
@VolodLytvynenko I prepared a new PR status-im/status-go#6304 on top of status-im/status-go#6271 to estimate L1 fees for Optimism more accurately (for the reference - that was discussed today in the #wallet-crew channel). Please have a look, review, test. |
|
Hey @VolodLytvynenko |
29% of end-end tests have passedFailed tests (40)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestFallbackMultipleDevice:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (17)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestWalletOneDevice:
|
|
@saledjenic take a look please found issue. Swap works fine on our mobile develop PR_ISSUE 1: 'not available for chain id' error is shown when to build route on swap page using Base networkSteps:
Actual result:
OS:IOS, Android Devices:
Logshttps://drive.google.com/file/d/1HPETmVyBNafudQoMU9-A8tzz6BUaNmNz/view?usp=drive_link |
54% of end-end tests have passedFailed tests (25)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestFallbackMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Expected to fail tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (32)Click to expandClass TestWalletOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePR:
Class TestWalletMultipleDevice:
Class TestFallbackMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
|
|
I have finished the testing. Now, @yevh-berdnyk helps to check e2e run failures. Some updates after testing: Usually, on Optimism , the estimated fee is <$0.01 (@saledjenic is it expected?), and the actual fee has never exceeded the estimate. However, this might be due there is no significant market movements today. Here is a table: Send flow Swap flow Bridge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes for #21954
Created to test status go PR status-im/status-go#6271 and status-im/status-go#6304