Never send transactions to the Notary that aren't signed by all parties. Toughen up to use validating Notary in general and put Client precheck into NotaryProtocol.

Rename method to better reflect its actions

Handle comments from PR

Correct indentation

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

@@ -88,9 +88,10 @@ object TwoPartyTradeProtocol {
             val partialTX: SignedTransaction = receiveAndCheckProposedTransaction()
 
             // These two steps could be done in parallel, in theory. Our framework doesn't support that yet though.
-            val ourSignature = signWithOurKey(partialTX)
+            val ourSignature = calculateOurSignature(partialTX)
-            val notarySignature = getNotarySignature(partialTX)
+            val allPartySignedTx = partialTX + ourSignature
-            return sendSignatures(partialTX, ourSignature, notarySignature)
+            val notarySignature = getNotarySignature(allPartySignedTx)
+            return sendSignatures(allPartySignedTx, ourSignature, notarySignature)
         }
 
         @Suspendable
@@ -138,16 +139,16 @@ object TwoPartyTradeProtocol {
             }
         }
 
-        open fun signWithOurKey(partialTX: SignedTransaction): DigitalSignature.WithKey {
+        open fun calculateOurSignature(partialTX: SignedTransaction): DigitalSignature.WithKey {
             progressTracker.currentStep = SIGNING
             return myKeyPair.signWithECDSA(partialTX.txBits)
         }
 
         @Suspendable
-        private fun sendSignatures(partialTX: SignedTransaction, ourSignature: DigitalSignature.WithKey,
+        private fun sendSignatures(allPartySignedTx: SignedTransaction, ourSignature: DigitalSignature.WithKey,
                                    notarySignature: DigitalSignature.LegallyIdentifiable): SignedTransaction {
             progressTracker.currentStep = SENDING_SIGS
-            val fullySigned = partialTX + ourSignature + notarySignature
+            val fullySigned = allPartySignedTx + notarySignature
 
             logger.trace { "Built finished transaction, sending back to secondary!" }
 

core/src/main/kotlin/com/r3corda/protocols/AbstractStateReplacementProtocol.kt

@@ -79,7 +79,9 @@ abstract class AbstractStateReplacementProtocol<T> {
 
             val participantSignatures = parties.map { getParticipantSignature(it, stx) }
 
-            val allSignatures = participantSignatures + getNotarySignature(stx)
+            val allPartySignedTx = stx + participantSignatures
+
+            val allSignatures = participantSignatures + getNotarySignature(allPartySignedTx)
             parties.forEach { send(it, allSignatures) }
 
             return allSignatures

core/src/main/kotlin/com/r3corda/protocols/NotaryProtocol.kt

@@ -48,6 +48,11 @@ object NotaryProtocol {
             check(wtx.inputs.all { stateRef -> serviceHub.loadState(stateRef).notary == notaryParty }) {
                 "Input states must have the same Notary"
             }
+            try {
+                stx.verifySignatures(notaryParty.owningKey)
+            } catch (ex: SignedTransaction.SignaturesMissingException) {
+                throw NotaryException(NotaryError.SignaturesMissing(ex.missing))
+            }
 
             val request = SignRequest(stx, serviceHub.myInfo.legalIdentity)
             val response = sendAndReceive<Result>(notaryParty, request)

core/src/main/kotlin/com/r3corda/protocols/TwoPartyDealProtocol.kt

@@ -139,10 +139,11 @@ object TwoPartyDealProtocol {
             val stx: SignedTransaction = verifyPartialTransaction(getPartialTransaction())
 
             // These two steps could be done in parallel, in theory. Our framework doesn't support that yet though.
-            val ourSignature = signWithOurKey(stx)
+            val ourSignature = computeOurSignature(stx)
-            val notarySignature = getNotarySignature(stx)
+            val allPartySignedTx = stx + ourSignature
+            val notarySignature = getNotarySignature(allPartySignedTx)
 
-            val fullySigned = sendSignatures(stx, ourSignature, notarySignature)
+            val fullySigned = sendSignatures(allPartySignedTx, ourSignature, notarySignature)
 
             progressTracker.currentStep = RECORDING
 
@@ -167,16 +168,16 @@ object TwoPartyDealProtocol {
             return subProtocol(NotaryProtocol.Client(stx))
         }
 
-        open fun signWithOurKey(partialTX: SignedTransaction): DigitalSignature.WithKey {
+        open fun computeOurSignature(partialTX: SignedTransaction): DigitalSignature.WithKey {
             progressTracker.currentStep = SIGNING
             return myKeyPair.signWithECDSA(partialTX.txBits)
         }
 
         @Suspendable
-        private fun sendSignatures(partialTX: SignedTransaction, ourSignature: DigitalSignature.WithKey,
+        private fun sendSignatures(allPartySignedTx: SignedTransaction, ourSignature: DigitalSignature.WithKey,
                                    notarySignature: DigitalSignature.LegallyIdentifiable): SignedTransaction {
             progressTracker.currentStep = SENDING_SIGS
-            val fullySigned = partialTX + ourSignature + notarySignature
+            val fullySigned = allPartySignedTx + notarySignature
 
             logger.trace { "Built finished transaction, sending back to other party!" }
 

core/src/main/kotlin/com/r3corda/protocols/ValidatingNotaryProtocol.kt

@@ -38,7 +38,7 @@ class ValidatingNotaryProtocol(otherSide: Party,
 
     private fun checkSignatures(stx: SignedTransaction) {
         try {
-            stx.verifySignatures(serviceHub.myInfo.legalIdentity.owningKey)
+            stx.verifySignatures(serviceHub.myInfo.notaryIdentity.owningKey)
         } catch(e: SignedTransaction.SignaturesMissingException) {
             throw NotaryException(NotaryError.SignaturesMissing(e.missing))
         }

docs/source/protocol-state-machines.rst

@@ -242,17 +242,20 @@ Let's implement the ``Seller.call`` method. This will be run when the protocol i
       @Suspendable
       override fun call(): SignedTransaction {
           val partialTX: SignedTransaction = receiveAndCheckProposedTransaction()
-          val ourSignature: DigitalSignature.WithKey = signWithOurKey(partialTX)
+          val ourSignature: DigitalSignature.WithKey = computeOurSignature(partialTX)
-          val notarySignature = getNotarySignature(partialTX)
+          val allPartySignedTx = partialTX + ourSignature
-          val result: SignedTransaction = sendSignatures(partialTX, ourSignature, notarySignature)
+          val notarySignature = getNotarySignature(allPartySignedTx)
+          val result: SignedTransaction = sendSignatures(allPartySignedTx, ourSignature, notarySignature)
           return result
       }
 
 Here we see the outline of the procedure. We receive a proposed trade transaction from the buyer and check that it's
-valid. Then we sign with our own key and request a notary to assert with another signature that the
+valid. The buyer has already attached their signature before sending it. Then we calculate and attach our own signature so that the transaction is
+now signed by both the buyer and the seller. We then send this request to a notary to assert with another signature that the
 timestamp in the transaction (if any) is valid and there are no double spends, and send back both
-our signature and the notaries signature. Finally, we hand back to the code that invoked the protocol the
+our signature and the notaries signature. Note we should not send to the notary until all other required signatures have been appended
-finished transaction.
+as the notary may validate the signatures as well as verifying for itself the transactional integrity.
+Finally, we hand back to the code that invoked the protocol the finished transaction.
 
 Let's fill out the ``receiveAndCheckProposedTransaction()`` method.
 
@@ -369,12 +372,12 @@ Here's the rest of the code:
 
    .. sourcecode:: kotlin
 
-      open fun signWithOurKey(partialTX: SignedTransaction) = myKeyPair.signWithECDSA(partialTX.txBits)
+      open fun computeOurSignature(partialTX: SignedTransaction) = myKeyPair.signWithECDSA(partialTX.txBits)
 
       @Suspendable
-      private fun sendSignatures(partialTX: SignedTransaction, ourSignature: DigitalSignature.WithKey,
+      private fun sendSignatures(allPartySignedTX: SignedTransaction, ourSignature: DigitalSignature.WithKey,
                                  notarySignature: DigitalSignature.LegallyIdentifiable): SignedTransaction {
-          val fullySigned = partialTX + ourSignature + notarySignature
+          val fullySigned = allPartySignedTX + notarySignature
           logger.trace { "Built finished transaction, sending back to secondary!" }
           send(otherSide, SignaturesFromSeller(ourSignature, notarySignature))
           return fullySigned

node/src/test/kotlin/com/r3corda/node/services/ValidatingNotaryServiceTests.kt

@@ -66,9 +66,10 @@ class ValidatingNotaryServiceTests {
             tx.toSignedTransaction(false)
         }
 
+        val ex = assertFailsWith(ExecutionException::class) {
             val future = runClient(stx)
-
+            future.get()
-        val ex = assertFailsWith(ExecutionException::class) { future.get() }
+        }
         val notaryError = (ex.cause as NotaryException).error
         assertThat(notaryError).isInstanceOf(NotaryError.SignaturesMissing::class.java)
 

src/main/kotlin/com/r3corda/demos/IRSDemo.kt

@@ -27,6 +27,7 @@ import com.r3corda.node.services.config.NodeConfiguration
 import com.r3corda.node.services.messaging.NodeMessagingClient
 import com.r3corda.node.services.network.NetworkMapService
 import com.r3corda.node.services.transactions.SimpleNotaryService
+import com.r3corda.node.services.transactions.ValidatingNotaryService
 import com.r3corda.testing.node.MockNetwork
 import joptsimple.OptionParser
 import joptsimple.OptionSet
@@ -406,7 +407,7 @@ private fun startNode(params: CliParams.RunNode, networkMap: SingleMessageRecipi
     val networkMapId =
             when (params.node) {
                 IRSDemoNode.NodeA -> {
-                    advertisedServices = setOf(ServiceInfo(NetworkMapService.type), ServiceInfo(SimpleNotaryService.type))
+                    advertisedServices = setOf(ServiceInfo(NetworkMapService.type), ServiceInfo(ValidatingNotaryService.type))
                     null
                 }
                 IRSDemoNode.NodeB -> {

src/main/kotlin/com/r3corda/demos/TraderDemo.kt

@@ -28,7 +28,7 @@ import com.r3corda.node.services.config.FullNodeConfiguration
 import com.r3corda.node.services.messaging.NodeMessagingClient
 import com.r3corda.node.services.network.NetworkMapService
 import com.r3corda.node.services.persistence.NodeAttachmentService
-import com.r3corda.node.services.transactions.SimpleNotaryService
+import com.r3corda.node.services.transactions.ValidatingNotaryService
 import com.r3corda.node.utilities.databaseTransaction
 import com.r3corda.protocols.NotaryProtocol
 import com.r3corda.protocols.TwoPartyTradeProtocol
@@ -141,7 +141,7 @@ fun main(args: Array<String>) {
     // the map is not very helpful, but we need one anyway. So just make the buyer side run the network map as it's
     // the side that sticks around waiting for the seller.
     val networkMapId = if (role == Role.BUYER) {
-        advertisedServices = setOf(ServiceInfo(NetworkMapService.type), ServiceInfo(SimpleNotaryService.type))
+        advertisedServices = setOf(ServiceInfo(NetworkMapService.type), ServiceInfo(ValidatingNotaryService.type))
         null
     } else {
         advertisedServices = emptySet()

test-utils/src/main/kotlin/com/r3corda/testing/node/MockNode.kt

@@ -24,6 +24,7 @@ import com.r3corda.node.services.persistence.DBCheckpointStorage
 import com.r3corda.node.services.persistence.PerFileCheckpointStorage
 import com.r3corda.node.services.transactions.InMemoryUniquenessProvider
 import com.r3corda.node.services.transactions.SimpleNotaryService
+import com.r3corda.node.services.transactions.ValidatingNotaryService
 import com.r3corda.node.utilities.databaseTransaction
 import org.slf4j.Logger
 import java.nio.file.Files
@@ -240,7 +241,7 @@ class MockNetwork(private val networkSendManuallyPumped: Boolean = false,
     }
 
     fun createNotaryNode(legalName: String? = null, keyPair: KeyPair? = null): MockNode {
-        return createNode(null, -1, defaultFactory, true, legalName, keyPair, ServiceInfo(NetworkMapService.type), ServiceInfo(SimpleNotaryService.type))
+        return createNode(null, -1, defaultFactory, true, legalName, keyPair, ServiceInfo(NetworkMapService.type), ServiceInfo(ValidatingNotaryService.type))
     }
 
     fun createPartyNode(networkMapAddr: SingleMessageRecipient, legalName: String? = null, keyPair: KeyPair? = null): MockNode {