Repository: horizontalsystems/bitcoin-kit-android
Issue: Transactions without blocks during restore

On Testnet

1. Set checkpoint to 1441440
2. Run sync
3. See that first few transactions have no blocks

Changed files:
bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/BlockSyncer.kt
bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockchainTest.kt
bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/network/PeerTask/GetMerkleBlocksTask.kt
bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockSyncerTest.kt
bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/Blockchain.kt
bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/models/MerkleBlock.kt

diff --git a/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockSyncerTest.kt b/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockSyncerTest.kt
index dc4414b3..db87842d 100644
--- a/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockSyncerTest.kt
+++ b/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockSyncerTest.kt
@@ -67,12 +67,8 @@ class BlockSyncerTest {
     @Test
     fun handleMerkleBlock_forceAddValidOrphanBlock() {
         val height = 100
-        realm.executeTransaction {
-            realm.where(BlockHash::class.java)
-                    .equalTo("headerHash", merkleBlock.blockHash)
-                    .findFirst()
-                    ?.height = height
-        }
+
+        merkleBlock.height = height
 
         whenever(blockchain.forceAdd(merkleBlock, height, realm)).thenReturn(block)
         whenever(blockchain.connect(merkleBlock, realm)).thenThrow(BlockValidatorException.NoPreviousBlock())
@@ -106,7 +102,7 @@ class BlockSyncerTest {
 
         blockSyncer.handleMerkleBlock(merkleBlock)
 
-        verify(transactionProcessor).process(merkleBlock.associatedTransactions, block, true, realm)
+        verify(transactionProcessor).process(merkleBlock.associatedTransactions, block, false, realm)
 
         val realm1 = factories.realmFactory.realm
         assertBlockHashNotPresent(block.reversedHeaderHashHex, realm1)
@@ -115,7 +111,7 @@ class BlockSyncerTest {
     @Test
     fun handleMerkleBlock_BloomFilterExpired_blockHashNotDeleted() {
         whenever(blockchain.connect(merkleBlock, realm)).thenReturn(block)
-        whenever(transactionProcessor.process(merkleBlock.associatedTransactions, block, true, realm)).thenThrow(BloomFilterManager.BloomFilterExpired)
+        whenever(transactionProcessor.process(merkleBlock.associatedTransactions, block, false, realm)).thenThrow(BloomFilterManager.BloomFilterExpired)
 
         blockSyncer.handleMerkleBlock(merkleBlock)
 
diff --git a/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockchainTest.kt b/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockchainTest.kt
index c9dbfe70..8ed1bcae 100644
--- a/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockchainTest.kt
+++ b/bitcoinkit/src/androidTest/java/io/horizontalsystems/bitcoinkit/blocks/BlockchainTest.kt
@@ -2,6 +2,7 @@ package io.horizontalsystems.bitcoinkit.blocks
 
 import io.horizontalsystems.bitcoinkit.RealmFactoryMock
 import io.horizontalsystems.bitcoinkit.models.Block
+import io.horizontalsystems.bitcoinkit.models.Transaction
 import io.horizontalsystems.bitcoinkit.network.NetworkParameters
 import io.realm.Realm
 import junit.framework.Assert
@@ -36,7 +37,7 @@ class BlockchainTest {
     }
 
     @Test
-    fun handleFork_forkExist_newBlocksLonger() {
+    fun handleFork_forkExist_newBlocksLonger_transactions() {
         val blocksInChain = mapOf(1 to "InChain1", 2 to "InChain2", 3 to "InChain3")
         val newBlocks = mapOf(2 to "NewBlock2", 3 to "NewBlock3", 4 to "NewBlock4")
 
@@ -76,7 +77,7 @@ class BlockchainTest {
     }
 
     @Test
-    fun handleFork_xxx_noNewBlocks() {
+    fun handleFork_noNewBlocks() {
         val blocksInChain = mapOf(1 to "InChain1", 2 to "InChain2", 3 to "InChain3")
         val newBlocks = mapOf<Int, String>()
 
@@ -102,37 +103,50 @@ class BlockchainTest {
     private fun assertBlocksPresent(blocksInChain: Map<Int, String>, realm: Realm) {
         blocksInChain.forEach { (height, id) ->
             Assert.assertEquals("Block $id($height) not found", 1, realm.where(Block::class.java).equalTo("height", height).equalTo("reversedHeaderHashHex", id).count())
+            Assert.assertEquals("Transaction $id($height) not found", 1, realm.where(Transaction::class.java).equalTo("hashHexReversed", id).count())
         }
     }
 
     private fun assertBlocksNotPresent(blocksInChain: Map<Int, String>, realm: Realm) {
         blocksInChain.forEach { (height, id) ->
             Assert.assertEquals("Block $id($height) should not present", 0, realm.where(Block::class.java).equalTo("height", height).equalTo("reversedHeaderHashHex", id).count())
+            Assert.assertEquals("Transaction $id($height) should not present", 0, realm.where(Transaction::class.java).equalTo("hashHexReversed", id).count())
         }
     }
 
     private fun assertNotStaleBlocksPresent(blocksInChain: Map<Int, String>, realm: Realm) {
         blocksInChain.forEach { (height, id) ->
             Assert.assertEquals("Not stale block $id($height) not found", 1, realm.where(Block::class.java).equalTo("stale", false).equalTo("height", height).equalTo("reversedHeaderHashHex", id).count())
+            Assert.assertEquals("Transaction $id($height) not found", 1, realm.where(Transaction::class.java).equalTo("hashHexReversed", id).count())
         }
     }
 
     private fun insertBlocks(blocksInChain: Map<Int, String>, newBlocks: Map<Int, String>) {
         realm.executeTransaction {
             blocksInChain.forEach { (height, id) ->
-                realm.insert(Block().apply {
+                val block = realm.copyToRealm(Block().apply {
                     reversedHeaderHashHex = id
                     this.height = height
                     stale = false
                 })
+
+                realm.insert(Transaction().apply {
+                    hashHexReversed = id
+                    this.block = block
+                })
             }
 
             newBlocks.forEach { (height, id) ->
-                realm.insert(Block().apply {
+                val block = realm.copyToRealm(Block().apply {
                     reversedHeaderHashHex = id
                     this.height = height
                     stale = true
                 })
+
+                realm.insert(Transaction().apply {
+                    hashHexReversed = id
+                    this.block = block
+                })
             }
         }
     }
diff --git a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/BlockSyncer.kt b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/BlockSyncer.kt
index 042c7ec0..d8dad34b 100644
--- a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/BlockSyncer.kt
+++ b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/BlockSyncer.kt
@@ -1,6 +1,5 @@
 package io.horizontalsystems.bitcoinkit.blocks
 
-import io.horizontalsystems.bitcoinkit.blocks.validators.BlockValidatorException
 import io.horizontalsystems.bitcoinkit.core.RealmFactory
 import io.horizontalsystems.bitcoinkit.managers.AddressManager
 import io.horizontalsystems.bitcoinkit.managers.BloomFilterManager
@@ -72,13 +71,13 @@ class BlockSyncer(private val realmFactory: RealmFactory,
         }
     }
 
-    fun getBlockHashes(): List<ByteArray> {
+    fun getBlockHashes(): List<BlockHash> {
         val realm = realmFactory.realm
         val blockHashes = realm.where(BlockHash::class.java)
                 .sort("order")
                 .findAll()
 
-        val result = blockHashes.take(500).map { it.headerHash }
+        val result = blockHashes.take(500).map { realm.copyFromRealm(it) }
         realm.close()
 
         return result
@@ -154,26 +153,15 @@ class BlockSyncer(private val realmFactory: RealmFactory,
         val realm = realmFactory.realm
 
         realm.executeTransaction {
-            var forceAdd = false
+            val height = merkleBlock.height
 
-            val block = try {
-                blockchain.connect(merkleBlock, realm)
-            } catch (e: BlockValidatorException.NoPreviousBlock) {
-                val height = realm.where(BlockHash::class.java)
-                        .equalTo("headerHash", merkleBlock.blockHash)
-                        .findFirst()
-                        ?.height ?: 0
-
-                if (height > 0) {
-                    forceAdd = true
-                    blockchain.forceAdd(merkleBlock, height, realm)
-                } else {
-                    throw e
-                }
+            val block = when (height) {
+                null -> blockchain.connect(merkleBlock, realm)
+                else -> blockchain.forceAdd(merkleBlock, height, realm)
             }
 
             try {
-                transactionProcessor.process(merkleBlock.associatedTransactions, block, needToRedownload || forceAdd, realm)
+                transactionProcessor.process(merkleBlock.associatedTransactions, block, needToRedownload, realm)
             } catch (e: BloomFilterManager.BloomFilterExpired) {
                 needToRedownload = true
             }
@@ -198,18 +186,7 @@ class BlockSyncer(private val realmFactory: RealmFactory,
                 .findAll().map { it.reversedHeaderHashHex }.toTypedArray()
 
         realm.executeTransaction {
-            realm.where(Block::class.java).`in`("reversedHeaderHashHex", toDelete).findAll().let { blocksToDelete ->
-                blocksToDelete.forEach { block ->
-                    block.transactions?.let { transactions ->
-                        transactions.forEach { transaction ->
-                            transaction.inputs.deleteAllFromRealm()
-                            transaction.outputs.deleteAllFromRealm()
-                        }
-                        transactions.deleteAllFromRealm()
-                    }
-                }
-                blocksToDelete.deleteAllFromRealm()
-            }
+            blockchain.deleteBlocks(realm.where(Block::class.java).`in`("reversedHeaderHashHex", toDelete).findAll())
         }
 
         realm.close()
diff --git a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/Blockchain.kt b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/Blockchain.kt
index 2b239c85..36bd3177 100644
--- a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/Blockchain.kt
+++ b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/blocks/Blockchain.kt
@@ -5,6 +5,7 @@ import io.horizontalsystems.bitcoinkit.models.Block
 import io.horizontalsystems.bitcoinkit.models.MerkleBlock
 import io.horizontalsystems.bitcoinkit.network.NetworkParameters
 import io.realm.Realm
+import io.realm.RealmResults
 import io.realm.Sort
 
 class Blockchain(private val network: NetworkParameters) {
@@ -52,18 +53,18 @@ class Blockchain(private val network: NetworkParameters) {
                         .sort("height", Sort.DESCENDING)
                         .findFirst()?.height!!
 
-                if (lastStaleHeight > lastNotStaleHeight) {
+                val blocksToDelete = if (lastStaleHeight > lastNotStaleHeight) {
                     realm.where(Block::class.java)
                             .equalTo("stale", false)
                             .greaterThanOrEqualTo("height", firstStaleHeight)
                             .findAll()
-                            .deleteAllFromRealm()
                 } else {
                     realm.where(Block::class.java)
                             .equalTo("stale", true)
                             .findAll()
-                            .deleteAllFromRealm()
                 }
+
+                deleteBlocks(blocksToDelete)
             }
 
             realm.where(Block::class.java)
@@ -75,4 +76,17 @@ class Blockchain(private val network: NetworkParameters) {
         }
     }
 
+    fun deleteBlocks(blocksToDelete: RealmResults<Block>) {
+        blocksToDelete.forEach { block ->
+            block.transactions?.let { transactions ->
+                transactions.forEach { transaction ->
+                    transaction.inputs.deleteAllFromRealm()
+                    transaction.outputs.deleteAllFromRealm()
+                }
+                transactions.deleteAllFromRealm()
+            }
+        }
+        blocksToDelete.deleteAllFromRealm()
+    }
+
 }
diff --git a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/models/MerkleBlock.kt b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/models/MerkleBlock.kt
index a02c7622..45b571a0 100644
--- a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/models/MerkleBlock.kt
+++ b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/models/MerkleBlock.kt
@@ -23,6 +23,7 @@ class MerkleBlock() {
     var hashes: Array<ByteArray> = arrayOf()
     var flags: ByteArray = byteArrayOf()
 
+    var height: Int? = null
     var associatedTransactionHexes = listOf<String>()
     val associatedTransactions = mutableListOf<Transaction>()
     val blockHash: ByteArray by lazy {
diff --git a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/network/PeerTask/GetMerkleBlocksTask.kt b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/network/PeerTask/GetMerkleBlocksTask.kt
index f07e8da2..9ebb2788 100644
--- a/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/network/PeerTask/GetMerkleBlocksTask.kt
+++ b/bitcoinkit/src/main/kotlin/io/horizontalsystems/bitcoinkit/network/PeerTask/GetMerkleBlocksTask.kt
@@ -1,27 +1,28 @@
 package io.horizontalsystems.bitcoinkit.network.PeerTask
 
 import io.horizontalsystems.bitcoinkit.core.toHexString
+import io.horizontalsystems.bitcoinkit.models.BlockHash
 import io.horizontalsystems.bitcoinkit.models.InventoryItem
 import io.horizontalsystems.bitcoinkit.models.MerkleBlock
 import io.horizontalsystems.bitcoinkit.models.Transaction
 
-class GetMerkleBlocksTask(hashes: List<ByteArray>) : PeerTask() {
+class GetMerkleBlocksTask(hashes: List<BlockHash>) : PeerTask() {
 
-    private var hashes = hashes.toMutableList()
+    private var blockHashes = hashes.toMutableList()
     private var pendingMerkleBlocks = mutableListOf<MerkleBlock>()
 
     override fun start() {
-        val items = hashes.map { hash ->
-            InventoryItem(InventoryItem.MSG_FILTERED_BLOCK, hash)
+        val items = blockHashes.map { hash ->
+            InventoryItem(InventoryItem.MSG_FILTERED_BLOCK, hash.headerHash)
         }
 
         requester?.getData(items)
     }
 
     override fun handleMerkleBlock(merkleBlock: MerkleBlock): Boolean {
-        if (hashes.firstOrNull { merkleBlock.blockHash.contentEquals(it) } == null) {
-            return false
-        }
+        val blockHash = blockHashes.firstOrNull { merkleBlock.blockHash.contentEquals(it.headerHash) } ?: return false
+
+        merkleBlock.height = if (blockHash.height > 0) blockHash.height else null
 
         if (merkleBlock.complete) {
             handleCompletedMerkleBlock(merkleBlock)
@@ -47,19 +48,15 @@ class GetMerkleBlocksTask(hashes: List<ByteArray>) : PeerTask() {
     }
 
     private fun handleCompletedMerkleBlock(merkleBlock: MerkleBlock) {
-        hashes.firstOrNull { it.contentEquals(merkleBlock.blockHash) }?.let {
-            hashes.remove(it)
+        blockHashes.firstOrNull { it.headerHash.contentEquals(merkleBlock.blockHash) }?.let {
+            blockHashes.remove(it)
         }
 
         delegate?.handleMerkleBlock(merkleBlock)
 
-        if (hashes.isEmpty()) {
+        if (blockHashes.isEmpty()) {
             delegate?.onTaskCompleted(this)
         }
     }
 
-//    override func isRequestingInventory(hash: Data) -> Bool {
-//        return hashes.contains(hash)
-//    }
-
 }