[#1092] Adopt proposal API

[LukasKorba]

Closes #1092

• TCA sdkSynchronizer dependency extended with 3 new Proposal APIs
• Typical Fee < 0.001 localized and updated in the UI
• Awaiting all transaction results with use of new proposal.transactionCount() method
• Implemented new PartialProposalError reducer and view
• Contact support mail with transaction IDs logic implemented
• Fallback to share logic implemented
• PPE integrated into SendFlow
• PPE integrated into Shielding
• Changelog updated

This code review checklist is intended to serve as a starting point for the author and reviewer, although it may not be appropriate for all types of changes (e.g. fixing a spelling typo in documentation). For more in-depth discussion of how we think about code review, please see Code Review Guidelines.

Author

• Self-review: Did you review your own code in GitHub's web interface? Code often looks different when reviewing the diff in a browser, making it easier to spot potential bugs.
• Does the code abide by the Coding Guidelines?
• Automated tests: Did you add appropriate automated tests for any code changes?
• Code coverage: Did you check the code coverage report for the automated tests? While we are not looking for perfect coverage, the tool can point out potential cases that have been missed.
• Documentation: Did you update Docs as appropiate? (E.g README.md, etc.)
• Run the app: Did you run the app and try the changes?
• Did you provide Screenshots of what the App looks like before and after your changes as part of the description of this PR? (only applicable to UI Changes)
• Rebase and squash: Did you pull in the latest changes from the main branch and squash your commits before assigning a reviewer? Having your code up to date and squashed will make it easier for others to review. Use best judgement when squashing commits, as some changes (such as refactoring) might be easier to review as a separate commit.

Reviewer

• Checklist review: Did you go through the code with the Code Review Guidelines checklist?
• Ad hoc review: Did you perform an ad hoc review? In addition to a first pass using the code review guidelines, do a second pass using your best judgement and experience which may identify additional questions or comments. Research shows that code review is most effective when done in multiple passes, where reviewers look for different things through each pass.
• Automated tests: Did you review the automated tests?
• Manual tests: Did you review the manual tests?You will find manual testing guidelines under our manual testing section
• How is Code Coverage affected by this PR? We encourage you to compare coverage before and after your changes and when possible, leave it in a better place. Learn More...
• Documentation: Did you review Docs, README.md, LICENSE.md, and Architecture.md as appropriate?
• Run the app: Did you run the app and try the changes? While the CI server runs the app to look for build failures or crashes, humans running the app are more likely to notice unexpected log messages, UI inconsistencies, or bad output data.

[daira]

For ZIP 320 support, the case where only the first transaction succeeds is essentially identical to the case where the exchange sends back the funds: both leave the funds in the ephemeral t-addr. I was under the impression that handling the latter was required for Zashi 1.0. So why can't we handle the former correctly?

[str4d]

For ZIP 320 support, the case where only the first transaction succeeds is essentially identical to the case where the exchange sends back the funds: both leave the funds in the ephemeral t-addr.

For ZIP 320 support, from a control of funds position, yes. But they are semantically two very different cases, and we have not done the UX work to distinguish them, or handle how the resulting transaction history is conveyed to users.

I was under the impression that handling the latter was required for Zashi 1.0.

I was not: my understanding is that we need to be able to send to TEX addresses (which covers the majority use case), but not that we need to handle exchanges sending funds back (which is an edge case).

I think we should handle both exchanges sending funds back, and transaction submission failures, in-app eventually. But I also think that handling this properly with good UX requires more engineering than we have time for in the 1.0 timeline, so it would go in a post-1.0 release. Having an error indicating to users when they have hit this edge case is the minimal amount of engineering we can do here, while avoiding those edge case users believing they have lost funds.

So why can't we handle the former correctly?

There are at least four different states we could be in here that I can think of (for ZIP 320 proposals; more if we also consider arbitrary proposals which the SDK might generate or need to handle in future):

• Transient failure to submit the original transactions; logical transaction is unfinished.
• Permanent failure to submit the original transactions; logical transaction is unfinished.
• Logical transaction is rejected; funds sent back.
• Drive-by funding: someone saw the ephemeral address and sent a (maybe intentionally identifiable) amount of funds to it.

Each state needs to be handled differently; some states have multiple possible resolutions. None of this has been discussed or planned yet, and I don't think we have time to do so for 1.0.

[daira]

I completely agree that an ideal UX would distinguish those (except possibly that the refund and drive-by cases might sometimes be indistinguishable), and that an ideal UX for this in Zashi can wait until after 1.0.

Where I disagree is that I don't think any of them need to result in any possibility of funds loss. Just allocate the ephemeral address in a way that guarantees that we scan it. That's a matter of how zcash_client_backend interacts with the protocol, not a UX decision for Zashi.

[daira]

utACK modulo text changes.

[str4d]

Where I disagree is that I don't think any of them need to result in any possibility of funds loss.

I agree, and am not arguing for this!

Just allocate the ephemeral address in a way that guarantees that we scan it.

I agree with "Just allocate the ephemeral address in a way that guarantees that we can scan it" - this is sufficient to guarantee no loss of funds. I disagree with actually scanning it in 1.0, because per my comment above that would require UX work to explain to the user what is going on in all of the different states that scan might occur from, that we do not have time for.

Only one of the four states I outlined is transient (it decays to the second state after about an hour), but three of the states are permanent, distinguishable, and "automatically re-shield" is not necessarily the correct resolution to all three. (Plus it opens us up to race conditions in other edge cases, the avoidance of which requires more engineering that we don't have time for.)

For 1.0, informing the user and telling them to contact us means that a) we don't make permanent changes to their wallet history that might be undesirable, and b) means that we learn about these edge cases if they occur, which helps us to craft better UX here.

[HonzaR]

Approving with two non-blocking questions inline. Thank you.