CORDA-1563, CORDA-2029 - backports to 3.3 (#3990)

* CORDA-1563 : Don't remove own node info - fix (#3219)

* CORDA-2029: Don't remove own node info

Don't remove own node info from cache on network map update.

* ENT-2014 Deletes of NodeInfo can fail to propagate leading to infinite retries (#1101)

* ENT-2014 Deletes of NodeInfo can fail to propagate leading to infinite retries
ENT-1880 Move identity key generation to network registration process

(cherry picked from commit c3ac203)

NOTE: Moved ENT Jira's to OS project

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

@@ -24,6 +24,7 @@ import net.corda.core.node.services.*
 import net.corda.core.serialization.SerializationWhitelist
 import net.corda.core.serialization.SerializeAsToken
 import net.corda.core.serialization.SingletonSerializeAsToken
+import net.corda.core.serialization.serialize
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.*
 import net.corda.node.CordaClock
@@ -266,6 +267,7 @@ abstract class AbstractNode(val configuration: NodeConfiguration,
                 NodeInfoWatcher(configuration.baseDirectory, getRxIoScheduler(), Duration.ofMillis(configuration.additionalNodeInfoPollingFrequencyMsec)),
                 networkMapClient,
                 networkParameteresReader.hash,
+                services.myInfo.serialize().hash,
                 configuration.baseDirectory)
         runOnStop += networkMapUpdater::close
 

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

@@ -255,6 +255,13 @@ open class NodeStartup(val args: Array<String>) {
         println("*                                                                *")
         println("******************************************************************")
         NetworkRegistrationHelper(conf, HTTPNetworkRegistrationService(compatibilityZoneURL, getVersionInfo()), nodeRegistrationConfig).buildKeystore()
+
+        // Minimal changes to make registration tool create node identity.
+        // TODO: Move node identity generation logic from node to registration helper.
+        createNode(conf, getVersionInfo()).generateAndSaveNodeInfo()
+
+        println("Successfully registered Corda node with compatibility zone, node identity keys and certificates are stored in '${conf.certificatesDirectory}', it is advised to backup the private keys and certificates.")
+        println("Corda node will now terminate.")
     }
 
     open protected fun loadConfigFile(cmdlineOptions: CmdLineOptions): NodeConfiguration = cmdlineOptions.loadConfig()

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

@@ -33,6 +33,7 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
                         private val fileWatcher: NodeInfoWatcher,
                         private val networkMapClient: NetworkMapClient?,
                         private val currentParametersHash: SecureHash,
+                        private val ourNodeInfoHash: SecureHash?,
                         private val baseDirectory: Path
 ) : AutoCloseable {
     companion object {
@@ -66,8 +67,10 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
                     networkMapCache.addNode(it.nodeInfo)
                 }
                 is NodeInfoUpdate.Remove -> {
-                    val nodeInfo = networkMapCache.getNodeByHash(it.hash)
+                    if (it.hash != ourNodeInfoHash) {
-                    nodeInfo?.let { networkMapCache.removeNode(it) }
+                        val nodeInfo = networkMapCache.getNodeByHash(it.hash)
+                        nodeInfo?.let { networkMapCache.removeNode(it) }
+                    }
                 }
             }
         }
@@ -100,6 +103,13 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
 
         val currentNodeHashes = networkMapCache.allNodeHashes
         val hashesFromNetworkMap = networkMap.nodeInfoHashes
+        (currentNodeHashes - hashesFromNetworkMap - fileWatcher.processedNodeInfoHashes)
+                .mapNotNull {
+                    if (it != ourNodeInfoHash) {
+                        networkMapCache.getNodeByHash(it)
+                    } else null
+                }.forEach(networkMapCache::removeNode)
+
         (hashesFromNetworkMap - currentNodeHashes).mapNotNull {
             // Download new node info from network map
             try {
@@ -114,11 +124,6 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
             networkMapCache.addNode(it)
         }
 
-        // Remove node info from network map.
-        (currentNodeHashes - hashesFromNetworkMap - fileWatcher.processedNodeInfoHashes)
-                .mapNotNull(networkMapCache::getNodeByHash)
-                .forEach(networkMapCache::removeNode)
-
         return cacheTimeout
     }
 

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

@@ -251,7 +251,8 @@ open class PersistentNetworkMapCache(
     }
 
     private fun removeInfoDB(session: Session, nodeInfo: NodeInfo) {
-        val info = findByIdentityKey(session, nodeInfo.legalIdentitiesAndCerts.first().owningKey).singleOrNull()
+        // findByIdentityKey might returns multiple node info with the same key, need to pick the right one by comparing serial.
+        val info = findByIdentityKey(session, nodeInfo.legalIdentitiesAndCerts.first().owningKey).singleOrNull { it.serial == nodeInfo.serial }
         info?.let { session.remove(it) }
         // invalidate cache last - this way, we might serve up the wrong info for a short time, but it will get refreshed
         // on the next load

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

@@ -2,10 +2,7 @@ package net.corda.node.services.network
 
 import com.google.common.jimfs.Configuration.unix
 import com.google.common.jimfs.Jimfs
-import com.nhaarman.mockito_kotlin.any
+import com.nhaarman.mockito_kotlin.*
-import com.nhaarman.mockito_kotlin.mock
-import com.nhaarman.mockito_kotlin.times
-import com.nhaarman.mockito_kotlin.verify
 import net.corda.cordform.CordformNode.NODE_INFO_DIRECTORY
 import net.corda.core.crypto.Crypto
 import net.corda.core.crypto.SecureHash
@@ -64,7 +61,7 @@ class NetworkMapUpdaterTest {
     private val scheduler = TestScheduler()
     private val networkParametersHash = SecureHash.randomSHA256()
     private val fileWatcher = NodeInfoWatcher(baseDir, scheduler)
-    private val updater = NetworkMapUpdater(networkMapCache, fileWatcher, networkMapClient, networkParametersHash, baseDir)
+    private lateinit var updater: NetworkMapUpdater
     private var parametersUpdate: ParametersUpdate? = null
 
     @After
@@ -73,8 +70,13 @@ class NetworkMapUpdaterTest {
         fs.close()
     }
 
+    private fun setUpdater(ourNodeHash: SecureHash? = null) {
+        updater = NetworkMapUpdater(networkMapCache, fileWatcher, networkMapClient, networkParametersHash, ourNodeHash, baseDir)
+    }
+
     @Test
     fun `process add node updates from network map, with additional node infos from dir`() {
+        setUpdater()
         val (nodeInfo1, signedNodeInfo1) = createNodeInfoAndSigned("Info 1")
         val (nodeInfo2, signedNodeInfo2) = createNodeInfoAndSigned("Info 2")
         val (nodeInfo3, signedNodeInfo3) = createNodeInfoAndSigned("Info 3")
@@ -112,6 +114,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `process remove node updates from network map, with additional node infos from dir`() {
+        setUpdater()
         val (nodeInfo1, signedNodeInfo1) = createNodeInfoAndSigned("Info 1")
         val (nodeInfo2, signedNodeInfo2) = createNodeInfoAndSigned("Info 2")
         val (nodeInfo3, signedNodeInfo3) = createNodeInfoAndSigned("Info 3")
@@ -151,6 +154,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `receive node infos from directory, without a network map`() {
+        setUpdater()
         val fileNodeInfoAndSigned = createNodeInfoAndSigned("Info from file")
 
         // Not subscribed yet.
@@ -169,6 +173,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `emit new parameters update info on parameters update from network map`() {
+        setUpdater()
         val paramsFeed = updater.trackParametersUpdate()
         val snapshot = paramsFeed.snapshot
         val updates = paramsFeed.updates.bufferUntilSubscribed()
@@ -181,7 +186,7 @@ class NetworkMapUpdaterTest {
             sequence(
                     expect { update: ParametersUpdateInfo ->
                         assertEquals(update.updateDeadline, updateDeadline)
-                        assertEquals(update.description,"Test update")
+                        assertEquals(update.description, "Test update")
                         assertEquals(update.hash, newParameters.serialize().hash)
                         assertEquals(update.parameters, newParameters)
                     }
@@ -191,6 +196,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `ack network parameters update`() {
+        setUpdater()
         val newParameters = testNetworkParameters(epoch = 314)
         scheduleParametersUpdate(newParameters, "Test update", Instant.MIN)
         updater.subscribeToNetworkMap()
@@ -198,7 +204,7 @@ class NetworkMapUpdaterTest {
         Thread.sleep(2L * cacheExpiryMs)
         val newHash = newParameters.serialize().hash
         val keyPair = Crypto.generateKeyPair()
-        updater.acceptNewNetworkParameters(newHash, { hash -> hash.serialize().sign(keyPair)})
+        updater.acceptNewNetworkParameters(newHash, { hash -> hash.serialize().sign(keyPair) })
         verify(networkMapClient).ackNetworkParametersUpdate(any())
         val updateFile = baseDir / NETWORK_PARAMS_UPDATE_FILE_NAME
         val signedNetworkParams = updateFile.readObject<SignedNetworkParameters>()
@@ -239,6 +245,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `remove node from filesystem deletes it from network map cache`() {
+        setUpdater()
         val fileNodeInfoAndSigned1 = createNodeInfoAndSigned("Info from file 1")
         val fileNodeInfoAndSigned2 = createNodeInfoAndSigned("Info from file 2")
         updater.subscribeToNetworkMap()
@@ -261,6 +268,7 @@ class NetworkMapUpdaterTest {
 
     @Test
     fun `remove node info file, but node in network map server`() {
+        setUpdater()
         val nodeInfoBuilder = TestNodeInfoBuilder()
         val (_, key) = nodeInfoBuilder.addIdentity(CordaX500Name("Info", "London", "GB"))
         val (serverNodeInfo, serverSignedNodeInfo) = nodeInfoBuilder.buildWithSigned(1, 1)
@@ -289,6 +297,56 @@ class NetworkMapUpdaterTest {
         assertThat(networkMapCache.allNodeHashes).containsOnly(serverSignedNodeInfo.raw.hash)
     }
 
+    // Test fix for ENT-1882
+    // This scenario can happen when signing of network map server is performed much longer after the node joined the network.
+    // Network map will advertise hashes without that node.
+    @Test
+    fun `not remove own node info when it is not in network map yet`() {
+        val (myInfo, signedMyInfo) = createNodeInfoAndSigned("My node info")
+        val (_, signedOtherInfo) = createNodeInfoAndSigned("Other info")
+        setUpdater(ourNodeHash = signedMyInfo.raw.hash)
+        networkMapCache.addNode(myInfo) // Simulate behaviour on node startup when our node info is added to cache
+        networkMapClient.publish(signedOtherInfo)
+        updater.subscribeToNetworkMap()
+        Thread.sleep(2L * cacheExpiryMs)
+        verify(networkMapCache, never()).removeNode(myInfo)
+        assertThat(nodeInfoMap.keys).containsOnly(signedOtherInfo.raw.hash)
+        assertThat(networkMapCache.allNodeHashes).containsExactlyInAnyOrder(signedMyInfo.raw.hash, signedOtherInfo.raw.hash)
+    }
+
+    @Test
+    fun `network map updater removes the correct node info after node info changes`() {
+        setUpdater()
+
+        val builder = TestNodeInfoBuilder()
+
+        builder.addIdentity(CordaX500Name("Test", "London", "GB"))
+
+        val signedNodeInfo1 = builder.buildWithSigned(1).signed
+        val signedNodeInfo2 = builder.buildWithSigned(2).signed
+
+        // Test adding new node.
+        networkMapClient.publish(signedNodeInfo1)
+        // Not subscribed yet.
+        verify(networkMapCache, times(0)).addNode(any())
+
+        updater.subscribeToNetworkMap()
+
+        // TODO: Remove sleep in unit test.
+        Thread.sleep(2L * cacheExpiryMs)
+        verify(networkMapCache, times(1)).addNode(signedNodeInfo1.verified())
+        assert(networkMapCache.allNodeHashes.size == 1)
+        networkMapClient.publish(signedNodeInfo2)
+        nodeInfoMap.remove(signedNodeInfo1.raw.hash)
+        Thread.sleep(2L * cacheExpiryMs)
+        scheduler.advanceTimeBy(10, TimeUnit.SECONDS)
+
+        verify(networkMapCache, times(1)).addNode(signedNodeInfo2.verified())
+        verify(networkMapCache, times(1)).removeNode(signedNodeInfo1.verified())
+
+        assert(networkMapCache.allNodeHashes.size == 1)
+    }
+
     private fun createMockNetworkMapCache(): NetworkMapCacheInternal {
         return mock {
             val data = ConcurrentHashMap<Party, NodeInfo>()