Trading: in the two party trade protocol, check that the proposed transaction is contract-valid, and that the missing signatures are what is expected.

core/src/main/kotlin/core/TransactionVerification.kt

@@ -23,7 +23,6 @@ class TransactionConflictException(val conflictRef: StateRef, val tx1: LedgerTra
  * [nonVerifiedRoots] set. Transactions in the non-verified set are ignored other than for looking up input states.
  */
 class TransactionGroup(val transactions: Set<LedgerTransaction>, val nonVerifiedRoots: Set<LedgerTransaction>) {
-
     /**
      * Verifies the group and returns the set of resolved transactions.
      */

core/src/main/kotlin/core/Transactions.kt

@@ -121,18 +121,25 @@ data class SignedTransaction(val txBits: SerializedBytes<WireTransaction>, val s
 
     /**
      * Verify the signatures, deserialise the wire transaction and then check that the set of signatures found matches
-     * the set of pubkeys in the commands.
+     * the set of pubkeys in the commands. If any signatures are missing, either throws an exception (by default) or
+     * returns the list of keys that have missing signatures, depending on the parameter.
      *
-     * @throws SignatureException if the signature is invalid or does not match.
+     * @throws SignatureException if a signature is invalid, does not match or if any signature is missing.
      */
-    fun verify() {
+    fun verify(throwIfSignaturesAreMissing: Boolean = true): Set<PublicKey> {
         verifySignatures()
         // Verify that every command key was in the set that we just verified: there should be no commands that were
         // unverified.
         val cmdKeys = tx.commands.flatMap { it.pubkeys }.toSet()
         val sigKeys = sigs.map { it.by }.toSet()
-        if (!sigKeys.containsAll(cmdKeys))
-            throw SignatureException("Missing signatures on transaction ${id.prefixChars()} for: ${(cmdKeys - sigKeys).map { it.toStringShort() }}")
+        if (sigKeys == cmdKeys)
+            return emptySet()
+
+        val missing = cmdKeys - sigKeys
+        if (throwIfSignaturesAreMissing)
+            throw SignatureException("Missing signatures on transaction ${id.prefixChars()} for: ${missing.map { it.toStringShort() }}")
+        else
+            return missing
     }
 
     /**

src/main/kotlin/contracts/protocols/TwoPartyTradeProtocol.kt

@@ -23,6 +23,7 @@ import core.node.TimestampingProtocol
 import core.utilities.trace
 import java.security.KeyPair
 import java.security.PublicKey
+import java.security.SignatureException
 import java.time.Instant
 
 /**
@@ -110,16 +111,22 @@ object TwoPartyTradeProtocol {
             val maybeSTX = sendAndReceive<SignedTransaction>(TRADE_TOPIC, otherSide, buyerSessionID, sessionID, hello)
 
             maybeSTX.validate {
-                it.verifySignatures()
+                // Check that the tx proposed by the buyer is valid.
+                val missingSigs = it.verify(throwIfSignaturesAreMissing = false)
+                if (missingSigs != setOf(myKeyPair.public, timestampingAuthority.identity.owningKey))
+                    throw SignatureException("The set of missing signatures is not as expected: $missingSigs")
+
                 val wtx: WireTransaction = it.tx
                 logger.trace { "Received partially signed transaction: ${it.id}" }
 
                 checkDependencies(it)
 
-                // TODO: Verify that the transaction is contract-valid even though it lacks sufficient signatures.
+                // This verifies that the transaction is contract-valid, even though it is missing signatures.
+                serviceHub.verifyTransaction(wtx.toLedgerTransaction(serviceHub.identityService))
+
+                if (wtx.outputs.sumCashBy(myKeyPair.public) != price)
+                    throw IllegalArgumentException("Transaction is not sending us the right amounnt of cash")
 
-                requireThat {
-                    "transaction sends us the right amount of cash" by (wtx.outputs.sumCashBy(myKeyPair.public) == price)
                 // There are all sorts of funny games a malicious secondary might play here, we should fix them:
                 //
                 // - This tx may attempt to send some assets we aren't intending to sell to the secondary, if
@@ -127,9 +134,8 @@ object TwoPartyTradeProtocol {
                 // - This tx may include output states that impose odd conditions on the movement of the cash,
                 //   once we implement state pairing.
                 //
-                    // but the goal of this code is not to be fully secure, but rather, just to find good ways to
+                // but the goal of this code is not to be fully secure (yet), but rather, just to find good ways to
                 // express protocol state machines on top of the messaging layer.
-                }
 
                 return it
             }
@@ -220,12 +226,7 @@ object TwoPartyTradeProtocol {
                 ptx.signWith(KeyPair(k, priv))
             }
 
-            val stx = ptx.toSignedTransaction(checkSufficientSignatures = false)
-            stx.verifySignatures()  // Verifies that we generated a signed transaction correctly.
-
-            // TODO: Could run verify() here to make sure the only signature missing is the sellers.
-
-            return stx
+            return ptx.toSignedTransaction(checkSufficientSignatures = false)
         }
 
         private fun assembleSharedTX(tradeRequest: SellerTradeInfo): Pair<TransactionBuilder, List<PublicKey>> {

src/main/kotlin/core/Services.kt

@@ -8,13 +8,10 @@
 
 package core
 
-import co.paralleluniverse.fibers.Suspendable
-import core.crypto.DigitalSignature
 import core.crypto.SecureHash
 import core.crypto.generateKeyPair
 import core.messaging.MessagingService
 import core.messaging.NetworkMap
-import core.serialization.SerializedBytes
 import java.security.KeyPair
 import java.security.PrivateKey
 import java.security.PublicKey
@@ -113,4 +110,17 @@ interface ServiceHub {
     val storageService: StorageService
     val networkService: MessagingService
     val networkMapService: NetworkMap
+
+    /**
+     * Given a [LedgerTransaction], looks up all its dependencies in the local database, uses the identity service to map
+     * the [SignedTransaction]s the DB gives back into [LedgerTransaction]s, and then runs the smart contracts for the
+     * transaction. If no exception is thrown, the transaction is valid.
+     */
+    fun verifyTransaction(ltx: LedgerTransaction) {
+        val dependencies = ltx.inputs.map {
+            storageService.validatedTransactions[it.txhash] ?: throw TransactionResolutionException(it.txhash)
+        }
+        val ltxns = dependencies.map { it.verifyToLedgerTransaction(identityService) }
+        TransactionGroup(setOf(ltx), ltxns.toSet()).verify(storageService.contractPrograms)
+    }
 }

src/test/kotlin/core/messaging/TwoPartyTradeProtocolTests.kt

@@ -261,7 +261,12 @@ class TwoPartyTradeProtocolTests : TestWithInMemoryNetwork() {
                         // Bob's transactions are valid, so she commits to the database
                         RecordingMap.Put(bobsFakeCash[1].id, bobsSignedTxns[bobsFakeCash[1].id]),
                         RecordingMap.Put(bobsFakeCash[2].id, bobsSignedTxns[bobsFakeCash[2].id]),
-                        RecordingMap.Put(bobsFakeCash[0].id, bobsSignedTxns[bobsFakeCash[0].id])
+                        RecordingMap.Put(bobsFakeCash[0].id, bobsSignedTxns[bobsFakeCash[0].id]),
+                        // Now she verifies the transaction is contract-valid (not signature valid) which means
+                        // looking up the states again.
+                        RecordingMap.Get(bobsFakeCash[1].id),
+                        RecordingMap.Get(bobsFakeCash[2].id),
+                        RecordingMap.Get(alicesFakePaper[0].id)
                 )
                 assertEquals(expected, records)
             }