status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Transaction confirmation bottom sheet is not closed after confirm button is tapped during sending flow

Open VolodLytvynenko opened this issue 1 year ago • 13 comments

Unfortunately, this issue doesn't happen consistently, and there are no exact steps to reproduce it. Hopefully, the logs will help

Steps:

  1. Go to a transaction confirmation page
  2. Enter the password to confirm the transaction -> confirm

Actual result:

The transaction confirmation bottom sheet does not close after the confirm button is tapped. OR the current bottom sheet is shown too long after the user navigates to the activity tab.

https://github.com/user-attachments/assets/c5e806cb-0b8b-4c1c-bb0e-fd905e56f0dc

Expected result:

Transaction confirmation bottom sheet is closed after the password is confirmed

OS:

IOS, Android

Devices:

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

ENV:

Release 2.30

Logs

logs (3).zip

VolodLytvynenko avatar Aug 08 '24 09:08 VolodLytvynenko

thank you @Parveshdhull 🥇

churik avatar Aug 08 '24 11:08 churik

hi @volodymyr, Thank you very much for creating this issue.

From the code it seems, we close the transition confirmation screen and navigate to the activity tab once wallet_createMultiTransaction calls onSuccess callback. In the case of multi-transition creation failure, we show a toast and allow the user to modify the transition and resubmit.

I can modify the code to directly navigate to the activity tab as soon as the confirm button is pressed, but I am not sure if it's desirable as the user will not able to modify the transition if creation fails.

cc @J-Son89 @smohamedjavid

Parveshdhull avatar Aug 08 '24 12:08 Parveshdhull

as the user will not able to modify the transition if creation fails

I don't think that we handle the case when tx creation fails (apart from the case when validation is there, so the user enters the invalid password for instance, but this is prior creation).

Is it so? @smohamedjavid @ulisesmac

churik avatar Aug 08 '24 12:08 churik

The bottom sheet should be closed on receiving the tx-hash. Not sure why it doesn't dismiss 🤔 https://github.com/status-im/status-mobile/blob/ce3f2b5895f4a442026e6379a9de39a95a941941/src/status_im/contexts/wallet/send/events.cljs#L761-L764


@Parveshdhull - Thanks for looking into this issue 🙏

I can modify the code to directly navigate to the activity tab as soon as the confirm button is pressed, but I am not sure if it's desirable as the user will not able to modify the transition if creation fails.

I wouldn't recommend it as if the transaction failed, and the user lands in the activity tab, it would create a bad UX.

By the time when we create a multi-transaction which requires the password of the user (validated by the standard auth), we can dismiss the bottom sheet along with the creation of multi-transaction. An additional dispatch without worrying about the result of the transaction as it's already taken care of. https://github.com/status-im/status-mobile/blob/ce3f2b5895f4a442026e6379a9de39a95a941941/src/status_im/contexts/wallet/send/events.cljs#L759

(IMO, the standard auth should dismiss itself when the user password is validated and sent to the respective process instead of the process needs to dismiss the sheet)

I don't think that we handle the case when tx creation fails (apart from the case when validation is there, so the user enters the invalid password for instance, but this is prior creation).

You are right @churik. We show an error toast to the user at the moment. But, users can go back to the routes page and edit the amount or routes if they want.

smohamedjavid avatar Aug 08 '24 12:08 smohamedjavid

Thank you @javid

By the time when we create a multi-transaction which requires the password of the user (validated by the standard auth), we can dismiss the bottom sheet along with the creation of multi-transaction

Yes, we can dismiss the auth sheet. Also, I think we have to disable the slide to send button. But by the bottom sheet, I think @VolodLytvynenko here referring to the whole transaction confirmation sheet/screen.

Volodymyr — Today at 6:11 PM

In case you ask, the user is not navigated to the activity tab at all. User gets stuck on the transaction confirmation page with the opened "enter password" bottom sheet still displayed after tapping the "continue" button.

So it seems transaction creation sometime takes significant time or silently fails :thinking: .

I wouldn't recommend it as if the transaction failed, and the user lands in the activity tab, it would create a bad UX.

I agree.

Parveshdhull avatar Aug 08 '24 13:08 Parveshdhull

But by the bottom sheet, I think @VolodLytvynenko here referring to the whole transaction confirmation sheet/screen.

Or I am confused. cc @VolodLytvynenko

Parveshdhull avatar Aug 08 '24 13:08 Parveshdhull

But by the bottom sheet, I think @VolodLytvynenko here referring to the whole transaction confirmation sheet/screen.

@Parveshdhull I didn't quite understand what you meant. To clarify, yes, the bottom sheet I'm referring to is the one shown on the transaction confirmation page

VolodLytvynenko avatar Aug 08 '24 13:08 VolodLytvynenko

Thank you @VolodLytvynenko for clarifying. https://github.com/status-im/status-mobile/pull/20987 should be fix the issue and dismiss password bottom sheet as soon as confirm button is pressed

Parveshdhull avatar Aug 08 '24 13:08 Parveshdhull

@shivekkhurana, @status-im/mobile-qa - seems important for 2.31 or is this issue already resolved @Parveshdhull ??

J-Son89 avatar Aug 19 '24 12:08 J-Son89

Hi @J-Son89, the issue is not resolved yet.

Parveshdhull avatar Aug 19 '24 12:08 Parveshdhull

I guess this issue might be due to the flaky network of the device. @VolodLytvynenko - Is this issue still reproducible?

smohamedjavid avatar Aug 27 '24 11:08 smohamedjavid

I guess this issue might be due to the flaky network of the device. @VolodLytvynenko - Is this issue still reproducible?

Hey @smohamedjavid! I have recently faced the issue while testing release 2.30. I am not sure what is the rootcause. There are no clear steps yet.

In my case transaction in fact has been send to blockchain but confirmation bottom sheet remained open. As for me it does not look like network issue.

pavloburykh avatar Aug 27 '24 12:08 pavloburykh

@smohamedjavid may be there is something in the logs that returned some error or unexpected data right after tx confirmation?

yesterday @antdanchenko mentioned that if it would be possible to log requests and response from the status-go and proxy, that would help a lot with such issues, afaik he is looking on enabling it.

churik avatar Aug 28 '24 09:08 churik

Just an update from inferring the logs, there are lots of database is locked errors (it happens because of concurrent operations with DB) across the logs and more importantly there are few in transfer. This might cause this issue (most likely) but yet to be confirmed as there are no clear steps to reproduce this.

ERROR[08-08|09:01:17.010|github.com/status-im/status-go/services/wallet/transfer/commands.go:254]                                                  processMultiTransactions error           error="database is locked"

smohamedjavid avatar Sep 02 '24 17:09 smohamedjavid

Can't reproducible anymore

VolodLytvynenko avatar Oct 07 '24 15:10 VolodLytvynenko