Removed the superfluous NetworkMapCacheImpl class (#3882)

NetworkMapCacheImpl and NetworkMapCacheBaseInternal interface merged into PersistentNetworkMapCache and NetworkMapCacheInternal interface respectively.

node/src/integration-test/kotlin/net/corda/node/services/messaging/ArtemisMessagingTest.kt

@@ -10,7 +10,6 @@ import net.corda.node.internal.configureDatabase
 import net.corda.node.services.config.FlowTimeoutConfiguration
 import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.config.configureWithDevSSLCertificate
-import net.corda.node.services.network.NetworkMapCacheImpl
 import net.corda.node.services.network.PersistentNetworkMapCache
 import net.corda.node.services.transactions.PersistentUniquenessProvider
 import net.corda.node.utilities.AffinityExecutor.ServiceAffinityExecutor
@@ -66,7 +65,7 @@ class ArtemisMessagingTest {
     private var messagingClient: P2PMessagingClient? = null
     private var messagingServer: ArtemisMessagingServer? = null
 
-    private lateinit var networkMapCache: NetworkMapCacheImpl
+    private lateinit var networkMapCache: PersistentNetworkMapCache
 
     @Before
     fun setUp() {
@@ -89,8 +88,7 @@ class ArtemisMessagingTest {
         }
         LogHelper.setLevel(PersistentUniquenessProvider::class)
         database = configureDatabase(makeTestDataSourceProperties(), DatabaseConfig(), { null }, { null })
-        val persistentNetworkMapCache = PersistentNetworkMapCache(database, ALICE_NAME).apply { start(emptyList()) }
+        networkMapCache = PersistentNetworkMapCache(database, rigorousMock(), ALICE_NAME).apply { start(emptyList()) }
-        networkMapCache = NetworkMapCacheImpl(persistentNetworkMapCache, rigorousMock(), database).apply { start() }
     }
 
     @After

node/src/integration-test/kotlin/net/corda/node/services/network/PersistentNetworkMapCacheTest.kt

@@ -5,6 +5,8 @@ import net.corda.core.serialization.serialize
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.node.internal.configureDatabase
 import net.corda.node.internal.schemas.NodeInfoSchemaV1
+import net.corda.node.services.identity.InMemoryIdentityService
+import net.corda.nodeapi.internal.DEV_ROOT_CA
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.testing.core.*
 import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties
@@ -27,7 +29,7 @@ class PersistentNetworkMapCacheTest {
 
     private var portCounter = 1000
     private val database = configureDatabase(makeTestDataSourceProperties(), DatabaseConfig(), { null }, { null })
-    private val charlieNetMapCache = PersistentNetworkMapCache(database, CHARLIE.name)
+    private val charlieNetMapCache = PersistentNetworkMapCache(database, InMemoryIdentityService(trustRoot = DEV_ROOT_CA.certificate), CHARLIE.name)
 
     @After
     fun cleanUp() {

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

@@ -53,7 +53,10 @@ import net.corda.node.services.keys.KeyManagementServiceInternal
 import net.corda.node.services.keys.PersistentKeyManagementService
 import net.corda.node.services.messaging.DeduplicationHandler
 import net.corda.node.services.messaging.MessagingService
-import net.corda.node.services.network.*
+import net.corda.node.services.network.NetworkMapClient
+import net.corda.node.services.network.NetworkMapUpdater
+import net.corda.node.services.network.NodeInfoWatcher
+import net.corda.node.services.network.PersistentNetworkMapCache
 import net.corda.node.services.persistence.*
 import net.corda.node.services.schema.NodeSchemaService
 import net.corda.node.services.statemachine.*
@@ -142,8 +145,7 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
         // TODO Break cyclic dependency
         identityService.database = database
     }
-    private val persistentNetworkMapCache = PersistentNetworkMapCache(database, configuration.myLegalName)
+    val networkMapCache = PersistentNetworkMapCache(database, identityService, configuration.myLegalName).tokenize()
-    val networkMapCache = NetworkMapCacheImpl(persistentNetworkMapCache, identityService, database).tokenize()
     val checkpointStorage = DBCheckpointStorage()
     @Suppress("LeakingThis")
     val transactionStorage = makeTransactionStorage(configuration.transactionCacheSizeBytes).tokenize()
@@ -253,7 +255,6 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
         identityService.start(trustRoot, listOf(identity.certificate, nodeCa))
         return database.use {
             it.transaction {
-                persistentNetworkMapCache.start(notaries = emptyList())
                 val (_, nodeInfoAndSigned) = updateNodeInfo(identity, identityKeyPair, publish = false)
                 nodeInfoAndSigned.nodeInfo
             }
@@ -265,8 +266,7 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
         log.info("Starting clearing of network map cache entries...")
         startDatabase()
         database.use {
-            persistentNetworkMapCache.start(notaries = emptyList())
+            networkMapCache.clearNetworkMapCache()
-            persistentNetworkMapCache.clearNetworkMapCache()
         }
     }
 
@@ -307,8 +307,7 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
         identityService.start(trustRoot, listOf(identity.certificate, nodeCa))
 
         val (keyPairs, nodeInfoAndSigned, myNotaryIdentity) = database.transaction {
-            persistentNetworkMapCache.start(netParams.notaries)
+            networkMapCache.start(netParams.notaries)
-            networkMapCache.start()
             updateNodeInfo(identity, identityKeyPair, publish = true)
         }
 

node/src/main/kotlin/net/corda/node/services/api/ServiceHubInternal.kt

@@ -25,9 +25,9 @@ import net.corda.node.services.persistence.AttachmentStorageInternal
 import net.corda.node.services.statemachine.ExternalEvent
 import net.corda.node.services.statemachine.FlowStateMachineImpl
 import net.corda.nodeapi.internal.persistence.CordaPersistence
+import java.security.PublicKey
 
-interface NetworkMapCacheInternal : NetworkMapCache, NetworkMapCacheBaseInternal
+interface NetworkMapCacheInternal : NetworkMapCache, NetworkMapCacheBase {
-interface NetworkMapCacheBaseInternal : NetworkMapCacheBase {
     val allNodeHashes: List<SecureHash>
 
     fun getNodeByHash(nodeHash: SecureHash): NodeInfo?

node/src/main/kotlin/net/corda/node/services/network/NetworkMapCacheImpl.kt

@@ -1,51 +0,0 @@
-package net.corda.node.services.network
-
-import net.corda.core.identity.AbstractParty
-import net.corda.core.node.NodeInfo
-import net.corda.core.node.services.IdentityService
-import net.corda.core.node.services.NetworkMapCache
-import net.corda.core.serialization.SingletonSerializeAsToken
-import net.corda.core.utilities.contextLogger
-import net.corda.node.services.api.NetworkMapCacheBaseInternal
-import net.corda.node.services.api.NetworkMapCacheInternal
-import net.corda.nodeapi.internal.persistence.CordaPersistence
-
-class NetworkMapCacheImpl(
-        private val networkMapCacheBase: NetworkMapCacheBaseInternal,
-        private val identityService: IdentityService,
-        private val database: CordaPersistence
-) : NetworkMapCacheBaseInternal by networkMapCacheBase, NetworkMapCacheInternal, SingletonSerializeAsToken() {
-    companion object {
-        private val logger = contextLogger()
-    }
-
-    fun start() {
-        for (nodeInfo in networkMapCacheBase.allNodes) {
-            for (identity in nodeInfo.legalIdentitiesAndCerts) {
-                identityService.verifyAndRegisterIdentity(identity)
-            }
-        }
-        networkMapCacheBase.changed.subscribe { mapChange ->
-            // TODO how should we handle network map removal
-            if (mapChange is NetworkMapCache.MapChange.Added) {
-                mapChange.node.legalIdentitiesAndCerts.forEach {
-                    try {
-                        identityService.verifyAndRegisterIdentity(it)
-                    } catch (ignore: Exception) {
-                        // Log a warning to indicate node info is not added to the network map cache.
-                        logger.warn("Node info for :'${it.name}' is not added to the network map due to verification error.")
-                    }
-                }
-            }
-        }
-    }
-
-    override fun getNodeByLegalIdentity(party: AbstractParty): NodeInfo? {
-        return database.transaction {
-            val wellKnownParty = identityService.wellKnownPartyFromAnonymous(party)
-            wellKnownParty?.let {
-                getNodesByLegalIdentityKey(it.owningKey).firstOrNull()
-            }
-        }
-    }
-}

node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt

@@ -3,6 +3,7 @@ package net.corda.node.services.network
 import net.corda.core.concurrent.CordaFuture
 import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.toStringShort
+import net.corda.core.identity.AbstractParty
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.identity.PartyAndCertificate
@@ -11,15 +12,17 @@ import net.corda.core.internal.concurrent.openFuture
 import net.corda.core.messaging.DataFeed
 import net.corda.core.node.NodeInfo
 import net.corda.core.node.NotaryInfo
+import net.corda.core.node.services.IdentityService
 import net.corda.core.node.services.NetworkMapCache.MapChange
 import net.corda.core.node.services.PartyInfo
 import net.corda.core.serialization.SingletonSerializeAsToken
 import net.corda.core.serialization.serialize
 import net.corda.core.utilities.NetworkHostAndPort
+import net.corda.core.utilities.Try
 import net.corda.core.utilities.contextLogger
 import net.corda.core.utilities.debug
 import net.corda.node.internal.schemas.NodeInfoSchemaV1
-import net.corda.node.services.api.NetworkMapCacheBaseInternal
+import net.corda.node.services.api.NetworkMapCacheInternal
 import net.corda.node.utilities.NonInvalidatingCache
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.bufferUntilDatabaseCommit
@@ -34,7 +37,8 @@ import javax.annotation.concurrent.ThreadSafe
 /** Database-based network map cache. */
 @ThreadSafe
 open class PersistentNetworkMapCache(private val database: CordaPersistence,
-                                     private val myLegalName: CordaX500Name) : SingletonSerializeAsToken(), NetworkMapCacheBaseInternal {
+                                     private val identityService: IdentityService,
+                                     private val myLegalName: CordaX500Name) : NetworkMapCacheInternal, SingletonSerializeAsToken() {
     companion object {
         private val logger = contextLogger()
     }
@@ -78,6 +82,15 @@ open class PersistentNetworkMapCache(private val database: CordaPersistence,
         }
     }
 
+    override fun getNodeByLegalIdentity(party: AbstractParty): NodeInfo? {
+        return database.transaction {
+            val wellKnownParty = identityService.wellKnownPartyFromAnonymous(party)
+            wellKnownParty?.let {
+                getNodesByLegalIdentityKey(it.owningKey).firstOrNull()
+            }
+        }
+    }
+
     override fun getNodeByHash(nodeHash: SecureHash): NodeInfo? {
         return database.transaction {
             val builder = session.criteriaBuilder
@@ -160,6 +173,7 @@ open class PersistentNetworkMapCache(private val database: CordaPersistence,
             val previousNode = getNodesByLegalIdentityKey(node.legalIdentities.first().owningKey).firstOrNull()
             if (previousNode == null) {
                 logger.info("No previous node found")
+                if (!verifyAndRegisterIdentities(node)) return
                 database.transaction {
                     updateInfoDB(node, session)
                     changePublisher.onNext(MapChange.Added(node))
@@ -169,6 +183,8 @@ open class PersistentNetworkMapCache(private val database: CordaPersistence,
                 return
             } else if (previousNode != node) {
                 logger.info("Previous node was found as: $previousNode")
+                // TODO We should be adding any new identities as well
+                if (!verifyIdentities(node)) return
                 database.transaction {
                     updateInfoDB(node, session)
                     changePublisher.onNext(MapChange.Modified(node, previousNode))
@@ -183,6 +199,27 @@ open class PersistentNetworkMapCache(private val database: CordaPersistence,
         logger.debug { "Done adding node with info: $node" }
     }
 
+    private fun verifyIdentities(node: NodeInfo): Boolean {
+        val failures = node.legalIdentitiesAndCerts.mapNotNull { Try.on { it.verify(identityService.trustAnchor) } as? Try.Failure }
+        if (failures.isNotEmpty()) {
+            logger.warn("$node has ${failures.size} invalid identities:")
+            failures.forEach { logger.warn("", it) }
+        }
+        return failures.isEmpty()
+    }
+
+    private fun verifyAndRegisterIdentities(node: NodeInfo): Boolean {
+        // First verify all the node's identities are valid before registering any of them
+        return if (verifyIdentities(node)) {
+            for (identity in node.legalIdentitiesAndCerts) {
+                identityService.verifyAndRegisterIdentity(identity)
+            }
+            true
+        } else {
+            false
+        }
+    }
+
     override fun removeNode(node: NodeInfo) {
         logger.info("Removing node with info: $node")
         synchronized(_changed) {