Fix validating notary flow to handle notary change transactions properly. (#1687)

Add a notary change test for checking longer chains involving both regular and notary change transactions.

docs/source/example-code/src/main/kotlin/net/corda/docs/CustomNotaryTutorial.kt

@@ -1,13 +1,14 @@
 package net.corda.docs
 
 import co.paralleluniverse.fibers.Suspendable
+import net.corda.core.contracts.TimeWindow
 import net.corda.core.contracts.TransactionVerificationException
 import net.corda.core.flows.*
 import net.corda.core.node.ServiceHub
 import net.corda.core.node.services.CordaService
 import net.corda.core.node.services.TimeWindowChecker
 import net.corda.core.node.services.TrustedAuthorityNotaryService
-import net.corda.core.transactions.SignedTransaction
+import net.corda.core.transactions.TransactionWithSignatures
 import net.corda.node.services.transactions.PersistentUniquenessProvider
 import java.security.PublicKey
 import java.security.SignatureException
@@ -35,10 +36,17 @@ class MyValidatingNotaryFlow(otherSide: FlowSession, service: MyCustomValidating
     override fun receiveAndVerifyTx(): TransactionParts {
         try {
             val stx = subFlow(ReceiveTransactionFlow(otherSideSession, checkSufficientSignatures = false))
-            checkNotary(stx.notary)
+            val notary = stx.notary
-            checkSignatures(stx)
+            checkNotary(notary)
-            val wtx = stx.tx
+            var timeWindow: TimeWindow? = null
-            return TransactionParts(wtx.id, wtx.inputs, wtx.timeWindow, wtx.notary)
+            val transactionWithSignatures = if (stx.isNotaryChangeTransaction()) {
+                stx.resolveNotaryChangeTransaction(serviceHub)
+            } else {
+                timeWindow = stx.tx.timeWindow
+                stx
+            }
+            checkSignatures(transactionWithSignatures)
+            return TransactionParts(stx.id, stx.inputs, timeWindow, notary!!)
         } catch (e: Exception) {
             throw when (e) {
                 is TransactionVerificationException,
@@ -48,9 +56,9 @@ class MyValidatingNotaryFlow(otherSide: FlowSession, service: MyCustomValidating
         }
     }
 
-    private fun checkSignatures(stx: SignedTransaction) {
+    private fun checkSignatures(tx: TransactionWithSignatures) {
         try {
-            stx.verifySignaturesExcept(service.notaryIdentityKey)
+            tx.verifySignaturesExcept(service.notaryIdentityKey)
         } catch (e: SignatureException) {
             throw NotaryException(NotaryError.TransactionInvalid(e))
         }

node/src/main/kotlin/net/corda/node/services/transactions/ValidatingNotaryFlow.kt

@@ -1,10 +1,11 @@
 package net.corda.node.services.transactions
 
 import co.paralleluniverse.fibers.Suspendable
+import net.corda.core.contracts.TimeWindow
 import net.corda.core.contracts.TransactionVerificationException
 import net.corda.core.flows.*
 import net.corda.core.node.services.TrustedAuthorityNotaryService
-import net.corda.core.transactions.SignedTransaction
+import net.corda.core.transactions.TransactionWithSignatures
 import java.security.SignatureException
 
 /**
@@ -15,9 +16,8 @@ import java.security.SignatureException
  */
 class ValidatingNotaryFlow(otherSideSession: FlowSession, service: TrustedAuthorityNotaryService) : NotaryFlow.Service(otherSideSession, service) {
     /**
-     * The received transaction is checked for contract-validity, which requires fully resolving it into a
+     * Fully resolves the received transaction and its dependencies, runs contract verification logic and checks that
-     * [TransactionForVerification], for which the caller also has to to reveal the whole transaction
+     * the transaction in question has all required signatures apart from the notary's.
-     * dependency chain.
      */
     @Suspendable
     override fun receiveAndVerifyTx(): TransactionParts {
@@ -25,9 +25,15 @@ class ValidatingNotaryFlow(otherSideSession: FlowSession, service: TrustedAuthor
             val stx = subFlow(ReceiveTransactionFlow(otherSideSession, checkSufficientSignatures = false))
             val notary = stx.notary
             checkNotary(notary)
-            checkSignatures(stx)
+            var timeWindow: TimeWindow? = null
-            val wtx = stx.tx
+            val transactionWithSignatures = if (stx.isNotaryChangeTransaction()) {
-            return TransactionParts(wtx.id, wtx.inputs, wtx.timeWindow, notary!!)
+                stx.resolveNotaryChangeTransaction(serviceHub)
+            } else {
+                timeWindow = stx.tx.timeWindow
+                stx
+            }
+            checkSignatures(transactionWithSignatures)
+            return TransactionParts(stx.id, stx.inputs, timeWindow, notary!!)
         } catch (e: Exception) {
             throw when (e) {
                 is TransactionVerificationException,
@@ -37,10 +43,10 @@ class ValidatingNotaryFlow(otherSideSession: FlowSession, service: TrustedAuthor
         }
     }
 
-    private fun checkSignatures(stx: SignedTransaction) {
+    private fun checkSignatures(tx: TransactionWithSignatures) {
         try {
-            stx.verifySignaturesExcept(service.notaryIdentityKey)
+            tx.verifySignaturesExcept(service.notaryIdentityKey)
-        } catch(e: SignatureException) {
+        } catch (e: SignatureException) {
             throw NotaryException(NotaryError.TransactionInvalid(e))
         }
     }

node/src/test/kotlin/net/corda/node/services/NotaryChangeTests.kt

@@ -3,6 +3,7 @@ package net.corda.node.services
 import net.corda.core.contracts.*
 import net.corda.core.crypto.generateKeyPair
 import net.corda.core.flows.NotaryChangeFlow
+import net.corda.core.flows.NotaryFlow
 import net.corda.core.flows.StateReplacementException
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
@@ -12,7 +13,7 @@ import net.corda.core.utilities.getOrThrow
 import net.corda.core.utilities.seconds
 import net.corda.node.internal.StartedNode
 import net.corda.node.services.network.NetworkMapService
-import net.corda.node.services.transactions.SimpleNotaryService
+import net.corda.node.services.transactions.ValidatingNotaryService
 import net.corda.nodeapi.internal.ServiceInfo
 import net.corda.testing.DUMMY_NOTARY
 import net.corda.testing.*
@@ -34,8 +35,8 @@ class NotaryChangeTests {
     lateinit var newNotaryNode: StartedNode<MockNetwork.MockNode>
     lateinit var clientNodeA: StartedNode<MockNetwork.MockNode>
     lateinit var clientNodeB: StartedNode<MockNetwork.MockNode>
-    lateinit var notaryNewId: Party
+    lateinit var newNotaryParty: Party
-    lateinit var notaryOldId: Party
+    lateinit var oldNotaryParty: Party
 
     @Before
     fun setUp() {
@@ -43,15 +44,15 @@ class NotaryChangeTests {
         mockNet = MockNetwork()
         oldNotaryNode = mockNet.createNode(
                 legalName = DUMMY_NOTARY.name,
-                advertisedServices = *arrayOf(ServiceInfo(NetworkMapService.type), ServiceInfo(SimpleNotaryService.type)))
+                advertisedServices = *arrayOf(ServiceInfo(NetworkMapService.type), ServiceInfo(ValidatingNotaryService.type)))
         clientNodeA = mockNet.createNode(networkMapAddress = oldNotaryNode.network.myAddress)
         clientNodeB = mockNet.createNode(networkMapAddress = oldNotaryNode.network.myAddress)
-        newNotaryNode = mockNet.createNode(networkMapAddress = oldNotaryNode.network.myAddress, advertisedServices = ServiceInfo(SimpleNotaryService.type))
+        newNotaryNode = mockNet.createNode(networkMapAddress = oldNotaryNode.network.myAddress, advertisedServices = ServiceInfo(ValidatingNotaryService.type))
         mockNet.registerIdentities()
         mockNet.runNetwork() // Clear network map registration messages
         oldNotaryNode.internals.ensureRegistered()
-        notaryNewId = newNotaryNode.info.legalIdentities[1]
+        newNotaryParty = newNotaryNode.info.legalIdentities[1]
-        notaryOldId = oldNotaryNode.info.legalIdentities[1]
+        oldNotaryParty = oldNotaryNode.info.legalIdentities[1]
     }
 
     @After
@@ -62,21 +63,16 @@ class NotaryChangeTests {
 
     @Test
     fun `should change notary for a state with single participant`() {
-        val state = issueState(clientNodeA, oldNotaryNode, notaryOldId)
+        val state = issueState(clientNodeA, oldNotaryParty)
-        val newNotary = notaryNewId
+        assertEquals(state.state.notary, oldNotaryParty)
-        val flow = NotaryChangeFlow(state, newNotary)
+        val newState = changeNotary(state, clientNodeA, newNotaryParty)
-        val future = clientNodeA.services.startFlow(flow)
+        assertEquals(newState.state.notary, newNotaryParty)
-
-        mockNet.runNetwork()
-
-        val newState = future.resultFuture.getOrThrow()
-        assertEquals(newState.state.notary, newNotary)
     }
 
     @Test
     fun `should change notary for a state with multiple participants`() {
-        val state = issueMultiPartyState(clientNodeA, clientNodeB, oldNotaryNode, notaryOldId)
+        val state = issueMultiPartyState(clientNodeA, clientNodeB, oldNotaryNode, oldNotaryParty)
-        val newNotary = notaryNewId
+        val newNotary = newNotaryParty
         val flow = NotaryChangeFlow(state, newNotary)
         val future = clientNodeA.services.startFlow(flow)
 
@@ -91,7 +87,7 @@ class NotaryChangeTests {
 
     @Test
     fun `should throw when a participant refuses to change Notary`() {
-        val state = issueMultiPartyState(clientNodeA, clientNodeB, oldNotaryNode, notaryOldId)
+        val state = issueMultiPartyState(clientNodeA, clientNodeB, oldNotaryNode, oldNotaryParty)
         val newEvilNotary = getTestPartyAndCertificate(CordaX500Name(organisation = "Evil R3", locality = "London", country = "GB"), generateKeyPair().public)
         val flow = NotaryChangeFlow(state, newEvilNotary.party)
         val future = clientNodeA.services.startFlow(flow)
@@ -105,10 +101,10 @@ class NotaryChangeTests {
 
     @Test
     fun `should not break encumbrance links`() {
-        val issueTx = issueEncumberedState(clientNodeA, notaryOldId)
+        val issueTx = issueEncumberedState(clientNodeA, oldNotaryParty)
 
         val state = StateAndRef(issueTx.outputs.first(), StateRef(issueTx.id, 0))
-        val newNotary = notaryNewId
+        val newNotary = newNotaryParty
         val flow = NotaryChangeFlow(state, newNotary)
         val future = clientNodeA.services.startFlow(flow)
         mockNet.runNetwork()
@@ -137,6 +133,47 @@ class NotaryChangeTests {
         }
     }
 
+    @Test
+    fun `notary change and regular transactions are properly handled during resolution in longer chains`() {
+        val issued = issueState(clientNodeA, oldNotaryParty)
+        val moved = moveState(issued, clientNodeA, clientNodeB)
+
+        // We don't to tx resolution when moving state to another node, so need to add the issue transaction manually
+        // to node B. The resolution process is tested later during notarisation.
+        clientNodeB.services.recordTransactions(clientNodeA.services.validatedTransactions.getTransaction(issued.ref.txhash)!!)
+
+        val changedNotary = changeNotary(moved, clientNodeB, newNotaryParty)
+        val movedBack = moveState(changedNotary, clientNodeB, clientNodeA)
+        val changedNotaryBack = changeNotary(movedBack, clientNodeA, oldNotaryParty)
+
+        assertEquals(issued.state, changedNotaryBack.state)
+    }
+
+    private fun changeNotary(movedState: StateAndRef<DummyContract.SingleOwnerState>, node: StartedNode<*>, newNotary: Party): StateAndRef<DummyContract.SingleOwnerState> {
+        val flow = NotaryChangeFlow(movedState, newNotary)
+        val future = node.services.startFlow(flow)
+        mockNet.runNetwork()
+
+        return future.resultFuture.getOrThrow()
+    }
+
+    private fun moveState(state: StateAndRef<DummyContract.SingleOwnerState>, fromNode: StartedNode<*>, toNode: StartedNode<*>): StateAndRef<DummyContract.SingleOwnerState> {
+        val tx = DummyContract.move(state, toNode.info.chooseIdentity())
+        val stx = fromNode.services.signInitialTransaction(tx)
+
+        val notaryFlow = NotaryFlow.Client(stx)
+        val future = fromNode.services.startFlow(notaryFlow)
+        mockNet.runNetwork()
+
+        val notarySignature = future.resultFuture.getOrThrow()
+        val finalTransaction = stx + notarySignature
+
+        fromNode.services.recordTransactions(finalTransaction)
+        toNode.services.recordTransactions(finalTransaction)
+
+        return finalTransaction.tx.outRef(0)
+    }
+
     private fun issueEncumberedState(node: StartedNode<*>, notaryIdentity: Party): WireTransaction {
         val owner = node.info.chooseIdentity().ref(0)
         val stateA = DummyContract.SingleOwnerState(Random().nextInt(), owner.party)
@@ -163,30 +200,31 @@ class NotaryChangeTests {
     //       - The transaction type is not a notary change transaction at all.
 }
 
-fun issueState(node: StartedNode<*>, notaryNode: StartedNode<*>, notaryIdentity: Party): StateAndRef<*> {
+fun issueState(node: StartedNode<*>, notaryIdentity: Party): StateAndRef<DummyContract.SingleOwnerState> {
     val tx = DummyContract.generateInitial(Random().nextInt(), notaryIdentity, node.info.chooseIdentity().ref(0))
-    val signedByNode = node.services.signInitialTransaction(tx)
+    val stx = node.services.signInitialTransaction(tx)
-    val stx = notaryNode.services.addSignature(signedByNode, notaryIdentity.owningKey)
     node.services.recordTransactions(stx)
-    return StateAndRef(tx.outputStates().first(), StateRef(stx.id, 0))
+    return stx.tx.outRef(0)
 }
 
 fun issueMultiPartyState(nodeA: StartedNode<*>, nodeB: StartedNode<*>, notaryNode: StartedNode<*>, notaryIdentity: Party): StateAndRef<DummyContract.MultiOwnerState> {
-    val state = TransactionState(DummyContract.MultiOwnerState(0,
+    val participants = listOf(nodeA.info.chooseIdentity(), nodeB.info.chooseIdentity())
-            listOf(nodeA.info.chooseIdentity(), nodeB.info.chooseIdentity())), DUMMY_PROGRAM_ID, notaryIdentity)
+    val state = TransactionState(
-    val tx = TransactionBuilder(notary = notaryIdentity).withItems(state, dummyCommand())
+            DummyContract.MultiOwnerState(0, participants),
+            DUMMY_PROGRAM_ID, notaryIdentity)
+    val tx = TransactionBuilder(notary = notaryIdentity).withItems(state, dummyCommand(participants.first().owningKey))
     val signedByA = nodeA.services.signInitialTransaction(tx)
     val signedByAB = nodeB.services.addSignature(signedByA)
     val stx = notaryNode.services.addSignature(signedByAB, notaryIdentity.owningKey)
     nodeA.services.recordTransactions(stx)
     nodeB.services.recordTransactions(stx)
-    return StateAndRef(state, StateRef(stx.id, 0))
+    return stx.tx.outRef(0)
 }
 
-fun issueInvalidState(node: StartedNode<*>, notary: Party): StateAndRef<*> {
+fun issueInvalidState(node: StartedNode<*>, notary: Party): StateAndRef<DummyContract.SingleOwnerState> {
     val tx = DummyContract.generateInitial(Random().nextInt(), notary, node.info.chooseIdentity().ref(0))
     tx.setTimeWindow(Instant.now(), 30.seconds)
     val stx = node.services.signInitialTransaction(tx)
     node.services.recordTransactions(stx)
-    return StateAndRef(tx.outputStates().first(), StateRef(stx.id, 0))
+    return stx.tx.outRef(0)
-}
+}