Update sending transaction end point (#21480) #21541
Conversation
Jenkins BuildsClick to see older builds (85)
|
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
from
c2051ca
to
0d03673
Compare
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
from
1a5e40a
to
78f1308
Compare
alwx
changed the title
alwx
changed the title
|
This one is now ready to be reviewed. |
|
@alwx UPDATE: your PR does not fix #21555. Most likely your branch was outdated when I checked, that's why I didn't reproduce. But I am reproducing now in the latest rebased build. Issue should be fixed by status-im/status-go#6059 |
alwx
changed the title
|
@alwx - Can you help me update this PR to point to status-im/status-go#6059 PR ( It requires the new send flow #21600 (comment) |
|
@alwx could you update go version in your PR #21541 so it uses latest go develop? Sale has merged this PR status-im/status-go#6059 and we need to test in scope of your PR. |
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
from
039ac63
to
9e94347
Compare
|
@pavloburykh done! |
|
@status-im/wallet-mobile-devs I need some reviews for this |
|
hi @saledjenic Issue with the inability to swap ERC-20 is still reproducible in the current PR after the Status Go update ISSUE 1: ERC-20 and ETH can't be swappedSteps:
Actual result:"Price route not found error is shown" Expected result:Swap transaction is successful Additional info:Swap transactions have been fixed in the Status Go PR: status-im/status-go#6059, which I’ve just verified in the following Desktop PR: status-im/status-desktop#16745. It seems this issue is only occurring on the mobile side. |
|
@saledjenic Additional info: this issue #21541 (comment) happens for ETH assets as well. however on the last nightly ETH assets can be swapped |
|
Bridge erc20 transactions are fixed! @alwx @saledjenic thank you! |
|
hi @alwx any updates on this PR? |
|
@VolodLytvynenko will hopefully be done with it today. Will update you soon. |
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
from
6422569
to
67d1bf0
Compare
|
QA NOTE: @VolodLytvynenko once this PR is merged #21635 please rebase current PR in order to test swap. I only tested that paraswap contract is updated but was not able to test swap itself as it is broken in develop. |
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
2 times, most recently
from
e5d1325
to
55aa3cb
Compare
alwx
force-pushed
the
update-sending-transactions-endpoint
branch
from
55aa3cb
to
4b08052
Compare
|
@pavloburykh done and ready to be tested |
88% of end-end tests have passedExpected to fail tests (1)Click to expandClass TestCommunityMultipleDeviceMerged:
Passed tests (7)Click to expandClass TestWalletMultipleDevice:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
hi @alwx unfortunately additional issues are found Unfortunately, this issue doesn't happen consistently, and there are no exact steps to reproduce it. Hopefully, the logs will help ISSUE 2: "no transactions build" error is shown after ERC-20 double approving and balances are zeroingSteps:
Actual result:
approv.mp4Expected result:User is navigate to swap setup page where approving is in process is shown Logs: |
ISSUE 3: "Approve" button lacks smooth transition during approval process, and approved toast not displayedSteps to Reproduce:
Actual Result:
approve.mp4Expected Result:
Refer to the design or to the last nightly: |
ISSUE 4: "Error: context canceled" Displayed When Approve Button is Tapped During Auto-RefreshSteps:
Actual Result:"Error: context canceled" is displayed instead of proceeding with the approval process. swap_errr.mp4Expected Result:User is navigated to the approval signature page, regardless of the auto-refresh timing. nightly_approve.mp4 |
| {:keycard-supported? (get-in transaction-for-signing | ||
| [:signingDetails :signOnKeycard]) |
There was a problem hiding this comment.
If I understood correctly, this flag signifies whether the flow supports keycard signing or not (e.g. in swaps we would pass false, since it wasn't implemented yet). Whether keycard should actually be used or not is decided inside standard-auth events I think. @flexsurfer can you confirm?
|
@alwx Probably this issue is not even related to this PR. Unfortunately, I couldn't reproduce it on the Desktop. I posted logs in the desktop channel as well and unfortunately, this issue doesn't happen consistently, and there are no exact steps to reproduce it. Hopefully, the logs will help PR_ISSUE 5: Transactions fail randomly on Arbitrum networkSteps:
Actual result:The transaction failed. Current transactions are not shown in the blockchain inch.mp4Expected result:Transaction is successful OS:IOS, Android Devices:
Logs: |
|
I can't reproduce this issue on desktop. Seems this is only mobile related. Also, thanks to @anastasiyaig for pointing out the more interesting parts of the logs: and |
|
issue #21541 is not fixed in the scope of the current PR. Fee estimation still does not match to desktop |
| @@ -1,34 +0,0 @@ | |||
| (ns status-im.contexts.keycard.sign.events | |||
There was a problem hiding this comment.
@alwx why is this ns removed, specifically the :keycard/sign-hash event? I'm wondering cause I rebased on top of this branch to work on keycard signing for dapps, but can't find where the keycard signing functionality is anymore.
|
hi @clauxx thank you for noticing this. Indeed the personal_sign signature is broken PR_ISSUE 6: The "personal_sign" signature failsSteps to Reproduce:
Actual Result:
connect.mp4Expected Result:The "personal_sign" signature is processed successfully. OS:IOS, Android Devices:
Logs |
Fixes #21480
Fixes #21555
In scope of this PR we need to test go PR status-im/status-go#6059 which should fix #21555 (QA Note: before testing make sure current PR is pointing to the latest go develop so includes this fix status-im/status-go#6059 which has been already merged)
Basically utilizes the new transaction flow implemented on the go side:
wallet_CreateMultiTransactionandwallet_ProceedWithTransactionsSignaturesare now deprecated and not used anymore.wallet.sign.transactionssignal is deprecated and not used either.The new transaction flow looks like this:
wallet_BuildTransactionsFromRoutewallet.router.sign-transactionssignalwallet_SignMessagecall or sign on keycardwallet_SendRouterTransactionsWithSignatureswith the signatures of signed hashes from the previous stepwallet.router.transactions-sentsignal will be sent after sending transactions or an error occursPlatforms
Areas that maybe impacted
Functional