Integrate with lightwalletd and librustzcash #16

gmale · 2019-03-12T17:06:55Z

Working branch for integration with the lightwalletd server and librustzcash core library

Update the scanBlocks request and integrate with the data that it generates inside dataDb

App is functional and displays transactions

…nit5 setup streaming is working and tests are functional

On the rust side, we now have access to the init DB commands. On the Go side we have time available for blocks

gmale · 2019-03-12T17:44:19Z

custom-tasks.gradle

@@ -1,5 +1,6 @@
-def protoSrcDir = "src/main/proto"


NOTE: I've put zero energy into cleaning this file because it will be deleted once #15 is merged.

str4d · 2019-03-13T01:37:49Z

src/androidTest/java/cash/z/wallet/sdk/dao/TransactionDaoTest.kt

+    }
+
+    @Test
+    fun testDaoInsert() {


This test suggests that inserts into the data db from the Android side are okay. Add a comment that this is only for testing purposes.

Addressed in 81141d9

str4d · 2019-03-13T01:39:49Z

src/main/java/cash/z/wallet/sdk/dao/NoteDao.kt

+    fun findById(id: Int): Note?
+
+    @Query("DELETE FROM received_notes WHERE id_note = :id")
+    fun deleteById(id: Int)


This allows modifications to the data DB from the Android side. Do we want this as part of the public API? What is it used for?

Addressed in 81141d9

str4d · 2019-03-13T01:41:02Z

src/main/java/cash/z/wallet/sdk/dao/NoteDao.kt

+    fun getAll(): List<Note>
+
+    @Delete
+    fun delete(block: Note)


I don't know how @Delete works, but I assume it does some kind of matching. This allows modifications to the data DB from the Android side.

Also, s/block/note

Addressed in 81141d9

str4d · 2019-03-13T01:41:33Z

src/main/java/cash/z/wallet/sdk/dao/BlockDao.kt

+    fun findById(height: Int): Block?
+
+    @Query("DELETE FROM blocks WHERE height = :height")
+    fun deleteById(height: Int)


Ditto re: modifications to data DB from Android side.

Addressed in 81141d9

str4d · 2019-03-13T01:43:41Z

src/main/rust/protos/ValueReceived.rs

+use protobuf::ProtobufEnum as ProtobufEnum_imported_for_functions;
+
+#[derive(PartialEq,Clone,Default)]
+pub struct ValueReceived {


This file was removed in an earlier commit in master, and has been included here by accident. Remove from this commit.

Addressed in c7d85f2

src/main/java/cash/z/wallet/sdk/data/CompactBlockDownloader.kt

str4d · 2019-03-13T02:28:28Z

build.gradle

@@ -56,35 +70,107 @@ android {
    sourceSets {
        main {
            java {
-                srcDirs "build/generated/source/wire"
+                srcDirs "build/generated/source/grpc"


Shouldn't this be src/generated/source/grpc?

as far as I can tell, it is.

str4d · 2019-03-13T02:30:41Z

src/main/java/cash/z/wallet/sdk/data/SampleSpendingKeyProvider.kt

+    }
+
+    override fun getValue(thisRef: Any?, property: KProperty<*>): String {
+        // dynamically generating keyes, based on seed is out of scope for this sample


s/keyes/keys

Addressed in c7d85f2

str4d · 2019-03-13T02:38:33Z

src/main/java/cash/z/wallet/sdk/dao/TransactionDao.kt

+    fun findById(id: Long): Transaction?
+
+    @Query("DELETE FROM transactions WHERE id_tx = :id")
+    fun deleteById(id: Long)


Ditto re: modifications to data DB from Android side.

This is the only one we allow, maybe. When a user cancels a pending transaction (before it has been sent to the network) we clean it up. Sounds like we should push that behavior down into the rust layer instead of letting apps do cancelled transaction cleanup.

str4d · 2019-03-13T02:38:39Z

src/main/java/cash/z/wallet/sdk/dao/TransactionDao.kt

+    fun getAll(): List<WalletTransaction>
+
+    @Delete
+    fun delete(transaction: Transaction)


Ditto re: modifications to data DB from Android side.

Ditto re: cancellation. We may want to discuss and then maybe bubble this up to mary/linda as a near-future feature to add to librustzcash (cancellation support by way of dataDB cleanup)

…tors The synchronizer now primarily collaborates with a downloader, processor and repository; each with a more focused set of responsibilities. The downloader streams blocks into a channel, the processor saves blocks from that channel and scans for transactions, the repository exposes transaction change events.

When a transaction is sent it transitions through a lifecycle, beginning with creating the raw transaction and ending with it being mined and added to the blockchain

Send is now functional and shows up in active transactions. This involved: - reducing the exposure of the seed - consistently using Ints for blockheight everywhere to match zcash - adding the use of spending keys - adding account initialization on startup - using accounts but defaulting to account 0 - internalizing birthday so we no longer need a reference outside of the library - enabling cancellation during send - cleanup active transaction manager

After experiencing several issues that make it more difficult to test send behavior, including the amount of time required to wait for blocks to get mined when testnet is slow, it became obvious that it was time to investigate mocking. Most of the behavior in the SDK is driven by channels so the mock only has to focus on putting useful data in the expected channels at the right time. One tradeoff here was the need to make all the synchronizer properties private, that way any implementation can achieve compatability without necessasily leveraging the same combinations of building blocks. This tradeoff felt acceptable given that these dependencies can be injected and available as singletons, if needed. This also had a side effect of elevating several channels into the Synchronizer interface, rather than reaching into the synchronizer to directly access those dependencies. Another benefit is that it's now easier to see what matters most to the app, particularly which channels are essential.

Working with bigdecimals anytime we need to multiply or divide values because it was causing issues when repeatedly toggling currency on the send screen. Coupled this with lots of improvements on the app side to do less processing while changing currencies

It is not really practical to try and use JUnit 5 for Android Instrumentation tests at this time. See documentation in build.gradle for details.

Never start downloading blocks prior to sapling activation height, only allow firstrun when dataDb is empty and also add birthday support. Also added corrections to isFirstRun logic.

This allows the app to show an error dialog rather than crashing silently

This commit will get iteratively polished until the PR is ready - Addresses PR feedback #16 (comment)

Remove unused functionality and disable modification to tables that should only be written by librustzcash

gmale · 2019-03-14T16:38:10Z

All comments here have been addressed but the work is still in an unpolished state. Closing this PR and renaming the branch to preview per this sprint task. That way, we can continue iterating on the code without committing unfinished work to master and still provide a clear place for 3rd party developers to find the latest stable code.

This commit will get iteratively polished until the PR is ready - Addresses PR feedback #16 (comment)

gmale added 5 commits March 11, 2019 14:16

Add data access layer for dataDb

c99f9a3

Update the scanBlocks request and integrate with the data that it generates inside dataDb

Create synchronizer and add layer of business logic onto the data layer

2c0966a

App is functional and displays transactions

Switch to JUnit 5

986ee50

Create CompactBlockDownloader, iterate on business logic using new JU…

5915ad3

…nit5 setup streaming is working and tests are functional

Integrate with dataDb init commands and grpc service updates.

9448f43

On the rust side, we now have access to the init DB commands. On the Go side we have time available for blocks

gmale requested a review from str4d March 12, 2019 17:06

gmale commented Mar 12, 2019

View reviewed changes

gmale force-pushed the tasks/glue branch 2 times, most recently from 228b39e to b020276 Compare March 12, 2019 18:18

str4d reviewed Mar 13, 2019

View reviewed changes

str4d mentioned this pull request Mar 13, 2019

Update README documentation #17

Merged

gmale added 14 commits March 13, 2019 01:54

Create ActiveTransactionManager to monitor active transactions

84b8756

When a transaction is sent it transitions through a lifecycle, beginning with creating the raw transaction and ending with it being mined and added to the blockchain

Fix bugs

21f09fa

Twig: Refactor to be much more useful.

913ecb4

Add additional logging to mock synchronizer

37c9cf7

Add address information to notes query and other cleanup

df9f020

Prevent change from being returned in transaction queries

c5e5941

Remove unused things and document limits of Junit 5 usage

09de20c

It is not really practical to try and use JUnit 5 for Android Instrumentation tests at this time. See documentation in build.gradle for details.

Improve logic and behavior during app startup.

56fada2

Never start downloading blocks prior to sapling activation height, only allow firstrun when dataDb is empty and also add birthday support. Also added corrections to isFirstRun logic.

Add a layer of logic for bubbling up errors

ed5e9c0

This allows the app to show an error dialog rather than crashing silently

Update the logic for determining balance

d38c348

gmale force-pushed the tasks/glue branch from b020276 to ad214cb Compare March 13, 2019 05:55

str4d mentioned this pull request Mar 13, 2019

Rust updates #15

Merged

gmale added 2 commits March 13, 2019 03:45

Cleanup

c7d85f2

This commit will get iteratively polished until the PR is ready - Addresses PR feedback #16 (comment)

Clean up data API

764455d

Remove unused functionality and disable modification to tables that should only be written by librustzcash

gmale force-pushed the tasks/glue branch from 81141d9 to 764455d Compare March 13, 2019 07:46

gmale closed this Mar 14, 2019

gmale deleted the tasks/glue branch March 14, 2019 16:52

gmale added a commit that referenced this pull request Mar 27, 2019

Cleanup

e7108ca

This commit will get iteratively polished until the PR is ready - Addresses PR feedback #16 (comment)

str4d mentioned this pull request May 19, 2022

Bring in UA-specific migrations #529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate with lightwalletd and librustzcash #16

Integrate with lightwalletd and librustzcash #16

gmale commented Mar 12, 2019

gmale Mar 12, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

str4d Mar 13, 2019

gmale Mar 13, 2019

gmale commented Mar 14, 2019

Integrate with lightwalletd and librustzcash #16

Integrate with lightwalletd and librustzcash #16

Conversation

gmale commented Mar 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmale commented Mar 14, 2019