Integration test for network parameter updates and improved logging (#2863)

node-api/src/main/kotlin/net/corda/nodeapi/internal/network/NetworkMap.kt

@@ -31,7 +31,14 @@ data class NetworkMap(
         val nodeInfoHashes: List<SecureHash>,
         val networkParameterHash: SecureHash,
         val parametersUpdate: ParametersUpdate?
-)
+) {
+    override fun toString(): String {
+        return """${NetworkMap::class.java.simpleName}(nodeInfoHashes=
+${nodeInfoHashes.joinToString("\n")}
+networkParameterHash=$networkParameterHash
+parametersUpdate=$parametersUpdate)"""
+    }
+}
 
 /**
  * Data class representing scheduled network parameters update.

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

@@ -2,35 +2,34 @@ package net.corda.node.services.network
 
 import net.corda.cordform.CordformNode
 import net.corda.core.crypto.random63BitValue
+import net.corda.core.internal.*
 import net.corda.core.internal.concurrent.transpose
-import net.corda.core.internal.div
+import net.corda.core.messaging.ParametersUpdateInfo
-import net.corda.core.internal.exists
-import net.corda.core.internal.list
-import net.corda.core.internal.readObject
 import net.corda.core.node.NodeInfo
+import net.corda.core.serialization.serialize
 import net.corda.core.utilities.getOrThrow
 import net.corda.core.utilities.seconds
 import net.corda.nodeapi.internal.network.NETWORK_PARAMS_FILE_NAME
+import net.corda.nodeapi.internal.network.NETWORK_PARAMS_UPDATE_FILE_NAME
 import net.corda.nodeapi.internal.network.SignedNetworkParameters
 import net.corda.testing.common.internal.testNetworkParameters
-import net.corda.testing.core.ALICE_NAME
+import net.corda.testing.core.*
-import net.corda.testing.core.BOB_NAME
-import net.corda.testing.core.SerializationEnvironmentRule
 import net.corda.testing.driver.NodeHandle
-import net.corda.testing.driver.PortAllocation
+import net.corda.testing.driver.internal.NodeHandleInternal
 import net.corda.testing.driver.internal.RandomFree
 import net.corda.testing.node.internal.CompatibilityZoneParams
 import net.corda.testing.node.internal.internalDriver
 import net.corda.testing.node.internal.network.NetworkMapServer
 import org.assertj.core.api.Assertions.assertThat
+import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.junit.After
+import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import java.net.URL
 import java.time.Instant
 import kotlin.streams.toList
-import kotlin.test.assertEquals
 
 class NetworkMapTest {
     @Rule
@@ -57,6 +56,53 @@ class NetworkMapTest {
         networkMapServer.close()
     }
 
+    @Test
+    fun `parameters update test`() {
+        internalDriver(
+                portAllocation = portAllocation,
+                compatibilityZone = compatibilityZone,
+                initialiseSerialization = false,
+                notarySpecs = emptyList()
+        ) {
+            val alice = startNode(providedName = ALICE_NAME).getOrThrow() as NodeHandleInternal
+            val nextParams = networkMapServer.networkParameters.copy(epoch = 3, modifiedTime = Instant.ofEpochMilli(random63BitValue()))
+            val nextHash = nextParams.serialize().hash
+            val snapshot = alice.rpc.networkParametersFeed().snapshot
+            val updates = alice.rpc.networkParametersFeed().updates.bufferUntilSubscribed()
+            assertEquals(null, snapshot)
+            assertThat(updates.isEmpty)
+            networkMapServer.scheduleParametersUpdate(nextParams, "Next parameters", Instant.ofEpochMilli(random63BitValue()))
+            // Wait for network map client to poll for the next update.
+            Thread.sleep(cacheTimeout.toMillis() * 2)
+            val laterParams = networkMapServer.networkParameters.copy(epoch = 4, modifiedTime = Instant.ofEpochMilli(random63BitValue()))
+            val laterHash = laterParams.serialize().hash
+            networkMapServer.scheduleParametersUpdate(laterParams, "Another update", Instant.ofEpochMilli(random63BitValue()))
+            Thread.sleep(cacheTimeout.toMillis() * 2)
+            updates.expectEvents(isStrict = false) {
+                sequence(
+                        expect { update: ParametersUpdateInfo ->
+                            assertEquals(update.description, "Next parameters")
+                            assertEquals(update.hash, nextHash)
+                            assertEquals(update.parameters, nextParams)
+                        },
+                        expect { update: ParametersUpdateInfo ->
+                            assertEquals(update.description, "Another update")
+                            assertEquals(update.hash, laterHash)
+                            assertEquals(update.parameters, laterParams)
+                        }
+                )
+            }
+            // This should throw, because the nextHash has been replaced by laterHash
+            assertThatThrownBy { alice.rpc.acceptNewNetworkParameters(nextHash) }.hasMessageContaining("Refused to accept parameters with hash")
+            alice.rpc.acceptNewNetworkParameters(laterHash)
+            assertEquals(laterHash, networkMapServer.latestParametersAccepted(alice.nodeInfo.legalIdentities.first().owningKey))
+            networkMapServer.advertiseNewParameters()
+            val networkParameters = (alice.configuration.baseDirectory / NETWORK_PARAMS_UPDATE_FILE_NAME)
+                    .readObject<SignedNetworkParameters>().verified()
+            assertEquals(networkParameters, laterParams)
+        }
+    }
+
     @Test
     fun `node correctly downloads and saves network parameters file on startup`() {
         internalDriver(

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

@@ -38,8 +38,7 @@ class NetworkParametersReader(private val trustRoot: X509Certificate,
             //  you get them from network map, but you have to run the approval step.
             if (signedParametersFromFile == null) { // Node joins for the first time.
                 downloadParameters(trustRoot, advertisedParametersHash)
-            }
+            } else if (signedParametersFromFile.raw.hash == advertisedParametersHash) { // Restarted with the same parameters.
-            else if (signedParametersFromFile.raw.hash == advertisedParametersHash) { // Restarted with the same parameters.
                 signedParametersFromFile.verifiedNetworkMapCert(trustRoot)
             } else { // Update case.
                 readParametersUpdate(advertisedParametersHash, signedParametersFromFile.raw.hash).verifiedNetworkMapCert(trustRoot)
@@ -64,6 +63,7 @@ class NetworkParametersReader(private val trustRoot: X509Certificate,
                     "Please update node to use correct network parameters file.")
         }
         parametersUpdateFile.moveTo(networkParamsFile, StandardCopyOption.REPLACE_EXISTING)
+        logger.info("Scheduled update to network parameters has occurred - node now updated to these new parameters.")
         return signedUpdatedParameters
     }
 

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

@@ -2,7 +2,9 @@ package net.corda.node.services.network
 
 import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.SignedData
-import net.corda.core.internal.*
+import net.corda.core.internal.openHttpConnection
+import net.corda.core.internal.post
+import net.corda.core.internal.responseAs
 import net.corda.core.node.NodeInfo
 import net.corda.core.serialization.deserialize
 import net.corda.core.serialization.serialize
@@ -11,7 +13,10 @@ import net.corda.core.utilities.seconds
 import net.corda.core.utilities.trace
 import net.corda.node.utilities.registration.cacheControl
 import net.corda.nodeapi.internal.SignedNodeInfo
-import net.corda.nodeapi.internal.network.*
+import net.corda.nodeapi.internal.network.NetworkMap
+import net.corda.nodeapi.internal.network.SignedNetworkMap
+import net.corda.nodeapi.internal.network.SignedNetworkParameters
+import net.corda.nodeapi.internal.network.verifiedNetworkMapCert
 import java.io.BufferedReader
 import java.net.URL
 import java.security.cert.X509Certificate
@@ -44,10 +49,7 @@ class NetworkMapClient(compatibilityZoneURL: URL, val trustedRoot: X509Certifica
         val signedNetworkMap = connection.responseAs<SignedNetworkMap>()
         val networkMap = signedNetworkMap.verifiedNetworkMapCert(trustedRoot)
         val timeout = connection.cacheControl().maxAgeSeconds().seconds
-        logger.trace {
+        logger.trace { "Fetched network map update from $networkMapUrl successfully: $networkMap" }
-            "Fetched network map update from $networkMapUrl successfully, retrieved ${networkMap.nodeInfoHashes.size} " +
-                    "node info hashes. Node Info hashes:\n${networkMap.nodeInfoHashes.joinToString("\n")}"
-        }
         return NetworkMapResponse(networkMap, timeout)
     }
 

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

@@ -147,12 +147,13 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
             // This update was handled already.
             return
         }
-        val newParameters = networkMapClient.getNetworkParameters(update.newParametersHash)
+        val newSignedNetParams = networkMapClient.getNetworkParameters(update.newParametersHash)
-        logger.info("Downloaded new network parameters: $newParameters from the update: $update")
+        val newNetParams = newSignedNetParams.verifiedNetworkMapCert(networkMapClient.trustedRoot)
-        newNetworkParameters = Pair(update, newParameters)
+        logger.info("Downloaded new network parameters: $newNetParams from the update: $update")
+        newNetworkParameters = Pair(update, newSignedNetParams)
         val updateInfo = ParametersUpdateInfo(
                 update.newParametersHash,
-                newParameters.verifiedNetworkMapCert(networkMapClient.trustedRoot),
+                newNetParams,
                 update.description,
                 update.updateDeadline)
         parametersUpdatesTrack.onNext(updateInfo)
@@ -162,15 +163,17 @@ class NetworkMapUpdater(private val networkMapCache: NetworkMapCacheInternal,
         networkMapClient ?: throw IllegalStateException("Network parameters updates are not support without compatibility zone configured")
         // TODO This scenario will happen if node was restarted and didn't download parameters yet, but we accepted them.
         // Add persisting of newest parameters from update.
-        val (_, newParams) = requireNotNull(newNetworkParameters) { "Couldn't find parameters update for the hash: $parametersHash" }
+        val (update, signedNewNetParams) = requireNotNull(newNetworkParameters) { "Couldn't find parameters update for the hash: $parametersHash" }
         // We should check that we sign the right data structure hash.
-        val newParametersHash = newParams.verifiedNetworkMapCert(networkMapClient.trustedRoot).serialize().hash
+        val newNetParams = signedNewNetParams.verifiedNetworkMapCert(networkMapClient.trustedRoot)
+        val newParametersHash = newNetParams.serialize().hash
         if (parametersHash == newParametersHash) {
             // The latest parameters have priority.
-            newParams.serialize()
+            signedNewNetParams.serialize()
                     .open()
                     .copyTo(baseDirectory / NETWORK_PARAMS_UPDATE_FILE_NAME, StandardCopyOption.REPLACE_EXISTING)
             networkMapClient.ackNetworkParametersUpdate(sign(parametersHash))
+            logger.info("Accepted network parameter update $update: $newNetParams")
         } else {
             throw IllegalArgumentException("Refused to accept parameters with hash $parametersHash because network map " +
                     "advertises update with hash $newParametersHash. Please check newest version")

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

@@ -205,10 +205,10 @@ class NetworkMapUpdaterTest {
         updates.expectEvents(isStrict = false) {
             sequence(
                     expect { update: ParametersUpdateInfo ->
-                        assertThat(update.updateDeadline == updateDeadline)
+                        assertEquals(update.updateDeadline, updateDeadline)
-                        assertThat(update.description == "Test update")
+                        assertEquals(update.description,"Test update")
-                        assertThat(update.hash == newParameters.serialize().hash)
+                        assertEquals(update.hash, newParameters.serialize().hash)
-                        assertThat(update.parameters == newParameters)
+                        assertEquals(update.parameters, newParameters)
                     }
             )
         }

testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/network/NetworkMapServer.kt

@@ -44,10 +44,6 @@ class NetworkMapServer(private val cacheTimeout: Duration,
 
     private val server: Server
     var networkParameters: NetworkParameters = stubNetworkParameters
-        set(networkParameters) {
-            check(field == stubNetworkParameters) { "Network parameters can be set only once" }
-            field = networkParameters
-        }
     private val service = InMemoryNetworkMapService()
     private var parametersUpdate: ParametersUpdate? = null
     private var nextNetworkParameters: NetworkParameters? = null