search

airgap-it / beacon-sdk

The beacon sdk allows developers of dApps and wallets on Tezos to implement the wallet interaction standard tzip-10.
https://walletbeacon.io
MIT License
103 stars 65 forks source link

Permission request not possible after pairing has expired #620

Closed klassare closed 9 months ago

klassare commented 1 year ago

Describe the bug (current behavior) It's not possible to send a permission request without a pairing

To Reproduce Steps to reproduce the behavior:

  1. Par with Kukai from the example dapp
  2. Remove the pairing (or let it expire) from Kukai
  3. Try to send a permission request

Expected (correct) behavior Allow permission request to be sent when a session exist, but no pairing. Do a session update instead of creating a new session.

Environment

Additional context Branch: feat/transport-kukai-wc2

IsaccoSordo commented 1 year ago

Hi @klassare, to trigger this flow we implemented a new optional method tezos_requestNewAccount that needs to be approved. We are also sending, through params, the current messageId (as id: this.currentMessageId!) Relevant lines of code: WalletConnectCommunicationClient.ts#L198-#L219

klassare commented 1 year ago

Toast notification is unresponsive to the session update. Doesn't disapear as expected.

AndreasGassmann commented 1 year ago

I think I accidentally cleared the session instead of the pairing in step 2, which is why I thought that this was fixed, sorry about that.

I can confirm that the UI doesn't behave correctly. It does something, but instead of showing the "permission granted" UI, it shows the "Awaiting response" message again.

Did you add the acknowledge message mentioned in this ticket now? https://github.com/airgap-it/beacon-sdk/issues/645 Because I think that wasn't there before IIRC. We may have an error in the logic handling the acknowledge message.

AndreasGassmann commented 1 year ago

@klassare we have a fix in the works in https://github.com/airgap-it/beacon-sdk/pull/653 and are currently testing it.

We noticed that the new "Change Session Account" alert on Kukai has a different behaviour compared to the other alerts:

  1. It is only showing up on one tab, not all of them
  2. There is no indicator in the browser tab icon that the user is prompted to do something.

There is the issue that an "abort" of the "Change Session Account" alert doesn't do anything. Here https://github.com/airgap-it/beacon-sdk/issues/646#issuecomment-1798672900 you suggested that we could represent the rejection as state change A => A. In this case, this could work. So we could check the current active account and if it's the same, we simply hide the UI (I think no error would be the best UX). Do you agree with this?

klassare commented 1 year ago

I'm aware that it needs some cleanup on the Kukai side. Just wanted to make it possible to test the flow completely on your end first.

AndreasGassmann commented 1 year ago

We were able to test it now and merged it back to the main branch, thanks.

We still have to decide on how we handle the case where the user cancels the request.

IsaccoSordo commented 9 months ago

This flow has been discarded. Possible future consideration.