airgap-it / beacon-android-sdk

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

Beacon instability #12

Closed hawkbee1 closed 1 year ago

hawkbee1 commented 1 year ago

I'm using beacon through Flutter plugin (https://github.com/TalaoDAO/beacon). On fresh install of our wallet ( https://github.com/TalaoDAO/AltMe ) everything is working as expected. After some random time we don't receive events from beacon anymore. I we clear android cache of the app or install the app again, beacon works as expected... for some time.

When issue happens I see the call to beacon for pairing but nothing is caught by the listener and I don't see any beacon logs about it. For several days I'm looking for issue in our wallet code and I'm now wondering if I miss something on beacon end. Are you aware of such behavior ? I see requests to https://beacon-node-1.hope-2.papers.tech in logs and we are all using the same Tezos address to test. Could we have such behavior because we are using the same tezos address most of the time ?

jsamol commented 1 year ago

Could we have such behavior because we are using the same tezos address most of the time ?

No, this shouldn't be an issue.

When the issue happens, have you checked if:

Also, just to make sure, since you mentioned that you're using Beacon through a Flutter plugin, have you already been able to confirm that it's the SDK and not the plugin that causes the issue?

If all result in yes as the answer, I'll need some more information to debug this, I've never experienced such a behavior while testing internally nor had anyone reported it before 🤔 Could you maybe provide me with a minimal reproducible example with steps on how to reproduce the bug then?

bibash28 commented 1 year ago

No, this is not a plugin issue... just two-way communications are interrupted...

Issue goes away after clearing the cache/ re-installing the app

bibash28 commented 1 year ago

I think i can avoid this instability...

by disconnecting beaconClient before starting it like in my commit - https://github.com/TalaoDAO/beacon/commit/8188e805af33ae19ea1b29d13644f46c7fdfc571

jsamol commented 1 year ago

@bibash28 BeaconWalletClient#stop from the client-wallet-compat package does nothing more than cancelling listeners and collectors that were set up with BeaconWalletClient#connect from the same package. If you haven't run BeaconWalletClient#connect (the compat version) in the first place (which I didn't see in your plugin), BeaconWalletClient#stop won't have any effect.

Please, look for the details I've already mentioned:

When the issue happens, have you checked if:

  • the BeaconWalletClient#connect is actively subscribed to
  • the wallet has peers that it's subscribed to and the dApp that you're trying to send the request from is amongst them
  • the dApp's public key matches the public key of the peer you've identified as describing the dApp?

or provide me a minimal reproducible example, ideally without the plugin in the middle, so I can debug it myself.

jsamol commented 1 year ago

@bibash28 I took a brief look at the plugin in general and noticed that any error that the client may emit gets dimissed.

BeaconWalletClient#connect returns a flow of Result<BeaconRequest>, internally it catches any exception that gets thrown, processing the error optionally, and propagates it to the developer wrapped as a Result instance. Not only does it mean that the catch operator is redundant, but also that you lose potential error information by processing only successful results in collect. Could you, please, check if there are any errors emitted when the issue happens?

hawkbee1 commented 1 year ago

Thanks a lot for this information, it will certainly help a lot. I'm not able to give a minimal example because issue is not happening on an event, it happens after some time.

bibash28 commented 1 year ago

Thanks I will have a look....

bibash28 commented 1 year ago

Looks like the issue is occurring from the SDK itself

Action:

  1. Pairing for the first time works nicely
  2. Waiting 15-20 min, then pairing does not occur
    • after scanning the QR of c , the QR code does not disappear (which means dApp is not getting anything)
  3. If I do a second attempt, it works nicely

A temporary solution for us can be

@jsamol Let me know if you didn't understand the issue...

jsamol commented 1 year ago

Thanks! I'll try to reproduce it and will update you on my findings.

jsamol commented 1 year ago

Ok, I've been able to reproduce the issue with the steps from yesterday. I think I fixed it in 3.2.3-beta02. Could you give it a try?

bibash28 commented 1 year ago

Sure...

hawkbee1 commented 1 year ago

Looks like you perfectly solved the issue @jsamol. I've seen testimony from Altme and Naan stating that we don't lose beacon anymore and I haven't get myself any issue with 3.2.3-beta02. Thanks a lot @jsamol

jsamol commented 1 year ago

@hawkbee1 I'm glad to hear, I'll prepare a proper release soon.

I'm closing the issue as it seems to be resolved. If you think you're still experiencing it in version 3.2.3 or newer, please, submit a new report.

bibash28 commented 1 year ago

@jsamol Thank you for beta update

But my team has confirmed that they are facing use after 2 3 hours now.. Could it be cache issue?