ENT-1684: Remove nodes that didn't accept update from network map (#692)

On flag day all nodes that didn't accept new parameters update will be kicked out of the network map.

network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersUpdateTest.kt

@@ -94,6 +94,7 @@ class NetworkParametersUpdateTest : IntegrationTest() {
         ) {
             var (alice) = listOf(
                     startNode(providedName = ALICE_NAME),
+                    startNode(providedName = BOB_NAME),
                     defaultNotaryNode
             ).transpose().getOrThrow()
             alice as NodeHandleInternal
@@ -137,6 +138,8 @@ class NetworkParametersUpdateTest : IntegrationTest() {
                     .readObject<SignedNetworkParameters>().verified()
             assertEquals(networkParameters, paramUpdateInfo.parameters)
             assertThat(alice.rpc.networkParametersFeed().snapshot).isNull() // Check that NMS doesn't advertise updates anymore.
+            // Check that Bob is no longer on the network as it didn't accept the new parameteres.
+            assertThat(alice.rpc.networkMapSnapshot().map { it.legalIdentities[0].name }).doesNotContain(BOB_NAME)
         }
     }
 

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NetworkMapStorage.kt

@@ -36,6 +36,7 @@ interface NetworkMapStorage {
 
     /**
      * Retrieves node info hashes where [NodeInfoEntity.isCurrent] is true and the certificate status is [CertificateStatus.VALID]
+     * Nodes should have declared that they are using correct set of parameters.
      */
     // TODO "Active" is the wrong word here
     fun getActiveNodeInfoHashes(): List<SecureHash>
@@ -70,4 +71,11 @@ interface NetworkMapStorage {
     fun getCurrentParametersUpdate(): ParametersUpdateEntity?
 
     fun setParametersUpdateStatus(update: ParametersUpdateEntity, newStatus: UpdateStatus)
+
+    /**
+     * Perform the switch of parameters on the flagDay.
+     * 1. Change status of ParametersUpdateEntity to [UpdateStatus.APPLIED]
+     * 2. Mark all the node infos that didn't accept the update as not current (so they won't be advertised in the network map)
+     */
+    fun switchFlagDay(update: ParametersUpdateEntity)
 }

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/NodeInfoStorage.kt

@@ -15,6 +15,7 @@ import net.corda.core.crypto.SecureHash
 import net.corda.core.node.NodeInfo
 import net.corda.nodeapi.internal.NodeInfoAndSigned
 import net.corda.nodeapi.internal.SignedNodeInfo
+import java.security.PublicKey
 import java.security.cert.CertPath
 
 /**
@@ -48,8 +49,8 @@ interface NodeInfoStorage {
 
     /**
      * Store information about latest accepted [NetworkParameters] hash.
-     * @param publicKeyHash Hash of public key that accepted network parameters. This public key should belong to [NodeInfo]
+     * @param publicKey Public key that accepted network parameters. This public key should belong to [NodeInfo]
      * @param acceptedParametersHash Hash of latest accepted network parameters.
      */
-    fun ackNodeInfoParametersUpdate(publicKeyHash: SecureHash, acceptedParametersHash: SecureHash)
+    fun ackNodeInfoParametersUpdate(publicKey: PublicKey, acceptedParametersHash: SecureHash)
 }

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt

@@ -156,6 +156,15 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw
             session.merge(update.copy(status = newStatus))
         }
     }
+
+    override fun switchFlagDay(update: ParametersUpdateEntity) {
+        database.transaction {
+            setParametersUpdateStatus(update, UpdateStatus.APPLIED)
+            session.createQuery("update ${NodeInfoEntity::class.java.name} n set n.isCurrent = false " +
+                    "where (n.acceptedParametersUpdate != :acceptedParamUp or n.acceptedParametersUpdate is null) and n.isCurrent = true")
+                    .setParameter("acceptedParamUp", update).executeUpdate()
+        }
+    }
 }
 
 internal fun DatabaseTransaction.getNetworkParametersEntity(hash: SecureHash): NetworkParametersEntity? {

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorage.kt

@@ -25,6 +25,7 @@ import net.corda.nodeapi.internal.SignedNodeInfo
 import net.corda.nodeapi.internal.crypto.x509Certificates
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseTransaction
+import java.security.PublicKey
 import java.security.cert.CertPath
 
 /**
@@ -39,7 +40,7 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
 
         database.transaction {
             val count = session.createQuery(
-                    "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash", java.lang.Long::class.java)
+                    "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash and isCurrent = true", java.lang.Long::class.java)
                     .setParameter("nodeInfoHash", nodeInfoHash.toString())
                     .singleResult
                     .toLong()
@@ -64,7 +65,7 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
             // Update any [NodeInfoEntity] instance for this CSR as not current.
             existingNodeInfos.forEach { session.merge(it.copy(isCurrent = false)) }
 
-            session.save(NodeInfoEntity(
+            session.saveOrUpdate(NodeInfoEntity(
                     nodeInfoHash = nodeInfoHash.toString(),
                     publicKeyHash = nodeInfo.legalIdentities[0].owningKey.hashString(),
                     certificateSigningRequest = request,
@@ -96,11 +97,11 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
         }
     }
 
-    override fun ackNodeInfoParametersUpdate(publicKeyHash: SecureHash, acceptedParametersHash: SecureHash) {
+    override fun ackNodeInfoParametersUpdate(publicKey: PublicKey, acceptedParametersHash: SecureHash) {
         return database.transaction {
             val nodeInfoEntity = session.fromQuery<NodeInfoEntity>(
                     "n where n.publicKeyHash = :publicKeyHash and isCurrent = true")
-                    .setParameter("publicKeyHash", publicKeyHash.toString())
+                    .setParameter("publicKeyHash", publicKey.hashString())
                     .singleResult
             val parametersUpdateEntity = session.fromQuery<ParametersUpdateEntity>(
                     "u where u.networkParameters.hash = :acceptedParametersHash").

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt

@@ -51,9 +51,6 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private
         val activeNetworkParameters = activeNetworkMap?.networkParameters
         logger.debug { "Current network map parameters: ${activeNetworkParameters?.networkParameters}" }
 
-        val nodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes()
-        logger.debug { "Retrieved node info hashes:\n${nodeInfoHashes.joinToString("\n")}" }
-
         // We persist signed parameters only if they were not persisted before (they are not in currentSignedNetworkMap as
         // normal parameters or as an update)
         if (!latestNetworkParameters.isSigned) {
@@ -63,12 +60,15 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private
         }
 
         val parametersToNetworkMap = if (parametersUpdate?.status == FLAG_DAY || activeNetworkParameters == null) {
-            parametersUpdate?.let { networkMapStorage.setParametersUpdateStatus(it, APPLIED) }
+            parametersUpdate?.let { networkMapStorage.switchFlagDay(it) }
             latestNetworkParameters
         } else {
             activeNetworkParameters
         }
 
+        val nodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes()
+        logger.debug { "Retrieved node info hashes:\n${nodeInfoHashes.joinToString("\n")}" }
+
         val newNetworkMap = NetworkMap(
                 nodeInfoHashes,
                 SecureHash.parse(parametersToNetworkMap.hash),

network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebService.kt

@@ -115,7 +115,7 @@ class NetworkMapWebService(private val nodeInfoStorage: NodeInfoStorage,
             val hash = signedParametersHash.verified()
             networkMapStorage.getSignedNetworkParameters(hash) ?: throw IllegalArgumentException("No network parameters with hash $hash")
             logger.debug { "Received ack-parameters with $hash from ${signedParametersHash.sig.by}" }
-            nodeInfoStorage.ackNodeInfoParametersUpdate(signedParametersHash.sig.by.encoded.sha256(), hash)
+            nodeInfoStorage.ackNodeInfoParametersUpdate(signedParametersHash.sig.by, hash)
             ok()
         } catch (e: SignatureException) {
             status(Response.Status.FORBIDDEN).entity(e.message)

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorageTest.kt

@@ -14,7 +14,10 @@ import com.r3.corda.networkmanage.TestBase
 import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity
 import com.r3.corda.networkmanage.common.persistence.entity.ParametersUpdateEntity
 import com.r3.corda.networkmanage.common.persistence.entity.UpdateStatus
+import com.r3.corda.networkmanage.common.utils.hashString
 import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.sha256
+import net.corda.core.serialization.serialize
 import net.corda.core.utilities.days
 import net.corda.nodeapi.internal.createDevNetworkMapCa
 import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
@@ -149,4 +152,54 @@ class PersistentNetworkMapStorageTest : TestBase() {
         assertThat(firstUpdate.status).isEqualTo(UpdateStatus.CANCELLED)
         assertThat(networkMapStorage.getCurrentParametersUpdate()?.description).isEqualTo("Update of update")
     }
+
+    @Test
+    fun `switch ParametersUpdate on flag day`() {
+        // Update
+        val networkParameters1 = testNetworkParameters()
+        val updateDeadline = Instant.now() + 10.days
+        networkMapStorage.saveNewParametersUpdate(networkParameters1, "Update 1", updateDeadline)
+        // given
+        val (signedNodeInfoA) = createValidSignedNodeInfo("TestA", requestStorage) // null as acceptedParametersUpdate
+        val (signedNodeInfoB) = createValidSignedNodeInfo("TestB", requestStorage) // accepts update
+
+        // Put signed node info data
+        nodeInfoStorage.putNodeInfo(signedNodeInfoA)
+        val nodeInfoHashB = nodeInfoStorage.putNodeInfo(signedNodeInfoB)
+
+        nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoB.nodeInfo.legalIdentities[0].owningKey, networkParameters1.serialize().hash)
+        val parameterUpdate = networkMapStorage.getCurrentParametersUpdate()!!
+        networkMapStorage.switchFlagDay(parameterUpdate)
+        // when
+        val validNodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes()
+        // then
+        assertThat(validNodeInfoHashes).containsOnly(nodeInfoHashB)
+    }
+
+    @Test
+    fun `accept second set of parameters and switch on flag day`() {
+        // Update 1
+        val networkParameters1 = testNetworkParameters()
+        val updateDeadline = Instant.now() + 10.days
+        networkMapStorage.saveNewParametersUpdate(networkParameters1, "Update 1", updateDeadline)
+        // given
+        val (signedNodeInfoA) = createValidSignedNodeInfo("TestA", requestStorage) // Update 1 as acceptedParametersUpdate
+        val (signedNodeInfoB) = createValidSignedNodeInfo("TestB", requestStorage) // Update 2 as acceptedParametersUpdate
+
+        // Put signed node info data
+        nodeInfoStorage.putNodeInfo(signedNodeInfoA)
+        val nodeInfoHashB = nodeInfoStorage.putNodeInfo(signedNodeInfoB)
+
+        nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoA.nodeInfo.legalIdentities[0].owningKey, networkParameters1.serialize().hash)
+        // Update 2
+        val networkParameters2 = testNetworkParameters(epoch = 2)
+        networkMapStorage.saveNewParametersUpdate(networkParameters2, "Update 2", updateDeadline + 10.days)
+        nodeInfoStorage.ackNodeInfoParametersUpdate(signedNodeInfoB.nodeInfo.legalIdentities[0].owningKey, networkParameters2.serialize().hash)
+        val parameterUpdate = networkMapStorage.getCurrentParametersUpdate()!!
+        networkMapStorage.switchFlagDay(parameterUpdate)
+        // when
+        val validNodeInfoHashes = networkMapStorage.getActiveNodeInfoHashes()
+        // then
+        assertThat(validNodeInfoHashes).containsOnly(nodeInfoHashB)
+    }
 }

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNodeInfoStorageTest.kt

@@ -164,6 +164,24 @@ class PersistentNodeInfoStorageTest : TestBase() {
         assertThat(singleNodeInfo().isCurrent).isTrue()
     }
 
+    @Test
+    fun `publish same node info twice after isCurrent change`() {
+        fun singleNodeInfo() = persistence.transaction { session.fromQuery<NodeInfoEntity>("").singleResult }
+
+        val (nodeInfoAndSigned) = createValidSignedNodeInfo("Test", requestStorage)
+        nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
+        // Change isCurrent to false (that happens on flagDay change)
+        persistence.transaction {
+            val ni = singleNodeInfo()
+            session.merge(ni.copy(isCurrent = false))
+        }
+        assertThat(singleNodeInfo().isCurrent).isFalse()
+        val nodeInfo = singleNodeInfo()
+        nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
+        assertThat(nodeInfo.publishedAt).isBeforeOrEqualTo(singleNodeInfo().publishedAt)
+        assertThat(singleNodeInfo().isCurrent).isTrue()
+    }
+
     @Test
     fun `accept parameters updates node info correctly`() {
         // given
@@ -174,7 +192,7 @@ class PersistentNodeInfoStorageTest : TestBase() {
         val netParamsHash = networkParameters.serialize().hash
         networkMapStorage.saveNewParametersUpdate(networkParameters, "Update", Instant.now() + 1.days)
         val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
-        nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey.encoded.sha256(), netParamsHash)
+        nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey, netParamsHash)
 
         // then
         val acceptedUpdate = nodeInfoStorage.getAcceptedParametersUpdate(nodeInfoHash)
@@ -190,7 +208,7 @@ class PersistentNodeInfoStorageTest : TestBase() {
         val (nodeInfoAndSigned, privateKey) = createValidSignedNodeInfo("Test", requestStorage)
         nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
 
-        nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey.encoded.sha256(), netParamsHash)
+        nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey, netParamsHash)
 
         val nodeInfo2 = nodeInfoAndSigned.nodeInfo.copy(serial = 2)
         val nodeInfoAndSigned2 = NodeInfoAndSigned(nodeInfo2.signWith(listOf(privateKey)))

network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/webservice/NetworkMapWebServiceTest.kt

@@ -217,7 +217,7 @@ class NetworkMapWebServiceTest {
             val keyPair = Crypto.generateKeyPair()
             val signedHash = hash.serialize().sign { keyPair.sign(it) }
             it.doPost("ack-parameters", signedHash.serialize())
-            verify(nodeInfoStorage).ackNodeInfoParametersUpdate(keyPair.public.encoded.sha256(), hash)
+            verify(nodeInfoStorage).ackNodeInfoParametersUpdate(keyPair.public, hash)
             val randomSigned = SecureHash.randomSHA256().serialize().sign { keyPair.sign(it) }
             assertThatThrownBy { it.doPost("ack-parameters", randomSigned.serialize()) }
                     .hasMessageContaining("HTTP ERROR 500")