Electric-Coin-Company / zcash-swift-wallet-sdk

iOS light client Framework proof-of-concept
MIT License
36 stars 33 forks source link

Transparent balance is not updated after a shielding transaction until at least 1 confirmation is received #1287

Closed tw0po1nt closed 1 year ago

tw0po1nt commented 1 year ago

Describe the issue

Please provide a general summary of the issue you're experiencing After calling SDKSynchronizer.shieldFunds(), I would expect that the transparent balance would be updated immediately in the wallet, even before confirmation. Otherwise, we have the problem where the user will still see a transparent balance - even after success - and might try to shield again, which will fail. Not great UX.

Can you reliably reproduce the issue?

If so, please list the steps to reproduce below:

  1. Clone Nighthawk 2.0 on the specified branch.
  2. Run the testnet target and create a wallet
  3. Send funds to the wallet's transparent address (via Testnet Zecfaucet)
  4. Wait for 10 confirmations of the sent funds
  5. Navigate to the transparent balance view on the wallet tab, observe the "Shield now" button
  6. Tap the "Shield now" button
  7. Tap the "Great!" button to confirm shielding
  8. On success, tap the "Done" button

Expected behaviour

Tell us what should happen

The shielding flow should dismiss, the transparent balance should be reporting as 0, the "Shield now" button should be gone, and the wallet should have an as-yet-unconfirmed incoming T-to-Z transaction that they can view in their transaction history.

Actual behaviour + errors

Tell us what happens instead including any noticeable error output (any messages displayed on-screen when e.g. a crash occurred)

The SDK reports the previous transparent balance, which causes the "Shield now" button to still be visible. Once the transaction has 1 detected confirmation, the balance updates and the "Shield now" button vanishes.

Any extra information that might be useful in the debugging process.

str4d commented 1 year ago

I've identified a bug in the Rust code backing Synchronizer.getTransparentBalance() that has the same symptoms as you describe; I'll get that fixed.

str4d commented 1 year ago

Opened https://github.com/zcash/librustzcash/issues/983 for the issue I think might be causing this.

LukasKorba commented 1 year ago

@tw0po1nt there are 2 sides of the issue. Like str4d mentioned, there is a bug that'll be fixed but I quickly checked your code and I think the code relies on synchronizer.latestState for .transparentBalance. This is really a value from the last sync and because the next sync is scheduled to happen between 10-30s, latestState always provides information from the past.

As a solution, you can call a public API synchronizer.getTransparentBalance(accountIndex: 0) to get the value immediately. For now this public API uses the same rust backend API that has the mentioned bug.

So for this issue to be fully fixed:

  1. wait on fix
  2. redo the logic around shielding a bit to not rely on latestState but rather call for the transparent balance immediately.
tw0po1nt commented 1 year ago

@str4d @LukasKorba The combination of the librustzcash fix and the suggested approach appear to have resolved this issue. Thanks 🙏