Vault Service API final clean-up (#1348)

* Remove notify/notifyAll from public Vault Service API.

* 1st pass - remove ContractUpgrade public API calls from VaultService into self contained ContractUpgradeService.

* Fix compile error caused by cut'n'paste refactoring.

.idea/runConfigurations/Quasar_exclude_pattern_extraction_from_node_tests__need_not_pass__check_output_at_the_bottom_of_console_.xml

@@ -1,25 +0,0 @@
-<component name="ProjectRunConfigurationManager">
-  <configuration default="false" name="Quasar exclude pattern extraction from node tests (need not pass, check output at the bottom of console)" type="JUnit" factoryName="JUnit">
-    <module name="" />
-    <option name="ALTERNATIVE_JRE_PATH_ENABLED" value="false" />
-    <option name="ALTERNATIVE_JRE_PATH" />
-    <option name="PACKAGE_NAME" value="net.corda.node" />
-    <option name="MAIN_CLASS_NAME" value="" />
-    <option name="METHOD_NAME" value="" />
-    <option name="TEST_OBJECT" value="package" />
-    <option name="VM_PARAMETERS" value="-ea -javaagent:$PROJECT_DIR$/experimental/quasar-hook/build/libs/quasar-hook.jar=&quot;expand=com,de,org,co,io;truncate=net.corda;alwaysExcluded=com.opengamma,io.atomix&quot; -javaagent:lib/quasar.jar -Xmx4G" />
-    <option name="PARAMETERS" value="" />
-    <option name="WORKING_DIRECTORY" value="file://$PROJECT_DIR$" />
-    <option name="ENV_VARIABLES" />
-    <option name="PASS_PARENT_ENVS" value="true" />
-    <option name="TEST_SEARCH_SCOPE">
-      <value defaultName="wholeProject" />
-    </option>
-    <envs />
-    <dir value="$PROJECT_DIR$" />
-    <patterns />
-    <method>
-      <option name="Gradle.BeforeRunTask" enabled="true" tasks="jar" externalProjectPath="$PROJECT_DIR$/experimental/quasar-hook" vmOptions="" scriptParameters="" />
-    </method>
-  </configuration>
-</component>

core/src/main/kotlin/net/corda/core/node/ServiceHub.kt

@@ -46,6 +46,7 @@ interface ServiceHub : ServicesForResolution {
     val vaultService: VaultService
     val vaultQueryService: VaultQueryService
     val keyManagementService: KeyManagementService
+    val contractUpgradeService: ContractUpgradeService
 
     /**
      * A map of hash->tx where tx has been signature/contract validated and the states are known to be correct.

core/src/main/kotlin/net/corda/core/node/services/ContractUpgradeService.kt

@@ -0,0 +1,30 @@
+package net.corda.core.node.services
+
+import net.corda.core.contracts.StateAndRef
+import net.corda.core.contracts.StateRef
+import net.corda.core.contracts.UpgradedContract
+
+/**
+ * The [ContractUpgradeService] is responsible for securely upgrading contract state objects according to
+ * a specified and mutually agreed (amongst participants) contract version.
+ * See also [ContractUpgradeFlow] to understand the workflow associated with contract upgrades.
+ */
+interface ContractUpgradeService {
+
+    /** Get contracts we would be willing to upgrade the suggested contract to. */
+    fun getAuthorisedContractUpgrade(ref: StateRef): Class<out UpgradedContract<*, *>>?
+
+    /**
+     * Authorise a contract state upgrade.
+     * This will store the upgrade authorisation in the vault, and will be queried by [ContractUpgradeFlow.Acceptor] during contract upgrade process.
+     * Invoking this method indicate the node is willing to upgrade the [state] using the [upgradedContractClass].
+     * This method will NOT initiate the upgrade process. To start the upgrade process, see [ContractUpgradeFlow.Instigator].
+     */
+    fun authoriseContractUpgrade(stateAndRef: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>)
+
+    /**
+     * Authorise a contract state upgrade.
+     * This will remove the upgrade authorisation from the vault.
+     */
+    fun deauthoriseContractUpgrade(stateAndRef: StateAndRef<*>)
+}

core/src/main/kotlin/net/corda/core/node/services/VaultService.kt

@@ -173,17 +173,6 @@ interface VaultService {
      */
     val updatesPublisher: PublishSubject<Vault.Update<ContractState>>
 
-    /**
-     * Possibly update the vault by marking as spent states that these transactions consume, and adding any relevant
-     * new states that they create. You should only insert transactions that have been successfully verified here!
-     *
-     * TODO: Consider if there's a good way to enforce the must-be-verified requirement in the type system.
-     */
-    fun notifyAll(txns: Iterable<CoreTransaction>)
-
-    /** Same as notifyAll but with a single transaction. */
-    fun notify(tx: CoreTransaction) = notifyAll(listOf(tx))
-
     /**
      * Provide a [CordaFuture] for when a [StateRef] is consumed, which can be very useful in building tests.
      */
@@ -191,24 +180,6 @@ interface VaultService {
         return updates.filter { it.consumed.any { it.ref == ref } }.toFuture()
     }
 
-    /** Get contracts we would be willing to upgrade the suggested contract to. */
-    // TODO: We need a better place to put business logic functions
-    fun getAuthorisedContractUpgrade(ref: StateRef): Class<out UpgradedContract<*, *>>?
-
-    /**
-     * Authorise a contract state upgrade.
-     * This will store the upgrade authorisation in the vault, and will be queried by [ContractUpgradeFlow.Acceptor] during contract upgrade process.
-     * Invoking this method indicate the node is willing to upgrade the [state] using the [upgradedContractClass].
-     * This method will NOT initiate the upgrade process. To start the upgrade process, see [ContractUpgradeFlow.Instigator].
-     */
-    fun authoriseContractUpgrade(stateAndRef: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>)
-
-    /**
-     * Authorise a contract state upgrade.
-     * This will remove the upgrade authorisation from the vault.
-     */
-    fun deauthoriseContractUpgrade(stateAndRef: StateAndRef<*>)
-
     /**
      *  Add a note to an existing [LedgerTransaction] given by its unique [SecureHash] id
      *  Multiple notes may be attached to the same [LedgerTransaction].

core/src/test/kotlin/net/corda/core/flows/ContractUpgradeFlowTest.kt

@@ -79,7 +79,7 @@ class ContractUpgradeFlowTest {
         assertFailsWith(UnexpectedFlowEndException::class) { rejectedFuture.getOrThrow() }
 
         // Party B authorise the contract state upgrade.
-        b.services.vaultService.authoriseContractUpgrade(btx!!.tx.outRef<ContractState>(0), DummyContractV2::class.java)
+        b.services.contractUpgradeService.authoriseContractUpgrade(btx!!.tx.outRef<ContractState>(0), DummyContractV2::class.java)
 
         // Party A initiates contract upgrade flow, expected to succeed this time.
         val resultFuture = a.services.startFlow(ContractUpgradeFlow(atx.tx.outRef(0), DummyContractV2::class.java)).resultFuture

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

@@ -56,6 +56,7 @@ import net.corda.node.services.statemachine.FlowStateMachineImpl
 import net.corda.node.services.statemachine.StateMachineManager
 import net.corda.node.services.statemachine.flowVersionAndInitiatingClass
 import net.corda.node.services.transactions.*
+import net.corda.node.services.upgrade.ContractUpgradeServiceImpl
 import net.corda.node.services.vault.HibernateVaultQueryImpl
 import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.services.vault.VaultSoftLockManager
@@ -770,6 +771,7 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         override val schemaService by lazy { NodeSchemaService(pluginRegistries.flatMap { it.requiredSchemas }.toSet()) }
         override val networkMapCache by lazy { PersistentNetworkMapCache(this) }
         override val vaultService by lazy { NodeVaultService(this) }
+        override val contractUpgradeService by lazy { ContractUpgradeServiceImpl() }
         override val vaultQueryService by lazy {
             HibernateVaultQueryImpl(database.hibernateConfig, vaultService)
         }

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

@@ -170,8 +170,8 @@ class CordaRPCOpsImpl(
         }
     }
 
-    override fun authoriseContractUpgrade(state: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>) = services.vaultService.authoriseContractUpgrade(state, upgradedContractClass)
+    override fun authoriseContractUpgrade(state: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>) = services.contractUpgradeService.authoriseContractUpgrade(state, upgradedContractClass)
-    override fun deauthoriseContractUpgrade(state: StateAndRef<*>) = services.vaultService.deauthoriseContractUpgrade(state)
+    override fun deauthoriseContractUpgrade(state: StateAndRef<*>) = services.contractUpgradeService.deauthoriseContractUpgrade(state)
     override fun currentNodeTime(): Instant = Instant.now(services.clock)
     override fun waitUntilNetworkReady() = services.networkMapCache.nodeReady
     override fun partyFromAnonymous(party: AbstractParty): Party? = services.identityService.partyFromAnonymous(party)

node/src/main/kotlin/net/corda/node/services/CoreFlowHandlers.kt

@@ -58,7 +58,7 @@ class ContractUpgradeHandler(otherSide: Party) : AbstractStateReplacementFlow.Ac
         val ourSTX = serviceHub.validatedTransactions.getTransaction(proposal.stateRef.txhash)
         requireNotNull(ourSTX) { "We don't have a copy of the referenced state" }
         val oldStateAndRef = ourSTX!!.tx.outRef<ContractState>(proposal.stateRef.index)
-        val authorisedUpgrade = serviceHub.vaultService.getAuthorisedContractUpgrade(oldStateAndRef.ref) ?:
+        val authorisedUpgrade = serviceHub.contractUpgradeService.getAuthorisedContractUpgrade(oldStateAndRef.ref) ?:
                 throw IllegalStateException("Contract state upgrade is unauthorised. State hash : ${oldStateAndRef.ref}")
         val proposedTx = stx.tx
         val expectedTx = ContractUpgradeFlow.assembleBareTx(oldStateAndRef, proposal.modification, proposedTx.privacySalt).toWireTransaction()

node/src/main/kotlin/net/corda/node/services/api/ServiceHubInternal.kt

@@ -22,6 +22,7 @@ import net.corda.node.services.config.NodeConfiguration
 import net.corda.node.services.messaging.MessagingService
 import net.corda.node.services.statemachine.FlowLogicRefFactoryImpl
 import net.corda.node.services.statemachine.FlowStateMachineImpl
+import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.utilities.CordaPersistence
 
 interface NetworkMapCacheInternal : NetworkMapCache {
@@ -100,7 +101,7 @@ interface ServiceHubInternal : PluginServiceHub {
 
         if (notifyVault) {
             val toNotify = recordedTransactions.map { if (it.isNotaryChangeTransaction()) it.notaryChangeTx else it.tx }
-            vaultService.notifyAll(toNotify)
+            (vaultService as NodeVaultService).notifyAll(toNotify)
         }
     }
 

node/src/main/kotlin/net/corda/node/services/upgrade/ContractUpgradeServiceImpl.kt

@@ -0,0 +1,26 @@
+package net.corda.node.services.upgrade
+
+import net.corda.core.contracts.StateAndRef
+import net.corda.core.contracts.StateRef
+import net.corda.core.contracts.UpgradedContract
+import net.corda.core.node.services.ContractUpgradeService
+
+class ContractUpgradeServiceImpl : ContractUpgradeService {
+
+    // TODO : Persist this in DB.
+    private val authorisedUpgrade = mutableMapOf<StateRef, Class<out UpgradedContract<*, *>>>()
+
+    override fun getAuthorisedContractUpgrade(ref: StateRef) = authorisedUpgrade[ref]
+
+    override fun authoriseContractUpgrade(stateAndRef: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>) {
+        val upgrade = upgradedContractClass.newInstance()
+        if (upgrade.legacyContract != stateAndRef.state.data.contract.javaClass) {
+            throw IllegalArgumentException("The contract state cannot be upgraded using provided UpgradedContract.")
+        }
+        authorisedUpgrade.put(stateAndRef.ref, upgradedContractClass)
+    }
+
+    override fun deauthoriseContractUpgrade(stateAndRef: StateAndRef<*>) {
+        authorisedUpgrade.remove(stateAndRef.ref)
+    }
+}

node/src/main/kotlin/net/corda/node/services/vault/NodeVaultService.kt

@@ -114,7 +114,7 @@ class NodeVaultService(private val services: ServiceHub) : SingletonSerializeAsT
      * indicate whether an update consists entirely of regular or notary change transactions, which may require
      * different processing logic.
      */
-    override fun notifyAll(txns: Iterable<CoreTransaction>) {
+    fun notifyAll(txns: Iterable<CoreTransaction>) {
         // It'd be easier to just group by type, but then we'd lose ordering.
         val regularTxns = mutableListOf<WireTransaction>()
         val notaryChangeTxns = mutableListOf<NotaryChangeWireTransaction>()
@@ -142,6 +142,9 @@ class NodeVaultService(private val services: ServiceHub) : SingletonSerializeAsT
         if (notaryChangeTxns.isNotEmpty()) notifyNotaryChange(notaryChangeTxns.toList())
     }
 
+    /** Same as notifyAll but with a single transaction. */
+    fun notify(tx: CoreTransaction) = notifyAll(listOf(tx))
+
     private fun notifyRegular(txns: Iterable<WireTransaction>) {
         fun makeUpdate(tx: WireTransaction): Vault.Update<ContractState> {
             val myKeys = services.keyManagementService.filterMyKeys(tx.outputs.flatMap { it.data.participants.map { it.owningKey } })
@@ -369,22 +372,7 @@ class NodeVaultService(private val services: ServiceHub) : SingletonSerializeAsT
         return claimedStates
     }
 
-    // TODO : Persists this in DB.
-    private val authorisedUpgrade = mutableMapOf<StateRef, Class<out UpgradedContract<*, *>>>()
 
-    override fun getAuthorisedContractUpgrade(ref: StateRef) = authorisedUpgrade[ref]
-
-    override fun authoriseContractUpgrade(stateAndRef: StateAndRef<*>, upgradedContractClass: Class<out UpgradedContract<*, *>>) {
-        val upgrade = upgradedContractClass.newInstance()
-        if (upgrade.legacyContract != stateAndRef.state.data.contract.javaClass) {
-            throw IllegalArgumentException("The contract state cannot be upgraded using provided UpgradedContract.")
-        }
-        authorisedUpgrade.put(stateAndRef.ref, upgradedContractClass)
-    }
-
-    override fun deauthoriseContractUpgrade(stateAndRef: StateAndRef<*>) {
-        authorisedUpgrade.remove(stateAndRef.ref)
-    }
 
     @VisibleForTesting
     internal fun isRelevant(state: ContractState, myKeys: Set<PublicKey>): Boolean {

node/src/test/kotlin/net/corda/node/services/persistence/DBTransactionStorageTests.kt

@@ -64,7 +64,7 @@ class DBTransactionStorageTests : TestDependencyInjectionBase() {
                         validatedTransactions.addTransaction(stx)
                     }
                     // Refactored to use notifyAll() as we have no other unit test for that method with multiple transactions.
-                    vaultService.notifyAll(txs.map { it.tx })
+                    (vaultService as NodeVaultService).notifyAll(txs.map { it.tx })
                 }
             }
         }

node/src/test/kotlin/net/corda/node/services/persistence/HibernateConfigurationTest.kt

@@ -25,6 +25,7 @@ import net.corda.finance.schemas.SampleCashSchemaV3
 import net.corda.finance.utils.sumCash
 import net.corda.node.services.schema.HibernateObserver
 import net.corda.node.services.schema.NodeSchemaService
+import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.services.vault.VaultSchemaV1
 import net.corda.node.utilities.CordaPersistence
 import net.corda.node.utilities.configureDatabase
@@ -88,7 +89,7 @@ class HibernateConfigurationTest : TestDependencyInjectionBase() {
                         validatedTransactions.addTransaction(stx)
                     }
                     // Refactored to use notifyAll() as we have no other unit test for that method with multiple transactions.
-                    vaultService.notifyAll(txs.map { it.tx })
+                    (vaultService as NodeVaultService).notifyAll(txs.map { it.tx })
                 }
                 override fun jdbcSession() = database.createSession()
             }

node/src/test/kotlin/net/corda/node/services/vault/NodeVaultServiceTest.kt

@@ -96,7 +96,7 @@ class NodeVaultServiceTest : TestDependencyInjectionBase() {
             val originalVault = vaultSvc
             val originalVaultQuery = vaultQuery
             val services2 = object : MockServices() {
-                override val vaultService: VaultService get() = originalVault
+                override val vaultService: NodeVaultService get() = originalVault as NodeVaultService
                 override fun recordTransactions(notifyVault: Boolean, txs: Iterable<SignedTransaction>) {
                     for (stx in txs) {
                         validatedTransactions.addTransaction(stx)

test-utils/src/main/kotlin/net/corda/node/testing/MockServiceHubInternal.kt

@@ -42,6 +42,7 @@ open class MockServiceHubInternal(
         val scheduler: SchedulerService? = null,
         val overrideClock: Clock? = NodeClock(),
         val schemas: SchemaService? = NodeSchemaService(),
+        val customContractUpgradeService: ContractUpgradeService? = null,
         val customTransactionVerifierService: TransactionVerifierService? = InMemoryTransactionVerifierService(2)
 ) : ServiceHubInternal {
     override val vaultQueryService: VaultQueryService
@@ -50,6 +51,8 @@ open class MockServiceHubInternal(
         get() = customTransactionVerifierService ?: throw UnsupportedOperationException()
     override val vaultService: VaultService
         get() = customVault ?: throw UnsupportedOperationException()
+    override val contractUpgradeService: ContractUpgradeService
+        get() = customContractUpgradeService ?: throw UnsupportedOperationException()
     override val keyManagementService: KeyManagementService
         get() = keyManagement ?: throw UnsupportedOperationException()
     override val identityService: IdentityService

test-utils/src/main/kotlin/net/corda/testing/node/MockServices.kt

@@ -26,6 +26,7 @@ import net.corda.node.services.persistence.InMemoryStateMachineRecordedTransacti
 import net.corda.node.services.schema.HibernateObserver
 import net.corda.node.services.schema.NodeSchemaService
 import net.corda.node.services.transactions.InMemoryTransactionVerifierService
+import net.corda.node.services.upgrade.ContractUpgradeServiceImpl
 import net.corda.node.services.vault.HibernateVaultQueryImpl
 import net.corda.node.services.vault.NodeVaultService
 import net.corda.node.utilities.CordaPersistence
@@ -123,7 +124,7 @@ open class MockServices(vararg val keys: KeyPair) : ServiceHub {
                             validatedTransactions.addTransaction(stx)
                         }
                         // Refactored to use notifyAll() as we have no other unit test for that method with multiple transactions.
-                        vaultService.notifyAll(txs.map { it.tx })
+                        (vaultService as NodeVaultService).notifyAll(txs.map { it.tx })
                     }
 
                     override val vaultQueryService: VaultQueryService = HibernateVaultQueryImpl(database.hibernateConfig, vaultService)
@@ -155,6 +156,7 @@ open class MockServices(vararg val keys: KeyPair) : ServiceHub {
     override val keyManagementService: KeyManagementService = MockKeyManagementService(identityService, *keys)
 
     override val vaultService: VaultService get() = throw UnsupportedOperationException()
+    override val contractUpgradeService: ContractUpgradeService = ContractUpgradeServiceImpl()
     override val vaultQueryService: VaultQueryService get() = throw UnsupportedOperationException()
     override val networkMapCache: NetworkMapCache get() = throw UnsupportedOperationException()
     override val clock: Clock get() = Clock.systemUTC()