electric-coin-company / zcash-swift-wallet-sdk Goto Github PK

Create an integration test utilizing the darksidewalletd changes to test transactions being reorg-ed away in LightClientKit.

Requirements:

• Transaction moved to different block in event of a reorg
• Transaction disappears in event of a reorg
• Balance difference is accurately reflected after a reorg

Reference:
https://docs.google.com/document/d/1_pC-uD7FdG8sMqVuuAXOoi9jFJlT9o3_Bg-WvJMfpwo/edit

Master Wallet Testing Doc with Pseudocode for tests:
https://docs.google.com/document/d/1Kv53ZnH_NVvP-b-6zMnZSCFk3k8sD-VlyGp40LMHczM/edit#

refreshPendingTransactions() marks all transactions in the outbound transaction DB (transactionManager) as mined when it sees they have been mined in the data DB. It's possible for transactions to become unmined in the data DB (when it's rewinded due to a reorg), and when that happens those transactions won't be marked as unmined in the outbound transaction DB.

refreshPendingTransactions should be updated to also unmine transactions that have been unmined in the data DB. Or would it be possible to deduplicate the databases, so that it always looks in the data DB to find out if a transaction has been mined?

This is wiring the UI and transaction sending functionality together.

• Take input from the iOS UI,

• pass it into librustzcash to create a z->z transaction,

• transaction is submitted to the network

• display the status of the created and sent transaction in the UI. <---- Blocked by Background thread issue

• Separate constants that are net dependent
• Create Testnet / main net podspec
• create demo app target for mainnet / testnet

Conduct OKR setting and vision setting meeting

Issue 1: Xcode uses a non-interactive script shell, so it cannot find the ZCASH_NETWORK_ENVIRONMENT=TESTNET environment variable listed in ~/.bash_profile. There is a way to set the environment variable for the app build, and also to set the environment variable in the cocoapod dependencies. The former is not the solution, the latter is.

Issue 2: sapling-spend-params and sapling-output-params, two parameters required for the build, are not in the ZcashLightClientKit. It’s not included because we want the parameters needing to be in one place and never bundled into a repo. The user must download them from https://z.cash/downloads/sapling-spend.params
and https://z.cash/downloads/sapling-output.params and move it to the correct folder. The readme should include these instructions in the meantime. The permanent fix is to write something in the build instructions that fetches the parameters.

we looked at other generated html sites that used Jazzy outputs, which looked good enough. But we haven’t generated our own and inspected the results.

Acceptance criteria:

• Jazzy output of current comments as-is (no requirement on content or coverage)
• Determine if the HTML output is good enough/modern/stylizable (Brad approval req)

As a mobile wallet developer I need an Android library that handles various types of zcash transactions so that I can easily integrate zcash into my wallet and interact with a majority of the existing zcash ecosystem.

Criteria:

• Create, send, and receive Sapling z-address to Sapling z-address
• • Construct transaction to send (success/fail indication)
• • Submit transaction to server (success/fail indication)
• • Confirmation that the tx was included in a block
• Receive to a t-address
• Send from a t-address to a Sapling z-address
• Send from a Sapling z-address to a t-address
• The state of transactions is clearly surfaced for developers

For all of the above:

1. The developer can create the transaction by passing the recipient address and amount
2. The transaction can be broadcast to the zcash network
3. Documentation on how to create and broadcast the transaction exists

Using the query for a paginated list of transactions, construct the methods and UI to exercise that query and display the transactions in a list that the user can page through.

Research spike for iOS code review.

timebox: 5 hours

• Walk through code (2 hours)
• Answer questions as they arise ( 2 hours)
• Wrap up call to go over issues (1 hour)

The chars.append(hexDigits[Int(byte / 16)]) line in hexEncodedString can attempt to access an out of bounds index, because byte is of type unichar (equivalent to UInt16) and can contain values greater than 255.

A couple options for fixing this: (a) Reject non-ASCII strings, or (b) encode the string as UTF-8 first (this will have the same result for all ASCII strings, and non-ASCII characters will get encoded as multiple bytes).

addresses #71

As a wallet developer I would like an example of sending functionality within the demo app

Criteria:

• Table entry for sending screen exists GUI
• Can send funds in screen
• status of sent funds is exposed to developer

As a developer I want to do what the title says

Story: As an iOS mobile wallet developer I would like to be able to query for the correct current blockheight so that I can demonstrate how to connect to lightwalletd.

Criteria:

• iOS Demo app that shows the latest block height.
• The app should be using the iOS SDK to achieve criteria 1

As a wallet developer, I would like to be able to easily query for transactions in my wallet and page them.

Criteria:

• Query for transactions
• Paginate the transactions in the query
• Demonstrate the display of transactions by exposing paged transactions in the demo UI

The comment says "Typically, this should be about half the average block time." The block time was halved recently so the constant can be halved too, if desired.

yas.

https://github.com/zcash/ZcashLightClientKit/blob/master/ZcashLightClientKit/Synchronizer/SDKSynchronizer.swift#L249 -- in a 32-bit build, Int is only 32 bits, so an amount of zatoshi passed in as an Int64 might be truncated. Fix this by always explicitly using Int64 to represent an amount of zatoshi, never the platform-dependent Int.

In getAddress and getBalance, the account ID provided by the user gets truncated into a smaller integer type:

    public func getAddress(index account: Int = 0) -> String? {
        rustBackend.getAddress(dbData: dataDbURL, account: Int32(account))
    }
    
    public func getBalance(account index: Int = 0) -> Int64 {
        rustBackend.getBalance(dbData: dataDbURL, account: Int32(index))
    }

Since the function accepts an Int, developers might expect accounts IDs larger than the maximum value of an Int32 to be valid and separate from other account IDs, but that cast might make them become negative or truncate them to be identical to a smaller account ID.

getAddress and getBalance should take an Int32 instead of an Int to make it impossible to call them with an invalid account ID.

Because obviously. Credit to @gtank.

Implement logging in the LightClientKit SDK to catch SDK failures and crashes.

Requirements:

• Logs related to Send
• Logs for Scanning
• Logs of DB state, balance, transaction history are surfaced

add key derivation and validation methods to lib.rs and update cargo.toml files to use note-spending-v7 branches as android does

pod trunk push failed because xcodebuild couldn't find that header file.

We fixed it by making all headers public and removing that file from .gitignore and copying it over to a static path.

How it should be:

prepare command touches a ZcashLightClientKit/zcashlc/zcashlc.h file
cargo builds generates the h file
the build system copies it over to the final destination or fails if the file is empty
build succeeds.

Create a data access object for reading transaction information from librustzcash database

Create demo app documentation to guide a user through building and deploying the demo apps onto an emulator

Create integration tests utilizing the darksidewalletd changes to test blockchain reorgs of various lengths in LightClientKit.

Requirements:

• Implement interface to darksidewalletd API
• A chain reorg of <10 blocks without a wallet-associated transaction
• A chain reorg of >10 blocks without a wallet-associated transaction

Reference:
https://docs.google.com/document/d/1_pC-uD7FdG8sMqVuuAXOoi9jFJlT9o3_Bg-WvJMfpwo/edit

Achieve 100% comment coverage of functions that wallet developers will or should use.

Pacu should write comments inline his Swift code next to each API call so that Jazzy can generate documentation that explains each function call. This is an initial pass, documentation can be polished off in a separate ticket.

Acceptance criteria:

• comments next to each function/call
• comments are in markup format

In validationFailed() a notification is sent out indicating that there has been a reorg. Do the event listeners get run at the time of this call to post()? If so, the wallet developer's event listener will run before the databases have been rewound. If they were using the event handler to update the UI with the results of a reorg (e.g. a transaction got unmined) their code wouldn't update the UI correctly since the database is still in the old state.

Work with marshall to get a test page up for the wallet team to see.

Acceptance criteria:

• generate HTML page hosted by read the docs
• show team resulting page in a parking lot

  func determineLowerBound(errorHeight: Int) -> BlockHeight {
        let offset = min(ZcashSDK.MAX_REORG_SIZE, ZcashSDK.DEFAULT_REWIND_DISTANCE * (consecutiveChainValidationErrors + 1))
        return max(errorHeight - offset, lowerBoundHeight ?? ZcashSDK.SAPLING_ACTIVATION_HEIGHT)
    }

that last max() should be a safeguard to rewinding past sapling activation, but has the side effect of not returning the correct rewind height.

The Fix

func determineLowerBound(errorHeight: Int) -> BlockHeight {
        let offset = min(ZcashSDK.MAX_REORG_SIZE, ZcashSDK.DEFAULT_REWIND_DISTANCE * (consecutiveChainValidationErrors + 1))
        return max(errorHeight - offset, self.config.walletBirthday)
    }

this wallet should never rewind further than its birthday which in the worst case will be SAPLING_ACTIVATION.
I was so defensive that I introduced a bug.

A notification is generated when there's a reorg, but rewinding the data DB can in theory fail with an exception, which will cause a different kind of error, from the SDK user's perspective. The user will need to know to also handle this error case, and not rely on the data in the DB when it happens. If they don't handle it, they might display data from a not-really-rewound DB.

This is just one example of how handling reorgs might be tricky for the user. There should be documentation explaining to users exactly what they need to do to handle reorgs appropriately. Writing that documentation isn't urgent and can wait until the code is stable.

This issue was originally discovered and reported by Florian Tramèr. I'm making it public since no users are currently at risk.

The issue is that the SDK will:

1. Download new blocks,
2. process those blocks, taking an amount of time that depends on how many notes belong to this wallet,
3. wait a fixed length of time,
4. make a request to lightwalletd for more new blocks.

A passive network adversary can measure the time between (1) and (4) to determine whether any notes in the blocks belonged to the wallet. To fix this, either

a. Decouple the network code from the block processing code, such that downloaded blocks are queued up to be processed on a separate thread and the processing won't affect the timing of network requests,
b. Florian's suggestion, to request blocks at absolute time intervals, i.e. instead of sleeping for 20 seconds between requests, request new blocks at times *:00, *:20, *:40. In order for this to not leak information, the block processing MUST always complete before the next request is due to be made.

I recommend (b) since it's easier and we're about to run into even harder side-channel challenges with memo downloading and mempool access.

submit constructed sapling transactions to lightwalletd and gracefully handle failure during submission.

Criteria:

• Submit transaction
• handle success/failure during submission

As a wallet developer I would like to be able to send sapling z-address to sapling z-address from the ios SDK

Criteria:

• Construct transaction to send (success/fail indication)
• Submit transaction to server (success/fail indication)
• Confirmation that the tx was included in a block

Be able to observe when a transaction is included in a block and alert the SDK

Criteria:

• alert sdk of successfully including tx in block

Create basic sending ui in iOS demo app that allows the user to send an arbitrary amount of zcash to a user defined Sapling z-address

I predict this will be a common security bug pattern:

1. There's a reorg.
2. That reorg invalidates some data in memory, in a database, or in the UI.
3. That data isn't deleted / marked as invalid, because the code to do so was forgotten / wasn't written, or there's a power failure / unhandled exception in that code.

#44 is an instance of this pattern. Let's think a little bit about what we can do to make these bugs less likely. @pacu and @gmale, you know what developers expect/like best so your input will be really important here!

The current design is to have the developer listen for events indicating there's been a reorg and write some code to update their data structures / UI in response to what changed, which is really susceptible to this kind of bug because event handlers might not be guaranteed to run (in case of power failure / crash) or the developer might forget to listen for them.

Another idea is to expose a single source of truth for wallet data, which is kept up to date by the SDK, and an event simply notifes developers when "something changed" (reorg, new transaction, etc) and in that event the developer completely re-generates their entire UI, other data structures, etc, from the single source of truth, so that there's no chance of the UI or anything becoming partially out of sync with the source of truth. This would probably create performance issues, so I'm wondering what other options there are. At the very least, can we have the SDK double-check the developer is listening to all the events they need to? Any other ideas?

As a mobile wallet developer I would like an SDK that provides me with all of the tools necessary to observe blockchain state and my wallet reliably so that I can easily integrate zcash shielded support into my wallet.

Criteria:

• The SDK can download and decrypt blocks in order to find transactions that pertain to the wallet
• The SDK must be able to recognize, acknowledge, and rectify the effects of a blockchain reorganization
• Able to query for all transactions related to the wallet
• Able to query for the current wallet balance
• Able to query for the current block height
• Able to generate Sapling keys from a ZIP32 seed
• Able to generate transparent keys from seed
• Able to generate ZIP32 seed from BIP39 style mnemonic
• Able to generate addresses from their associated keys
• Able to initialize accounts (#15)

We need to improve the way errors bubble up to the surface.

Example: when a connectivity issue arises, the SDK synchronizer would receive a SwiftGRPC.RPCError, when it should not know of its existence

This creates 2 main problems:

1. errors are implementation related
2. user experience is degraded because it is not possible to provide a comprehensive failure.

As a developer I would like to see what the title says

The current GRPC SDK that we're using is being deprecated. We need to update to the latest version of Swift GRPC NIO.

Findings:

• SwiftGRPC NIO has no support for CocoaPods
• can't be integrated as Swift Package at an SDK Level (except using Carthage, no thanks!)

Things to Explore:

• Fork the project and contribute with Cocoapod Support
• Support some sort of no so plug and play Swift Package Manager Support on the sdk

I created a question on their repo to find out the effort of supporting cocoapods on their end, so that we can decide whether we should contribute to those efforts or proceed to research how to support SPM in some manner

grpc/grpc-swift#767

Well It turns out there's a pr in process for this!
grpc/grpc-swift#764
let's see how it goes. I'll probably contribute on it if required

Research how to implement mainnet compatibility into the iOS SDK and Demo App. Have the discussion and create the tickets to implement this functionality.

Time box: 2 hours

As a wallet developer I would like to be able to receive a paginated list of transactions so I can easily and quickly display those transactions to the user.

As an iOS developer I would like to be able to initialize an account by calling into librustzcash so that I can easily create a new wallet or expand an existing one.

add details on download and managment

As a wallet developer I need to have an SDK that handles blockchain reorgs gracefully

Criteria:

• Detect reorg by validating the chain
• Attempt fix by rewinding 100 blocks and updating local storage (TODO validate with str4d)
• resync last 100 blocks

• Add a table of demo / code file / description of code
• Linda generates readme in markdown for Pacu and hands it off with necessary pictures
• Linda submits PR