Add a test for notary change transaction updates in the vault.

Fix a minor related issue with update type propagation.

Add static NoNotaryUpdate object

Added isEmpty check to Vault.Update

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

@@ -34,7 +34,6 @@ import java.util.*
  */
 @CordaSerializable
 class Vault<out T : ContractState>(val states: Iterable<StateAndRef<T>>) {
-
     /**
      * Represents an update observed by the vault that will be notified to observers.  Include the [StateRef]s of
      * transaction outputs that were consumed (inputs) and the [ContractState]s produced (outputs) to/by the transaction
@@ -44,15 +43,17 @@ class Vault<out T : ContractState>(val states: Iterable<StateAndRef<T>>) {
      * other transactions observed, then the changes are observed "net" of those.
      */
     @CordaSerializable
-    data class Update<U : ContractState>(val consumed: Set<StateAndRef<U>>,
+    data class Update<U : ContractState>(
-                      val produced: Set<StateAndRef<U>>,
+            val consumed: Set<StateAndRef<U>>,
-                      val flowId: UUID? = null,
+            val produced: Set<StateAndRef<U>>,
-                      /**
+            val flowId: UUID? = null,
-                       * Specifies the type of update, currently supported types are general and notary change. Notary
+            /**
-                       * change transactions only modify the notary field on states, and potentially need to be handled
+             * Specifies the type of update, currently supported types are general and notary change. Notary
-                       * differently.
+             * change transactions only modify the notary field on states, and potentially need to be handled
-                       */
+             * differently.
-                      val type: UpdateType = UpdateType.GENERAL) {
+             */
+            val type: UpdateType = UpdateType.GENERAL
+    ) {
         /** Checks whether the update contains a state of the specified type. */
         inline fun <reified T : ContractState> containsType() = consumed.any { it.state.data is T } || produced.any { it.state.data is T }
 
@@ -65,6 +66,8 @@ class Vault<out T : ContractState>(val states: Iterable<StateAndRef<T>>) {
                             || produced.any { clazz.isAssignableFrom(it.state.data.javaClass) }
                 }
 
+        fun isEmpty() = consumed.isEmpty() && produced.isEmpty()
+
         /**
          * Combine two updates into a single update with the combined inputs and outputs of the two updates but net
          * any outputs of the left-hand-side (this) that are consumed by the inputs of the right-hand-side (rhs).
@@ -72,17 +75,22 @@ class Vault<out T : ContractState>(val states: Iterable<StateAndRef<T>>) {
          * i.e. the net effect in terms of state live-ness of receiving the combined update is the same as receiving this followed by rhs.
          */
         operator fun plus(rhs: Update<U>): Update<U> {
-            val combined = Vault.Update<U>(
+            require(rhs.type == type) { "Cannot combine updates of different types" }
-                    consumed + (rhs.consumed - produced),
+            val combinedConsumed = consumed + (rhs.consumed - produced)
-                    // The ordering below matters to preserve ordering of consumed/produced Sets when they are insertion order dependent implementations.
+            // The ordering below matters to preserve ordering of consumed/produced Sets when they are insertion order dependent implementations.
-                    produced.filter { it !in rhs.consumed }.toSet() + rhs.produced)
+            val combinedProduced = produced.filter { it !in rhs.consumed }.toSet() + rhs.produced
-            return combined
+            return copy(consumed = combinedConsumed, produced = combinedProduced)
         }
 
         override fun toString(): String {
             val sb = StringBuilder()
             sb.appendln("${consumed.size} consumed, ${produced.size} produced")
             sb.appendln("")
+            sb.appendln("Consumed:")
+            consumed.forEach {
+                sb.appendln("${it.ref}: ${it.state}")
+            }
+            sb.appendln("")
             sb.appendln("Produced:")
             produced.forEach {
                 sb.appendln("${it.ref}: ${it.state}")
@@ -92,7 +100,8 @@ class Vault<out T : ContractState>(val states: Iterable<StateAndRef<T>>) {
     }
 
     companion object {
-        val NoUpdate = Update<ContractState>(emptySet(), emptySet())
+        val NoUpdate = Update(emptySet(), emptySet(), type = Vault.UpdateType.GENERAL)
+        val NoNotaryUpdate = Vault.Update(emptySet(), emptySet(),  type = Vault.UpdateType.NOTARY_CHANGE)
     }
 
     @CordaSerializable

core/src/test/kotlin/net/corda/core/node/VaultUpdateTests.kt

@@ -8,6 +8,7 @@ import net.corda.core.transactions.LedgerTransaction
 import net.corda.testing.DUMMY_NOTARY
 import org.junit.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 
 
 class VaultUpdateTests {
@@ -83,4 +84,11 @@ class VaultUpdateTests {
         val expected = Vault.Update<ContractState>(setOf(stateAndRef2, stateAndRef3), setOf(stateAndRef4))
         assertEquals(expected, after)
     }
+
+    @Test
+    fun `can't combine updates of different types`() {
+        val regularUpdate = Vault.Update<ContractState>(setOf(stateAndRef0, stateAndRef1), setOf(stateAndRef4))
+        val notaryChangeUpdate = Vault.Update<ContractState>(setOf(stateAndRef2, stateAndRef3), setOf(stateAndRef0, stateAndRef1), type = Vault.UpdateType.NOTARY_CHANGE)
+        assertFailsWith<IllegalArgumentException> { regularUpdate + notaryChangeUpdate }
+    }
 }

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

@@ -31,10 +31,7 @@ import net.corda.core.serialization.SerializationDefaults.STORAGE_CONTEXT
 import net.corda.core.serialization.SingletonSerializeAsToken
 import net.corda.core.serialization.deserialize
 import net.corda.core.serialization.serialize
-import net.corda.core.transactions.CoreTransaction
+import net.corda.core.transactions.*
-import net.corda.core.transactions.NotaryChangeWireTransaction
-import net.corda.core.transactions.TransactionBuilder
-import net.corda.core.transactions.WireTransaction
 import net.corda.core.utilities.*
 import net.corda.node.services.database.RequeryConfiguration
 import net.corda.node.services.statemachine.FlowStateMachineImpl
@@ -86,7 +83,7 @@ class NodeVaultService(private val services: ServiceHub, dataSourceProperties: P
     private val mutex = ThreadBox(InnerState())
 
     private fun recordUpdate(update: Vault.Update<ContractState>): Vault.Update<ContractState> {
-        if (update != Vault.NoUpdate) {
+        if (!update.isEmpty()) {
             val producedStateRefs = update.produced.map { it.ref }
             val producedStateRefsMap = update.produced.associateBy { it.ref }
             val consumedStateRefs = update.consumed.map { it.ref }
@@ -248,7 +245,7 @@ class NodeVaultService(private val services: ServiceHub, dataSourceProperties: P
         fun makeUpdate(tx: NotaryChangeWireTransaction): Vault.Update<ContractState> {
             // We need to resolve the full transaction here because outputs are calculated from inputs
             // We also can't do filtering beforehand, since output encumbrance pointers get recalculated based on
-            // input position
+            // input positions
             val ltx = tx.resolve(services, emptyList())
 
             val (consumedStateAndRefs, producedStates) = ltx.inputs.
@@ -263,13 +260,13 @@ class NodeVaultService(private val services: ServiceHub, dataSourceProperties: P
 
             if (consumedStateAndRefs.isEmpty() && producedStateAndRefs.isEmpty()) {
                 log.trace { "tx ${tx.id} was irrelevant to this vault, ignoring" }
-                return Vault.NoUpdate
+                return Vault.NoNotaryUpdate
             }
 
-            return  Vault.Update(consumedStateAndRefs.toHashSet(), producedStateAndRefs.toHashSet())
+            return Vault.Update(consumedStateAndRefs.toHashSet(), producedStateAndRefs.toHashSet(), null, Vault.UpdateType.NOTARY_CHANGE)
         }
 
-        val netDelta = txns.fold(Vault.NoUpdate) { netDelta, txn -> netDelta + makeUpdate(txn) }
+        val netDelta = txns.fold(Vault.NoNotaryUpdate) { netDelta, txn -> netDelta + makeUpdate(txn) }
         processAndNotify(netDelta)
     }
 
@@ -292,7 +289,7 @@ class NodeVaultService(private val services: ServiceHub, dataSourceProperties: P
     }
 
     private fun processAndNotify(update: Vault.Update<ContractState>) {
-        if (update != Vault.NoUpdate) {
+        if (!update.isEmpty()) {
             recordUpdate(update)
             mutex.locked {
                 // flowId required by SoftLockManager to perform auto-registration of soft locks for new states

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

@@ -5,8 +5,10 @@ import net.corda.contracts.asset.DUMMY_CASH_ISSUER
 import net.corda.contracts.getCashBalance
 import net.corda.core.contracts.*
 import net.corda.core.crypto.generateKeyPair
+import net.corda.core.crypto.sign
 import net.corda.core.identity.AnonymousParty
 import net.corda.core.node.services.*
+import net.corda.core.transactions.NotaryChangeWireTransaction
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.utilities.NonEmptySet
@@ -447,7 +449,7 @@ class NodeVaultServiceTest : TestDependencyInjectionBase() {
     // TODO: Unit test linear state relevancy checks
 
     @Test
-    fun `make update`() {
+    fun `correct updates are generated for general transactions`() {
         val service = (services.vaultService as NodeVaultService)
         val vaultSubscriber = TestSubscriber<Vault.Update<*>>().apply {
             service.updates.subscribe(this)
@@ -478,4 +480,56 @@ class NodeVaultServiceTest : TestDependencyInjectionBase() {
         val observedUpdates = vaultSubscriber.onNextEvents
         assertEquals(observedUpdates, listOf(expectedIssueUpdate, expectedMoveUpdate))
     }
+
+    @Test
+    fun `correct updates are generated when changing notaries`() {
+        val service = (services.vaultService as NodeVaultService)
+        val notary = services.myInfo.legalIdentity
+
+        val vaultSubscriber = TestSubscriber<Vault.Update<*>>().apply {
+            service.updates.subscribe(this)
+        }
+
+        val anonymousIdentity = services.keyManagementService.freshKeyAndCert(services.myInfo.legalIdentityAndCert, false)
+        val thirdPartyIdentity = AnonymousParty(generateKeyPair().public)
+        val amount = Amount(1000, Issued(BOC.ref(1), GBP))
+
+        // Issue some cash
+        val issueTx = TransactionBuilder(notary).apply {
+            Cash().generateIssue(this, amount, anonymousIdentity.party, notary)
+        }.toWireTransaction()
+
+        // We need to record the issue transaction so inputs can be resolved for the notary change transaction
+        val signedIssueTx = SignedTransaction(issueTx, listOf(BOC_KEY.sign(issueTx.id)))
+        services.validatedTransactions.addTransaction(signedIssueTx)
+
+        val initialCashState = StateAndRef(issueTx.outputs.single(), StateRef(issueTx.id, 0))
+
+        // Change notary
+        val newNotary = DUMMY_NOTARY
+        val changeNotaryTx = NotaryChangeWireTransaction(listOf(initialCashState.ref), issueTx.notary!!, newNotary)
+        val cashStateWithNewNotary = StateAndRef(initialCashState.state.copy(notary = newNotary), StateRef(changeNotaryTx.id, 0))
+
+        database.transaction {
+            service.notifyAll(listOf(issueTx, changeNotaryTx))
+        }
+
+        // Move cash
+        val moveTx = database.transaction {
+            TransactionBuilder(newNotary).apply {
+                service.generateSpend(this, Amount(1000, GBP), thirdPartyIdentity)
+            }.toWireTransaction()
+        }
+
+        database.transaction {
+            service.notify(moveTx)
+        }
+
+        val expectedIssueUpdate = Vault.Update(emptySet(), setOf(initialCashState), null)
+        val expectedNotaryChangeUpdate = Vault.Update(setOf(initialCashState), setOf(cashStateWithNewNotary), null, Vault.UpdateType.NOTARY_CHANGE)
+        val expectedMoveUpdate = Vault.Update(setOf(cashStateWithNewNotary), emptySet(), null)
+
+        val observedUpdates = vaultSubscriber.onNextEvents
+        assertEquals(observedUpdates, listOf(expectedIssueUpdate, expectedNotaryChangeUpdate, expectedMoveUpdate))
+    }
 }