Remove createTwoNodes()

Remove `createTwoNodes()` from mock network as its behaviour is inconsistent with creating a set of nodes. `createSomeNodes()` is generally a better fit and creates a network map and notary, and registers all nodes. Where that's not the intention, it's acceptable to manually create each node.

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

@@ -26,9 +26,9 @@ class CashExitFlowTests {
 
     @Before
     fun start() {
-        val nodes = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(1)
-        notaryNode = nodes.first
+        notaryNode = nodes.notaryNode
-        bankOfCordaNode = nodes.second
+        bankOfCordaNode = nodes.partyNodes[0]
         notary = notaryNode.info.notaryIdentity
         bankOfCorda = bankOfCordaNode.info.legalIdentity
 

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

@@ -24,9 +24,9 @@ class CashIssueFlowTests {
 
     @Before
     fun start() {
-        val nodes = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(1)
-        notaryNode = nodes.first
+        notaryNode = nodes.notaryNode
-        bankOfCordaNode = nodes.second
+        bankOfCordaNode = nodes.partyNodes[0]
         notary = notaryNode.info.notaryIdentity
         bankOfCorda = bankOfCordaNode.info.legalIdentity
 

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

@@ -31,14 +31,12 @@ class CashPaymentFlowTests {
 
     @Before
     fun start() {
-        val nodes = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(1)
-        notaryNode = nodes.first
+        notaryNode = nodes.notaryNode
-        bankOfCordaNode = nodes.second
+        bankOfCordaNode = nodes.partyNodes[0]
         notary = notaryNode.info.notaryIdentity
         bankOfCorda = bankOfCordaNode.info.legalIdentity
 
-        notaryNode.services.identityService.registerIdentity(bankOfCordaNode.info.legalIdentityAndCert)
-        bankOfCordaNode.services.identityService.registerIdentity(notaryNode.info.legalIdentityAndCert)
         val future = bankOfCordaNode.services.startFlow(CashIssueFlow(initialBalance, ref,
                 bankOfCorda,
                 notary)).resultFuture

node/src/test/kotlin/net/corda/node/messaging/AttachmentTests.kt

@@ -55,7 +55,9 @@ class AttachmentTests {
 
     @Test
     fun `download and store`() {
-        val (n0, n1) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(2)
+        val n0 = nodes.partyNodes[0]
+        val n1 = nodes.partyNodes[1]
 
         // Insert an attachment into node zero's store directly.
         val id = n0.database.transaction {
@@ -84,7 +86,9 @@ class AttachmentTests {
 
     @Test
     fun `missing`() {
-        val (n0, n1) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(2)
+        val n0 = nodes.partyNodes[0]
+        val n1 = nodes.partyNodes[1]
 
         // Get node one to fetch a non-existent attachment.
         val hash = SecureHash.randomSHA256()

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

@@ -1,6 +1,7 @@
 package net.corda.node.services.network
 
 import com.google.common.util.concurrent.ListenableFuture
+import net.corda.core.crypto.random63BitValue
 import net.corda.core.getOrThrow
 import net.corda.core.messaging.SingleMessageRecipient
 import net.corda.core.node.NodeInfo
@@ -19,6 +20,7 @@ import net.corda.node.services.network.NetworkMapService.Companion.PUSH_TOPIC
 import net.corda.node.services.network.NetworkMapService.Companion.QUERY_TOPIC
 import net.corda.node.services.network.NetworkMapService.Companion.REGISTER_TOPIC
 import net.corda.node.services.network.NetworkMapService.Companion.SUBSCRIPTION_TOPIC
+import net.corda.node.services.transactions.SimpleNotaryService
 import net.corda.node.utilities.AddOrRemove
 import net.corda.node.utilities.AddOrRemove.ADD
 import net.corda.node.utilities.AddOrRemove.REMOVE
@@ -51,10 +53,8 @@ abstract class AbstractNetworkMapServiceTest<out S : AbstractNetworkMapService>
     @Before
     fun setup() {
         mockNet = MockNetwork(defaultFactory = nodeFactory)
-        mockNet.createTwoNodes(firstNodeName = DUMMY_MAP.name, secondNodeName = ALICE.name).apply {
+        mapServiceNode = mockNet.createNode(null, -1, nodeFactory, true, DUMMY_MAP.name, null, BigInteger.valueOf(random63BitValue()), ServiceInfo(NetworkMapService.type), ServiceInfo(SimpleNotaryService.type))
-            mapServiceNode = first
+        alice = mockNet.createNode(mapServiceNode.network.myAddress, -1, nodeFactory, true, ALICE.name)
-            alice = second
-        }
         mockNet.runNetwork()
         lastSerial = System.currentTimeMillis()
     }

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

@@ -21,7 +21,9 @@ class InMemoryNetworkMapCacheTest {
 
     @Test
     fun registerWithNetwork() {
-        val (n0, n1) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(1)
+        val n0 = nodes.mapNode
+        val n1 = nodes.partyNodes[0]
         val future = n1.services.networkMapCache.addMapService(n1.network, n0.network.myAddress, false, null)
         mockNet.runNetwork()
         future.getOrThrow()
@@ -48,7 +50,9 @@ class InMemoryNetworkMapCacheTest {
 
     @Test
     fun `getNodeByLegalIdentity`() {
-        val (n0, n1) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(1)
+        val n0 = nodes.mapNode
+        val n1 = nodes.partyNodes[0]
         val node0Cache: NetworkMapCache = n0.services.networkMapCache
         val expected = n1.info
 

node/src/test/kotlin/net/corda/node/services/persistence/DataVendingServiceTests.kt

@@ -42,7 +42,9 @@ class DataVendingServiceTests {
 
     @Test
     fun `notify of transaction`() {
-        val (vaultServiceNode, registerNode) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(2)
+        val vaultServiceNode = nodes.partyNodes[0]
+        val registerNode = nodes.partyNodes[1]
         val beneficiary = vaultServiceNode.info.legalIdentity
         val deposit = registerNode.info.legalIdentity.ref(1)
         mockNet.runNetwork()
@@ -70,7 +72,9 @@ class DataVendingServiceTests {
      */
     @Test
     fun `notify failure`() {
-        val (vaultServiceNode, registerNode) = mockNet.createTwoNodes()
+        val nodes = mockNet.createSomeNodes(2)
+        val vaultServiceNode = nodes.partyNodes[0]
+        val registerNode = nodes.partyNodes[1]
         val beneficiary = vaultServiceNode.info.legalIdentity
         val deposit = MEGA_CORP.ref(1)
         mockNet.runNetwork()

node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt

@@ -31,6 +31,7 @@ import net.corda.core.utilities.unwrap
 import net.corda.flows.CashIssueFlow
 import net.corda.flows.CashPaymentFlow
 import net.corda.node.internal.InitiatedFlowFactory
+import net.corda.node.services.network.NetworkMapService
 import net.corda.node.services.persistence.checkpoints
 import net.corda.node.services.transactions.ValidatingNotaryService
 import net.corda.testing.*
@@ -71,19 +72,27 @@ class FlowFrameworkTests {
 
     @Before
     fun start() {
-        val nodes = mockNet.createTwoNodes()
+        node1 = mockNet.createNode(advertisedServices = ServiceInfo(NetworkMapService.type))
-        node1 = nodes.first
+        node2 = mockNet.createNode(networkMapAddress = node1.network.myAddress)
-        node2 = nodes.second
+        // We intentionally create our own notary and ignore the one provided by the network
         val notaryKeyPair = generateKeyPair()
         val notaryService = ServiceInfo(ValidatingNotaryService.type, getTestX509Name("notary-service-2000"))
         val overrideServices = mapOf(Pair(notaryService, notaryKeyPair))
         // Note that these notaries don't operate correctly as they don't share their state. They are only used for testing
         // service addressing.
-        notary1 = mockNet.createNotaryNode(networkMapAddr = node1.network.myAddress, overrideServices = overrideServices, serviceName = notaryService.name)
+        notary1 = mockNet.createNotaryNode(networkMapAddress = node1.network.myAddress, overrideServices = overrideServices, serviceName = notaryService.name)
-        notary2 = mockNet.createNotaryNode(networkMapAddr = node1.network.myAddress, overrideServices = overrideServices, serviceName = notaryService.name)
+        notary2 = mockNet.createNotaryNode(networkMapAddress = node1.network.myAddress, overrideServices = overrideServices, serviceName = notaryService.name)
 
         mockNet.messagingNetwork.receivedMessages.toSessionTransfers().forEach { sessionTransfers += it }
         mockNet.runNetwork()
+
+        // We don't create a network map, so manually handle registrations
+        val nodes = listOf(node1, node2, notary1, notary2)
+        nodes.forEach { node ->
+            nodes.map { it.services.myInfo.legalIdentityAndCert }.forEach { identity ->
+                node.services.identityService.registerIdentity(identity)
+            }
+        }
     }
 
     @After
@@ -327,8 +336,9 @@ class FlowFrameworkTests {
                 anonymous = false))
         // We pay a couple of times, the notary picking should go round robin
         for (i in 1..3) {
-            node1.services.startFlow(CashPaymentFlow(500.DOLLARS, node2.info.legalIdentity, anonymous = false))
+            val flow = node1.services.startFlow(CashPaymentFlow(500.DOLLARS, node2.info.legalIdentity, anonymous = false))
             mockNet.runNetwork()
+            flow.resultFuture.getOrThrow()
         }
         val endpoint = mockNet.messagingNetwork.endpoint(notary1.network.myAddress as InMemoryMessagingNetwork.PeerHandle)!!
         val party1Info = notary1.services.networkMapCache.getPartyInfo(notary1.info.notaryIdentity)!!

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

@@ -331,27 +331,6 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
         }
     }
 
-    // TODO: Move this to using createSomeNodes which doesn't conflate network services with network users.
-    /**
-     * Sets up a two node network, in which the first node runs network map and notary services and the other
-     * doesn't.
-     */
-    fun createTwoNodes(firstNodeName: X500Name? = null,
-                       secondNodeName: X500Name? = null,
-                       nodeFactory: Factory = defaultFactory,
-                       notaryKeyPair: KeyPair? = null): Pair<MockNode, MockNode> {
-        require(nodes.isEmpty())
-        val notaryServiceInfo = ServiceInfo(SimpleNotaryService.type)
-        val notaryOverride = if (notaryKeyPair != null)
-            mapOf(Pair(notaryServiceInfo, notaryKeyPair))
-        else
-            null
-        return Pair(
-                createNode(null, -1, nodeFactory, true, firstNodeName, notaryOverride, BigInteger.valueOf(random63BitValue()), ServiceInfo(NetworkMapService.type), notaryServiceInfo),
-                createNode(nodes[0].network.myAddress, -1, nodeFactory, true, secondNodeName)
-        )
-    }
-
     /**
      * A bundle that separates the generic user nodes and service-providing nodes. A real network might not be so
      * clearly separated, but this is convenient for testing.
@@ -384,18 +363,18 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
         return BasketOfNodes(nodes, notaryNode, mapNode)
     }
 
-    fun createNotaryNode(networkMapAddr: SingleMessageRecipient? = null,
+    fun createNotaryNode(networkMapAddress: SingleMessageRecipient? = null,
                          legalName: X500Name? = null,
                          overrideServices: Map<ServiceInfo, KeyPair>? = null,
                          serviceName: X500Name? = null): MockNode {
-        return createNode(networkMapAddr, -1, defaultFactory, true, legalName, overrideServices, BigInteger.valueOf(random63BitValue()),
+        return createNode(networkMapAddress, -1, defaultFactory, true, legalName, overrideServices, BigInteger.valueOf(random63BitValue()),
                 ServiceInfo(NetworkMapService.type), ServiceInfo(ValidatingNotaryService.type, serviceName))
     }
 
-    fun createPartyNode(networkMapAddr: SingleMessageRecipient,
+    fun createPartyNode(networkMapAddress: SingleMessageRecipient,
                         legalName: X500Name? = null,
                         overrideServices: Map<ServiceInfo, KeyPair>? = null): MockNode {
-        return createNode(networkMapAddr, -1, defaultFactory, true, legalName, overrideServices)
+        return createNode(networkMapAddress, -1, defaultFactory, true, legalName, overrideServices)
     }
 
     @Suppress("unused") // This is used from the network visualiser tool.