From 26855967989557e4c078bb08dd528231d30fad8b Mon Sep 17 00:00:00 2001 From: Rick Parker Date: Tue, 2 Apr 2019 18:23:43 +0100 Subject: [PATCH] =?UTF-8?q?CORDA-2817=20Revert=20CORDA-2162=20but=20modify?= =?UTF-8?q?=20Cash=20move=20to=20allow=20multiple=20m=E2=80=A6=20(#4971)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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/transactions/TransactionBuilder.kt | 12 +----- .../transactions/TransactionBuilderTest.kt | 23 ----------- .../net/corda/finance/contracts/asset/Cash.kt | 38 ++++++++++++++++++- .../finance/contracts/asset/CashTests.kt | 22 ++++++++++- .../contracts/asset/ObligationTests.kt | 2 +- 5 files changed, 59 insertions(+), 38 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt index 2551fade8b..69f9013678 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/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> = 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). */ - fun commands(): List> { - 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) - } - } + /** Returns an immutable list of [Command]s. */ + fun commands(): List> = ArrayList(commands) /** * Sign the built transaction and return it. This is an internal function for use by the service hub, please use diff --git a/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt b/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt index 4881ebf5e7..abadaedeb7 100644 --- a/core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt +++ b/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 diff --git a/finance/contracts/src/main/kotlin/net/corda/finance/contracts/asset/Cash.kt b/finance/contracts/src/main/kotlin/net/corda/finance/contracts/asset/Cash.kt index 20d0d78774..1730bf5013 100644 --- a/finance/contracts/src/main/kotlin/net/corda/finance/contracts/asset/Cash.kt +++ b/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() { (inputAmount == outputAmount + amountExitingLedger) } - verifyMoveCommand(inputs, tx.commands) + verifyFlattenedMoveCommand(inputs, tx.commands) } } } @@ -207,3 +207,37 @@ class Cash : OnLedgerAsset() { val Amount.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>.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 verifyFlattenedMoveCommand(inputs: List, + commands: List>) + : 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() + // 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 Collection>.groupCommands() = groupCommands(T::class.java) + +/** Group commands by instances of the given type. */ +internal fun Collection>.groupCommands(klass: Class) = select(klass).groupBy { it.value }.map { it.key to it.value.flatMap { it.signers }.toSet() }.toMap() diff --git a/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/CashTests.kt b/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/CashTests.kt index b1c7ef250e..ad96837eb4 100644 --- a/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/CashTests.kt +++ b/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 = 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()) - this.verifies() + command(megaCorp.publicKey, Cash.Commands.Issue()) + 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) } } diff --git a/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/ObligationTests.kt b/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/ObligationTests.kt index 669525a72c..13c1428a54 100644 --- a/finance/contracts/src/test/kotlin/net/corda/finance/contracts/asset/ObligationTests.kt +++ b/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() }