Clean up transaction key flow

* Identities returned from TxKeyFlow were backwards, meaning keys were incorrectly assigned to the remote and local identities. Added unit test covering this case and corrected the flow logic.
* Rename TxKeyFlow to TransactionKeyFlow
* Correct registration of transaction key flows
* Move TransactionKeyFlow.Provider into CoreFlowHandlers
* Move TransactionKeyFlow.Request up to the top level class instead of being a class within an object.
* Remove AbstractIdentityFlow and move the validation logic into individual flows to make it clearer that it's registering the received identities.
* Cash flows now return the recipient identity instead of full identity lookup, as this is what
the caller actually needs and simplifies a lot of cases.

core/src/main/kotlin/net/corda/flows/TransactionKeyFlow.kt

@@ -0,0 +1,50 @@
+package net.corda.flows
+
+import co.paralleluniverse.fibers.Suspendable
+import net.corda.core.flows.FlowLogic
+import net.corda.core.flows.InitiatingFlow
+import net.corda.core.flows.StartableByRPC
+import net.corda.core.identity.Party
+import net.corda.core.utilities.ProgressTracker
+import net.corda.core.utilities.unwrap
+
+/**
+ * Very basic flow which exchanges transaction key and certificate paths between two parties in a transaction.
+ * This is intended for use as a subflow of another flow.
+ */
+@StartableByRPC
+@InitiatingFlow
+class TransactionKeyFlow(val otherSide: Party,
+                         val revocationEnabled: Boolean,
+                         override val progressTracker: ProgressTracker) : FlowLogic<LinkedHashMap<Party, AnonymisedIdentity>>() {
+    constructor(otherSide: Party) : this(otherSide, false, tracker())
+
+    companion object {
+        object AWAITING_KEY : ProgressTracker.Step("Awaiting key")
+
+        fun tracker() = ProgressTracker(AWAITING_KEY)
+        fun validateIdentity(otherSide: Party, anonymousOtherSide: AnonymisedIdentity): AnonymisedIdentity {
+            require(anonymousOtherSide.certificate.subject == otherSide.name)
+            return anonymousOtherSide
+        }
+    }
+
+    @Suspendable
+    override fun call(): LinkedHashMap<Party, AnonymisedIdentity> {
+        progressTracker.currentStep = AWAITING_KEY
+        val legalIdentityAnonymous = serviceHub.keyManagementService.freshKeyAndCert(serviceHub.myInfo.legalIdentityAndCert, revocationEnabled)
+        serviceHub.identityService.registerAnonymousIdentity(legalIdentityAnonymous.identity, serviceHub.myInfo.legalIdentity, legalIdentityAnonymous.certPath)
+
+        // Special case that if we're both parties, a single identity is generated
+        val identities = LinkedHashMap<Party, AnonymisedIdentity>()
+        if (otherSide == serviceHub.myInfo.legalIdentity) {
+            identities.put(otherSide, legalIdentityAnonymous)
+        } else {
+            val otherSideAnonymous = sendAndReceive<AnonymisedIdentity>(otherSide, legalIdentityAnonymous).unwrap { validateIdentity(otherSide, it) }
+            identities.put(serviceHub.myInfo.legalIdentity, legalIdentityAnonymous)
+            identities.put(otherSide, otherSideAnonymous)
+        }
+        return identities
+    }
+
+}

core/src/main/kotlin/net/corda/flows/TxKeyFlow.kt

@@ -1,91 +0,0 @@
-package net.corda.flows
-
-import co.paralleluniverse.fibers.Suspendable
-import net.corda.core.flows.FlowLogic
-import net.corda.core.flows.InitiatedBy
-import net.corda.core.flows.InitiatingFlow
-import net.corda.core.flows.StartableByRPC
-import net.corda.core.identity.Party
-import net.corda.core.serialization.CordaSerializable
-import net.corda.core.utilities.ProgressTracker
-import net.corda.core.utilities.unwrap
-
-/**
- * Very basic flow which exchanges transaction key and certificate paths between two parties in a transaction.
- * This is intended for use as a subflow of another flow.
- */
-object TxKeyFlow {
-    abstract class AbstractIdentityFlow<out T>(val otherSide: Party, val revocationEnabled: Boolean): FlowLogic<T>() {
-        fun validateIdentity(untrustedIdentity: AnonymisedIdentity): AnonymisedIdentity {
-            val (certPath, theirCert, txIdentity) = untrustedIdentity
-            if (theirCert.subject == otherSide.name) {
-                serviceHub.identityService.registerAnonymousIdentity(txIdentity, otherSide, certPath)
-                return AnonymisedIdentity(certPath, theirCert, txIdentity)
-            } else
-                throw IllegalStateException("Expected certificate subject to be ${otherSide.name} but found ${theirCert.subject}")
-        }
-    }
-
-    @StartableByRPC
-    @InitiatingFlow
-    class Requester(otherSide: Party,
-                    override val progressTracker: ProgressTracker) : AbstractIdentityFlow<TxIdentities>(otherSide, false) {
-        constructor(otherSide: Party) : this(otherSide, tracker())
-        companion object {
-            object AWAITING_KEY : ProgressTracker.Step("Awaiting key")
-
-            fun tracker() = ProgressTracker(AWAITING_KEY)
-        }
-
-        @Suspendable
-        override fun call(): TxIdentities {
-            progressTracker.currentStep = AWAITING_KEY
-            val myIdentity = serviceHub.keyManagementService.freshKeyAndCert(serviceHub.myInfo.legalIdentityAndCert, revocationEnabled)
-            serviceHub.identityService.registerAnonymousIdentity(myIdentity.identity, serviceHub.myInfo.legalIdentity, myIdentity.certPath)
-
-            // Special case that if we're both parties, a single identity is generated
-            return if (otherSide == serviceHub.myInfo.legalIdentity) {
-                TxIdentities(Pair(otherSide, myIdentity))
-            } else {
-                val theirIdentity = receive<AnonymisedIdentity>(otherSide).unwrap { validateIdentity(it) }
-                send(otherSide, myIdentity)
-                TxIdentities(Pair(otherSide, myIdentity),
-                        Pair(serviceHub.myInfo.legalIdentity, theirIdentity))
-            }
-        }
-    }
-
-    /**
-     * Flow which waits for a key request from a counterparty, generates a new key and then returns it to the
-     * counterparty and as the result from the flow.
-     */
-    @InitiatedBy(Requester::class)
-    class Provider(otherSide: Party) : AbstractIdentityFlow<TxIdentities>(otherSide, false) {
-        companion object {
-            object SENDING_KEY : ProgressTracker.Step("Sending key")
-        }
-
-        override val progressTracker: ProgressTracker = ProgressTracker(SENDING_KEY)
-
-        @Suspendable
-        override fun call(): TxIdentities {
-            val revocationEnabled = false
-            progressTracker.currentStep = SENDING_KEY
-            val myIdentity = serviceHub.keyManagementService.freshKeyAndCert(serviceHub.myInfo.legalIdentityAndCert, revocationEnabled)
-            send(otherSide, myIdentity)
-            val theirIdentity = receive<AnonymisedIdentity>(otherSide).unwrap { validateIdentity(it) }
-            return TxIdentities(Pair(otherSide, myIdentity),
-                    Pair(serviceHub.myInfo.legalIdentity, theirIdentity))
-        }
-    }
-
-    @CordaSerializable
-    data class TxIdentities(val identities: List<Pair<Party, AnonymisedIdentity>>) {
-        constructor(vararg identities: Pair<Party, AnonymisedIdentity>) : this(identities.toList())
-        init {
-            require(identities.size == identities.map { it.first }.toSet().size) { "Identities must be unique: ${identities.map { it.first }}" }
-        }
-        fun forParty(party: Party): AnonymisedIdentity = identities.single { it.first == party }.second
-        fun toMap(): Map<Party, AnonymisedIdentity> = this.identities.toMap()
-    }
-}

core/src/test/kotlin/net/corda/flows/TransactionKeyFlowTests.kt

@@ -10,9 +10,11 @@ import net.corda.testing.node.MockNetwork
 import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
 
-class TxKeyFlowTests {
+class TransactionKeyFlowTests {
     lateinit var mockNet: MockNetwork
 
     @Before
@@ -37,7 +39,7 @@ class TxKeyFlowTests {
         bobNode.services.identityService.registerIdentity(notaryNode.info.legalIdentityAndCert)
 
         // Run the flows
-        val requesterFlow = aliceNode.services.startFlow(TxKeyFlow.Requester(bob))
+        val requesterFlow = aliceNode.services.startFlow(TransactionKeyFlow(bob))
 
         // Get the results
         val actual: Map<Party, AnonymisedIdentity> = requesterFlow.resultFuture.getOrThrow().toMap()
@@ -47,5 +49,15 @@ class TxKeyFlowTests {
         val bobAnonymousIdentity = actual[bob] ?: throw IllegalStateException()
         assertNotEquals<AbstractParty>(alice, aliceAnonymousIdentity.identity)
         assertNotEquals<AbstractParty>(bob, bobAnonymousIdentity.identity)
+
+        // Verify that the anonymous identities look sane
+        assertEquals(alice.name, aliceAnonymousIdentity.certificate.subject)
+        assertEquals(bob.name, bobAnonymousIdentity.certificate.subject)
+
+        // Verify that the nodes have the right anonymous identities
+        assertTrue { aliceAnonymousIdentity.identity.owningKey in aliceNode.services.keyManagementService.keys }
+        assertTrue { bobAnonymousIdentity.identity.owningKey in bobNode.services.keyManagementService.keys }
+        assertFalse { aliceAnonymousIdentity.identity.owningKey in bobNode.services.keyManagementService.keys }
+        assertFalse { bobAnonymousIdentity.identity.owningKey in aliceNode.services.keyManagementService.keys }
     }
 }

finance/src/main/kotlin/net/corda/flows/AbstractCashFlow.kt

@@ -3,9 +3,8 @@ package net.corda.flows
 import co.paralleluniverse.fibers.Suspendable
 import net.corda.core.flows.FlowException
 import net.corda.core.flows.FlowLogic
-import net.corda.core.identity.AnonymousParty
+import net.corda.core.identity.AbstractParty
 import net.corda.core.identity.Party
-import net.corda.core.identity.PartyAndCertificate
 import net.corda.core.serialization.CordaSerializable
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.ProgressTracker
@@ -37,10 +36,12 @@ abstract class AbstractCashFlow<T>(override val progressTracker: ProgressTracker
      * Specialised flows for unit tests differ from this.
      *
      * @param stx the signed transaction.
-     * @param identities a mapping from the original identities of the parties to the anonymised equivalents.
+     * @param recipient the identity used for the other side of the transaction, where applicable (i.e. this is
+     * null for exit transactions). For anonymous transactions this is the confidential identity generated for the
+     * transaction, otherwise this is the well known identity.
      */
     @CordaSerializable
-    data class Result(val stx: SignedTransaction, val identities: TxKeyFlow.TxIdentities)
+    data class Result(val stx: SignedTransaction, val recipient: AbstractParty?)
 }
 
 class CashException(message: String, cause: Throwable) : FlowException(message, cause)

finance/src/main/kotlin/net/corda/flows/CashExitFlow.kt

@@ -70,6 +70,6 @@ class CashExitFlow(val amount: Amount<Currency>, val issueRef: OpaqueBytes, prog
         // Commit the transaction
         progressTracker.currentStep = FINALISING_TX
         finaliseTx(participants, tx, "Unable to notarise exit")
-        return Result(tx, TxKeyFlow.TxIdentities())
+        return Result(tx, null)
     }
 }

finance/src/main/kotlin/net/corda/flows/CashIssueFlow.kt

@@ -41,15 +41,11 @@ class CashIssueFlow(val amount: Amount<Currency>,
     override fun call(): AbstractCashFlow.Result {
         progressTracker.currentStep = GENERATING_ID
         val txIdentities = if (anonymous) {
-            subFlow(TxKeyFlow.Requester(recipient))
+            subFlow(TransactionKeyFlow(recipient))
         } else {
-            TxKeyFlow.TxIdentities(emptyList())
-        }
-        val anonymousRecipient = if (anonymous) {
-            txIdentities.forParty(recipient).identity
-        } else {
-            recipient
+            emptyMap<Party, AnonymisedIdentity>()
         }
+        val anonymousRecipient = txIdentities.get(recipient)?.identity ?: recipient
         progressTracker.currentStep = GENERATING_TX
         val builder: TransactionBuilder = TransactionType.General.Builder(notary = notary)
         val issuer = serviceHub.myInfo.legalIdentity.ref(issueRef)
@@ -58,6 +54,6 @@ class CashIssueFlow(val amount: Amount<Currency>,
         val tx = serviceHub.signInitialTransaction(builder, signers)
         progressTracker.currentStep = FINALISING_TX
         subFlow(FinalityFlow(tx))
-        return Result(tx, txIdentities)
+        return Result(tx, anonymousRecipient)
     }
 }

finance/src/main/kotlin/net/corda/flows/CashPaymentFlow.kt

@@ -35,15 +35,11 @@ open class CashPaymentFlow(
     override fun call(): AbstractCashFlow.Result {
         progressTracker.currentStep = GENERATING_ID
         val txIdentities = if (anonymous) {
-            subFlow(TxKeyFlow.Requester(recipient))
+            subFlow(TransactionKeyFlow(recipient))
         } else {
-            TxKeyFlow.TxIdentities(emptyList())
-        }
-        val anonymousRecipient = if (anonymous) {
-            txIdentities.forParty(recipient).identity
-        } else {
-            recipient
+            emptyMap<Party, AnonymisedIdentity>()
         }
+        val anonymousRecipient = txIdentities.get(recipient)?.identity ?: recipient
         progressTracker.currentStep = GENERATING_TX
         val builder: TransactionBuilder = TransactionType.General.Builder(null as Party?)
         // TODO: Have some way of restricting this to states the caller controls
@@ -62,6 +58,6 @@ open class CashPaymentFlow(
 
         progressTracker.currentStep = FINALISING_TX
         finaliseTx(setOf(recipient), tx, "Unable to notarise spend")
-        return Result(tx, txIdentities)
+        return Result(tx, anonymousRecipient)
     }
 }

finance/src/main/kotlin/net/corda/flows/IssuerFlow.kt

@@ -4,7 +4,6 @@ import co.paralleluniverse.fibers.Suspendable
 import net.corda.contracts.asset.Cash
 import net.corda.core.contracts.*
 import net.corda.core.flows.*
-import net.corda.core.identity.AbstractParty
 import net.corda.core.identity.Party
 import net.corda.core.serialization.CordaSerializable
 import net.corda.core.serialization.OpaqueBytes
@@ -47,17 +46,12 @@ object IssuerFlow {
             val issueRequest = IssuanceRequestState(amount, issueToParty, issueToPartyRef, anonymous)
             return sendAndReceive<AbstractCashFlow.Result>(issuerBankParty, issueRequest).unwrap { res ->
                 val tx = res.stx.tx
-                val recipient = if (anonymous) {
-                    res.identities.forParty(issueToParty).identity
-                } else {
-                    issueToParty
-                }
                 val expectedAmount = Amount(amount.quantity, Issued(issuerBankParty.ref(issueToPartyRef), amount.token))
                 val cashOutputs = tx.outputs
                         .map { it.data}
                         .filterIsInstance<Cash.State>()
-                        .filter { state -> state.owner == recipient }
-                require(cashOutputs.size == 1) { "Require a single cash output paying $recipient, found ${tx.outputs}" }
+                        .filter { state -> state.owner == res.recipient }
+                require(cashOutputs.size == 1) { "Require a single cash output paying ${res.recipient}, found ${tx.outputs}" }
                 require(cashOutputs.single().amount == expectedAmount) { "Require payment of $expectedAmount"}
                 res
             }

finance/src/test/kotlin/net/corda/flows/CashPaymentFlowTests.kt

@@ -32,7 +32,6 @@ class CashPaymentFlowTests {
         notary = notaryNode.info.notaryIdentity
         bankOfCorda = bankOfCordaNode.info.legalIdentity
 
-        notaryNode.registerInitiatedFlow(TxKeyFlow.Provider::class.java)
         notaryNode.services.identityService.registerIdentity(bankOfCordaNode.info.legalIdentityAndCert)
         bankOfCordaNode.services.identityService.registerIdentity(notaryNode.info.legalIdentityAndCert)
         val future = bankOfCordaNode.services.startFlow(CashIssueFlow(initialBalance, ref,
@@ -55,9 +54,9 @@ class CashPaymentFlowTests {
         val future = bankOfCordaNode.services.startFlow(CashPaymentFlow(expectedPayment,
                 payTo)).resultFuture
         mockNet.runNetwork()
-        val (paymentTx, identities) = future.getOrThrow()
+        val (paymentTx, receipient) = future.getOrThrow()
         val states = paymentTx.tx.outputs.map { it.data }.filterIsInstance<Cash.State>()
-        val paymentState: Cash.State = states.single { it.owner == identities.forParty(payTo).identity }
+        val paymentState: Cash.State = states.single { it.owner == receipient }
         val changeState: Cash.State = states.single { it != paymentState }
         assertEquals(expectedChange.`issued by`(bankOfCorda.ref(ref)), changeState.amount)
         assertEquals(expectedPayment.`issued by`(bankOfCorda.ref(ref)), paymentState.amount)

finance/src/test/kotlin/net/corda/flows/IssuerFlowTest.kt

@@ -43,7 +43,6 @@ class IssuerFlowTest {
 
         nodes.forEach { node ->
             nodes.map { it.info.legalIdentityAndCert }.forEach(node.services.identityService::registerIdentity)
-            node.registerInitiatedFlow(TxKeyFlow.Provider::class.java)
         }
     }
 

node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt

@@ -212,8 +212,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
                 findRPCFlows(scanResult)
             }
 
-            // TODO: Investigate having class path scanning find this flow
-            registerInitiatedFlow(TxKeyFlow.Provider::class.java)
             // TODO Remove this once the cash stuff is in its own CorDapp
             registerInitiatedFlow(IssuerFlow.Issuer::class.java)
 
@@ -413,6 +411,7 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         installCoreFlow(BroadcastTransactionFlow::class) { otherParty, _ -> NotifyTransactionHandler(otherParty) }
         installCoreFlow(NotaryChangeFlow::class) { otherParty, _ -> NotaryChangeHandler(otherParty) }
         installCoreFlow(ContractUpgradeFlow::class) { otherParty, _ -> ContractUpgradeHandler(otherParty) }
+        installCoreFlow(TransactionKeyFlow::class) { otherParty, _ -> TransactionKeyHandler(otherParty) }
     }
 
     /**

node/src/main/kotlin/net/corda/node/services/CoreFlowHandlers.kt

@@ -10,6 +10,7 @@ import net.corda.core.flows.FlowException
 import net.corda.core.flows.FlowLogic
 import net.corda.core.identity.Party
 import net.corda.core.transactions.SignedTransaction
+import net.corda.core.utilities.ProgressTracker
 import net.corda.core.utilities.unwrap
 import net.corda.flows.*
 
@@ -122,3 +123,24 @@ class ContractUpgradeHandler(otherSide: Party) : AbstractStateReplacementFlow.Ac
         ContractUpgradeFlow.verify(oldStateAndRef.state.data, expectedTx.outRef<ContractState>(0).state.data, expectedTx.commands.single())
     }
 }
+
+class TransactionKeyHandler(val otherSide: Party, val revocationEnabled: Boolean) : FlowLogic<Unit>() {
+    constructor(otherSide: Party) : this(otherSide, false)
+    companion object {
+        object SENDING_KEY : ProgressTracker.Step("Sending key")
+    }
+
+    override val progressTracker: ProgressTracker = ProgressTracker(SENDING_KEY)
+
+    @Suspendable
+    override fun call(): Unit {
+        val revocationEnabled = false
+        progressTracker.currentStep = SENDING_KEY
+        val legalIdentityAnonymous = serviceHub.keyManagementService.freshKeyAndCert(serviceHub.myInfo.legalIdentityAndCert, revocationEnabled)
+        val otherSideAnonymous = sendAndReceive<AnonymisedIdentity>(otherSide, legalIdentityAnonymous).unwrap { TransactionKeyFlow.validateIdentity(otherSide, it) }
+        val (certPath, theirCert, txIdentity) = otherSideAnonymous
+        // Validate then store their identity so that we can prove the key in the transaction is owned by the
+        // counterparty.
+        serviceHub.identityService.registerAnonymousIdentity(txIdentity, otherSide, certPath)
+    }
+}

node/src/test/kotlin/net/corda/node/services/network/InMemoryIdentityServiceTests.kt

@@ -7,7 +7,6 @@ import net.corda.core.identity.PartyAndCertificate
 import net.corda.core.node.services.IdentityService
 import net.corda.core.utilities.*
 import net.corda.flows.AnonymisedIdentity
-import net.corda.flows.TxKeyFlow
 import net.corda.node.services.identity.InMemoryIdentityService
 import net.corda.testing.ALICE_PUBKEY
 import net.corda.testing.BOB_PUBKEY

test-utils/src/main/kotlin/net/corda/testing/node/MockNode.kt

@@ -23,7 +23,7 @@ import net.corda.core.node.services.ServiceInfo
 import net.corda.core.utilities.DUMMY_NOTARY_KEY
 import net.corda.core.utilities.getTestPartyAndCertificate
 import net.corda.core.utilities.loggerFor
-import net.corda.flows.TxKeyFlow
+import net.corda.flows.TransactionKeyFlow
 import net.corda.node.internal.AbstractNode
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.identity.InMemoryIdentityService
@@ -301,8 +301,6 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
         val node = nodeFactory.create(config, this, networkMapAddress, advertisedServices.toSet(), id, overrideServices, entropyRoot)
         if (start) {
             node.setup().start()
-            // Register flows that are normally found via plugins
-            node.registerInitiatedFlow(TxKeyFlow.Provider::class.java)
             if (threadPerNode && networkMapAddress != null)
                 node.networkMapRegistrationFuture.getOrThrow()   // Block and wait for the node to register in the net map.
         }

tools/explorer/src/main/kotlin/net/corda/explorer/ExplorerSimulation.kt

@@ -18,7 +18,6 @@ import net.corda.core.node.services.ServiceInfo
 import net.corda.core.node.services.ServiceType
 import net.corda.core.serialization.OpaqueBytes
 import net.corda.core.success
-import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.ALICE
 import net.corda.core.utilities.BOB
 import net.corda.core.utilities.DUMMY_NOTARY