From 56c6ec967c9643ec78bed2bbec15f3f405d10d48 Mon Sep 17 00:00:00 2001 From: Katarzyna Streich Date: Wed, 9 May 2018 15:01:57 +0100 Subject: [PATCH] ENT-1803: Allow change of updateDeadline on parameters update (#814) * Allow change of updateDeadline on parameters update ENT-1803. Parameters that where already advertised still need explicit cancellation. --- .../doorman/ParametersUpdateHandler.kt | 21 ++--- .../com/r3/corda/networkmanage/TestUtils.kt | 12 +++ .../doorman/NetworkParametersCmdTest.kt | 12 +-- .../doorman/ParametersUpdateHandlerTest.kt | 79 ++++++++++++++++++- 4 files changed, 102 insertions(+), 22 deletions(-) diff --git a/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandler.kt b/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandler.kt index b757dcd2c3..20534e73b9 100644 --- a/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandler.kt +++ b/network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandler.kt @@ -48,7 +48,9 @@ class ParametersUpdateHandler(val csrStorage: CertificateSigningRequestStorage, private fun handleSetNetworkParameters(setNetParams: NetworkParametersCmd.Set) { logger.info("maxMessageSize is not currently wired in the nodes") - val activeNetParams = networkMapStorage.getNetworkMaps().publicNetworkMap?.networkParameters?.networkParameters + val activeMap = networkMapStorage.getNetworkMaps().publicNetworkMap + val activeNetParams = activeMap?.networkParameters?.networkParameters + // Setting initial parameters case. if (activeNetParams == null) { require(setNetParams.parametersUpdate == null) { "'parametersUpdate' specified in network parameters file but there are no network parameters to update" @@ -57,27 +59,26 @@ class ParametersUpdateHandler(val csrStorage: CertificateSigningRequestStorage, logger.info("Saving initial network parameters to be signed:\n$initialNetParams") networkMapStorage.saveNetworkParameters(initialNetParams, null) println("Saved initial network parameters to be signed:\n$initialNetParams") - } else { + } else { // An update. val parametersUpdate = requireNotNull(setNetParams.parametersUpdate) { "'parametersUpdate' not specified in network parameters file but there is already an active set of network parameters" } - setNetParams.checkCompatibility(activeNetParams) - val latestNetParams = checkNotNull(networkMapStorage.getLatestNetworkParameters()?.networkParameters) { "Something has gone wrong! We have an active set of network parameters ($activeNetParams) but apparently no latest network parameters!" } - // It's not necessary that latestNetParams is the current active network parameters. It can be the network - // parameters from a previous update attempt which has't activated yet. We still take the epoch value for this + // parameters from a previous update attempt which hasn't activated yet. We still take the epoch value for this // new set from latestNetParams to make sure the advertised update attempts have incrementing epochs. // This has the implication that *active* network parameters may have gaps in their epochs. val newNetParams = setNetParams.toNetworkParameters(modifiedTime = Instant.now(), epoch = latestNetParams.epoch + 1) - + val activeUpdate = activeMap.networkMap.parametersUpdate + if (sameNetworkParameters(latestNetParams, newNetParams) && activeUpdate != null) { + // TODO We don't have cancel event propagation on client side. + throw IllegalArgumentException("New network parameters are the same as the latest ones and there is an update scheduled: $activeUpdate\n" + + "If you want to just change updateDeadline or update description - cancel old update first.") + } logger.info("Enabling update to network parameters:\n$newNetParams\n$parametersUpdate") - - require(!sameNetworkParameters(latestNetParams, newNetParams)) { "New network parameters are the same as the latest ones" } - networkMapStorage.saveNewParametersUpdate(newNetParams, parametersUpdate.description, parametersUpdate.updateDeadline) logger.info("Update enabled") diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestUtils.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestUtils.kt index aa03ef442c..4388c045d7 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestUtils.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/TestUtils.kt @@ -3,6 +3,8 @@ package com.r3.corda.networkmanage import com.r3.corda.networkmanage.common.persistence.NetworkMaps import com.r3.corda.networkmanage.common.persistence.entity.NetworkMapEntity import com.r3.corda.networkmanage.common.persistence.entity.NetworkParametersEntity +import com.r3.corda.networkmanage.doorman.NetworkParametersCmd +import com.r3.corda.networkmanage.doorman.ParametersUpdateConfig import net.corda.core.crypto.SecureHash import net.corda.core.node.NetworkParameters import net.corda.core.serialization.serialize @@ -71,3 +73,13 @@ fun createNetworkMaps(signingCertAndKeyPair: CertificateAndKeyPair = createDevNe val netParamsEntity = createNetworkParametersEntity(signingCertAndKeyPair, networkParameters) return createNetworkMaps(signingCertAndKeyPair, netParamsEntity, nodeInfoHashes, privateNodeInfoHashes, timestamp = timestamp) } + +fun NetworkParameters.toCmd(parametersUpdate: ParametersUpdateConfig? = null): NetworkParametersCmd.Set { + return NetworkParametersCmd.Set( + minimumPlatformVersion = minimumPlatformVersion, + notaries = notaries, + maxMessageSize = maxMessageSize, + maxTransactionSize = maxTransactionSize, + parametersUpdate = parametersUpdate + ) +} diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersCmdTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersCmdTest.kt index 1e28204869..06d78ae59a 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersCmdTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/NetworkParametersCmdTest.kt @@ -1,8 +1,8 @@ package com.r3.corda.networkmanage.doorman +import com.r3.corda.networkmanage.toCmd import net.corda.core.identity.CordaX500Name import net.corda.core.identity.Party -import net.corda.core.node.NetworkParameters import net.corda.core.node.NotaryInfo import net.corda.testing.common.internal.testNetworkParameters import net.corda.testing.core.ALICE_NAME @@ -56,15 +56,5 @@ class NetworkParametersCmdTest { .withMessageContaining("notaries") } - private fun NetworkParameters.toCmd(): NetworkParametersCmd.Set { - return NetworkParametersCmd.Set( - minimumPlatformVersion = minimumPlatformVersion, - notaries = notaries, - maxMessageSize = maxMessageSize, - maxTransactionSize = maxTransactionSize, - parametersUpdate = null - ) - } - private fun freshParty(name: CordaX500Name) = TestIdentity(name).party } \ No newline at end of file diff --git a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandlerTest.kt b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandlerTest.kt index 5db3964db9..ca904e8691 100644 --- a/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandlerTest.kt +++ b/network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/ParametersUpdateHandlerTest.kt @@ -2,22 +2,34 @@ package com.r3.corda.networkmanage.doorman import com.nhaarman.mockito_kotlin.mock import com.r3.corda.networkmanage.common.persistence.* +import com.r3.corda.networkmanage.toCmd import com.typesafe.config.* +import net.corda.core.crypto.SecureHash import net.corda.core.internal.* +import net.corda.core.node.NetworkParameters import net.corda.core.serialization.serialize +import net.corda.core.utilities.days +import net.corda.nodeapi.internal.createDevNetworkMapCa +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.persistence.CordaPersistence import net.corda.nodeapi.internal.persistence.DatabaseConfig +import net.corda.testing.common.internal.testNetworkParameters import net.corda.testing.core.SerializationEnvironmentRule +import net.corda.testing.internal.createDevIntermediateCaCertPath import net.corda.testing.internal.signWith import net.corda.testing.node.MockServices import org.assertj.core.api.Assertions import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.After import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import java.nio.file.Path +import java.time.Instant import java.util.* class ParametersUpdateHandlerTest { @@ -31,12 +43,14 @@ class ParametersUpdateHandlerTest { private lateinit var persistence: CordaPersistence private lateinit var csrStorage: CertificateSigningRequestStorage private lateinit var netParamsUpdateHandler: ParametersUpdateHandler + private lateinit var networkMapStorage: PersistentNetworkMapStorage @Before fun init() { persistence = configureDatabase(MockServices.makeTestDataSourceProperties(), DatabaseConfig(runMigration = true)) csrStorage = PersistentCertificateSigningRequestStorage(persistence) - netParamsUpdateHandler = ParametersUpdateHandler(csrStorage, mock()) + networkMapStorage = PersistentNetworkMapStorage(persistence) + netParamsUpdateHandler = ParametersUpdateHandler(csrStorage, networkMapStorage) } @After @@ -75,6 +89,69 @@ class ParametersUpdateHandlerTest { .hasMessageContaining("is not registered with the doorman") } + @Test + fun `set twice initial parameters`() { + val testParams = testNetworkParameters() + val setInitialCmd = testParams.toCmd() + netParamsUpdateHandler.processNetworkParameters(setInitialCmd) + val testParams2 = testParams.copy(minimumPlatformVersion = 7) + netParamsUpdateHandler.processNetworkParameters(testParams2.toCmd()) + assertThat(networkMapStorage.getLatestNetworkParameters()!!.networkParameters.minimumPlatformVersion).isEqualTo(7) + assertThatThrownBy { netParamsUpdateHandler.processNetworkParameters( + testParams2.toCmd(parametersUpdate = ParametersUpdateConfig("It needs changing. Now!", updateDeadline = Instant.now()))) + }.hasMessageContaining("'parametersUpdate' specified in network parameters file but there are no network parameters to update") + } + + @Test + fun `cancel parameters and then set the same`() { + loadInitialParams() + val paramsForUpdate = testNetworkParameters(minimumPlatformVersion = 101) + val updateDeadline1 = Instant.now() + 10.days + val updateParamsCmd = paramsForUpdate.toCmd(ParametersUpdateConfig("Update", updateDeadline1)) + netParamsUpdateHandler.processNetworkParameters(updateParamsCmd) + val cancelCmd = NetworkParametersCmd.CancelUpdate + netParamsUpdateHandler.processNetworkParameters(cancelCmd) + // Use just cancelled parameters + netParamsUpdateHandler.processNetworkParameters(updateParamsCmd) + } + + @Test + fun `set parameters update and then change update deadline`() { + loadInitialParams() + val paramsForUpdate = testNetworkParameters(minimumPlatformVersion = 101) + val updateDeadline1 = Instant.now() + 10.days + val updateParamsCmd = paramsForUpdate.toCmd(ParametersUpdateConfig("Update", updateDeadline1)) + netParamsUpdateHandler.processNetworkParameters(updateParamsCmd) + // Just change description + netParamsUpdateHandler.processNetworkParameters(paramsForUpdate.toCmd(ParametersUpdateConfig("Update again", updateDeadline1))) + // Just change update deadline + netParamsUpdateHandler.processNetworkParameters(paramsForUpdate.toCmd(ParametersUpdateConfig("Update again", updateDeadline1 + 10.days))) + } + + @Test + fun `try to change active update`() { + val paramsForUpdate = testNetworkParameters(minimumPlatformVersion = 101) + val updateDeadline = Instant.now() + 10.days + val description = "Update" + val paramsUpdate = ParametersUpdate(paramsForUpdate.serialize().hash, description, updateDeadline) + loadInitialParams(parametersUpdate = paramsUpdate) + networkMapStorage.saveNetworkParameters(paramsForUpdate, null) + val updateParamsCmd = paramsForUpdate.toCmd(ParametersUpdateConfig(description, updateDeadline)) + assertThatThrownBy { netParamsUpdateHandler.processNetworkParameters(updateParamsCmd) } + .hasMessageContaining("New network parameters are the same as the latest ones") + } + + // Load initial parameters to db and sign them, set them to be in active network map. + private fun loadInitialParams(params: NetworkParameters = testNetworkParameters(), parametersUpdate: ParametersUpdate? = null) { + val (rootCa) = createDevIntermediateCaCertPath() + val networkMapCertAndKeyPair = createDevNetworkMapCa(rootCa) + val networkParametersSig = networkMapCertAndKeyPair.sign(params).sig + val networkParametersHash = networkMapStorage.saveNetworkParameters(params, networkParametersSig).hash + val networkMap = NetworkMap(emptyList(), SecureHash.parse(networkParametersHash), parametersUpdate) + val networkMapAndSigned = NetworkMapAndSigned(networkMap) { networkMapCertAndKeyPair.sign(networkMap).sig } + networkMapStorage.saveNewNetworkMap(networkMapAndSigned = networkMapAndSigned) + } + private fun saveConfig(configFile: Path, notaryFiles: List) { val config = ConfigValueFactory.fromMap( mapOf("minimumPlatformVersion" to 1,