FIX - Modify generateSpend() to send change to a fresh confidential identity (#1533)

* Modify generateSpend() to send change to a fresh confidential identity

* Update cash tests to handle change being sent to a new address

* Add comments explaining intent

* Fix broken master build.

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

@@ -327,7 +327,13 @@ class Cash : OnLedgerAsset<Currency, Cash.Commands, Cash.State>() {
             val totalAmount = payments.map { it.amount }.sumOrThrow()
             val cashSelection = CashSelection.getInstance({ services.jdbcSession().metaData })
             val acceptableCoins = cashSelection.unconsumedCashStatesForSpending(services, totalAmount, onlyFromParties, tx.notary, tx.lockId)
+            val revocationEnabled = false // Revocation is currently unsupported
+            // Generate a new identity that change will be sent to for confidentiality purposes. This means that a
+            // third party with a copy of the transaction (such as the notary) cannot identify who the change was
+            // sent to
+            val changeIdentity = services.keyManagementService.freshKeyAndCert(services.myInfo.legalIdentitiesAndCerts.first(), revocationEnabled)
             return OnLedgerAsset.generateSpend(tx, payments, acceptableCoins,
+                    changeIdentity.party.anonymise(),
                     { state, quantity, owner -> deriveState(state, quantity, owner) },
                     { Cash().generateMoveCommand() })
         }

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

@@ -2,7 +2,6 @@ package net.corda.finance.contracts.asset
 
 import net.corda.core.contracts.*
 import net.corda.core.contracts.Amount.Companion.sumOrThrow
-import net.corda.core.contracts.Amount.Companion.zero
 import net.corda.core.identity.AbstractParty
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.utilities.loggerFor
@@ -45,6 +44,8 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
          * @param amount How much currency to send.
          * @param to a key of the recipient.
          * @param acceptableStates a list of acceptable input states to use.
+         * @param payChangeTo party to pay any change to; this is normally a confidential identity of the calling
+         * party.
          * @param deriveState a function to derive an output state based on an input state, amount for the output
          * and public key to pay to.
          * @return A [Pair] of the same transaction builder passed in as [tx], and the list of keys that need to sign
@@ -58,9 +59,10 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
                                                          amount: Amount<T>,
                                                          to: AbstractParty,
                                                          acceptableStates: List<StateAndRef<S>>,
+                                                         payChangeTo: AbstractParty,
                                                          deriveState: (TransactionState<S>, Amount<Issued<T>>, AbstractParty) -> TransactionState<S>,
                                                          generateMoveCommand: () -> CommandData): Pair<TransactionBuilder, List<PublicKey>> {
-            return generateSpend(tx, listOf(PartyAndAmount(to, amount)), acceptableStates, deriveState, generateMoveCommand)
+            return generateSpend(tx, listOf(PartyAndAmount(to, amount)), acceptableStates, payChangeTo, deriveState, generateMoveCommand)
         }
 
         /**
@@ -76,6 +78,8 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
          * @param amount How much currency to send.
          * @param to a key of the recipient.
          * @param acceptableStates a list of acceptable input states to use.
+         * @param payChangeTo party to pay any change to; this is normally a confidential identity of the calling
+         * party. We use a new confidential identity here so that the recipient is not identifiable.
          * @param deriveState a function to derive an output state based on an input state, amount for the output
          * and public key to pay to.
          * @param T A type representing a token
@@ -90,6 +94,7 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
         fun <S : FungibleAsset<T>, T: Any> generateSpend(tx: TransactionBuilder,
                                                          payments: List<PartyAndAmount<T>>,
                                                          acceptableStates: List<StateAndRef<S>>,
+                                                         payChangeTo: AbstractParty,
                                                          deriveState: (TransactionState<S>, Amount<Issued<T>>, AbstractParty) -> TransactionState<S>,
                                                          generateMoveCommand: () -> CommandData): Pair<TransactionBuilder, List<PublicKey>> {
             // Discussion
@@ -133,7 +138,7 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
             // how much we've gathered for each issuer: this map will keep track of how much we've used from each
             // as we work our way through the payments.
             val statesGroupedByIssuer = gathered.groupBy { it.state.data.amount.token }
-            val remainingFromEachIssuer= statesGroupedByIssuer
+            val remainingFromEachIssuer = statesGroupedByIssuer
                     .mapValues {
                         it.value.map {
                             it.state.data.amount
@@ -141,7 +146,7 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
                     }.toList().toMutableList()
             val outputStates = mutableListOf<TransactionState<S>>()
             for ((party, paymentAmount) in payments) {
-                var remainingToPay= paymentAmount.quantity
+                var remainingToPay = paymentAmount.quantity
                 while (remainingToPay > 0) {
                     val (token, remainingFromCurrentIssuer) = remainingFromEachIssuer.last()
                     val templateState = statesGroupedByIssuer[token]!!.first().state
@@ -171,10 +176,9 @@ abstract class OnLedgerAsset<T : Any, C : CommandData, S : FungibleAsset<T>> : C
             }
 
             // Whatever values we have left over for each issuer must become change outputs.
-            val myself = gathered.first().state.data.owner
             for ((token, amount) in remainingFromEachIssuer) {
                 val templateState = statesGroupedByIssuer[token]!!.first().state
-                outputStates += deriveState(templateState, amount, myself)
+                outputStates += deriveState(templateState, amount, payChangeTo)
             }
 
             for (state in gathered) tx.addInputState(state)

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

@@ -602,9 +602,19 @@ class CashTests : TestDependencyInjectionBase() {
         database.transaction {
             @Suppress("UNCHECKED_CAST")
             val vaultState = vaultStatesUnconsumed.elementAt(0)
+            val changeAmount = 90.DOLLARS `issued by` defaultIssuer
+            val likelyChangeState = wtx.outputs.map(TransactionState<*>::data).filter { state ->
+                if (state is Cash.State) {
+                    state.amount == changeAmount
+                } else {
+                    false
+                }
+            }.single()
+            val changeOwner = (likelyChangeState as Cash.State).owner
+            assertEquals(1, miniCorpServices.keyManagementService.filterMyKeys(setOf(changeOwner.owningKey)).toList().size)
             assertEquals(vaultState.ref, wtx.inputs[0])
             assertEquals(vaultState.state.data.copy(owner = THEIR_IDENTITY_1, amount = 10.DOLLARS `issued by` defaultIssuer), wtx.outputs[0].data)
-            assertEquals(vaultState.state.data.copy(amount = 90.DOLLARS `issued by` defaultIssuer), wtx.outputs[1].data)
+            assertEquals(vaultState.state.data.copy(amount = changeAmount, owner = changeOwner), wtx.outputs[1].data)
             assertEquals(OUR_IDENTITY_1.owningKey, wtx.commands.single { it.value is Cash.Commands.Move }.signers[0])
         }
     }