From d6f82637ad6d89ab2b2c4bc656b3f384214150b9 Mon Sep 17 00:00:00 2001 From: Andras Slemmer Date: Wed, 7 Sep 2016 18:14:05 +0100 Subject: [PATCH 1/4] core: Fix tx resolution bug by topological sorting of downloaded transactions --- .../r3corda/core/contracts/DummyContract.kt | 12 +++--- .../protocols/ResolveTransactionsProtocol.kt | 40 ++++++++++++++++++- .../ResolveTransactionsProtocolTest.kt | 24 +++++++++++ 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/core/src/main/kotlin/com/r3corda/core/contracts/DummyContract.kt b/core/src/main/kotlin/com/r3corda/core/contracts/DummyContract.kt index a1c437bd18..95f6c868aa 100644 --- a/core/src/main/kotlin/com/r3corda/core/contracts/DummyContract.kt +++ b/core/src/main/kotlin/com/r3corda/core/contracts/DummyContract.kt @@ -54,14 +54,16 @@ class DummyContract : Contract { return TransactionType.General.Builder(notary = notary).withItems(state, Command(Commands.Create(), owner.party.owningKey)) } - fun move(prior: StateAndRef, newOwner: PublicKey): TransactionBuilder { - val priorState = prior.state.data + fun move(prior: StateAndRef, newOwner: PublicKey) = move(listOf(prior), newOwner) + fun move(priors: List>, newOwner: PublicKey): TransactionBuilder { + require(priors.size > 0) + val priorState = priors[0].state.data val (cmd, state) = priorState.withNewOwner(newOwner) - return TransactionType.General.Builder(notary = prior.state.notary).withItems( - /* INPUT */ prior, + return TransactionType.General.Builder(notary = priors[0].state.notary).withItems( + /* INPUTS */ *priors.toTypedArray(), /* COMMAND */ Command(cmd, priorState.owner), /* OUTPUT */ state ) } } -} \ No newline at end of file +} diff --git a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt index ea1c90607b..267013bba3 100644 --- a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt +++ b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt @@ -32,6 +32,44 @@ class ResolveTransactionsProtocol(private val txHashes: Set, companion object { private fun dependencyIDs(wtx: WireTransaction) = wtx.inputs.map { it.txhash }.toSet() + + private fun topologicalSort(transactions: Iterable): List { + + // Construct txhash -> dependent-txhashes map + val forwardGraph = HashMap>() + transactions.forEach { tx -> + tx.tx.inputs.forEach { input -> + forwardGraph.getOrPut(input.txhash) { HashSet() }.add(tx.id) + } + } + + // Construct txhash -> tx map + val transactionMap = HashMap() + transactions.forEach { transactionMap[it.tx.id] = it } + + val visited = HashSet() + val result = LinkedList() + + fun visit(transactionHash: SecureHash) { + if (transactionHash in visited) { + return + } + visited.add(transactionHash) + forwardGraph[transactionHash]?.forEach { + visit(it) + } + result.addFirst(transactionMap[transactionHash]!!) + } + + transactions.forEach { + visit(it.tx.id) + } + + require(result.size == transactionMap.size) + require(visited.size == transactionMap.size) + return result + } + } class ExcessivelyLargeTransactionGraph() : Exception() @@ -60,7 +98,7 @@ class ResolveTransactionsProtocol(private val txHashes: Set, @Suspendable override fun call(): List { - val newTxns: Iterable = downloadDependencies(txHashes) + val newTxns: Iterable = topologicalSort(downloadDependencies(txHashes)) // For each transaction, verify it and insert it into the database. As we are iterating over them in a // depth-first order, we should not encounter any verification failures due to missing data. If we fail diff --git a/core/src/test/kotlin/com/r3corda/core/protocols/ResolveTransactionsProtocolTest.kt b/core/src/test/kotlin/com/r3corda/core/protocols/ResolveTransactionsProtocolTest.kt index bf766a2998..43ae3d36e5 100644 --- a/core/src/test/kotlin/com/r3corda/core/protocols/ResolveTransactionsProtocolTest.kt +++ b/core/src/test/kotlin/com/r3corda/core/protocols/ResolveTransactionsProtocolTest.kt @@ -97,6 +97,30 @@ class ResolveTransactionsProtocolTest { } } + @Test + fun `triangle of transactions resolves fine`() { + val stx1 = makeTransactions().first + + val stx2 = DummyContract.move(stx1.tx.outRef(0), MINI_CORP_PUBKEY).run { + signWith(MEGA_CORP_KEY) + signWith(DUMMY_NOTARY_KEY) + toSignedTransaction() + } + + val stx3 = DummyContract.move(listOf(stx1.tx.outRef(0), stx2.tx.outRef(0)), MINI_CORP_PUBKEY).run { + signWith(MEGA_CORP_KEY) + signWith(DUMMY_NOTARY_KEY) + toSignedTransaction() + } + + a.services.recordTransactions(stx2, stx3) + + val p = ResolveTransactionsProtocol(setOf(stx3.id), a.info.identity) + val future = b.services.startProtocol("resolve", p) + net.runNetwork() + future.get() + } + @Test fun attachment() { val id = a.services.storageService.attachments.importAttachment("Some test file".toByteArray().opaque().open()) From 5596ced5c3e3f59b1d73d896fab945cbf0e75cff Mon Sep 17 00:00:00 2001 From: Andras Slemmer Date: Thu, 8 Sep 2016 10:04:18 +0100 Subject: [PATCH 2/4] core: Make topological sort more efficient --- .../protocols/ResolveTransactionsProtocol.kt | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt index 267013bba3..456a7dd60f 100644 --- a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt +++ b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt @@ -33,40 +33,36 @@ class ResolveTransactionsProtocol(private val txHashes: Set, companion object { private fun dependencyIDs(wtx: WireTransaction) = wtx.inputs.map { it.txhash }.toSet() - private fun topologicalSort(transactions: Iterable): List { + private fun topologicalSort(transactions: Collection): List { // Construct txhash -> dependent-txhashes map - val forwardGraph = HashMap>() + val forwardGraph = HashMap>() transactions.forEach { tx -> tx.tx.inputs.forEach { input -> - forwardGraph.getOrPut(input.txhash) { HashSet() }.add(tx.id) + forwardGraph.getOrPut(input.txhash) { HashSet() }.add(tx) } } - // Construct txhash -> tx map - val transactionMap = HashMap() - transactions.forEach { transactionMap[it.tx.id] = it } + val visited = HashSet(transactions.size) + val result = ArrayList(transactions.size) - val visited = HashSet() - val result = LinkedList() - - fun visit(transactionHash: SecureHash) { - if (transactionHash in visited) { + fun visit(transaction: SignedTransaction) { + if (transaction.id in visited) { return } - visited.add(transactionHash) - forwardGraph[transactionHash]?.forEach { + visited.add(transaction.id) + forwardGraph[transaction.id]?.forEach { visit(it) } - result.addFirst(transactionMap[transactionHash]!!) + result.add(transaction) } transactions.forEach { - visit(it.tx.id) + visit(it) } - require(result.size == transactionMap.size) - require(visited.size == transactionMap.size) + require(result.size == transactions.size) + result.reverse() return result } @@ -134,7 +130,7 @@ class ResolveTransactionsProtocol(private val txHashes: Set, override val topic: String get() = throw UnsupportedOperationException() @Suspendable - private fun downloadDependencies(depsToCheck: Set): List { + private fun downloadDependencies(depsToCheck: Set): Collection { // Maintain a work queue of all hashes to load/download, initialised with our starting set. Then do a breadth // first traversal across the dependency graph. // @@ -184,7 +180,7 @@ class ResolveTransactionsProtocol(private val txHashes: Set, throw ExcessivelyLargeTransactionGraph() } - return resultQ.values.reversed() + return resultQ.values } /** From 8e6198cf91336c6223dd01a67965894169abdeb4 Mon Sep 17 00:00:00 2001 From: Andras Slemmer Date: Thu, 8 Sep 2016 10:15:59 +0100 Subject: [PATCH 3/4] core: Make topological sort result deterministic --- .../com/r3corda/protocols/ResolveTransactionsProtocol.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt index 456a7dd60f..4230e4899f 100644 --- a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt +++ b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt @@ -39,7 +39,8 @@ class ResolveTransactionsProtocol(private val txHashes: Set, val forwardGraph = HashMap>() transactions.forEach { tx -> tx.tx.inputs.forEach { input -> - forwardGraph.getOrPut(input.txhash) { HashSet() }.add(tx) + // Note that we use a LinkedHashSet here to make the traversal deterministic (as long as the input list is) + forwardGraph.getOrPut(input.txhash) { LinkedHashSet() }.add(tx) } } From fcd477e332c342e51cc82c742aa79915a8bfd449 Mon Sep 17 00:00:00 2001 From: Andras Slemmer Date: Thu, 8 Sep 2016 10:17:00 +0100 Subject: [PATCH 4/4] core: Address topo sort style suggestions --- .../protocols/ResolveTransactionsProtocol.kt | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt index 4230e4899f..bc1b7a25c8 100644 --- a/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt +++ b/core/src/main/kotlin/com/r3corda/protocols/ResolveTransactionsProtocol.kt @@ -34,8 +34,7 @@ class ResolveTransactionsProtocol(private val txHashes: Set, private fun dependencyIDs(wtx: WireTransaction) = wtx.inputs.map { it.txhash }.toSet() private fun topologicalSort(transactions: Collection): List { - - // Construct txhash -> dependent-txhashes map + // Construct txhash -> dependent-txs map val forwardGraph = HashMap>() transactions.forEach { tx -> tx.tx.inputs.forEach { input -> @@ -48,22 +47,21 @@ class ResolveTransactionsProtocol(private val txHashes: Set, val result = ArrayList(transactions.size) fun visit(transaction: SignedTransaction) { - if (transaction.id in visited) { - return + if (transaction.id !in visited) { + visited.add(transaction.id) + forwardGraph[transaction.id]?.forEach { + visit(it) + } + result.add(transaction) } - visited.add(transaction.id) - forwardGraph[transaction.id]?.forEach { - visit(it) - } - result.add(transaction) } transactions.forEach { visit(it) } - require(result.size == transactions.size) result.reverse() + require(result.size == transactions.size) return result }