CORDA-1171: When a double-spend occurs, do not send the consuming transaction id and requesting party back to the client - this might lead to privacy leak. Only the transaction id hash is now returned.

node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt

@@ -6,6 +6,7 @@ import net.corda.core.contracts.AlwaysAcceptAttachmentConstraint
 import net.corda.core.contracts.ContractState
 import net.corda.core.contracts.StateRef
 import net.corda.core.crypto.CompositeKey
+import net.corda.core.crypto.sha256
 import net.corda.core.flows.NotaryError
 import net.corda.core.flows.NotaryException
 import net.corda.core.flows.NotaryFlow
@@ -28,8 +29,8 @@ import net.corda.nodeapi.internal.DevIdentityGenerator
 import net.corda.nodeapi.internal.network.NetworkParametersCopier
 import net.corda.testing.common.internal.testNetworkParameters
 import net.corda.testing.contracts.DummyContract
-import net.corda.testing.core.singleIdentity
 import net.corda.testing.core.dummyCommand
+import net.corda.testing.core.singleIdentity
 import net.corda.testing.node.internal.InternalMockNetwork
 import net.corda.testing.node.internal.InternalMockNetwork.MockNode
 import net.corda.testing.node.internal.InternalMockNodeParameters
@@ -142,13 +143,12 @@ class BFTNotaryServiceTests {
             }.single()
             spendTxs.zip(results).forEach { (tx, result) ->
                 if (result is Try.Failure) {
-                    val error = (result.exception as NotaryException).error as NotaryError.Conflict
+                    val exception = result.exception as NotaryException
+                    val error = exception.error as NotaryError.Conflict
                     assertEquals(tx.id, error.txId)
-                    val (stateRef, consumingTx) = error.conflict.verified().stateHistory.entries.single()
+                    val (stateRef, cause) = error.consumedStates.entries.single()
                     assertEquals(StateRef(issueTx.id, 0), stateRef)
-                    assertEquals(spendTxs[successfulIndex].id, consumingTx.id)
-                    assertEquals(0, consumingTx.inputIndex)
-                    assertEquals(info.singleIdentity(), consumingTx.requestingParty)
+                    assertEquals(spendTxs[successfulIndex].id.sha256(), cause.hashOfTransactionId)
                 }
             }
         }

node/src/main/kotlin/net/corda/node/services/transactions/BFTNonValidatingNotaryService.kt

@@ -4,16 +4,11 @@ import co.paralleluniverse.fibers.Suspendable
 import com.google.common.util.concurrent.SettableFuture
 import net.corda.core.contracts.StateRef
 import net.corda.core.crypto.Crypto
-import net.corda.core.crypto.DigitalSignature
 import net.corda.core.crypto.SecureHash
-import net.corda.core.flows.FlowLogic
-import net.corda.core.flows.FlowSession
-import net.corda.core.flows.NotaryError
-import net.corda.core.flows.NotaryException
+import net.corda.core.crypto.SignedData
+import net.corda.core.flows.*
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
-import net.corda.core.flows.NotarisationPayload
-import net.corda.core.flows.NotarisationRequest
 import net.corda.core.node.services.NotaryService
 import net.corda.core.node.services.UniquenessProvider
 import net.corda.core.schemas.PersistentStateRef
@@ -81,18 +76,22 @@ class BFTNonValidatingNotaryService(
         @Suspendable
         override fun call(): Void? {
             val payload = otherSideSession.receive<NotarisationPayload>().unwrap { it }
-            val signatures = commit(payload)
-            otherSideSession.send(signatures)
+            val response = commit(payload)
+            otherSideSession.send(response)
             return null
         }
 
-        private fun commit(payload: NotarisationPayload): List<DigitalSignature> {
+        private fun commit(payload: NotarisationPayload): NotarisationResponse {
             val response = service.commitTransaction(payload, otherSideSession.counterparty)
             when (response) {
-                is BFTSMaRt.ClusterResponse.Error -> throw NotaryException(response.error)
+                is BFTSMaRt.ClusterResponse.Error -> {
+                    // TODO: here we assume that all error will be the same, but there might be invalid onces from mailicious nodes
+                    val responseError = response.errors.first().verified()
+                    throw NotaryException(responseError, payload.coreTransaction.id)
+                }
                 is BFTSMaRt.ClusterResponse.Signatures -> {
                     log.debug("All input states of transaction ${payload.coreTransaction.id} have been committed")
-                    return response.txSignatures
+                    return NotarisationResponse(response.txSignatures)
                 }
             }
         }
@@ -150,13 +149,16 @@ class BFTNonValidatingNotaryService(
                 val inputs = transaction.inputs
                 val notary = transaction.notary
                 if (transaction is FilteredTransaction) NotaryService.validateTimeWindow(services.clock, transaction.timeWindow)
-                if (notary !in services.myInfo.legalIdentities) throw NotaryException(NotaryError.WrongNotary)
+                if (notary !in services.myInfo.legalIdentities) throw NotaryInternalException(NotaryError.WrongNotary)
                 commitInputStates(inputs, id, callerIdentity)
                 log.debug { "Inputs committed successfully, signing $id" }
                 BFTSMaRt.ReplicaResponse.Signature(sign(id))
-            } catch (e: NotaryException) {
+            } catch (e: NotaryInternalException) {
                 log.debug { "Error processing transaction: ${e.error}" }
-                BFTSMaRt.ReplicaResponse.Error(e.error)
+                val serializedError = e.error.serialize()
+                val errorSignature = sign(serializedError.bytes)
+                val signedError = SignedData(serializedError, errorSignature)
+                BFTSMaRt.ReplicaResponse.Error(signedError)
             }
         }
 

node/src/main/kotlin/net/corda/node/services/transactions/BFTSMaRt.kt

@@ -14,10 +14,8 @@ import bftsmart.tom.server.defaultservices.DefaultReplier
 import bftsmart.tom.util.Extractor
 import net.corda.core.contracts.StateRef
 import net.corda.core.crypto.*
-import net.corda.core.flows.NotaryError
-import net.corda.core.flows.NotaryException
+import net.corda.core.flows.*
 import net.corda.core.identity.Party
-import net.corda.core.flows.NotarisationPayload
 import net.corda.core.internal.declaredField
 import net.corda.core.internal.toTypedArray
 import net.corda.core.node.services.UniquenessProvider
@@ -56,15 +54,15 @@ object BFTSMaRt {
     /** Sent from [Replica] to [Client]. */
     @CordaSerializable
     sealed class ReplicaResponse {
-        data class Error(val error: NotaryError) : ReplicaResponse()
-        data class Signature(val txSignature: DigitalSignature) : ReplicaResponse()
+        data class Error(val error: SignedData<NotaryError>) : ReplicaResponse()
+        data class Signature(val txSignature: TransactionSignature) : ReplicaResponse()
     }
 
     /** An aggregate response from all replica ([Replica]) replies sent from [Client] back to the calling application. */
     @CordaSerializable
     sealed class ClusterResponse {
-        data class Error(val error: NotaryError) : ClusterResponse()
-        data class Signatures(val txSignatures: List<DigitalSignature>) : ClusterResponse()
+        data class Error(val errors: List<SignedData<NotaryError>>) : ClusterResponse()
+        data class Signatures(val txSignatures: List<TransactionSignature>) : ClusterResponse()
     }
 
     interface Cluster {
@@ -136,7 +134,7 @@ object BFTSMaRt {
                     ClusterResponse.Signatures(accepted.map { it.txSignature })
                 } else {
                     log.debug { "Cluster response - error: ${rejected.first().error}" }
-                    ClusterResponse.Error(rejected.first().error)
+                    ClusterResponse.Error(rejected.map { it.error })
                 }
 
                 val messageContent = aggregateResponse.serialize().bytes
@@ -232,10 +230,9 @@ object BFTSMaRt {
                     }
                 } else {
                     log.debug { "Conflict detected – the following inputs have already been committed: ${conflicts.keys.joinToString()}" }
-                    val conflict = UniquenessProvider.Conflict(conflicts)
-                    val conflictData = conflict.serialize()
-                    val signedConflict = SignedData(conflictData, sign(conflictData.bytes))
-                    throw NotaryException(NotaryError.Conflict(txId, signedConflict))
+                    val conflict = conflicts.mapValues { StateConsumptionDetails(it.value.id.sha256()) }
+                    val error = NotaryError.Conflict(txId, conflict)
+                    throw NotaryInternalException(error)
                 }
             }
         }

node/src/main/kotlin/net/corda/node/services/transactions/PersistentUniquenessProvider.kt

@@ -3,10 +3,13 @@ package net.corda.node.services.transactions
 import net.corda.core.contracts.StateRef
 import net.corda.core.crypto.Crypto
 import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.sha256
+import net.corda.core.flows.NotaryError
+import net.corda.core.flows.NotaryInternalException
+import net.corda.core.flows.StateConsumptionDetails
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.internal.ThreadBox
-import net.corda.core.node.services.UniquenessException
 import net.corda.core.node.services.UniquenessProvider
 import net.corda.core.schemas.PersistentStateRef
 import net.corda.core.serialization.SingletonSerializeAsToken
@@ -68,8 +71,9 @@ class PersistentUniquenessProvider : UniquenessProvider, SingletonSerializeAsTok
                         toPersistentEntityKey = { PersistentStateRef(it.txhash.toString(), it.index) },
                         fromPersistentEntity = {
                             //TODO null check will become obsolete after making DB/JPA columns not nullable
-                            var txId = it.id.txId ?: throw IllegalStateException("DB returned null SecureHash transactionId")
-                            var index = it.id.index ?: throw IllegalStateException("DB returned null SecureHash index")
+                            val txId = it.id.txId
+                                    ?: throw IllegalStateException("DB returned null SecureHash transactionId")
+                            val index = it.id.index ?: throw IllegalStateException("DB returned null SecureHash index")
                             Pair(StateRef(txhash = SecureHash.parse(txId), index = index),
                                     UniquenessProvider.ConsumingTx(
                                             id = SecureHash.parse(it.consumingTxHash),
@@ -91,7 +95,6 @@ class PersistentUniquenessProvider : UniquenessProvider, SingletonSerializeAsTok
     }
 
     override fun commit(states: List<StateRef>, txId: SecureHash, callerIdentity: Party) {
-
         val conflict = mutex.locked {
             val conflictingStates = LinkedHashMap<StateRef, UniquenessProvider.ConsumingTx>()
             for (inputState in states) {
@@ -100,7 +103,8 @@ class PersistentUniquenessProvider : UniquenessProvider, SingletonSerializeAsTok
             }
             if (conflictingStates.isNotEmpty()) {
                 log.debug("Failure, input states already committed: ${conflictingStates.keys}")
-                UniquenessProvider.Conflict(conflictingStates)
+                val conflict = conflictingStates.mapValues { StateConsumptionDetails(it.value.id.sha256()) }
+                conflict
             } else {
                 states.forEachIndexed { i, stateRef ->
                     committedStates[stateRef] = UniquenessProvider.ConsumingTx(txId, i, callerIdentity)
@@ -110,6 +114,6 @@ class PersistentUniquenessProvider : UniquenessProvider, SingletonSerializeAsTok
             }
         }
 
-        if (conflict != null) throw UniquenessException(conflict)
+        if (conflict != null) throw NotaryInternalException(NotaryError.Conflict(txId, conflict))
     }
 }

node/src/main/kotlin/net/corda/node/services/transactions/RaftUniquenessProvider.kt

@@ -19,8 +19,11 @@ import io.atomix.copycat.server.storage.Storage
 import io.atomix.copycat.server.storage.StorageLevel
 import net.corda.core.contracts.StateRef
 import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.sha256
+import net.corda.core.flows.NotaryError
+import net.corda.core.flows.NotaryInternalException
+import net.corda.core.flows.StateConsumptionDetails
 import net.corda.core.identity.Party
-import net.corda.core.node.services.UniquenessException
 import net.corda.core.node.services.UniquenessProvider
 import net.corda.core.serialization.SerializationDefaults
 import net.corda.core.serialization.SingletonSerializeAsToken
@@ -204,7 +207,11 @@ class RaftUniquenessProvider(private val transportConfiguration: NodeSSLConfigur
         val commitCommand = DistributedImmutableMap.Commands.PutAll(encode(entries))
         val conflicts = client.submit(commitCommand).get()
 
-        if (conflicts.isNotEmpty()) throw UniquenessException(UniquenessProvider.Conflict(decode(conflicts)))
+        if (conflicts.isNotEmpty()) {
+            val conflictingStates = decode(conflicts).mapValues { StateConsumptionDetails(it.value.id.sha256()) }
+            val error = NotaryError.Conflict(txId, conflictingStates)
+            throw NotaryInternalException(error)
+        }
         log.debug("All input states of transaction $txId have been committed")
     }
 

node/src/main/kotlin/net/corda/node/services/transactions/ValidatingNotaryFlow.kt

@@ -37,7 +37,7 @@ class ValidatingNotaryFlow(otherSideSession: FlowSession, service: TrustedAuthor
         } catch (e: Exception) {
             throw when (e) {
                 is TransactionVerificationException,
-                is SignatureException -> NotaryException(NotaryError.TransactionInvalid(e))
+                is SignatureException -> NotaryInternalException(NotaryError.TransactionInvalid(e))
                 else -> e
             }
         }
@@ -67,7 +67,7 @@ class ValidatingNotaryFlow(otherSideSession: FlowSession, service: TrustedAuthor
         try {
             tx.verifySignaturesExcept(service.notaryIdentityKey)
         } catch (e: SignatureException) {
-            throw NotaryException(NotaryError.TransactionInvalid(e))
+            throw NotaryInternalException(NotaryError.TransactionInvalid(e))
         }
     }
 }

node/src/test/kotlin/net/corda/node/services/transactions/NotaryServiceTests.kt

@@ -3,10 +3,7 @@ package net.corda.node.services.transactions
 import net.corda.core.concurrent.CordaFuture
 import net.corda.core.contracts.StateAndRef
 import net.corda.core.contracts.StateRef
-import net.corda.core.crypto.Crypto
-import net.corda.core.crypto.SecureHash
-import net.corda.core.crypto.TransactionSignature
-import net.corda.core.crypto.sign
+import net.corda.core.crypto.*
 import net.corda.core.flows.*
 import net.corda.core.identity.Party
 import net.corda.core.internal.generateSignature
@@ -137,32 +134,45 @@ class NotaryServiceTests {
 
     @Test
     fun `should report conflict when inputs are reused across transactions`() {
-        val inputState = issueState(aliceNode.services, alice)
-        val stx = run {
-            val tx = TransactionBuilder(notary)
-                    .addInputState(inputState)
-                    .addCommand(dummyCommand(alice.owningKey))
-            aliceNode.services.signInitialTransaction(tx)
+        val firstState = issueState(aliceNode.services, alice)
+        val secondState = issueState(aliceNode.services, alice)
+
+        fun spendState(state: StateAndRef<*>): SignedTransaction {
+            val stx = run {
+                val tx = TransactionBuilder(notary)
+                        .addInputState(state)
+                        .addCommand(dummyCommand(alice.owningKey))
+                aliceNode.services.signInitialTransaction(tx)
+            }
+            aliceNode.services.startFlow(NotaryFlow.Client(stx))
+            mockNet.runNetwork()
+            return stx
         }
-        val stx2 = run {
+
+        val firstSpendTx = spendState(firstState)
+        val secondSpendTx = spendState(secondState)
+
+        val doubleSpendTx = run {
             val tx = TransactionBuilder(notary)
-                    .addInputState(inputState)
                     .addInputState(issueState(aliceNode.services, alice))
+                    .addInputState(firstState)
+                    .addInputState(secondState)
                     .addCommand(dummyCommand(alice.owningKey))
             aliceNode.services.signInitialTransaction(tx)
         }
 
-        val firstSpend = NotaryFlow.Client(stx)
-        val secondSpend = NotaryFlow.Client(stx2) // Double spend the inputState in a second transaction.
-        aliceNode.services.startFlow(firstSpend)
-        val future = aliceNode.services.startFlow(secondSpend)
-
+        val doubleSpend = NotaryFlow.Client(doubleSpendTx) // Double spend the inputState in a second transaction.
+        val future = aliceNode.services.startFlow(doubleSpend)
         mockNet.runNetwork()
 
         val ex = assertFailsWith(NotaryException::class) { future.resultFuture.getOrThrow() }
         val notaryError = ex.error as NotaryError.Conflict
-        assertEquals(notaryError.txId, stx2.id)
-        notaryError.conflict.verified()
+        assertEquals(notaryError.txId, doubleSpendTx.id)
+        with(notaryError) {
+            assertEquals(consumedStates.size, 2)
+            assertEquals(consumedStates[firstState.ref]!!.hashOfTransactionId, firstSpendTx.id.sha256())
+            assertEquals(consumedStates[secondState.ref]!!.hashOfTransactionId, secondSpendTx.id.sha256())
+        }
     }
 
     @Test

node/src/test/kotlin/net/corda/node/services/transactions/PersistentUniquenessProviderTests.kt

@@ -1,8 +1,10 @@
 package net.corda.node.services.transactions
 
 import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.sha256
+import net.corda.core.flows.NotaryInternalException
+import net.corda.core.flows.NotaryError
 import net.corda.core.identity.CordaX500Name
-import net.corda.core.node.services.UniquenessException
 import net.corda.node.internal.configureDatabase
 import net.corda.node.services.schema.NodeSchemaService
 import net.corda.nodeapi.internal.persistence.CordaPersistence
@@ -60,12 +62,11 @@ class PersistentUniquenessProviderTests {
             val inputs = listOf(inputState)
             provider.commit(inputs, txID, identity)
 
-            val ex = assertFailsWith<UniquenessException> { provider.commit(inputs, txID, identity) }
+            val ex = assertFailsWith<NotaryInternalException> { provider.commit(inputs, txID, identity) }
+            val error = ex.error as NotaryError.Conflict
 
-            val consumingTx = ex.error.stateHistory[inputState]!!
-            assertEquals(consumingTx.id, txID)
-            assertEquals(consumingTx.inputIndex, inputs.indexOf(inputState))
-            assertEquals(consumingTx.requestingParty, identity)
+            val conflictCause = error.consumedStates[inputState]!!
+            assertEquals(conflictCause.hashOfTransactionId, txID.sha256())
         }
     }
 }

node/src/test/kotlin/net/corda/node/services/transactions/ValidatingNotaryServiceTests.kt

@@ -14,7 +14,6 @@ import net.corda.core.node.ServiceHub
 import net.corda.core.transactions.SignedTransaction
 import net.corda.core.transactions.TransactionBuilder
 import net.corda.core.utilities.getOrThrow
-import net.corda.node.services.api.StartedNodeServices
 import net.corda.node.services.issueInvalidState
 import net.corda.testing.contracts.DummyContract
 import net.corda.testing.core.ALICE_NAME
@@ -80,14 +79,13 @@ class ValidatingNotaryServiceTests {
             aliceNode.services.signInitialTransaction(tx)
         }
 
-        val ex = assertFailsWith(NotaryException::class) {
+        // Expecting SignaturesMissingException instead of NotaryException, since the exception should originate from
+        // the client flow.
+        val ex = assertFailsWith<SignedTransaction.SignaturesMissingException> {
             val future = runClient(stx)
             future.getOrThrow()
         }
-        val notaryError = ex.error as NotaryError.TransactionInvalid
-        assertThat(notaryError.cause).isInstanceOf(SignedTransaction.SignaturesMissingException::class.java)
-
-        val missingKeys = (notaryError.cause as SignedTransaction.SignaturesMissingException).missing
+        val missingKeys = ex.missing
         assertEquals(setOf(expectedMissingKey), missingKeys)
     }