From 8df18e954f093900cb4744ce0d08f2509968d97d Mon Sep 17 00:00:00 2001 From: Patrick Kuo Date: Thu, 28 Jun 2018 15:10:52 +0100 Subject: [PATCH] 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) --- .../net/corda/node/internal/NodeStartup.kt | 7 ++++ .../services/network/NetworkMapUpdater.kt | 17 ++++---- .../network/PersistentNetworkMapCache.kt | 3 +- .../registration/NetworkRegistrationHelper.kt | 3 -- .../services/network/NetworkMapUpdaterTest.kt | 41 +++++++++++++++++-- .../node/internal/network/NetworkMapServer.kt | 9 ++-- 6 files changed, 62 insertions(+), 18 deletions(-) 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 95a03d4c48..f9a045445b 100644 --- a/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt +++ b/node/src/main/kotlin/net/corda/node/internal/NodeStartup.kt @@ -311,6 +311,13 @@ open class NodeStartup(val args: Array) { println("* *") println("******************************************************************") NodeRegistrationHelper(conf, HTTPNetworkRegistrationService(compatibilityZoneURL), 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.") } protected open fun loadConfigFile(cmdlineOptions: CmdLineOptions): Pair> = 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 2541a527ba..b30e3472e8 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 @@ -114,6 +114,15 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal, } val currentNodeHashes = networkMapCache.allNodeHashes + + // Remove node info from network map. + (currentNodeHashes - allHashesFromNetworkMap - fileWatcher.processedNodeInfoHashes) + .mapNotNull { + if (it != ourNodeInfoHash) { + networkMapCache.getNodeByHash(it) + } else null + }.forEach(networkMapCache::removeNode) + (allHashesFromNetworkMap - currentNodeHashes).mapNotNull { // Download new node info from network map try { @@ -128,14 +137,6 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal, networkMapCache.addNode(it) } - // Remove node info from network map. - (currentNodeHashes - allHashesFromNetworkMap - fileWatcher.processedNodeInfoHashes) - .mapNotNull { - if (it != ourNodeInfoHash) { - networkMapCache.getNodeByHash(it) - } else null - }.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 bd4f0f8b4d..fb815df849 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 @@ -255,7 +255,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/main/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelper.kt b/node/src/main/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelper.kt index 6a804bfc8f..987da3d6fa 100644 --- a/node/src/main/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelper.kt +++ b/node/src/main/kotlin/net/corda/node/utilities/registration/NetworkRegistrationHelper.kt @@ -97,9 +97,6 @@ open class NetworkRegistrationHelper(private val config: SSLConfiguration, onSuccess(keyPair, certificates) // All done, clean up temp files. requestIdStore.deleteIfExists() - - println("Successfully registered Corda node with compatibility zone, node identity keys and certificates are stored in '${config.certificatesDirectory}', it is advised to backup the private keys and certificates.") - println("Corda node will now terminate.") } private fun validateCertificates(registeringPublicKey: PublicKey, certificates: List) { 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 fd08de8cef..e7efd37ae2 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 @@ -16,7 +16,10 @@ import net.corda.core.serialization.serialize import net.corda.core.utilities.millis import net.corda.node.services.api.NetworkMapCacheInternal import net.corda.nodeapi.internal.NodeInfoAndSigned -import net.corda.nodeapi.internal.network.* +import net.corda.nodeapi.internal.network.NETWORK_PARAMS_UPDATE_FILE_NAME +import net.corda.nodeapi.internal.network.NodeInfoFilesCopier +import net.corda.nodeapi.internal.network.SignedNetworkParameters +import net.corda.nodeapi.internal.network.verifiedNetworkMapCert import net.corda.testing.common.internal.testNetworkParameters import net.corda.testing.core.* import net.corda.testing.driver.PortAllocation @@ -186,7 +189,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) } @@ -204,7 +207,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) }) val updateFile = baseDir / NETWORK_PARAMS_UPDATE_FILE_NAME val signedNetworkParams = updateFile.readObject() val paramsFromFile = signedNetworkParams.verifiedNetworkMapCert(DEV_ROOT_CA.certificate) @@ -299,6 +302,38 @@ class NetworkMapUpdaterTest { 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.addLegalIdentity(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) + 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() diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt index d74b35053c..c69c9060a4 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt +++ b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt @@ -8,8 +8,6 @@ import net.corda.core.node.NetworkParameters import net.corda.core.node.NodeInfo import net.corda.core.serialization.serialize import net.corda.core.utilities.NetworkHostAndPort -import net.corda.core.utilities.contextLogger -import net.corda.core.utilities.days import net.corda.nodeapi.internal.SignedNodeInfo import net.corda.nodeapi.internal.createDevNetworkMapCa import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair @@ -128,7 +126,12 @@ class NetworkMapServer(private val pollInterval: Duration, val hash = signedNodeInfo.raw.hash val nodeInfo = signedNodeInfo.verified() val privateNetwork = nodeNamesUUID[nodeInfo.legalIdentities[0].name] - networkMaps.computeIfAbsent(privateNetwork, { mutableSetOf() }).add(hash) + val map = networkMaps.computeIfAbsent(privateNetwork) { mutableSetOf() } + map.add(hash) + nodeInfoMap.filter { it.value.verified().legalIdentities.first().name == signedNodeInfo.verified().legalIdentities.first().name }.forEach { t, _ -> + nodeInfoMap.remove(t) + map.remove(t) + } nodeInfoMap[hash] = signedNodeInfo ok() } catch (e: Exception) {