Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust layer replies with error to sent transaction memo query #1154

Closed
HonzaR opened this issue Jul 31, 2023 · 3 comments · Fixed by #1116
Closed

Rust layer replies with error to sent transaction memo query #1154

HonzaR opened this issue Jul 31, 2023 · 3 comments · Fixed by #1116
Labels
bug Something isn't working

Comments

@HonzaR
Copy link
Contributor

HonzaR commented Jul 31, 2023

Describe the issue

Rust layer replies with an error to the sent transaction memo query.

Can you reliably reproduce the issue?

If so, please list the steps to reproduce below:

  1. Run Demo-app
  2. Select one of the predefined wallets (Alice/Ben: both have sapling funds available)
  3. Let it sync
  4. Hit Send and send some sapling funds to the opposite sapling address
  5. Fill in the Memo field
  6. Hit Send button and wait for confirmation
  7. Then go to the Transaction screen
  8. And hit any displayed sent transaction to query its memo - the memo should then be printed into the console

Expected behaviour

The same as for received transactions. It replies with an empty string/error in case of no memo in the transaction and with a valid string memo in case of a transaction with a memo.

Actual behavior + errors

java.lang.RuntimeException: An error occurred retrieving the memo, Query returned no rows

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

Reported in zcash/librustzcash#834.

@nuttycom
Copy link
Contributor

nuttycom commented Aug 1, 2023

The issue appears to be that the Android SDK is attempting to use the transaction ID as the note identifier for calling get_memo in the Rust backend: https://github.com/zcash/zcash-android-wallet-sdk/blob/1a0508f57dfa6eec68668f4705990f59f62ff3c5/sdk-lib/src/main/java/cash/z/ecc/android/sdk/internal/db/derived/TxOutputsView.kt#L46

The note ID is not currently returned from the v_tx_outputs view. We can either change that, or the memos can be retrieved directly from that view rather than making an extra get_memo call.

@nuttycom
Copy link
Contributor

nuttycom commented Aug 2, 2023

After some thought, the best option here is to use the (txid, pool_type, output_index) tuple for querying for memos. All of these values are already returned by the view, and the Sqlite WalletRead impl's associated NoteRef type can be updated to use this representation.

@HonzaR HonzaR linked a pull request Aug 16, 2023 that will close this issue
13 tasks
@HonzaR
Copy link
Contributor Author

HonzaR commented Sep 6, 2023

Closing as resolved by this commit.

@HonzaR HonzaR closed this as completed Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants