CORDA-2109: Fix a bug that prevents consecutive multiparty contract upgrades

The contract upgrade handler assumes that the state to be upgraded is
created by a WireTransaction. This breaks the upgrade process if it was
in fact issued by a ContractUpgradeWireTransactions or a NotaryChangeWireTransaction.

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

@@ -3,15 +3,12 @@ package net.corda.core.flows
 import com.natpryce.hamkrest.*
 import com.natpryce.hamkrest.assertion.assert
 import net.corda.core.contracts.*
-import net.corda.testing.internal.matchers.flow.willReturn
-import net.corda.testing.internal.matchers.flow.willThrow
 import net.corda.core.flows.mixins.WithContracts
 import net.corda.core.flows.mixins.WithFinality
 import net.corda.core.identity.AbstractParty
 import net.corda.core.internal.Emoji
 import net.corda.core.transactions.ContractUpgradeLedgerTransaction
 import net.corda.core.transactions.LedgerTransaction
-import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.OpaqueBytes
 import net.corda.core.utilities.getOrThrow
 import net.corda.finance.USD
@@ -20,9 +17,12 @@ import net.corda.finance.contracts.asset.Cash
 import net.corda.finance.flows.CashIssueFlow
 import net.corda.testing.contracts.DummyContract
 import net.corda.testing.contracts.DummyContractV2
+import net.corda.testing.contracts.DummyContractV3
 import net.corda.testing.core.ALICE_NAME
 import net.corda.testing.core.BOB_NAME
 import net.corda.testing.core.singleIdentity
+import net.corda.testing.internal.matchers.flow.willReturn
+import net.corda.testing.internal.matchers.flow.willThrow
 import net.corda.testing.node.internal.InternalMockNetwork
 import net.corda.testing.node.internal.TestStartedNode
 import net.corda.testing.node.internal.cordappsForPackages
@@ -57,32 +57,45 @@ class ContractUpgradeFlowTest : WithContracts, WithFinality {
 
         aliceNode.finalise(stx, bob)
 
-        val atx = aliceNode.getValidatedTransaction(stx)
+        val aliceTx = aliceNode.getValidatedTransaction(stx)
-        val btx = bobNode.getValidatedTransaction(stx)
+        val bobTx = bobNode.getValidatedTransaction(stx)
 
         // The request is expected to be rejected because party B hasn't authorised the upgrade yet.
         assert.that(
-            aliceNode.initiateDummyContractUpgrade(atx),
+                aliceNode.initiateContractUpgrade(aliceTx, DummyContractV2::class),
                 willThrow<UnexpectedFlowEndException>())
 
-        // Party B authorise the contract state upgrade, and immediately deauthorise the same.
+        // Party B authorises the contract state upgrade, and immediately de-authorises the same.
-        assert.that(bobNode.authoriseDummyContractUpgrade(btx), willReturn())
+        assert.that(bobNode.authoriseContractUpgrade(bobTx, DummyContractV2::class), willReturn())
-        assert.that(bobNode.deauthoriseContractUpgrade(btx), willReturn())
+        assert.that(bobNode.deauthoriseContractUpgrade(bobTx), willReturn())
 
-        // The request is expected to be rejected because party B has subsequently deauthorised a previously authorised upgrade.
+        // The request is expected to be rejected because party B has subsequently de-authorised a previously authorised upgrade.
         assert.that(
-            aliceNode.initiateDummyContractUpgrade(atx),
+                aliceNode.initiateContractUpgrade(aliceTx, DummyContractV2::class),
                 willThrow<UnexpectedFlowEndException>())
 
-        // Party B authorise the contract state upgrade
+        // Party B authorises the contract state upgrade.
-        assert.that(bobNode.authoriseDummyContractUpgrade(btx), willReturn())
+        assert.that(bobNode.authoriseContractUpgrade(bobTx, DummyContractV2::class), willReturn())
 
         // Party A initiates contract upgrade flow, expected to succeed this time.
         assert.that(
-            aliceNode.initiateDummyContractUpgrade(atx),
+                aliceNode.initiateContractUpgrade(aliceTx, DummyContractV2::class),
                 willReturn(
-                        aliceNode.hasDummyContractUpgradeTransaction()
+                        aliceNode.hasContractUpgradeTransaction<DummyContract.State, DummyContractV2.State>()
-                                and bobNode.hasDummyContractUpgradeTransaction()))
+                                and bobNode.hasContractUpgradeTransaction<DummyContract.State, DummyContractV2.State>()))
+
+        val upgradedState = aliceNode.getStateFromVault(DummyContractV2.State::class)
+
+        // We now test that the upgraded state can be upgraded further, to V3.
+        // Party B authorises the contract state upgrade.
+        assert.that(bobNode.authoriseContractUpgrade(upgradedState, DummyContractV3::class), willReturn())
+
+        // Party A initiates contract upgrade flow which is expected to succeed.
+        assert.that(
+                aliceNode.initiateContractUpgrade(upgradedState, DummyContractV3::class),
+                willReturn(
+                        aliceNode.hasContractUpgradeTransaction<DummyContractV2.State, DummyContractV3.State>()
+                                and bobNode.hasContractUpgradeTransaction<DummyContractV2.State, DummyContractV3.State>()))
     }
 
     private fun TestStartedNode.issueCash(amount: Amount<Currency> = Amount(1000, USD)) =
@@ -161,19 +174,8 @@ class ContractUpgradeFlowTest : WithContracts, WithFinality {
         override fun verify(tx: LedgerTransaction) {}
     }
 
-    //region Operations
-    private fun TestStartedNode.initiateDummyContractUpgrade(tx: SignedTransaction) =
-            initiateContractUpgrade(tx, DummyContractV2::class)
-
-    private fun TestStartedNode.authoriseDummyContractUpgrade(tx: SignedTransaction) =
-            authoriseContractUpgrade(tx, DummyContractV2::class)
-    //endregion
-
     //region Matchers
-    private fun TestStartedNode.hasDummyContractUpgradeTransaction() =
+    private inline fun <reified FROM : Any, reified TO : Any> TestStartedNode.hasContractUpgradeTransaction() =
-            hasContractUpgradeTransaction<DummyContract.State, DummyContractV2.State>()
-
-    private inline fun <reified FROM : Any, reified TO: Any> TestStartedNode.hasContractUpgradeTransaction() =
             has<StateAndRef<ContractState>, ContractUpgradeLedgerTransaction>(
                     "a contract upgrade transaction",
                     { getContractUpgradeTransaction(it) },
@@ -186,10 +188,10 @@ class ContractUpgradeFlowTest : WithContracts, WithFinality {
     private inline fun <reified FROM : Any, reified TO : Any> isUpgrade() =
             isUpgradeFrom<FROM>() and isUpgradeTo<TO>()
 
-    private inline fun <reified T: Any> isUpgradeFrom() =
+    private inline fun <reified T : Any> isUpgradeFrom() =
             has<ContractUpgradeLedgerTransaction, Any>("input data", { it.inputs.single().state.data }, isA<T>(anything))
 
-    private inline fun <reified T: Any> isUpgradeTo() =
+    private inline fun <reified T : Any> isUpgradeTo() =
             has<ContractUpgradeLedgerTransaction, Any>("output data", { it.outputs.single().data }, isA<T>(anything))
     //endregion
 }

core/src/test/kotlin/net/corda/core/flows/mixins/WithContracts.kt

@@ -51,8 +51,12 @@ interface WithContracts : WithMockNet {
 
     fun <T : UpgradedContract<*, *>> TestStartedNode.authoriseContractUpgrade(
         tx: SignedTransaction, toClass: KClass<T>) =
+        authoriseContractUpgrade(tx.tx.outRef(0), toClass)
+
+    fun <T : UpgradedContract<*, *>> TestStartedNode.authoriseContractUpgrade(
+            stateAndRef: StateAndRef<ContractState>, toClass: KClass<T>) =
             startFlow(
-            ContractUpgradeFlow.Authorise(tx.tx.outRef<ContractState>(0), toClass.java)
+                    ContractUpgradeFlow.Authorise(stateAndRef, toClass.java)
             )
 
     fun TestStartedNode.deauthoriseContractUpgrade(tx: SignedTransaction) = startFlow(

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

@@ -56,7 +56,7 @@ class ContractUpgradeHandler(otherSide: FlowSession) : AbstractStateReplacementF
         // verify outputs matches the proposed upgrade.
         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 oldStateAndRef = ourSTX!!.resolveBaseTransaction(serviceHub).outRef<ContractState>(proposal.stateRef.index)
         val authorisedUpgrade = serviceHub.contractUpgradeService.getAuthorisedContractUpgrade(oldStateAndRef.ref) ?: throw IllegalStateException("Contract state upgrade is unauthorised. State hash : ${oldStateAndRef.ref}")
         val proposedTx = stx.coreTransaction as ContractUpgradeWireTransaction
         val expectedTx = ContractUpgradeUtils.assembleUpgradeTx(oldStateAndRef, proposal.modification, proposedTx.privacySalt, serviceHub)

testing/test-utils/src/main/kotlin/net/corda/testing/contracts/DummyContractV3.kt

@@ -0,0 +1,36 @@
+package net.corda.testing.contracts
+
+import net.corda.core.contracts.*
+import net.corda.core.identity.AbstractParty
+import net.corda.core.transactions.LedgerTransaction
+
+// The dummy contract doesn't do anything useful. It exists for testing purposes.
+
+/**
+ * Dummy contract state for testing of the upgrade process.
+ */
+class DummyContractV3 : UpgradedContractWithLegacyConstraint<DummyContractV2.State, DummyContractV3.State> {
+    companion object {
+        const val PROGRAM_ID: ContractClassName = "net.corda.testing.contracts.DummyContractV3"
+    }
+
+    override val legacyContract: String = DummyContractV2.PROGRAM_ID
+    override val legacyContractConstraint: AttachmentConstraint = AlwaysAcceptAttachmentConstraint
+
+    data class State(val magicNumber: Int = 0, val owners: List<AbstractParty>) : ContractState {
+        override val participants: List<AbstractParty> = owners
+    }
+
+    interface Commands : CommandData {
+        class Create : TypeOnlyCommandData(), Commands
+        class Move : TypeOnlyCommandData(), Commands
+    }
+
+    override fun upgrade(state: DummyContractV2.State): State {
+        return State(state.magicNumber, state.participants)
+    }
+
+    override fun verify(tx: LedgerTransaction) {
+        // Other verifications.
+    }
+}