[CORDA-2162]: Cash.generateSpend cannot be used twice to generate two cash moves in the same tx (fix). (#4394)

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

@@ -132,7 +132,7 @@ open class TransactionBuilder @JvmOverloads constructor(
                     createComponentGroups(
                             inputStates(),
                             resolvedOutputs,
-                            commands,
+                            commands(),
                             (allContractAttachments + attachments).toSortedSet().toList(), // Sort the attachments to ensure transaction builds are stable.
                             notary,
                             window,
@@ -629,8 +629,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. */
+    /** Returns an immutable list of [Command]s, grouping by [CommandData] and joining signers. */
-    fun commands(): List<Command<*>> = ArrayList(commands)
+    fun commands(): List<Command<*>> = commands.groupBy { cmd -> cmd.value }.entries.map { (data, cmds) -> Command(data, cmds.flatMap(Command<*>::signers).toSet().toList()) }
 
     /**
      * 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

@@ -122,6 +122,25 @@ class TransactionBuilderTest {
         assertThat(wtx.references).containsOnly(referenceStateRef)
     }
 
+    @Test
+    fun `multiple commands with same data are joined without duplicates in terms of signers`() {
+        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

docs/source/changelog.rst

@@ -17,6 +17,8 @@ Unreleased
 
 * Fixed a problem with IRS demo not being able to simulate future dates as expected (https://github.com/corda/corda/issues/3851).
 
+* Fixed a problem that was preventing `Cash.generateSpend` to be used more than once per transaction (https://github.com/corda/corda/issues/4110).
+
 * ``SwapIdentitiesFlow``, from the experimental confidential-identities module, is now an inlined flow. Instead of passing in a ``Party`` with
   whom to exchange the anonymous identity, a ``FlowSession`` to that party is required instead. The flow running on the other side must
   also call ``SwapIdentitiesFlow``. This change was required as the previous API allowed any counterparty to generate anonoymous identities

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

@@ -19,7 +19,6 @@ import net.corda.finance.utils.sumCashOrNull
 import net.corda.finance.utils.sumCashOrZero
 import net.corda.node.services.vault.NodeVaultService
 import net.corda.nodeapi.internal.persistence.CordaPersistence
-import net.corda.testing.contracts.DummyState
 import net.corda.testing.core.*
 import net.corda.testing.dsl.EnforceVerifyOrFail
 import net.corda.testing.dsl.TransactionDSL
@@ -103,6 +102,10 @@ class CashTests {
             vaultFiller.fillWithSomeTestCash(400.DOLLARS, megaCorpServices, 1, megaCorp.ref(1), ourIdentity)
             vaultFiller.fillWithSomeTestCash(80.DOLLARS, miniCorpServices, 1, miniCorp.ref(1), ourIdentity)
             vaultFiller.fillWithSomeTestCash(80.SWISS_FRANCS, miniCorpServices, 1, miniCorp.ref(1), ourIdentity)
+
+            vaultFiller.fillWithSomeTestCash(100.POUNDS, megaCorpServices, 1, megaCorp.ref(1), ourIdentity)
+            vaultFiller.fillWithSomeTestCash(400.POUNDS, megaCorpServices, 1, megaCorp.ref(1), ourIdentity)
+            vaultFiller.fillWithSomeTestCash(80.POUNDS, miniCorpServices, 1, miniCorp.ref(1), ourIdentity)
         }
         database.transaction {
             vaultStatesUnconsumed = ourServices.vaultService.queryBy<Cash.State>().states
@@ -269,8 +272,8 @@ class CashTests {
             output(Cash.PROGRAM_ID, inState.copy(amount = inState.amount * 2))
             command(megaCorp.publicKey, Cash.Commands.Issue())
             tweak {
-                command(megaCorp.publicKey, Cash.Commands.Issue())
+                command(miniCorp.publicKey, Cash.Commands.Issue())
-                this `fails with` "there is only a single issue command"
+                this.verifies()
             }
             this.verifies()
         }
@@ -846,4 +849,45 @@ class CashTests {
         assertEquals(megaCorp.party, out(2).amount.token.issuer.party)
         assertEquals(megaCorp.party, out(3).amount.token.issuer.party)
     }
+
+    @Test
+    fun generateSpendTwiceWithinATransaction() {
+        val tx = TransactionBuilder(dummyNotary.party)
+        database.transaction {
+            val payments = listOf(
+                    PartyAndAmount(miniCorpAnonymised, 400.DOLLARS),
+                    PartyAndAmount(charlie.party.anonymise(), 150.DOLLARS)
+            )
+            Cash.generateSpend(ourServices, tx, payments, ourServices.myInfo.singleIdentityAndCert())
+        }
+        database.transaction {
+            val payments = listOf(
+                    PartyAndAmount(miniCorpAnonymised, 400.POUNDS),
+                    PartyAndAmount(charlie.party.anonymise(), 150.POUNDS)
+            )
+            Cash.generateSpend(ourServices, tx, payments, ourServices.myInfo.singleIdentityAndCert())
+        }
+
+        val wtx = tx.toWireTransaction(ourServices)
+        fun out(i: Int) = wtx.getOutput(i) as Cash.State
+        assertEquals(8, wtx.outputs.size)
+
+        assertEquals(80.DOLLARS, out(0).amount.withoutIssuer())
+        assertEquals(320.DOLLARS, out(1).amount.withoutIssuer())
+        assertEquals(150.DOLLARS, out(2).amount.withoutIssuer())
+        assertEquals(30.DOLLARS, out(3).amount.withoutIssuer())
+        assertEquals(miniCorp.party, out(0).amount.token.issuer.party)
+        assertEquals(megaCorp.party, out(1).amount.token.issuer.party)
+        assertEquals(megaCorp.party, out(2).amount.token.issuer.party)
+        assertEquals(megaCorp.party, out(3).amount.token.issuer.party)
+
+        assertEquals(80.POUNDS, out(4).amount.withoutIssuer())
+        assertEquals(320.POUNDS, out(5).amount.withoutIssuer())
+        assertEquals(150.POUNDS, out(6).amount.withoutIssuer())
+        assertEquals(30.POUNDS, out(7).amount.withoutIssuer())
+        assertEquals(miniCorp.party, out(4).amount.token.issuer.party)
+        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)
+    }
 }

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

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