Merged in rnicoll-cash-destroy-own (pull request #293)

Restrict cash exit commands to working on cash held by the issuer

contracts/src/main/kotlin/com/r3corda/contracts/asset/Cash.kt

@@ -85,7 +85,7 @@ class Cash : OnLedgerAsset<Currency, Cash.State>() {
         : this(Amount(amount.quantity, Issued(deposit, amount.token)), owner)
 
         override val deposit = amount.token.issuer
-        override val exitKeys = setOf(deposit.party.owningKey)
+        override val exitKeys = setOf(owner, deposit.party.owningKey)
         override val contract = CASH_PROGRAM_ID
         override val issuanceDef = amount.token
         override val participants = listOf(owner)

contracts/src/main/kotlin/com/r3corda/contracts/asset/FungibleAsset.kt

@@ -29,7 +29,10 @@ interface FungibleAsset<T> : OwnableState {
     val deposit: PartyAndReference
     val issuanceDef: Issued<T>
     val amount: Amount<Issued<T>>
-    /** There must be an ExitCommand signed by these keys to destroy the amount */
+    /**
+     * There must be an ExitCommand signed by these keys to destroy the amount. While all states require their
+     * owner to sign, some (i.e. cash) also require the issuer.
+     */
     val exitKeys: Collection<PublicKey>
     /** There must be a MoveCommand signed by this key to claim the amount */
     override val owner: PublicKey

contracts/src/main/kotlin/com/r3corda/contracts/asset/Obligation.kt

@@ -447,16 +447,14 @@ class Obligation<P> : Contract {
      *
      * @param tx transaction builder to add states and commands to.
      * @param amountIssued the amount to be exited, represented as a quantity of issued currency.
-     * @param changeKey the key to send any change to. This needs to be explicitly stated as the input states are not
-     * necessarily owned by us.
      * @param assetStates the asset states to take funds from. No checks are done about ownership of these states, it is
      * the responsibility of the caller to check that they do not exit funds held by others.
      * @return the public key of the assets issuer, who must sign the transaction for it to be valid.
      */
     @Suppress("unused")
     fun generateExit(tx: TransactionBuilder, amountIssued: Amount<Issued<Terms<P>>>,
-                     changeKey: PublicKey, assetStates: List<StateAndRef<Obligation.State<P>>>): PublicKey
+                     assetStates: List<StateAndRef<Obligation.State<P>>>): PublicKey
-            = Clauses.ConserveAmount<P>().generateExit(tx, amountIssued, changeKey, assetStates,
+            = Clauses.ConserveAmount<P>().generateExit(tx, amountIssued, assetStates,
             deriveState = { state, amount, owner -> state.copy(data = state.data.move(amount, owner)) },
             generateExitCommand = { amount -> Commands.Exit(amount) }
     )

contracts/src/main/kotlin/com/r3corda/contracts/asset/OnLedgerAsset.kt

@@ -44,8 +44,8 @@ abstract class OnLedgerAsset<T : Any, S : FungibleAsset<T>> : Contract {
      * @return the public key of the assets issuer, who must sign the transaction for it to be valid.
      */
     fun generateExit(tx: TransactionBuilder, amountIssued: Amount<Issued<T>>,
-                     changeKey: PublicKey, assetStates: List<StateAndRef<S>>): PublicKey
+                     assetStates: List<StateAndRef<S>>): PublicKey
-        = conserveClause.generateExit(tx, amountIssued, changeKey, assetStates,
+        = conserveClause.generateExit(tx, amountIssued, assetStates,
             deriveState = { state, amount, owner -> deriveState(state, amount, owner) },
             generateExitCommand = { amount -> generateExitCommand(amount) }
     )

contracts/src/main/kotlin/com/r3corda/contracts/clause/AbstractConserveAmount.kt

@@ -53,16 +53,15 @@ abstract class AbstractConserveAmount<S: FungibleAsset<T>, T: Any> : GroupClause
      *
      * @param tx transaction builder to add states and commands to.
      * @param amountIssued the amount to be exited, represented as a quantity of issued currency.
-     * @param changeKey the key to send any change to. This needs to be explicitly stated as the input states are not
-     * necessarily owned by us.
      * @param assetStates the asset states to take funds from. No checks are done about ownership of these states, it is
      * the responsibility of the caller to check that they do not exit funds held by others.
      * @return the public key of the assets issuer, who must sign the transaction for it to be valid.
      */
     fun generateExit(tx: TransactionBuilder, amountIssued: Amount<Issued<T>>,
-                     changeKey: PublicKey, assetStates: List<StateAndRef<S>>,
+                     assetStates: List<StateAndRef<S>>,
                      deriveState: (TransactionState<S>, Amount<Issued<T>>, PublicKey) -> TransactionState<S>,
                      generateExitCommand: (Amount<Issued<T>>) -> CommandData): PublicKey {
+        val owner = assetStates.map { it.state.data.owner }.toSet().single()
         val currency = amountIssued.token.product
         val amount = Amount(amountIssued.quantity, currency)
         var acceptableCoins = assetStates.filter { ref -> ref.state.data.amount.token == amountIssued.token }
@@ -82,7 +81,7 @@ abstract class AbstractConserveAmount<S: FungibleAsset<T>, T: Any> : GroupClause
 
         val outputs = if (change != null) {
             // Add a change output and adjust the last output downwards.
-            listOf(deriveState(gathered.last().state, change, changeKey))
+            listOf(deriveState(gathered.last().state, change, owner))
         } else emptyList()
 
         for (state in gathered) tx.addInputState(state)
@@ -184,15 +183,14 @@ abstract class AbstractConserveAmount<S: FungibleAsset<T>, T: Any> : GroupClause
         val deposit = token.issuer
         val outputAmount: Amount<Issued<T>> = outputs.sumFungibleOrZero(token)
 
-        // If we want to remove assets from the ledger, that must be signed for by the issuer.
+        // If we want to remove assets from the ledger, that must be signed for by the issuer and owner.
-        // A mis-signed or duplicated exit command will just be ignored here and result in the exit amount being zero.
         val exitKeys: Set<PublicKey> = inputs.flatMap { it.exitKeys }.toSet()
         val exitCommand = tx.commands.select<FungibleAsset.Commands.Exit<T>>(parties = null, signers = exitKeys).filter {it.value.amount.token == token}.singleOrNull()
         val amountExitingLedger: Amount<Issued<T>> = exitCommand?.value?.amount ?: Amount(0, token)
 
         requireThat {
             "there are no zero sized inputs" by inputs.none { it.amount.quantity == 0L }
-            "for reference ${deposit.reference} at issuer ${deposit.party.name} the amounts balance" by
+            "for reference ${deposit.reference} at issuer ${deposit.party.name} the amounts balance: ${inputAmount.quantity} - ${amountExitingLedger.quantity} != ${outputAmount.quantity}" by
                     (inputAmount == outputAmount + amountExitingLedger)
         }
 

contracts/src/test/kotlin/com/r3corda/contracts/asset/CashTests.kt

@@ -17,7 +17,9 @@ class CashTests {
             amount = 1000.DOLLARS `issued by` defaultIssuer,
             owner = DUMMY_PUBKEY_1
     )
-    val outState = inState.copy(owner = DUMMY_PUBKEY_2)
+    // Input state held by the issuer
+    val issuerInState = inState.copy(owner = defaultIssuer.party.owningKey)
+    val outState = issuerInState.copy(owner = DUMMY_PUBKEY_2)
 
     fun Cash.State.editDepositRef(ref: Byte) = copy(
             amount = Amount(amount.quantity, token = amount.token.copy(deposit.copy(reference = OpaqueBytes.of(ref))))
@@ -111,7 +113,7 @@ class CashTests {
 
         // We can consume $1000 in a transaction and output $2000 as long as it's signed by an issuer.
         transaction {
-            input { inState }
+            input { issuerInState }
             output { inState.copy(amount = inState.amount * 2) }
 
             // Move fails: not allowed to summon money.
@@ -279,12 +281,12 @@ class CashTests {
     fun exitLedger() {
         // Single input/output straightforward case.
         transaction {
-            input { inState }
+            input { issuerInState }
-            output { outState.copy(amount = inState.amount - (200.DOLLARS `issued by` defaultIssuer)) }
+            output { issuerInState.copy(amount = issuerInState.amount - (200.DOLLARS `issued by` defaultIssuer)) }
 
             tweak {
                 command(MEGA_CORP_PUBKEY) { Cash.Commands.Exit(100.DOLLARS `issued by` defaultIssuer) }
-                command(DUMMY_PUBKEY_1) { Cash.Commands.Move() }
+                command(MEGA_CORP_PUBKEY) { Cash.Commands.Move() }
                 this `fails with` "the amounts balance"
             }
 
@@ -293,20 +295,24 @@ class CashTests {
                 this `fails with` "required com.r3corda.contracts.asset.FungibleAsset.Commands.Move command"
 
                 tweak {
-                    command(DUMMY_PUBKEY_1) { Cash.Commands.Move() }
+                    command(MEGA_CORP_PUBKEY) { Cash.Commands.Move() }
                     this.verifies()
                 }
             }
         }
+    }
+
+    @Test
+    fun `exit ledger with multiple issuers`() {
         // Multi-issuer case.
         transaction {
-            input { inState }
+            input { issuerInState }
-            input { inState `issued by` MINI_CORP }
+            input { issuerInState.copy(owner = MINI_CORP_PUBKEY) `issued by` MINI_CORP }
 
-            output { inState.copy(amount = inState.amount - (200.DOLLARS `issued by` defaultIssuer)) `issued by` MINI_CORP }
+            output { issuerInState.copy(amount = issuerInState.amount - (200.DOLLARS `issued by` defaultIssuer)) `issued by` MINI_CORP }
-            output { inState.copy(amount = inState.amount - (200.DOLLARS `issued by` defaultIssuer)) }
+            output { issuerInState.copy(owner = MINI_CORP_PUBKEY, amount = issuerInState.amount - (200.DOLLARS `issued by` defaultIssuer)) }
 
-            command(DUMMY_PUBKEY_1) { Cash.Commands.Move() }
+            command(MEGA_CORP_PUBKEY, MINI_CORP_PUBKEY) { Cash.Commands.Move() }
 
             this `fails with` "at issuer MegaCorp the amounts balance"
 
@@ -318,6 +324,18 @@ class CashTests {
         }
     }
 
+    @Test
+    fun `exit cash not held by its issuer`() {
+        // Single input/output straightforward case.
+        transaction {
+            input { inState }
+            output { outState.copy(amount = inState.amount - (200.DOLLARS `issued by` defaultIssuer)) }
+            command(MEGA_CORP_PUBKEY) { Cash.Commands.Exit(200.DOLLARS `issued by` defaultIssuer) }
+            command(DUMMY_PUBKEY_1) { Cash.Commands.Move() }
+            this `fails with` "at issuer MegaCorp the amounts balance"
+        }
+    }
+
     @Test
     fun multiIssuer() {
         transaction {
@@ -385,7 +403,7 @@ class CashTests {
      */
     fun makeExit(amount: Amount<Currency>, corp: Party, depositRef: Byte = 1): WireTransaction {
         val tx = TransactionType.General.Builder(DUMMY_NOTARY)
-        Cash().generateExit(tx, Amount(amount.quantity, Issued(corp.ref(depositRef), amount.token)), OUR_PUBKEY_1, WALLET)
+        Cash().generateExit(tx, Amount(amount.quantity, Issued(corp.ref(depositRef), amount.token)), WALLET)
         return tx.toWireTransaction()
     }
 

node/src/main/kotlin/com/r3corda/node/services/monitor/WalletMonitorService.kt

@@ -193,7 +193,7 @@ class WalletMonitorService(net: MessagingService, val smm: StateMachineManager,
         val builder: TransactionBuilder = TransactionType.General.Builder(null)
         val issuer = PartyAndReference(services.storageService.myLegalIdentity, req.issueRef)
         Cash().generateExit(builder, Amount(req.pennies, Issued(issuer, req.currency)),
-                issuer.party.owningKey, services.walletService.currentWallet.statesOfType<Cash.State>())
+                services.walletService.currentWallet.statesOfType<Cash.State>().filter { it.state.data.owner == issuer.party.owningKey })
         builder.signWith(services.storageService.myLegalIdentityKey)
         val tx = builder.toSignedTransaction()
         services.walletService.notify(tx.tx)