diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index cc8b283997..9da81d32c1 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/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 diff --git a/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt b/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt index b1ee8134ae..94cac05b86 100644 --- a/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt +++ b/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt @@ -255,6 +255,13 @@ open class NodeStartup(val args: Array) { 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() diff --git a/node/src/main/kotlin/net/corda/node/services/network/NetworkMapUpdater.kt b/node/src/main/kotlin/net/corda/node/services/network/NetworkMapUpdater.kt index 9c7f8fc276..b525345936 100644 --- a/node/src/main/kotlin/net/corda/node/services/network/NetworkMapUpdater.kt +++ b/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) - nodeInfo?.let { networkMapCache.removeNode(it) } + if (it.hash != ourNodeInfoHash) { + 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 } diff --git a/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt b/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt index d6c99a300e..7891a68998 100644 --- a/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt +++ b/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 diff --git a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapUpdaterTest.kt b/node/src/test/kotlin/net/corda/node/services/network/NetworkMapUpdaterTest.kt index cf414bbfe5..1a02f5e20f 100644 --- a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapUpdaterTest.kt +++ b/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.mock -import com.nhaarman.mockito_kotlin.times -import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.* 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() @@ -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()