Code cleanup, add comments and tests to reflect changes.

core/src/main/kotlin/com/r3corda/core/crypto/PartialMerkleTree.kt

@@ -12,17 +12,47 @@ class MerkleTreeException(val reason: String): Exception() {
 /**
  * Building and verification of Partial Merkle Tree.
  * Partial Merkle Tree is a minimal tree needed to check that given set of leaves belongs to a full Merkle Tree.
- * todo example of partial tree
+ *
+ * Example of Merkle tree with 5 leaves.
+ *
+ *             h15
+ *          /       \
+ *         h14       h55
+ *        /  \      /  \
+ *      h12  h34   h5->d(h5)
+ *     / \   / \   / \
+ *    l1 l2 l3 l4 l5->d(l5)
+ *
+ * l* denote hashes of leaves, h* - hashes of nodes below.
+ * h5->d(h5) denotes duplication of left hand side node. These nodes are kept in a full tree as DuplicatedLeaf.
+ * When filtering the tree for l5, we don't want to keep both l5 and it's duplicate (it can also be solved using null
+ * values in a tree, but this solution is clearer).
+ *
+ * Example of Partial tree based on the tree above.
+ *
+ *             ___
+ *          /       \
+ *          _        _
+ *        /  \      /  \
+ *      h12   _     _   d(h5)
+ *           / \   / \
+ *          I3 l4 I5 d(l5)
+ *
+ * We want to check l3 and l5 - now turned into IncudedLeaf (I3 and I5 above). To verify that these two leaves belong to
+ * the tree with a hash root h15 we need to provide a Merkle branch (or partial tree). In our case we need hashes:
+ * h12, l4, d(l5) and d(h5). Verification is done by hashing the partial tree to obtain the root and checking it against
+ * the obtained h15 hash. Additionally we store included hashes used in calculation and compare them to leaves hashes we got
+ * (there can be a difference in obtained leaves ordering - that's why it's a set comparison not hashing leaves into a tree).
+ * If both equalities hold, we can assume that l3 and l5 belong to the transaction with root h15.
  */
-class PartialMerkleTree(
+
-    val root: PartialTree
+class PartialMerkleTree(val root: PartialTree) {
-) {
     /**
      * The structure is a little different than that of Merkle Tree.
-     * Partial Tree by might not be a full binary tree. Leaves represent either original Merkle tree leaves
+     * Partial Tree might not be a full binary tree. Leaves represent either original Merkle tree leaves
      * or cut subtree node with stored hash. We differentiate between the leaves that are included in a filtered
      * transaction and leaves that just keep hashes needed for calculation. Reason for this approach: during verification
-     * it's easier to extract hashes used as base for this tree.
+     * it's easier to extract hashes used as a base for this tree.
      */
     sealed class PartialTree() {
         class IncludedLeaf(val hash: SecureHash): PartialTree()
@@ -32,14 +62,14 @@ class PartialMerkleTree(
 
     companion object {
         /**
-         * @param merkleRoot
+         * @param merkleRoot Root of full Merkle tree.
-         * @param includeHashes
+         * @param includeHashes Hashes that should be included in a partial tree.
          * @return Partial Merkle tree root.
          */
         fun build(merkleRoot: MerkleTree, includeHashes: List<SecureHash>): PartialMerkleTree {
             val usedHashes = ArrayList<SecureHash>()
-            //Too much included hashes or different ones.
             val tree = buildPartialTree(merkleRoot, includeHashes, usedHashes)
+            //Too much included hashes or different ones.
             if(includeHashes.size != usedHashes.size)
                 throw MerkleTreeException("Some of the provided hashes are not in the tree.")
             return PartialMerkleTree(tree.second)
@@ -84,14 +114,13 @@ class PartialMerkleTree(
     }
 
     /**
-     * @param merkleRootHash 
+     * @param merkleRootHash Hash that should be checked for equality with root calculated from this partial tree.
-     * @param hashesToCheck
+     * @param hashesToCheck List of included leaves hashes that should be found in this partial tree.
      */
     fun verify(merkleRootHash: SecureHash, hashesToCheck: List<SecureHash>): Boolean {
         val usedHashes = ArrayList<SecureHash>()
         val verifyRoot = verify(root, usedHashes)
         //It means that we obtained more/less hashes than needed or different sets of hashes.
-        //Ordering insensitive.
         if(hashesToCheck.size != usedHashes.size || hashesToCheck.minus(usedHashes).isNotEmpty())
             return false
         return (verifyRoot == merkleRootHash)

core/src/main/kotlin/com/r3corda/core/transactions/MerkleTransaction.kt

@@ -8,17 +8,6 @@ import com.r3corda.core.crypto.*
 import com.r3corda.core.serialization.serialize
 import java.util.*
 
-/**
- * Creation and verification of a Merkle Tree for a Wire Transaction.
- *
- * See: https://en.wikipedia.org/wiki/Merkle_tree
- *
- * Transaction is split into following blocks: inputs, outputs, commands, attachments' refs. Merkle Tree is kept in
- * a recursive data structure. Building is done bottom up, from all leaves' hashes.
- * If a row in a tree has an odd number of elements - the final hash is hashed with itself.
- */
-
-//Todo It's just mess, move it to wtx
 /**
  * Build filtered transaction using provided filtering functions.
  */
@@ -39,10 +28,19 @@ fun WireTransaction.calculateLeavesHashes(): List<SecureHash> {
 fun SecureHash.hashConcat(other: SecureHash) = (this.bits + other.bits).sha256()
 fun <T: Any> serializedHash(x: T) = x.serialize().hash
 
+/**
+ * Creation and verification of a Merkle Tree for a Wire Transaction.
+ *
+ * See: https://en.wikipedia.org/wiki/Merkle_tree
+ *
+ * Transaction is split into following blocks: inputs, outputs, commands, attachments' refs. Merkle Tree is kept in
+ * a recursive data structure. Building is done bottom up, from all leaves' hashes.
+ * If a row in a tree has an odd number of elements - the final hash is hashed with itself.
+ */
 sealed class MerkleTree(val hash: SecureHash) {
     class Leaf(val value: SecureHash): MerkleTree(value)
     class Node(val value: SecureHash, val left: MerkleTree, val right: MerkleTree): MerkleTree(value)
-    //DuplicatedLeaf is storing a hash of the righmost node that had to be duplicated to obtain the tree.
+    //DuplicatedLeaf is storing a hash of the rightmost node that had to be duplicated to obtain the tree.
     //That duplication can cause problems while building and verifying partial tree (especially for trees with duplicate
     //attachments or commands).
     class DuplicatedLeaf(val value: SecureHash): MerkleTree(value)
@@ -51,8 +49,8 @@ sealed class MerkleTree(val hash: SecureHash) {
         val newHash = this.hash.hashConcat(right.hash)
         return Node(newHash, this, right)
     }
-}
+
-//todo -> to wire transaction
+    companion object {
         /**
          * Merkle tree building using hashes.
          */
@@ -66,7 +64,7 @@ fun getMerkleTree(allLeavesHashes: List<SecureHash>): MerkleTree {
          * @param lastNodesList MerkleTree nodes from previous level.
          * @return Tree root.
          */
-tailrec fun buildMerkleTree(lastNodesList: List<MerkleTree>): MerkleTree {
+        private tailrec fun buildMerkleTree(lastNodesList: List<MerkleTree>): MerkleTree {
             if (lastNodesList.size < 1)
                 throw MerkleTreeException("Cannot calculate Merkle root on empty hash list.")
             if (lastNodesList.size == 1) {
@@ -90,6 +88,8 @@ tailrec fun buildMerkleTree(lastNodesList: List<MerkleTree>): MerkleTree {
                 return buildMerkleTree(newLevelHashes)
             }
         }
+    }
+}
 
 /**
  * Class that holds filtered leaves for a partial Merkle transaction. We assume mixed leaves types.
@@ -138,8 +138,9 @@ class FilteredTransaction(
 ) {
     companion object {
         /**
-         * Construction of filtered transaction with Partial Merkle Tree, takes WireTransaction and filtering functions
+         * Construction of filtered transaction with Partial Merkle Tree.
-         * for inputs, outputs, attachments, commands.
+         * @param wtx WireTransaction to be filtered.
+         * @param filterFuns filtering functions for inputs, outputs, attachments, commands.
          */
         fun buildMerkleTransaction(wtx: WireTransaction,
                                    filterFuns: FilterFuns

core/src/test/kotlin/com/r3corda/core/crypto/PartialMerkleTreeTest.kt

@@ -2,31 +2,23 @@ package com.r3corda.core.crypto
 
 import com.r3corda.contracts.asset.*
 import com.r3corda.core.contracts.DOLLARS
-import com.r3corda.core.contracts.TransactionType
 import com.r3corda.core.contracts.`issued by`
-import com.r3corda.core.contracts.`with notary`
 import com.r3corda.core.serialization.serialize
-import com.r3corda.core.transactions.FilterFuns
+import com.r3corda.core.transactions.*
-import com.r3corda.core.transactions.buildFilteredTransaction
-import com.r3corda.core.transactions.getMerkleRoot
-import com.r3corda.core.transactions.hashConcat
-import com.r3corda.core.utilities.DUMMY_NOTARY
 import com.r3corda.core.utilities.DUMMY_PUBKEY_1
 import com.r3corda.testing.*
 
 
 import org.junit.Test
-import java.util.*
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 
 class PartialMerkleTreeTest {
     val nodes = "abcdef"
     val hashed = nodes.map { it.serialize().sha256() }
     val root = SecureHash.Companion.parse("F6D8FB3720114F8D040D64F633B0D9178EB09A55AA7D62FAE1A070D1BF561051")
-
+    val merkleTree = MerkleTree.getMerkleTree(hashed)
-    private fun makeTX() = TransactionType.General.Builder(DUMMY_NOTARY).withItems(
-            1000.DOLLARS.CASH `issued by` DUMMY_CASH_ISSUER `owned by` ALICE_PUBKEY `with notary` DUMMY_NOTARY)
 
     val testLedger = ledger {
         unverifiedTransaction {
@@ -51,16 +43,19 @@ class PartialMerkleTreeTest{
     //Building full Merkle Tree tests.
     @Test
     fun `building Merkle tree with 6 nodes - no rightmost nodes`() {
-        assertEquals(6, hashed.size)
+        assertEquals(root, merkleTree.hash)
-        val mr = getMerkleRoot(hashed)
+    }
-        assertEquals(root, mr)
+
+    @Test
+    fun `building Merkle tree - no hashes`() {
+        assertFailsWith<MerkleTreeException> { MerkleTree.getMerkleTree(emptyList()) }
     }
 
     @Test
     fun `building Merkle tree one node`() {
         val node = 'a'.serialize().sha256()
-        val mr = getMerkleRoot(listOf(node))
+        val mt = MerkleTree.getMerkleTree(listOf(node))
-        assertEquals(node, mr)
+        assertEquals(node, mt.hash)
     }
 
     @Test
@@ -69,8 +64,8 @@ class PartialMerkleTreeTest{
         val h1 = hashed[0].hashConcat(hashed[1])
         val h2 = hashed[2].hashConcat(hashed[2])
         val expected = h1.hashConcat(h2)
-        val root = getMerkleRoot(odd)
+        val mt = MerkleTree.getMerkleTree(odd)
-        assertEquals(root, expected)
+        assertEquals(mt.hash, expected)
     }
 
     @Test
@@ -83,65 +78,69 @@ class PartialMerkleTreeTest{
         assert(mt.verify(testTx.id))
     }
 
-    @Test
-    fun `ordering insensitive tree`(){
-        val filterFuns = FilterFuns(filterCommands =  { x -> true })
-        val tx1 = makeTX()
-        tx1.addCommand(Cash.Commands.Issue(1), ALICE_PUBKEY)
-        tx1.addCommand(Cash.Commands.Issue(0), ALICE_PUBKEY)
-        val wtx1 = tx1.toWireTransaction()
-
-        val tx2 = makeTX()
-        tx2.addCommand(Cash.Commands.Issue(0), ALICE_PUBKEY)
-        tx2.addCommand(Cash.Commands.Issue(1), ALICE_PUBKEY)
-        val wtx2 = tx2.toWireTransaction()
-        val mt1 = wtx1.buildFilteredTransaction(filterFuns)
-        val mt2 = wtx2.buildFilteredTransaction(filterFuns)
-        assertEquals(wtx1.id, wtx2.id)
-        assert(mt1.verify(wtx1.id))
-        assert(mt2.verify(wtx2.id))
-    }
-
     //Partial Merkle Tree building tests
     @Test
-    fun `Partial Merkle Tree, only left nodes branch`(){
+    fun `build Partial Merkle Tree, only left nodes branch`() {
-        val includeLeaves = listOf(false, false, false, true, false, true)
         val inclHashes = listOf(hashed[3], hashed[5])
-        val pmt = PartialMerkleTree.build(includeLeaves, hashed)
+        val pmt = PartialMerkleTree.build(merkleTree, inclHashes)
-        assert(!pmt.includeBranch[2] && !pmt.includeBranch[4]&& !pmt.includeBranch[8])
+        assert(pmt.verify(merkleTree.hash, inclHashes))
-        assert(pmt.verify(inclHashes, root))
-
     }
 
     @Test
-    fun `Partial Merkle Tree, include zero leaves`(){
+    fun `build Partial Merkle Tree, include zero leaves`() {
-        val includeLeaves = listOf(false, false, false, false, false, false)
+        val pmt = PartialMerkleTree.build(merkleTree, emptyList())
-        val pmt = PartialMerkleTree.build(includeLeaves, hashed)
+        assert(pmt.verify(merkleTree.hash, emptyList()))
-        assertEquals(root, pmt.branchHashes[0])
-        assert(pmt.verify(emptyList(), root))
     }
 
     @Test
-    fun `Partial Merkle Tree, include all leaves`(){
+    fun `build Partial Merkle Tree, include all leaves`() {
-        val includeLeaves = listOf(true, true, true, true, true, true)
+        val pmt = PartialMerkleTree.build(merkleTree, hashed)
-        val pmt = PartialMerkleTree.build(includeLeaves, hashed)
+        assert(pmt.verify(merkleTree.hash, hashed))
-        assert(pmt.verify(hashed, root))
+    }
+
+    @Test
+    fun `build Partial Merkle Tree - duplicate leaves failure`() {
+        val inclHashes = arrayListOf(hashed[3], hashed[5], hashed[3], hashed[5])
+        assertFailsWith<MerkleTreeException> { PartialMerkleTree.build(merkleTree, inclHashes) }
     }
 
     @Test
     fun `verify Partial Merkle Tree - too many leaves failure`() {
-        val includeLeaves = listOf(false, false, false, true, false, true)
+        val inclHashes = arrayListOf(hashed[3], hashed[5])
-        val inclHashes = listOf(hashed[3], hashed[5], hashed[0])
+        val pmt = PartialMerkleTree.build(merkleTree, inclHashes)
-        val pmt = PartialMerkleTree.build(includeLeaves, hashed)
+        inclHashes.add(hashed[0])
-        assertFalse(pmt.verify(inclHashes, root))
+        assertFalse(pmt.verify(merkleTree.hash, inclHashes))
+    }
+
+    @Test
+    fun `verify Partial Merkle Tree - too little leaves failure`() {
+        val inclHashes = arrayListOf(hashed[3], hashed[5], hashed[0])
+        val pmt = PartialMerkleTree.build(merkleTree, inclHashes)
+        inclHashes.remove(hashed[0])
+        assertFalse(pmt.verify(merkleTree.hash, inclHashes))
+    }
+
+    @Test
+    fun `verify Partial Merkle Tree - duplicate leaves failure`() {
+        val mt = MerkleTree.getMerkleTree(hashed.subList(0,5)) //Odd number of leaves. Last one is duplicated.
+        val inclHashes = arrayListOf(hashed[3], hashed[4])
+        val pmt = PartialMerkleTree.build(mt, inclHashes)
+        inclHashes.add(hashed[4])
+        assertFalse(pmt.verify(mt.hash, inclHashes))
+    }
+
+    @Test
+    fun `verify Partial Merkle Tree - different leaves failure`() {
+        val inclHashes = arrayListOf(hashed[3], hashed[5])
+        val pmt = PartialMerkleTree.build(merkleTree, inclHashes)
+        assertFalse(pmt.verify(merkleTree.hash, listOf(hashed[2], hashed[4])))
     }
 
     @Test
     fun `verify Partial Merkle Tree - wrong root`() {
-        val includeLeaves = listOf(false, false, false, true, false, true)
         val inclHashes = listOf(hashed[3], hashed[5])
-        val pmt = PartialMerkleTree.build(includeLeaves, hashed)
+        val pmt = PartialMerkleTree.build(merkleTree, inclHashes)
         val wrongRoot = hashed[3].hashConcat(hashed[5])
-        assertFalse(pmt.verify(inclHashes, wrongRoot))
+        assertFalse(pmt.verify(wrongRoot, inclHashes))
     }
 }