Update to ENT-1433: Network parameter update entities in the db are not deleted, but rather marked as CANCELLED or APPLIED. (#600)

Also, the node info entity refers to the update entity, and not to the network parameters entity, which brings in a natural check that the parameters hash that the node is accepting is a planned update.

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

@@ -10,22 +10,19 @@
 
 package com.r3.corda.networkmanage.common.persistence
 
-import com.r3.corda.networkmanage.common.persistence.entity.NetworkMapEntity
+import com.r3.corda.networkmanage.common.persistence.entity.*
-import com.r3.corda.networkmanage.common.persistence.entity.NetworkParametersEntity
-import com.r3.corda.networkmanage.common.persistence.entity.ParametersUpdateEntity
-import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity
 import net.corda.core.crypto.SecureHash
 import net.corda.core.internal.DigitalSignatureWithCert
 import net.corda.core.node.NetworkParameters
 import net.corda.nodeapi.internal.network.NetworkMapAndSigned
-import net.corda.nodeapi.internal.network.ParametersUpdate
 import net.corda.nodeapi.internal.network.SignedNetworkParameters
 import java.time.Instant
 
 /**
  * Data access object interface for NetworkMap persistence layer
  */
-// TODO This storage abstraction should be removed. It results in less readable code when constructing network map in NetworkMapSigner
+// TODO This storage abstraction needs some thought. Some of the methods clearly don't make sense e.g. setParametersUpdateStatus.
+// The NetworkMapSignerTest uses a mock of this which means we need to provide methods for every trivial db operation.
 interface NetworkMapStorage {
     /**
      * Returns the active network map, or null
@@ -52,22 +49,16 @@ interface NetworkMapStorage {
 
     /**
      *  Persists given network parameters with signature if provided.
-     *  @return hash corresponding to newly created network parameters entry
+     *  @return The newly inserted [NetworkParametersEntity]
      */
-    fun saveNetworkParameters(networkParameters: NetworkParameters, signature: DigitalSignatureWithCert?): SecureHash
+    fun saveNetworkParameters(networkParameters: NetworkParameters, signature: DigitalSignatureWithCert?): NetworkParametersEntity
 
     /**
-     * Save new parameters update information with corresponding network parameters. Only one parameters update entity can be present at any time. Any existing
+     * Save new parameters update information with corresponding network parameters. Only one parameters update entity
-     * parameters update is cleared and overwritten by this one.
+     * can be NEW or FLAG_DAY at any time - if one exists it will be cancelled.
      */
     fun saveNewParametersUpdate(networkParameters: NetworkParameters, description: String, updateDeadline: Instant)
 
-    /**
-     * Indicate that it is time to switch network parameters in network map from active ones to the ones from update.
-     * @param parametersHash hash of the parameters from update
-     */
-    fun setFlagDay(parametersHash: SecureHash)
-
     /**
      * Retrieves the latest (i.e. most recently inserted) network parameters entity
      * Note that they may not have been signed up yet.
@@ -75,13 +66,8 @@ interface NetworkMapStorage {
      */
     fun getLatestNetworkParameters(): NetworkParametersEntity?
 
-    /**
+    /** Returns the single new or flag day parameters update, or null if there isn't one. */
-     * Retrieve any parameters update that may be active, null if none are present.
+    fun getCurrentParametersUpdate(): ParametersUpdateEntity?
-     */
-    fun getParametersUpdate(): ParametersUpdateEntity?
 
-    /**
+    fun setParametersUpdateStatus(update: ParametersUpdateEntity, newStatus: UpdateStatus)
-     * Removes any scheduled parameters updates.
-     */
-    fun clearParametersUpdates()
 }

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

@@ -10,7 +10,7 @@
 
 package com.r3.corda.networkmanage.common.persistence
 
-import com.r3.corda.networkmanage.common.persistence.entity.NetworkParametersEntity
+import com.r3.corda.networkmanage.common.persistence.entity.ParametersUpdateEntity
 import net.corda.core.crypto.SecureHash
 import net.corda.core.node.NodeInfo
 import net.corda.nodeapi.internal.NodeInfoAndSigned
@@ -34,10 +34,10 @@ interface NodeInfoStorage {
     fun getNodeInfo(nodeInfoHash: SecureHash): SignedNodeInfo?
 
     /**
-     * Returns the network parameters that the node has accepted or null if couldn't find node info with given hash or
+     * Returns the parameters update the node has accepted or null if couldn't find node info with given hash or
-     * there is no information on accepted parameters hash stored for this entity
+     * there is no information on accepted parameters update stored for this entity.
      */
-    fun getAcceptedNetworkParameters(nodeInfoHash: SecureHash): NetworkParametersEntity?
+    fun getAcceptedParametersUpdate(nodeInfoHash: SecureHash): ParametersUpdateEntity?
 
     /**
      * The [nodeInfoAndSigned] is keyed by the public key, old node info with the same public key will be replaced by the new node info.

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

@@ -18,6 +18,7 @@ import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.nodeapi.internal.persistence.DatabaseTransaction
 import net.corda.nodeapi.internal.persistence.SchemaMigration
+import org.hibernate.Session
 import org.hibernate.query.Query
 import java.util.*
 import javax.persistence.LockModeType
@@ -25,6 +26,10 @@ import javax.persistence.criteria.CriteriaBuilder
 import javax.persistence.criteria.Path
 import javax.persistence.criteria.Predicate
 
+inline fun <reified T> Session.fromQuery(query: String): Query<T> {
+    return createQuery("from ${T::class.java.name} $query", T::class.java)
+}
+
 inline fun <reified T> DatabaseTransaction.uniqueEntityWhere(predicate: (CriteriaBuilder, Path<T>) -> Predicate): T? {
     return entitiesWhere(predicate).setMaxResults(1).uniqueResult()
 }

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

@@ -11,6 +11,7 @@
 package com.r3.corda.networkmanage.common.persistence
 
 import com.r3.corda.networkmanage.common.persistence.entity.*
+import com.r3.corda.networkmanage.common.utils.logger
 import net.corda.core.crypto.SecureHash
 import net.corda.core.internal.DigitalSignatureWithCert
 import net.corda.core.node.NetworkParameters
@@ -83,11 +84,11 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw
         }
     }
 
-    override fun saveNetworkParameters(networkParameters: NetworkParameters, signature: DigitalSignatureWithCert?): SecureHash {
+    override fun saveNetworkParameters(networkParameters: NetworkParameters, signature: DigitalSignatureWithCert?): NetworkParametersEntity {
         val serialized = networkParameters.serialize()
         signature?.verify(serialized)
         val hash = serialized.hash
-        database.transaction {
+        return database.transaction {
             val entity = getNetworkParametersEntity(hash)
             val newNetworkParamsEntity = if (entity != null) {
                 entity.copy(
@@ -102,9 +103,8 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw
                         certificate = signature?.by
                 )
             }
-            session.merge(newNetworkParamsEntity)
+            session.merge(newNetworkParamsEntity) as NetworkParametersEntity
         }
-        return hash
     }
 
     override fun getLatestNetworkParameters(): NetworkParametersEntity? {
@@ -123,47 +123,37 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw
 
     override fun saveNewParametersUpdate(networkParameters: NetworkParameters, description: String, updateDeadline: Instant) {
         database.transaction {
-            val hash = saveNetworkParameters(networkParameters, null)
+            val existingUpdate = getCurrentParametersUpdate()
-            val netParamsEntity = getNetworkParametersEntity(hash)!!
+            if (existingUpdate != null) {
-            clearParametersUpdates()
+                logger.info("Cancelling existing update: $existingUpdate")
-            session.save(ParametersUpdateEntity(0, netParamsEntity, description, updateDeadline))
+                session.merge(existingUpdate.copy(status = UpdateStatus.CANCELLED))
+            }
+            val netParamsEntity = saveNetworkParameters(networkParameters, null)
+            session.save(ParametersUpdateEntity(
+                    networkParameters = netParamsEntity,
+                    description = description,
+                    updateDeadline = updateDeadline
+            ))
         }
     }
 
-    override fun clearParametersUpdates() {
+    override fun getCurrentParametersUpdate(): ParametersUpdateEntity? {
-        database.transaction {
-            val delete = "delete from ${ParametersUpdateEntity::class.java.name}"
-            session.createQuery(delete).executeUpdate()
-        }
-    }
-
-    override fun getParametersUpdate(): ParametersUpdateEntity? {
         return database.transaction {
-            val currentParametersHash = getActiveNetworkMap()?.networkParameters?.hash
+            val newParamsUpdates = session.fromQuery<ParametersUpdateEntity>("u where u.status in :statuses")
-            val latestParameters = getLatestNetworkParameters()
+                    .setParameterList("statuses", listOf(UpdateStatus.NEW, UpdateStatus.FLAG_DAY))
-            val criteria = session.criteriaBuilder.createQuery(ParametersUpdateEntity::class.java)
+                    .resultList
-            val root = criteria.from(ParametersUpdateEntity::class.java)
+            when (newParamsUpdates.size) {
-            val query = criteria.select(root)
+                0 -> null
-            // We just want the last entry
+                1 -> newParamsUpdates[0]
-            val parametersUpdate = session.createQuery(query).setMaxResults(1).uniqueResult()
+                else -> throw IllegalStateException("More than one update found: $newParamsUpdates")
-            check(parametersUpdate == null || latestParameters == parametersUpdate.networkParameters) {
-                "ParametersUpdate doesn't correspond to latest network parameters"
             }
-            // Highly unlikely, but...
-            check(parametersUpdate == null || latestParameters?.hash != currentParametersHash) {
-                "Having update for parameters that are already in network map"
-            }
-            parametersUpdate
         }
     }
 
-    override fun setFlagDay(parametersHash: SecureHash) {
+    override fun setParametersUpdateStatus(update: ParametersUpdateEntity, newStatus: UpdateStatus) {
+        require(newStatus != UpdateStatus.NEW)
         database.transaction {
-            val parametersUpdateEntity = getParametersUpdate() ?: throw IllegalArgumentException("Setting flag day but no parameters update to switch to")
+            session.merge(update.copy(status = newStatus))
-            if (parametersHash.toString() != parametersUpdateEntity.networkParameters.hash) {
-                throw IllegalArgumentException("Setting flag day for parameters: $parametersHash, but in database we have update for: ${parametersUpdateEntity.networkParameters.hash}")
-            }
-            session.merge(parametersUpdateEntity.copy(flagDay = true))
         }
     }
 }

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

@@ -11,9 +11,11 @@
 package com.r3.corda.networkmanage.common.persistence
 
 import com.r3.corda.networkmanage.common.persistence.entity.CertificateSigningRequestEntity
-import com.r3.corda.networkmanage.common.persistence.entity.NetworkParametersEntity
 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 com.r3.corda.networkmanage.common.utils.logger
 import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.sha256
 import net.corda.core.internal.CertRole
@@ -33,7 +35,19 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
         val (nodeInfo, signedNodeInfo) = nodeInfoAndSigned
         val nodeCaCert = nodeInfo.legalIdentitiesAndCerts[0].certPath.x509Certificates.find { CertRole.extract(it) == NODE_CA }
         nodeCaCert ?: throw IllegalArgumentException("Missing Node CA")
+        val nodeInfoHash = signedNodeInfo.raw.hash
+
         database.transaction {
+            val count = session.createQuery(
+                    "select count(*) from ${NodeInfoEntity::class.java.name} where nodeInfoHash = :nodeInfoHash", java.lang.Long::class.java)
+                    .setParameter("nodeInfoHash", nodeInfoHash.toString())
+                    .singleResult
+                    .toLong()
+            if (count != 0L) {
+                logger.debug("Ignoring duplicate publish: $nodeInfo")
+                return@transaction nodeInfoHash
+            }
+
             // TODO Move these checks out of data access layer
             val request = requireNotNull(getSignedRequestByPublicHash(nodeCaCert.publicKey.encoded.sha256())) {
                 "Node-info not registered with us"
@@ -42,9 +56,8 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
                 require(it == CertificateStatus.VALID) { "Certificate is no longer valid: $it" }
             }
 
-            val existingNodeInfos = session.createQuery(
+            val existingNodeInfos = session.fromQuery<NodeInfoEntity>(
-                    "from ${NodeInfoEntity::class.java.name} n where n.certificateSigningRequest = :csr and n.isCurrent = true order by n.publishedAt desc",
+                    "n where n.certificateSigningRequest = :csr and n.isCurrent = true order by n.publishedAt desc")
-                    NodeInfoEntity::class.java)
                     .setParameter("csr", request)
                     .resultList
 
@@ -52,15 +65,16 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
             existingNodeInfos.forEach { session.merge(it.copy(isCurrent = false)) }
 
             session.save(NodeInfoEntity(
-                    nodeInfoHash = signedNodeInfo.raw.hash.toString(),
+                    nodeInfoHash = nodeInfoHash.toString(),
                     publicKeyHash = nodeInfo.legalIdentities[0].owningKey.hashString(),
                     certificateSigningRequest = request,
                     signedNodeInfo = signedNodeInfo,
                     isCurrent = true,
-                    acceptedNetworkParameters = existingNodeInfos.firstOrNull()?.acceptedNetworkParameters
+                    acceptedParametersUpdate = existingNodeInfos.firstOrNull()?.acceptedParametersUpdate
             ))
         }
-        return signedNodeInfo.raw.hash
+
+        return nodeInfoHash
     }
 
     override fun getNodeInfo(nodeInfoHash: SecureHash): SignedNodeInfo? {
@@ -69,9 +83,9 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
         }
     }
 
-    override fun getAcceptedNetworkParameters(nodeInfoHash: SecureHash): NetworkParametersEntity? {
+    override fun getAcceptedParametersUpdate(nodeInfoHash: SecureHash): ParametersUpdateEntity? {
         return database.transaction {
-            session.find(NodeInfoEntity::class.java, nodeInfoHash.toString())?.acceptedNetworkParameters
+            session.find(NodeInfoEntity::class.java, nodeInfoHash.toString())?.acceptedParametersUpdate
         }
     }
 
@@ -84,21 +98,18 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
 
     override fun ackNodeInfoParametersUpdate(publicKeyHash: SecureHash, acceptedParametersHash: SecureHash) {
         return database.transaction {
-            val builder = session.criteriaBuilder
+            val nodeInfoEntity = session.fromQuery<NodeInfoEntity>(
-            val query = builder.createQuery(NodeInfoEntity::class.java).run {
+                    "n where n.publicKeyHash = :publicKeyHash and isCurrent = true")
-                from(NodeInfoEntity::class.java).run {
+                    .setParameter("publicKeyHash", publicKeyHash.toString())
-                    where(builder.equal(get<NodeInfoEntity>(NodeInfoEntity::publicKeyHash.name), publicKeyHash.toString()))
+                    .singleResult
-                }
+            val parametersUpdateEntity = session.fromQuery<ParametersUpdateEntity>(
+                    "u where u.networkParameters.hash = :acceptedParametersHash").
+                    setParameter("acceptedParametersHash", acceptedParametersHash.toString())
+                    .singleResult
+            require(parametersUpdateEntity.status in listOf(UpdateStatus.NEW, UpdateStatus.FLAG_DAY)) {
+                "$parametersUpdateEntity can no longer be accepted as it's ${parametersUpdateEntity.status}"
             }
-            val nodeInfo = requireNotNull(session.createQuery(query).setMaxResults(1).uniqueResult()) {
+            session.merge(nodeInfoEntity.copy(acceptedParametersUpdate = parametersUpdateEntity))
-                "NodeInfo with public key hash $publicKeyHash doesn't exist"
-            }
-            val networkParameters = requireNotNull(getNetworkParametersEntity(acceptedParametersHash)) {
-                "Network parameters $acceptedParametersHash doesn't exist"
-            }
-            require(networkParameters.isSigned) { "Network parameters $acceptedParametersHash is not signed" }
-            val newInfo = nodeInfo.copy(acceptedNetworkParameters = networkParameters)
-            session.merge(newInfo)
         }
     }
 

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

@@ -41,6 +41,6 @@ data class NodeInfoEntity(
         val publishedAt: Instant = Instant.now(),
 
         @ManyToOne(fetch = FetchType.LAZY)
-        @JoinColumn(name = "accepted_network_parameters")
+        @JoinColumn(name = "accepted_params_update")
-        val acceptedNetworkParameters: NetworkParametersEntity?
+        val acceptedParametersUpdate: ParametersUpdateEntity?
 )

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

@@ -23,11 +23,26 @@ data class ParametersUpdateEntity(
         @Column(name = "update_deadline", nullable = false)
         val updateDeadline: Instant,
 
-        // This boolean flag is used when we want to explicitly point that it's time to switch parameters in network map.
+        @Column(name = "status", length = 16, nullable = false, columnDefinition = "NVARCHAR(16)")
-        @Column(name = "flag_day", nullable = false)
+        @Enumerated(EnumType.STRING)
-        val flagDay: Boolean = false
+        val status: UpdateStatus = UpdateStatus.NEW
 ) {
     fun toParametersUpdate(): ParametersUpdate {
         return ParametersUpdate(SecureHash.parse(networkParameters.hash), description, updateDeadline)
     }
-}
+}
+
+enum class UpdateStatus {
+    /** A newly created update. */
+    NEW,
+    /**
+     * An update that has passed its deadline and flagged to be made active on the next signing event. At most only one
+     * update with status either NEW or FLAG_DAY can exist.
+     */
+    FLAG_DAY,
+    /** Any previously flagged update that has been activated into the network map. */
+    APPLIED,
+    /** A new or flag day update that has been cancelled. */
+    CANCELLED
+}
+

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

@@ -11,6 +11,7 @@
 package com.r3.corda.networkmanage.common.signer
 
 import com.r3.corda.networkmanage.common.persistence.NetworkMapStorage
+import com.r3.corda.networkmanage.common.persistence.entity.UpdateStatus.*
 import net.corda.core.crypto.SecureHash
 import net.corda.core.node.NetworkParameters
 import net.corda.core.utilities.contextLogger
@@ -34,7 +35,7 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private
         }
         logger.debug { "Retrieved latest network parameters: ${latestNetworkParameters.networkParameters}" }
 
-        val parametersUpdate = networkMapStorage.getParametersUpdate()
+        val parametersUpdate = networkMapStorage.getCurrentParametersUpdate()
         logger.debug { "Retrieved parameters update: $parametersUpdate" }
         check(parametersUpdate == null || parametersUpdate.networkParameters.hash == latestNetworkParameters.hash) {
             "The latest network parameters are not the scheduled updated ones"
@@ -61,8 +62,8 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private
             logger.debug { "No need to sign any network parameters as they're up-to-date" }
         }
 
-        val parametersToNetworkMap = if (parametersUpdate?.flagDay == true || activeNetworkParameters == null) {
+        val parametersToNetworkMap = if (parametersUpdate?.status == FLAG_DAY || activeNetworkParameters == null) {
-            networkMapStorage.clearParametersUpdates()
+            parametersUpdate?.let { networkMapStorage.setParametersUpdateStatus(it, APPLIED) }
             latestNetworkParameters
         } else {
             activeNetworkParameters
@@ -71,7 +72,7 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage, private
         val newNetworkMap = NetworkMap(
                 nodeInfoHashes,
                 SecureHash.parse(parametersToNetworkMap.hash),
-                parametersUpdate?.let { if (!it.flagDay) it.toParametersUpdate() else null })
+                parametersUpdate?.let { if (it.status == NEW) it.toParametersUpdate() else null })
         logger.debug { "Potential new network map: $newNetworkMap" }
 
         if (activeNetworkMap?.networkMap != newNetworkMap) {

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

@@ -12,11 +12,11 @@ package com.r3.corda.networkmanage.doorman
 
 import com.atlassian.jira.rest.client.internal.async.AsynchronousJiraRestClientFactory
 import com.r3.corda.networkmanage.common.persistence.*
+import com.r3.corda.networkmanage.common.persistence.entity.UpdateStatus
 import com.r3.corda.networkmanage.common.signer.NetworkMapSigner
 import com.r3.corda.networkmanage.common.utils.CertPathAndKey
 import com.r3.corda.networkmanage.doorman.signer.*
 import com.r3.corda.networkmanage.doorman.webservice.*
-import net.corda.core.crypto.SecureHash
 import net.corda.core.node.NetworkParameters
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.core.utilities.contextLogger
@@ -238,36 +238,33 @@ class NetworkManagementServer(dataSourceProperties: Properties, databaseConfig:
     }
 
     private fun handleFlagDay() {
-        val parametersUpdate = checkNotNull(networkMapStorage.getParametersUpdate()) {
+        val parametersUpdate = checkNotNull(networkMapStorage.getCurrentParametersUpdate()) {
             "No network parameters updates are scheduled"
         }
         check(Instant.now() >= parametersUpdate.updateDeadline) {
             "Update deadline of ${parametersUpdate.updateDeadline} hasn't passed yet"
         }
+        val latestNetParamsEntity = networkMapStorage.getLatestNetworkParameters()
+        check(parametersUpdate.networkParameters.hash == networkMapStorage.getLatestNetworkParameters()?.hash) {
+            "The latest network parameters is not the scheduled one:\n${latestNetParamsEntity?.networkParameters}\n${parametersUpdate.toParametersUpdate()}"
+        }
         val activeNetParams = networkMapStorage.getActiveNetworkMap()?.networkParameters
-        val latestNetParamsEntity = networkMapStorage.getLatestNetworkParameters()!!
+        check(parametersUpdate.networkParameters.isSigned) {
-        check(latestNetParamsEntity.isSigned) {
             "Parameters we are trying to switch to haven't been signed yet"
         }
-        // TODO This check is stil not good enough as when it comes to signing, the NetworkMapSigner will just accept
+        logger.info("""Flag day has occurred, however the new network parameters won't be active until the new network map is signed.
-        check(latestNetParamsEntity.hash == parametersUpdate.networkParameters.hash) {
+From: $activeNetParams
-            "The latest network parameters is not the scheduled one:\n${latestNetParamsEntity.networkParameters}\n${parametersUpdate.toParametersUpdate()}"
+To: ${parametersUpdate.networkParameters}""")
-        }
+        networkMapStorage.setParametersUpdateStatus(parametersUpdate, UpdateStatus.FLAG_DAY)
-        logger.info("Flag day has occurred, however the new network parameters won't be active until the new network map is signed.\n" +
-                "Switching from: $activeNetParams\nTo: ${latestNetParamsEntity.networkParameters}")
-        networkMapStorage.setFlagDay(SecureHash.parse(parametersUpdate.networkParameters.hash))
-        println("Set the flag day")
     }
 
     private fun handleCancelUpdate() {
-        val parametersUpdate = networkMapStorage.getParametersUpdate()
+        val parametersUpdate = checkNotNull(networkMapStorage.getCurrentParametersUpdate()) {
-        if (parametersUpdate == null) {
+            "No network parameters updates are scheduled"
-            logger.info("Trying to cancel parameters update but no update is scheduled")
-        } else {
-            logger.info("Cancelling parameters update: ${parametersUpdate.toParametersUpdate()}")
-            // We leave parameters from that update in the database, for auditing reasons
-            networkMapStorage.clearParametersUpdates()
         }
+        logger.info("""Cancelling parameters update: ${parametersUpdate.toParametersUpdate()}.
+However, the network map will continue to advertise this update until the new one is signed.""")
+        networkMapStorage.setParametersUpdateStatus(parametersUpdate, UpdateStatus.CANCELLED)
         println("Done with cancel update")
     }
 }

network-management/src/main/resources/migration/network-manager.changelog-init.xml

@@ -137,7 +137,7 @@
             <column name="published_at" type="TIMESTAMP">
                 <constraints nullable="false"/>
             </column>
-            <column name="accepted_network_parameters" type="NVARCHAR(64)"/>
+            <column name="accepted_params_update" type="BIGINT"/>
         </createTable>
     </changeSet>
     <changeSet author="R3.Corda" id="1520338500424-8">
@@ -318,7 +318,7 @@
     </changeSet>
     <changeSet author="R3.Corda" id="1520338500424-38">
         <addForeignKeyConstraint baseColumnNames="network_parameters" baseTableName="network_map"
-                                 constraintName="FK_NM_NP"
+                                 constraintName="FK__NM__NP"
                                  referencedColumnNames="hash" referencedTableName="network_parameters"/>
     </changeSet>
     <changeSet author="R3.Corda" id="1520338500424-39">
@@ -335,18 +335,18 @@
             <column name="network_parameters" type="NVARCHAR(64)">
                 <constraints nullable="false"/>
             </column>
-            <column name="flag_day" type="BOOLEAN">
+            <column name="status" type="NVARCHAR(16)">
                 <constraints nullable="false"/>
             </column>
         </createTable>
         <addPrimaryKey columnNames="id" constraintName="CONSTRAINT_PARAMUPKEY" tableName="parameters_update"/>
         <addForeignKeyConstraint baseTableName="parameters_update" baseColumnNames="network_parameters"
-                                 constraintName="FK_PU_NP"
+                                 constraintName="FK__PU__NP"
                                  referencedTableName="network_parameters" referencedColumnNames="hash"/>
     </changeSet>
     <changeSet author="R3.Corda" id="1520338500424-40">
-        <addForeignKeyConstraint baseColumnNames="accepted_network_parameters" baseTableName="node_info"
+        <addForeignKeyConstraint baseColumnNames="accepted_params_update" baseTableName="node_info"
-                                 constraintName="FK__NI__NP"
+                                 constraintName="FK__NI__PU"
-                                 referencedColumnNames="hash" referencedTableName="network_parameters"/>
+                                 referencedColumnNames="id" referencedTableName="parameters_update"/>
     </changeSet>
 </databaseChangeLog>

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

@@ -12,14 +12,14 @@ package com.r3.corda.networkmanage.common.persistence
 
 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 net.corda.core.crypto.SecureHash
-import net.corda.core.crypto.random63BitValue
+import net.corda.core.utilities.days
-import net.corda.core.serialization.serialize
 import net.corda.nodeapi.internal.createDevNetworkMapCa
 import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
 import net.corda.nodeapi.internal.network.NetworkMap
 import net.corda.nodeapi.internal.network.NetworkMapAndSigned
-import net.corda.nodeapi.internal.network.ParametersUpdate
 import net.corda.nodeapi.internal.network.verifiedNetworkMapCert
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
@@ -68,8 +68,8 @@ class PersistentNetworkMapStorageTest : TestBase() {
         // Create network parameters
         val networkParameters = testNetworkParameters(maxTransactionSize = 1234567)
         val networkParametersSig = networkMapCertAndKeyPair.sign(networkParameters).sig
-        val networkParametersHash = networkMapStorage.saveNetworkParameters(networkParameters, networkParametersSig)
+        val networkParametersHash = networkMapStorage.saveNetworkParameters(networkParameters, networkParametersSig).hash
-        val networkMap = NetworkMap(listOf(nodeInfoHash), networkParametersHash, null)
+        val networkMap = NetworkMap(listOf(nodeInfoHash), SecureHash.parse(networkParametersHash), null)
         val networkMapAndSigned = NetworkMapAndSigned(networkMap) { networkMapCertAndKeyPair.sign(networkMap).sig }
 
         // when
@@ -87,8 +87,8 @@ class PersistentNetworkMapStorageTest : TestBase() {
         assertThat(activeSignedNetworkMap.sig).isEqualTo(networkMapAndSigned.signed.sig)
         assertThat(activeNetworkParameters).isEqualTo(networkParameters)
         assertThat(activeSignedNetworkParameters.sig).isEqualTo(networkParametersSig)
-        assertThat(SecureHash.parse(activeNetworkParametersEntity.hash))
+        assertThat(activeNetworkParametersEntity.hash)
-                .isEqualTo(activeNetworkMap.networkParameterHash)
+                .isEqualTo(activeNetworkMap.networkParameterHash.toString())
                 .isEqualTo(networkParametersHash)
     }
 
@@ -127,27 +127,26 @@ class PersistentNetworkMapStorageTest : TestBase() {
     }
 
     @Test
-    fun `saveNewParametersUpdate clears the previous updates from database`() {
+    fun `saveNewParametersUpdate marks update as NEW and persists network parameters as the latest`() {
-        val testParameters1 = testNetworkParameters(epoch = 1)
+        val networkParameters = testNetworkParameters()
-        val testParameters2 = testNetworkParameters(epoch = 2)
+        val updateDeadline = Instant.now() + 10.days
-        val hash1 = testParameters1.serialize().hash
+        networkMapStorage.saveNewParametersUpdate(networkParameters, "Update 1", updateDeadline)
-        val hash2 = testParameters2.serialize().hash
+        val parameterUpdate = networkMapStorage.getCurrentParametersUpdate()!!
-        val updateDeadline1 = Instant.ofEpochMilli(random63BitValue())
+        assertThat(parameterUpdate.description).isEqualTo("Update 1")
-        val updateDeadline2 = Instant.ofEpochMilli(random63BitValue())
+        assertThat(parameterUpdate.updateDeadline).isEqualTo(updateDeadline)
-        networkMapStorage.saveNewParametersUpdate(testParameters1, "Update 1", updateDeadline1)
+        assertThat(parameterUpdate.status).isEqualTo(UpdateStatus.NEW)
-        networkMapStorage.saveNewParametersUpdate(testParameters1, "Update of update", updateDeadline1)
+        assertThat(parameterUpdate.networkParameters.networkParameters).isEqualTo(networkParameters)
-        assertThat(networkMapStorage.getParametersUpdate()?.toParametersUpdate()).isEqualTo(ParametersUpdate(hash1, "Update of update", updateDeadline1))
+        assertThat(networkMapStorage.getLatestNetworkParameters()?.networkParameters).isEqualTo(networkParameters)
-        networkMapStorage.saveNewParametersUpdate(testParameters2, "Update 3", updateDeadline2)
-        assertThat(networkMapStorage.getParametersUpdate()?.toParametersUpdate()).isEqualTo(ParametersUpdate(hash2, "Update 3", updateDeadline2))
     }
 
     @Test
-    fun `clear parameters update removes all parameters updates`() {
+    fun `saveNewParametersUpdate marks previous update as cancelled`() {
-        val params1 = testNetworkParameters(minimumPlatformVersion = 1)
+        networkMapStorage.saveNewParametersUpdate(testNetworkParameters(epoch = 1), "Update 1", Instant.now() + 1.days)
-        val params2 = testNetworkParameters(minimumPlatformVersion = 2)
+        networkMapStorage.saveNewParametersUpdate(testNetworkParameters(epoch = 2), "Update of update", Instant.now() + 2.days)
-        networkMapStorage.saveNewParametersUpdate(params1, "Update 1", Instant.ofEpochMilli(random63BitValue()))
+        val firstUpdate = persistence.transaction {
-        networkMapStorage.saveNewParametersUpdate(params2, "Update 2", Instant.ofEpochMilli(random63BitValue()))
+            session.fromQuery<ParametersUpdateEntity>("u where u.description = 'Update 1'").singleResult
-        networkMapStorage.clearParametersUpdates()
+        }
-        assertThat(networkMapStorage.getParametersUpdate()).isNull()
+        assertThat(firstUpdate.status).isEqualTo(UpdateStatus.CANCELLED)
+        assertThat(networkMapStorage.getCurrentParametersUpdate()?.description).isEqualTo("Update of update")
     }
 }

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

@@ -11,6 +11,7 @@
 package com.r3.corda.networkmanage.common.persistence
 
 import com.r3.corda.networkmanage.TestBase
+import com.r3.corda.networkmanage.common.persistence.entity.NodeInfoEntity
 import com.r3.corda.networkmanage.common.utils.hashString
 import net.corda.core.crypto.Crypto
 import net.corda.core.crypto.SecureHash
@@ -18,8 +19,8 @@ import net.corda.core.crypto.sha256
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.internal.CertRole
 import net.corda.core.serialization.serialize
+import net.corda.core.utilities.days
 import net.corda.nodeapi.internal.NodeInfoAndSigned
-import net.corda.nodeapi.internal.createDevNetworkMapCa
 import net.corda.nodeapi.internal.crypto.CertificateAndKeyPair
 import net.corda.nodeapi.internal.crypto.CertificateType
 import net.corda.nodeapi.internal.crypto.X509Utilities
@@ -36,6 +37,7 @@ import org.junit.Before
 import org.junit.Test
 import java.security.PrivateKey
 import java.security.cert.X509Certificate
+import java.time.Instant
 import javax.security.auth.x500.X500Principal
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
@@ -49,14 +51,12 @@ class PersistentNodeInfoStorageTest : TestBase() {
     private lateinit var persistence: CordaPersistence
     private lateinit var rootCaCert: X509Certificate
     private lateinit var doormanCertAndKeyPair: CertificateAndKeyPair
-    private lateinit var networkMapCertAndKeyPair: CertificateAndKeyPair
 
     @Before
     fun startDb() {
         val (rootCa, intermediateCa) = createDevIntermediateCaCertPath()
         rootCaCert = rootCa.certificate
         this.doormanCertAndKeyPair = intermediateCa
-        networkMapCertAndKeyPair = createDevNetworkMapCa(rootCa)
         persistence = configureDatabase(MockServices.makeTestDataSourceProperties(), DatabaseConfig(runMigration = true))
         nodeInfoStorage = PersistentNodeInfoStorage(persistence)
         requestStorage = PersistentCertificateSigningRequestStorage(persistence)
@@ -142,14 +142,26 @@ class PersistentNodeInfoStorageTest : TestBase() {
     @Test
     fun `putNodeInfo persists SignedNodeInfo with its signature`() {
         // given
-        val (nodeInfoWithSigned) = createValidSignedNodeInfo("Test", requestStorage)
+        val (nodeInfoAndSigned) = createValidSignedNodeInfo("Test", requestStorage)
 
         // when
-        val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoWithSigned)
+        val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
 
         // then
         val persistedSignedNodeInfo = nodeInfoStorage.getNodeInfo(nodeInfoHash)
-        assertThat(persistedSignedNodeInfo?.signatures).isEqualTo(nodeInfoWithSigned.signed.signatures)
+        assertThat(persistedSignedNodeInfo?.signatures).isEqualTo(nodeInfoAndSigned.signed.signatures)
+    }
+
+    @Test
+    fun `publish same node info twice`() {
+        fun singleNodeInfo() = persistence.transaction { session.fromQuery<NodeInfoEntity>("").singleResult }
+
+        val (nodeInfoAndSigned) = createValidSignedNodeInfo("Test", requestStorage)
+        nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
+        val nodeInfo = singleNodeInfo()
+        nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
+        assertThat(nodeInfo.publishedAt).isEqualTo(singleNodeInfo().publishedAt)  // Check publishAt hasn't changed
+        assertThat(singleNodeInfo().isCurrent).isTrue()
     }
 
     @Test
@@ -159,21 +171,21 @@ class PersistentNodeInfoStorageTest : TestBase() {
 
         // when
         val networkParameters = testNetworkParameters()
-        val sigWithCert = networkMapCertAndKeyPair.sign(networkParameters).sig
+        val netParamsHash = networkParameters.serialize().hash
-        val netParamsHash = networkMapStorage.saveNetworkParameters(networkParameters, sigWithCert)
+        networkMapStorage.saveNewParametersUpdate(networkParameters, "Update", Instant.now() + 1.days)
         val nodeInfoHash = nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
         nodeInfoStorage.ackNodeInfoParametersUpdate(nodeInfoAndSigned.nodeInfo.legalIdentities[0].owningKey.encoded.sha256(), netParamsHash)
 
         // then
-        val acceptedNetworkParameters = nodeInfoStorage.getAcceptedNetworkParameters(nodeInfoHash)
+        val acceptedUpdate = nodeInfoStorage.getAcceptedParametersUpdate(nodeInfoHash)
-        assertThat(acceptedNetworkParameters?.hash).isEqualTo(netParamsHash.toString())
+        assertThat(acceptedUpdate?.networkParameters?.hash).isEqualTo(netParamsHash.toString())
     }
 
     @Test
     fun `updating node info after it's accepted network parameters`() {
         val networkParameters = testNetworkParameters()
-        val sigWithCert = networkMapCertAndKeyPair.sign(networkParameters).sig
+        val netParamsHash = networkParameters.serialize().hash
-        val netParamsHash = networkMapStorage.saveNetworkParameters(networkParameters, sigWithCert)
+        networkMapStorage.saveNewParametersUpdate(networkParameters, "Update", Instant.now() + 1.days)
 
         val (nodeInfoAndSigned, privateKey) = createValidSignedNodeInfo("Test", requestStorage)
         nodeInfoStorage.putNodeInfo(nodeInfoAndSigned)
@@ -184,8 +196,8 @@ class PersistentNodeInfoStorageTest : TestBase() {
         val nodeInfoAndSigned2 = NodeInfoAndSigned(nodeInfo2.signWith(listOf(privateKey)))
         val nodeInfoHash2 = nodeInfoStorage.putNodeInfo(nodeInfoAndSigned2)
 
-        val acceptedNetworkParameters = nodeInfoStorage.getAcceptedNetworkParameters(nodeInfoHash2)
+        val acceptedUpdate = nodeInfoStorage.getAcceptedParametersUpdate(nodeInfoHash2)
-        assertThat(acceptedNetworkParameters?.hash).isEqualTo(netParamsHash.toString())
+        assertThat(acceptedUpdate?.networkParameters?.hash).isEqualTo(netParamsHash.toString())
     }
 }
 

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSignerTest.kt

@@ -14,6 +14,7 @@ import com.nhaarman.mockito_kotlin.*
 import com.r3.corda.networkmanage.TestBase
 import com.r3.corda.networkmanage.common.persistence.NetworkMapStorage
 import com.r3.corda.networkmanage.common.persistence.entity.ParametersUpdateEntity
+import com.r3.corda.networkmanage.common.persistence.entity.UpdateStatus
 import com.r3.corda.networkmanage.createNetworkMapEntity
 import com.r3.corda.networkmanage.createNetworkParametersEntity
 import com.r3.corda.networkmanage.createNetworkParametersEntityUnsigned
@@ -72,7 +73,7 @@ class NetworkMapSignerTest : TestBase() {
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(latestNetParamsEntity)
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(nodeInfoHashes)
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(null)
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(null)
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(null)
 
         // when
         networkMapSigner.signNetworkMap()
@@ -81,7 +82,7 @@ class NetworkMapSignerTest : TestBase() {
         // Verify networkMapStorage calls
         verify(networkMapStorage).getActiveNodeInfoHashes()
         verify(networkMapStorage).getActiveNetworkMap()
-        verify(networkMapStorage).getParametersUpdate()
+        verify(networkMapStorage).getCurrentParametersUpdate()
         verify(networkMapStorage).getLatestNetworkParameters()
         argumentCaptor<NetworkMapAndSigned>().apply {
             verify(networkMapStorage).saveNewActiveNetworkMap(capture())
@@ -122,7 +123,7 @@ class NetworkMapSignerTest : TestBase() {
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(createNetworkParametersEntityUnsigned(netParams))
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(emptyList())
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(null)
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(null)
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(null)
 
         // when
         networkMapSigner.signNetworkMap()
@@ -132,7 +133,7 @@ class NetworkMapSignerTest : TestBase() {
         verify(networkMapStorage).getActiveNodeInfoHashes()
         verify(networkMapStorage).getActiveNetworkMap()
         verify(networkMapStorage).getLatestNetworkParameters()
-        verify(networkMapStorage).getParametersUpdate()
+        verify(networkMapStorage).getCurrentParametersUpdate()
         argumentCaptor<NetworkMapAndSigned>().apply {
             verify(networkMapStorage).saveNewActiveNetworkMap(capture())
             assertEquals(netParams.serialize().hash, firstValue.networkMap.networkParameterHash)
@@ -152,7 +153,7 @@ class NetworkMapSignerTest : TestBase() {
         val parametersUpdate = ParametersUpdateEntity(0, updateNetworkParameters,"Update time", Instant.ofEpochMilli(random63BitValue()))
         val netMapEntity = createNetworkMapEntity(signingCertAndKeyPair, currentNetworkParameters, emptyList(), null)
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(netMapEntity)
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(parametersUpdate)
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(parametersUpdate)
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(updateNetworkParameters)
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(emptyList())
 
@@ -164,7 +165,7 @@ class NetworkMapSignerTest : TestBase() {
         verify(networkMapStorage).getActiveNetworkMap()
         verify(networkMapStorage).getActiveNodeInfoHashes()
         verify(networkMapStorage).getLatestNetworkParameters()
-        verify(networkMapStorage).getParametersUpdate()
+        verify(networkMapStorage).getCurrentParametersUpdate()
 
         val paramsCaptor = argumentCaptor<NetworkParameters>()
         val signatureCaptor = argumentCaptor<DigitalSignatureWithCert>()
@@ -178,7 +179,7 @@ class NetworkMapSignerTest : TestBase() {
         val updateNetworkParameters = createNetworkParametersEntityUnsigned(testNetworkParameters(epoch = 2))
         val parametersUpdate = ParametersUpdateEntity(0, updateNetworkParameters,"Update time", Instant.ofEpochMilli(random63BitValue()))
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(null)
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(parametersUpdate)
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(parametersUpdate)
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(emptyList())
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(createNetworkParametersEntity())
 
@@ -195,7 +196,7 @@ class NetworkMapSignerTest : TestBase() {
 
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(activeNetworkMap)
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(emptyList())
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(parametersUpdate.copy(flagDay = true))
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(parametersUpdate.copy(status = UpdateStatus.FLAG_DAY))
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(updateNetworkParameters)
 
         // when
@@ -220,7 +221,7 @@ class NetworkMapSignerTest : TestBase() {
 
         whenever(networkMapStorage.getActiveNetworkMap()).thenReturn(activeNetworkMap)
         whenever(networkMapStorage.getActiveNodeInfoHashes()).thenReturn(emptyList())
-        whenever(networkMapStorage.getParametersUpdate()).thenReturn(null)
+        whenever(networkMapStorage.getCurrentParametersUpdate()).thenReturn(null)
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(createNetworkParametersEntity())
 
         // when