CORDA-641: A temporary fix for contract upgrade transactions (#1700)

* A temporary fix for contract upgrade transactions:
during LedgerTransaction verification run the right logic based on whether
it contains the UpgradeCommand.

* Move ContractUpgradeFlowTest away from createSomeNodes()

* Remove assembleBareTx as it's not used

core/src/main/kotlin/net/corda/core/contracts/Structures.kt

@@ -198,9 +198,6 @@ interface MoveCommand : CommandData {
     val contract: Class<out Contract>?
 }
 
-/** Indicates that this transaction replaces the inputs contract state to another contract state */
-data class UpgradeCommand(val upgradedContractClass: ContractClassName) : CommandData
-
 // DOCSTART 6
 /** A [Command] where the signing parties have been looked up if they have a well known/recognised institutional key. */
 @CordaSerializable

core/src/main/kotlin/net/corda/core/flows/ContractUpgradeFlow.kt

@@ -3,9 +3,6 @@ package net.corda.core.flows
 import co.paralleluniverse.fibers.Suspendable
 import net.corda.core.contracts.*
 import net.corda.core.internal.ContractUpgradeUtils
-import net.corda.core.transactions.LedgerTransaction
-import net.corda.core.transactions.TransactionBuilder
-import java.security.PublicKey
 
 /**
  * A flow to be used for authorising and upgrading state objects of an old contract to a new contract.
@@ -16,30 +13,6 @@ import java.security.PublicKey
  * use the new updated state for future transactions.
  */
 object ContractUpgradeFlow {
-
-    @JvmStatic
-    fun verify(tx: LedgerTransaction) {
-        // Contract Upgrade transaction should have 1 input, 1 output and 1 command.
-        verify(tx.inputs.single().state,
-                tx.outputs.single(),
-                tx.commandsOfType<UpgradeCommand>().single())
-    }
-
-    @JvmStatic
-    fun verify(input: TransactionState<ContractState>, output: TransactionState<ContractState>, commandData: Command<UpgradeCommand>) {
-        val command = commandData.value
-        val participantKeys: Set<PublicKey> = input.data.participants.map { it.owningKey }.toSet()
-        val keysThatSigned: Set<PublicKey> = commandData.signers.toSet()
-        @Suppress("UNCHECKED_CAST")
-        val upgradedContract = javaClass.classLoader.loadClass(command.upgradedContractClass).newInstance() as UpgradedContract<ContractState, *>
-        requireThat {
-            "The signing keys include all participant keys" using keysThatSigned.containsAll(participantKeys)
-            "Inputs state reference the legacy contract" using (input.contract == upgradedContract.legacyContract)
-            "Outputs state reference the upgraded contract" using (output.contract == command.upgradedContractClass)
-            "Output state must be an upgraded version of the input state" using (output.data == upgradedContract.upgrade(input.data))
-        }
-    }
-
     /**
      * Authorise a contract state upgrade.
      *
@@ -53,7 +26,7 @@ object ContractUpgradeFlow {
     class Authorise(
             val stateAndRef: StateAndRef<*>,
             private val upgradedContractClass: Class<out UpgradedContract<*, *>>
-        ) : FlowLogic<Void?>() {
+    ) : FlowLogic<Void?>() {
         @Suspendable
         override fun call(): Void? {
             val upgrade = upgradedContractClass.newInstance()
@@ -89,23 +62,6 @@ object ContractUpgradeFlow {
             newContractClass: Class<out UpgradedContract<OldState, NewState>>
     ) : AbstractStateReplacementFlow.Instigator<OldState, NewState, Class<out UpgradedContract<OldState, NewState>>>(originalState, newContractClass) {
 
-        companion object {
-            fun <OldState : ContractState, NewState : ContractState> assembleBareTx(
-                    stateRef: StateAndRef<OldState>,
-                    upgradedContractClass: Class<out UpgradedContract<OldState, NewState>>,
-                    privacySalt: PrivacySalt
-            ): TransactionBuilder {
-                val contractUpgrade = upgradedContractClass.newInstance()
-                return TransactionBuilder(stateRef.state.notary)
-                        .withItems(
-                                stateRef,
-                                StateAndContract(contractUpgrade.upgrade(stateRef.state.data), upgradedContractClass.name),
-                                Command(UpgradeCommand(upgradedContractClass.name), stateRef.state.data.participants.map { it.owningKey }),
-                                privacySalt
-                        )
-            }
-        }
-
         @Suspendable
         override fun assembleTx(): AbstractStateReplacementFlow.UpgradeTx {
             val baseTx = ContractUpgradeUtils.assembleBareTx(originalState, modification, PrivacySalt())

core/src/main/kotlin/net/corda/core/internal/UpgradeCommand.kt

@@ -0,0 +1,7 @@
+package net.corda.core.internal
+
+import net.corda.core.contracts.CommandData
+import net.corda.core.contracts.ContractClassName
+
+/** Indicates that this transaction replaces the inputs contract state to another contract state */
+data class UpgradeCommand(val upgradedContractClass: ContractClassName) : CommandData

core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt

@@ -3,8 +3,10 @@ package net.corda.core.transactions
 import net.corda.core.contracts.*
 import net.corda.core.crypto.SecureHash
 import net.corda.core.identity.Party
+import net.corda.core.internal.UpgradeCommand
 import net.corda.core.internal.castIfPossible
 import net.corda.core.serialization.CordaSerializable
+import java.security.PublicKey
 import java.util.*
 import java.util.function.Predicate
 
@@ -63,7 +65,13 @@ data class LedgerTransaction(
     @Throws(TransactionVerificationException::class)
     fun verify() {
         verifyConstraints()
-        verifyContracts()
+        // TODO: make contract upgrade transactions have a separate type
+        if (commands.any { it.value is UpgradeCommand }) {
+            verifyContractUpgrade()
+        }
+        else {
+            verifyContracts()
+        }
     }
 
     /**
@@ -153,6 +161,25 @@ data class LedgerTransaction(
         }
     }
 
+    private fun verifyContractUpgrade() {
+        // Contract Upgrade transaction should have 1 input, 1 output and 1 command.
+        val input = inputs.single().state
+        val output = outputs.single()
+        val commandData = commandsOfType<UpgradeCommand>().single()
+
+        val command = commandData.value
+        val participantKeys: Set<PublicKey> = input.data.participants.map { it.owningKey }.toSet()
+        val keysThatSigned: Set<PublicKey> = commandData.signers.toSet()
+        @Suppress("UNCHECKED_CAST")
+        val upgradedContract = javaClass.classLoader.loadClass(command.upgradedContractClass).newInstance() as UpgradedContract<ContractState, *>
+        requireThat {
+            "The signing keys include all participant keys" using keysThatSigned.containsAll(participantKeys)
+            "Inputs state reference the legacy contract" using (input.contract == upgradedContract.legacyContract)
+            "Outputs state reference the upgraded contract" using (output.contract == command.upgradedContractClass)
+            "Output state must be an upgraded version of the input state" using (output.data == upgradedContract.upgrade(input.data))
+        }
+    }
+
     /**
      * Given a type and a function that returns a grouping key, associates inputs and outputs together so that they
      * can be processed as one. The grouping key is any arbitrary object that can act as a map key (so must implement

core/src/test/kotlin/net/corda/core/contracts/DummyContractV2Tests.kt

@@ -1,20 +1,15 @@
 package net.corda.core.contracts
 
-import com.nhaarman.mockito_kotlin.mock
-import net.corda.testing.contracts.DummyContract
-import net.corda.testing.contracts.DummyContractV2
 import net.corda.core.crypto.SecureHash
-import net.corda.core.node.ServicesForResolution
+import net.corda.core.internal.UpgradeCommand
 import net.corda.testing.ALICE
 import net.corda.testing.DUMMY_NOTARY
+import net.corda.testing.TestDependencyInjectionBase
 import net.corda.testing.contracts.DUMMY_PROGRAM_ID
 import net.corda.testing.contracts.DUMMY_V2_PROGRAM_ID
-import net.corda.testing.TestDependencyInjectionBase
+import net.corda.testing.contracts.DummyContract
+import net.corda.testing.contracts.DummyContractV2
 import net.corda.testing.node.MockServices
-import net.corda.testing.setCordappPackages
-import net.corda.testing.unsetCordappPackages
-import org.junit.After
-import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertTrue

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

@@ -43,22 +43,15 @@ class ContractUpgradeFlowTest {
     fun setup() {
         setCordappPackages("net.corda.testing.contracts", "net.corda.finance.contracts.asset", "net.corda.core.flows")
         mockNet = MockNetwork()
-        val nodes = mockNet.createSomeNodes(notaryKeyPair = null) // prevent generation of notary override
+        val notaryNode = mockNet.createNotaryNode()
-        a = nodes.partyNodes[0]
+        a = mockNet.createPartyNode(notaryNode.network.myAddress, ALICE.name)
-        b = nodes.partyNodes[1]
+        b = mockNet.createPartyNode(notaryNode.network.myAddress, BOB.name)
 
         // Process registration
         mockNet.runNetwork()
         a.internals.ensureRegistered()
 
-        notary = a.services.getDefaultNotary()
+        notary = notaryNode.services.getDefaultNotary()
-        val nodeIdentity = nodes.notaryNode.info.legalIdentitiesAndCerts.single { it.party == notary }
-        a.database.transaction {
-            a.services.identityService.verifyAndRegisterIdentity(nodeIdentity)
-        }
-        b.database.transaction {
-            b.services.identityService.verifyAndRegisterIdentity(nodeIdentity)
-        }
     }
 
     @After

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

@@ -2,7 +2,6 @@ package net.corda.node.services
 
 import co.paralleluniverse.fibers.Suspendable
 import net.corda.core.contracts.ContractState
-import net.corda.core.contracts.UpgradeCommand
 import net.corda.core.contracts.UpgradedContract
 import net.corda.core.contracts.requireThat
 import net.corda.core.flows.*
@@ -64,9 +63,6 @@ class ContractUpgradeHandler(otherSide: FlowSession) : AbstractStateReplacementF
             "The proposed upgrade ${proposal.modification.javaClass} is a trusted upgrade path" using (proposal.modification.name == authorisedUpgrade)
             "The proposed tx matches the expected tx for this upgrade" using (proposedTx == expectedTx)
         }
-        ContractUpgradeFlow.verify(
+        proposedTx.toLedgerTransaction(serviceHub).verify()
-                oldStateAndRef.state,
-                expectedTx.outRef<ContractState>(0).state,
-                expectedTx.toLedgerTransaction(serviceHub).commandsOfType<UpgradeCommand>().single())
     }
 }

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockNode.kt

@@ -391,7 +391,7 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
     @JvmOverloads
     fun createSomeNodes(numPartyNodes: Int = 2, nodeFactory: Factory<*> = defaultFactory, notaryKeyPair: KeyPair? = DUMMY_NOTARY_KEY): BasketOfNodes {
         require(nodes.isEmpty())
-        val notaryServiceInfo = ServiceInfo(SimpleNotaryService.type)
+        val notaryServiceInfo = ServiceInfo(ValidatingNotaryService.type)
         val notaryOverride = if (notaryKeyPair != null)
             mapOf(Pair(notaryServiceInfo, notaryKeyPair))
         else

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

@@ -1,8 +1,8 @@
 package net.corda.testing.contracts
 
 import net.corda.core.contracts.*
-import net.corda.core.flows.ContractUpgradeFlow
 import net.corda.core.identity.AbstractParty
+import net.corda.core.internal.UpgradeCommand
 import net.corda.core.node.ServicesForResolution
 import net.corda.core.transactions.LedgerTransaction
 import net.corda.core.transactions.TransactionBuilder
@@ -32,7 +32,6 @@ class DummyContractV2 : UpgradedContract<DummyContract.State, DummyContractV2.St
     }
 
     override fun verify(tx: LedgerTransaction) {
-        if (tx.commands.any { it.value is UpgradeCommand }) ContractUpgradeFlow.verify(tx)
         // Other verifications.
     }
     // DOCEND 1