CORDA-2817 Revert CORDA-2162 but modify Cash move to allow multiple m… (#4971)

* CORDA-2817 Revert CORDA-2162 but modify Cash move to allow multiple move commands and thus multiple generateSpends in the same transaction.

* CORDA-2817 Remove API changes and internalise into Cash.

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

@@ -7,7 +7,6 @@ import net.corda.core.contracts.*
 import net.corda.core.crypto.*
 import net.corda.core.identity.Party
 import net.corda.core.internal.*
-import net.corda.core.internal.cordapp.CordappResolver
 import net.corda.core.node.NetworkParameters
 import net.corda.core.node.ServiceHub
 import net.corda.core.node.ServicesForResolution
@@ -672,15 +671,8 @@ with @BelongsToContract, or supply an explicit contract parameter to addOutputSt
     /** Returns an immutable list of output [TransactionState]s. */
     fun outputStates(): List<TransactionState<*>> = ArrayList(outputs)
 
-    /** Returns an immutable list of [Command]s, grouping by [CommandData] and joining signers (from v4, v3 and below return all commands with duplicates for different signers). */
+    /** Returns an immutable list of [Command]s. */
-    fun commands(): List<Command<*>> {
+    fun commands(): List<Command<*>> = ArrayList(commands)
-        return if (CordappResolver.currentTargetVersion >= CORDA_VERSION_THAT_INTRODUCED_FLATTENED_COMMANDS) {
-            commands.groupBy { cmd -> cmd.value }
-                    .entries.map { (data, cmds) -> Command(data, cmds.flatMap(Command<*>::signers).toSet().toList()) }
-        } else {
-            ArrayList(commands)
-        }
-    }
 
     /**
      * Sign the built transaction and return it. This is an internal function for use by the service hub, please use

core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt

@@ -10,8 +10,6 @@ import net.corda.core.crypto.SecureHash
 import net.corda.core.identity.Party
 import net.corda.core.internal.AbstractAttachment
 import net.corda.core.internal.PLATFORM_VERSION
-import net.corda.core.internal.cordapp.CordappImpl.Companion.DEFAULT_CORDAPP_VERSION
-import net.corda.core.internal.cordapp.CordappResolver
 import net.corda.core.node.ServicesForResolution
 import net.corda.core.node.ZoneVersionTooLowException
 import net.corda.core.node.services.AttachmentStorage
@@ -115,27 +113,6 @@ class TransactionBuilderTest {
         assertThat(wtx.references).containsOnly(referenceStateRef)
     }
 
-    @Test
-    fun `multiple commands with same data are joined without duplicates in terms of signers`() {
-        // This behaviour is only activated for platform version 4 onwards.
-        CordappResolver.withCordapp(targetPlatformVersion = 4) {
-            val aliceParty = TestIdentity(ALICE_NAME).party
-            val bobParty = TestIdentity(BOB_NAME).party
-            val tx = TransactionBuilder(notary)
-            tx.addCommand(DummyCommandData, notary.owningKey, aliceParty.owningKey)
-            tx.addCommand(DummyCommandData, aliceParty.owningKey, bobParty.owningKey)
-
-            val commands = tx.commands()
-
-            assertThat(commands).hasSize(1)
-            assertThat(commands.single()).satisfies { cmd ->
-                assertThat(cmd.value).isEqualTo(DummyCommandData)
-                assertThat(cmd.signers).hasSize(3)
-                assertThat(cmd.signers).contains(notary.owningKey, bobParty.owningKey, aliceParty.owningKey)
-            }
-        }
-    }
-
     @Test
     fun `automatic signature constraint`() {
         val aliceParty = TestIdentity(ALICE_NAME).party

finance/contracts/src/main/kotlin/net/corda/finance/contracts/asset/Cash.kt

@@ -14,10 +14,10 @@ import net.corda.core.schemas.PersistentState
 import net.corda.core.schemas.QueryableState
 import net.corda.core.transactions.LedgerTransaction
 import net.corda.core.transactions.TransactionBuilder
-import net.corda.finance.schemas.CashSchemaV1
 import net.corda.finance.contracts.utils.sumCash
 import net.corda.finance.contracts.utils.sumCashOrNull
 import net.corda.finance.contracts.utils.sumCashOrZero
+import net.corda.finance.schemas.CashSchemaV1
 import java.security.PublicKey
 import java.util.*
 
@@ -164,7 +164,7 @@ class Cash : OnLedgerAsset<Currency, Cash.Commands, Cash.State>() {
                             (inputAmount == outputAmount + amountExitingLedger)
                 }
 
-                verifyMoveCommand<Commands.Move>(inputs, tx.commands)
+                verifyFlattenedMoveCommand<Commands.Move>(inputs, tx.commands)
             }
         }
     }
@@ -207,3 +207,37 @@ class Cash : OnLedgerAsset<Currency, Cash.Commands, Cash.State>() {
 val Amount<Currency>.CASH: Cash.State get() = Cash.State(Amount(quantity, Issued(NULL_PARTY.ref(1), token)), NULL_PARTY)
 /** An extension property that lets you get a cash state from an issued token, under the [NULL_PARTY] */
 val Amount<Issued<Currency>>.STATE: Cash.State get() = Cash.State(this, NULL_PARTY)
+
+/**
+ * Simple functionality for verifying multiple move commands that differ only by signers. Verifies that each input has a signature from its owning key.
+ *
+ * @param T the type of the move command.
+ */
+@Throws(IllegalArgumentException::class)
+internal inline fun <reified T : MoveCommand> verifyFlattenedMoveCommand(inputs: List<OwnableState>,
+                                                                         commands: List<CommandWithParties<CommandData>>)
+        : MoveCommand {
+    // Now check the digital signatures on the move command. Every input has an owning public key, and we must
+    // see a signature from each of those keys. The actual signatures have been verified against the transaction
+    // data by the platform before execution.
+    val owningPubKeys = inputs.map { it.owner.owningKey }.toSet()
+    val commands = commands.groupCommands<T>()
+    // Does not use requireThat to maintain message compatibility with verifyMoveCommand.
+    if (commands.isEmpty()) {
+        throw IllegalStateException("Required ${T::class.qualifiedName} command")
+    }
+    requireThat {
+        "move commands can only differ by signing keys" using (commands.size == 1)
+    }
+    val keysThatSigned = commands.values.first()
+    requireThat {
+        "the owning keys are a subset of the signing keys" using keysThatSigned.containsAll(owningPubKeys)
+    }
+    return commands.keys.single()
+}
+
+/** Group commands by instances of the given type. */
+internal inline fun <reified T : CommandData> Collection<CommandWithParties<CommandData>>.groupCommands() = groupCommands(T::class.java)
+
+/** Group commands by instances of the given type. */
+internal fun <C : CommandData> Collection<CommandWithParties<CommandData>>.groupCommands(klass: Class<C>) = select(klass).groupBy { it.value }.map { it.key to it.value.flatMap { it.signers }.toSet() }.toMap()

finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/CashTests.kt

@@ -164,6 +164,22 @@ class CashTests {
         }
     }
 
+    @Test
+    fun twoMoves() {
+        transaction {
+            attachment(Cash.PROGRAM_ID)
+            input(Cash.PROGRAM_ID, inState)
+            input(Cash.PROGRAM_ID, inState.copy(owner = bob.party))
+            output(Cash.PROGRAM_ID, outState)
+            command(alice.publicKey, Cash.Commands.Move())
+            tweak {
+                output(Cash.PROGRAM_ID, outState)
+                command(bob.publicKey, Cash.Commands.Move())
+                this.verifies()
+            }
+        }
+    }
+
     @BelongsToContract(Cash::class)
     object DummyState: ContractState {
         override val participants: List<AbstractParty> = emptyList()
@@ -273,8 +289,8 @@ class CashTests {
             output(Cash.PROGRAM_ID, inState.copy(amount = inState.amount * 2))
             command(megaCorp.publicKey, Cash.Commands.Issue())
             tweak {
-                command(miniCorp.publicKey, Cash.Commands.Issue())
+                command(megaCorp.publicKey, Cash.Commands.Issue())
-                this.verifies()
+                this `fails with` "there is only a single issue command"
             }
             this.verifies()
         }
@@ -889,5 +905,7 @@ class CashTests {
         assertEquals(megaCorp.party, out(5).amount.token.issuer.party)
         assertEquals(megaCorp.party, out(6).amount.token.issuer.party)
         assertEquals(megaCorp.party, out(7).amount.token.issuer.party)
+
+        assertEquals(2, wtx.commands.size)
     }
 }

finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/ObligationTests.kt

@@ -244,7 +244,7 @@ class ObligationTests {
             command(MEGA_CORP_PUBKEY, Obligation.Commands.Issue())
             tweak {
                 command(MEGA_CORP_PUBKEY, Obligation.Commands.Issue())
-                this.verifies()
+                this `fails with` "there is only a single issue command"
             }
             this.verifies()
         }