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.

Sign up for GitHub

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

Jump to bottom

Add ZIP 321 URI parser #397

Open
ccjernigan opened this issue Feb 7, 2022 · 3 comments · May be fixed by #1409
Open

Add ZIP 321 URI parser #397

ccjernigan opened this issue Feb 7, 2022 · 3 comments · May be fixed by #1409
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Milestone
Android Zashi 1.1

Comments

@ccjernigan
Copy link
Contributor

ccjernigan commented Feb 7, 2022 •
edited

Is your feature request related to a problem? Please describe.

In order to support Electric-Coin-Company/zashi-android#134 and Electric-Coin-Company/zashi-android#202, we need a parser for https://zips.z.cash/zip-0321

Describe the solution you'd like

A parser has been implemented in Rust, so ideally the SDK would expose a Kotlin API for the app to consume this.

Implementation tips

  • A new JNI interface needs to be written. Relevant files include sdk-lib/src/main/rust/lib.rs sdk-lib/src/main/java/cash/z/ecc/android/sdk/jni/RustBackend
  • A new Kotlin API needs to be written. We can probably draw from sdk-ext-lib in the secant repo for some of these.
  • Because Rust libraries require disk IO to load, the Kotlin API will probably need a companion suspend new(toParse: String) function so that it can be initialized. Longer-term, we might want some kind of preloaded SDK object to make this less gross.
  • Looking at the existing implementation, this might be a place where we start to do something different. RustBackend needs a lot of parameters to initialize (e.g. context, database paths, etc) and many of its operations are stateful. Meanwhile, it seems like we have a need here for some simpler utility functions that delegate to the Rust native library which are stateless. It might be that we consider a separate set of StatelessRustBackend/StatelessRustBackendWelding or UtilityRustBackend/UtilityRustBackendWelding classes that don't require constructor parameters. Strong suggestions for the name don't exist yet. This Stateless version of the class can share the same instance of NativeLibraryLoader, although we might moving it elsewhere instead of RustBackend.Companion.rustLibraryLoader. There is an existing interface of Derivation used by DerivationTool which sort of has the same pattern.
@ccjernigan ccjernigan mentioned this issue Feb 7, 2022
Closed
@ccjernigan ccjernigan transferred this issue from Electric-Coin-Company/zashi-android Feb 7, 2022
@ccjernigan ccjernigan mentioned this issue Feb 16, 2022
Merged
15 tasks
@HonzaR
Copy link
Collaborator

HonzaR commented Mar 17, 2022

Hi @ccjernigan, have you decided on the naming of the new stateless/utility RustBacked classes yet? I'm in favor of utility naming, because I assume there will be more utility methods added later.

@ccjernigan
Copy link
Contributor Author

I don't really have a strong opinion—if you think utility works then that seems like a reasonable place to start. This particular class will be internal to the SDK, so as a non-public API we can rename or refactor it as we learn more about how it will get used in practice.

@HonzaR HonzaR mentioned this issue Mar 18, 2022
Closed
@HonzaR
Copy link
Collaborator

HonzaR commented Apr 4, 2022

Solution to this issue depends on updating lib.rs library in the Android SDK project. Waiting for it then. Electric-Coin-Company/zcash-swift-wallet-sdk#335

@ccjernigan ccjernigan mentioned this issue Apr 21, 2022
Closed
@nuttycom nuttycom added this to the Request ZEC Flow milestone Sep 9, 2023
@nuttycom nuttycom changed the title Add Zcash URI parser Add ZIP 321 URI parser Feb 9, 2024
pacu added a commit to pacu/zcash-android-wallet-sdk that referenced this issue Feb 27, 2024
@pacu
[Electric-Coin-Company#397] Add ZIP 321 URI parser
bb356e6 
adds a transaction proposal API for URI strings

Closes Electric-Coin-Company#397

Add `proposeFulfillingPaymentUri` to changelog
@pacu pacu mentioned this issue Feb 27, 2024
Closed
13 tasks
pacu added a commit to pacu/zcash-android-wallet-sdk that referenced this issue Feb 27, 2024
@pacu
[Electric-Coin-Company#397] Add ZIP 321 URI parser
b96a8dd 
adds a transaction proposal API for URI strings

Closes Electric-Coin-Company#397

Add `proposeFulfillingPaymentUri` to changelog
pacu added a commit to pacu/zcash-android-wallet-sdk that referenced this issue Mar 7, 2024
@pacu
[Electric-Coin-Company#397] Add ZIP 321 URI parser
f747161 
adds a transaction proposal API for URI strings

Closes Electric-Coin-Company#397

Add `proposeFulfillingPaymentUri` to changelog
pacu added a commit to pacu/zcash-android-wallet-sdk that referenced this issue Mar 9, 2024
@pacu
[Electric-Coin-Company#397] Add ZIP 321 URI parser
919aa65 
adds a transaction proposal API for URI strings

Closes Electric-Coin-Company#397

Add `proposeFulfillingPaymentUri` to changelog
@pacu pacu linked a pull request Mar 9, 2024 that will close this issue
Open
13 tasks
@nuttycom nuttycom modified the milestones: Request ZEC Flow, Android Zashi 1.1 Apr 9, 2024
@true-jared true-jared added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Milestone
Android Zashi 1.1
Development

Successfully merging a pull request may close this issue.

[#397] Add ZIP 321 URI parser pacu/zcash-android-wallet-sdk
4 participants
@nuttycom @ccjernigan @HonzaR @true-jared