Reorg-Handling Tests & lightwalletd<->client Reorg-Handling Contract

[defuse]

In #43, you can double-spend since transactions from reorged blocks aren't removed: #43 (review). A similar kind of problem exists in the current master branch.

Both this attack and the previous one would be caught by a test that does the following:

1. Send a payment to a client with a transaction that lands in a block that will be reorged away.
2. The reorg happens.
3. Check whether the client still believes it got the payment.

Close this ticket by implementing that test. The test code defining "believes it got the payment" can serve as documentation for how all clients should handle reorgs.

[defuse]

There are a couple different ways to close this ticket.

As mentioned above, one way is to write an light client in Go, which implements the logic for reorg handling similar to what exists in the iOS/Android SDK code and librustzcash, then write a test showing that light client handles reorgs correctly.

A different way is to write unit tests for the block injestor loop. We should target 100% coverage for that function, but an MVP test would make sure that when there's a reorg of varying depth, the blocks in the cache end up correct. Then, open a second ticket for integration testing reorgs with the whole stack (testing what happens when there's a reorg using the actual android / iOS SDK connected to an instance of lightwalletd).

[LarryRuane]

It may be worth giving an update. #149 includes a very thorough block cache unit test, 66811f5#diff-e701676ec027655032b02c89e29f720dR13, that I believe convincingly demonstrates that correct blocks -- and, most importantly, no stale or incorrect (invalid) blocks -- end up in the cache. But this is not a test of the block ingestor loop (mentioned in the previous comment); it's a test of cache.Add(), which is where the large majority of the reorg complexity handling resides. The block ingestor loop has some simple retry logic (in the case that its request to zcashd for a block fails), and it also decides to back up 2 blocks if cache.Add() tells it that a reorg has occurred (the ingestor loop doesn't even detect reorgs). The unit test, of course, backs up by 2 blocks and tests many other (more complex) scenarios.

Also, #164 will test the ingestor loop as well, but it isn't yet complete.

All this is to say that I think this ticket is mostly resolved, except integration testing.

[geffenz]

While this exact test may not have been created, a test suite has and is working well. This was included in the darkside wallet™ framework. Closing.