Tx validation detect duplicate inputs (#138)

* Move duplicate input detection to transaction verification

Duplicate detection was previously part of the NotaryFlow.

core/src/main/kotlin/net/corda/core/contracts/Structures.kt

@@ -136,7 +136,7 @@ data class TransactionState<out T : ContractState> @JvmOverloads constructor(
          *
          * The encumbered state refers to another by index, and the referred encumbrance state
          * is an output state in a particular position on the same transaction that created the encumbered state. An alternative
-         * implementation would be encumber by reference to a StateRef., which would allow the specification of encumbrance
+         * implementation would be encumbering by reference to a [StateRef], which would allow the specification of encumbrance
          * by a state created in a prior transaction.
          *
          * Note that an encumbered state that is being consumed must have its encumbrance consumed in the same transaction,

core/src/main/kotlin/net/corda/core/contracts/TransactionTypes.kt

@@ -19,6 +19,8 @@ sealed class TransactionType {
      */
     fun verify(tx: LedgerTransaction) {
         require(tx.notary != null || tx.timestamp == null) { "Transactions with timestamps must be notarised." }
+        val duplicates = detectDuplicateInputs(tx)
+        if (duplicates.isNotEmpty()) throw TransactionVerificationException.DuplicateInputStates(tx, duplicates)
         val missing = verifySigners(tx)
         if (missing.isNotEmpty()) throw TransactionVerificationException.SignersMissing(tx, missing.toList())
         verifyTransaction(tx)
@@ -35,6 +37,19 @@ sealed class TransactionType {
         return missing
     }
 
+    /** Check that the inputs are unique. */
+    private fun detectDuplicateInputs(tx: LedgerTransaction): Set<StateRef> {
+        var seenInputs = emptySet<StateRef>()
+        var duplicates = emptySet<StateRef>()
+        tx.inputs.forEach { state ->
+            if (seenInputs.contains(state.ref)) {
+                duplicates += state.ref
+            }
+            seenInputs += state.ref
+        }
+        return duplicates
+    }
+
     /**
      * Return the list of public keys that that require signatures for the transaction type.
      * Note: the notary key is checked separately for all transactions and need not be included.

core/src/main/kotlin/net/corda/core/contracts/TransactionVerification.kt

@@ -97,6 +97,9 @@ sealed class TransactionVerificationException(val tx: LedgerTransaction, cause:
     class SignersMissing(tx: LedgerTransaction, val missing: List<CompositeKey>) : TransactionVerificationException(tx, null) {
         override fun toString() = "Signers missing: ${missing.joinToString()}"
     }
+    class DuplicateInputStates(tx: LedgerTransaction, val duplicates: Set<StateRef>) : TransactionVerificationException(tx, null) {
+        override fun toString() = "Duplicate inputs: ${duplicates.joinToString()}"
+    }
 
     class InvalidNotaryChange(tx: LedgerTransaction) : TransactionVerificationException(tx, null)
     class NotaryChangeInWrongTransactionType(tx: LedgerTransaction, val outputNotary: Party) : TransactionVerificationException(tx, null) {

core/src/main/kotlin/net/corda/flows/NotaryFlow.kt

@@ -101,8 +101,6 @@ object NotaryFlow {
 
             val result = try {
                 validateTimestamp(wtx)
-                // TODO:  Move the duplicate input detection to `TransactionType.verify()`.
-                detectDuplicateInputs(wtx)
                 beforeCommit(stx)
                 commitInputStates(wtx)
                 val sig = sign(stx.id.bytes)
@@ -132,21 +130,6 @@ object NotaryFlow {
         open fun beforeCommit(stx: SignedTransaction) {
         }
 
-        /** Throw NotaryException if inputs are not unique. */
-        private fun detectDuplicateInputs(tx: WireTransaction) {
-            var seenInputs = emptySet<StateRef>()
-            var conflicts = emptyMap<StateRef, UniquenessProvider.ConsumingTx>()
-            tx.inputs.forEachIndexed { i, stateRef ->
-                if (seenInputs.contains(stateRef)) {
-                    conflicts += stateRef.to(UniquenessProvider.ConsumingTx(tx.id, i, otherSide))
-                }
-                seenInputs += stateRef
-            }
-            if (conflicts.isNotEmpty()) {
-                throw notaryException(tx, UniquenessException(UniquenessProvider.Conflict(conflicts)))
-            }
-        }
-
         /**
          * A NotaryException is thrown if any of the states have been consumed by a different transaction. Note that
          * this method does not throw an exception when input states are present multiple times within the transaction.

core/src/test/kotlin/net/corda/core/contracts/TransactionTests.kt

@@ -82,6 +82,33 @@ class TransactionTests {
         transaction.type.verify(transaction)
     }
 
+    @Test
+    fun `transaction verification fails for duplicate inputs`() {
+        val baseOutState = TransactionState(DummyContract.SingleOwnerState(0, ALICE_PUBKEY), DUMMY_NOTARY)
+        val stateRef = StateRef(SecureHash.randomSHA256(), 0)
+        val stateAndRef = StateAndRef(baseOutState, stateRef)
+        val inputs = listOf(stateAndRef, stateAndRef)
+        val outputs = listOf(baseOutState)
+        val commands = emptyList<AuthenticatedObject<CommandData>>()
+        val attachments = emptyList<Attachment>()
+        val id = SecureHash.randomSHA256()
+        val signers = listOf(DUMMY_NOTARY_KEY.public.composite)
+        val timestamp: Timestamp? = null
+        val transaction: LedgerTransaction = LedgerTransaction(
+                inputs,
+                outputs,
+                commands,
+                attachments,
+                id,
+                DUMMY_NOTARY,
+                signers,
+                timestamp,
+                TransactionType.General()
+        )
+
+        assertFailsWith<TransactionVerificationException.DuplicateInputStates> { transaction.type.verify(transaction) }
+    }
+
     @Test
     fun `general transactions cannot change notary`() {
         val notary: Party = DUMMY_NOTARY

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

@@ -103,25 +103,6 @@ class NotaryServiceTests {
         assertEquals(f1.resultFuture.getOrThrow(), f2.resultFuture.getOrThrow())
     }
 
-    @Test fun `should report conflict when inputs are reused within a single transactions`() {
-        val stx = run {
-            val inputState = issueState(clientNode)
-            // Use inputState twice as input of a single transaction.
-            val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState, inputState)
-            tx.signWith(clientNode.keyPair!!)
-            tx.toSignedTransaction(false)
-        }
-
-        val future = clientNode.services.startFlow(NotaryFlow.Client(stx))
-
-        net.runNetwork()
-
-        val ex = assertFailsWith(NotaryException::class) { future.resultFuture.getOrThrow() }
-        val notaryError = ex.error as NotaryError.Conflict
-        assertEquals(notaryError.tx, stx.tx)
-        notaryError.conflict.verified()
-    }
-
     @Test fun `should report conflict when inputs are reused across transactions`() {
         val inputState = issueState(clientNode)
         val stx = run {