From 159ca9884fa43e8afc94aa10dffd0e10a619f18e Mon Sep 17 00:00:00 2001 From: Thomas Schroeter Date: Fri, 30 Dec 2016 21:23:25 +0000 Subject: [PATCH 1/5] Make NotaryFlow idempotent Alternatively, we could make the underlying UniquenessProviders idempotent. --- .../main/kotlin/net/corda/flows/NotaryFlow.kt | 15 +++++++-- .../corda/node/services/NotaryServiceTests.kt | 31 ++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt index 1fef4059d5..bb0fe49505 100644 --- a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt +++ b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt @@ -133,9 +133,18 @@ object NotaryFlow { try { uniquenessProvider.commit(tx.inputs, tx.id, otherSide) } catch (e: UniquenessException) { - val conflictData = e.error.serialize() - val signedConflict = SignedData(conflictData, sign(conflictData.bytes)) - throw NotaryException(NotaryError.Conflict(tx, signedConflict)) + // Allow re-committing the transaction to make the NotaryFlow idempotent. Alternatively, we could make + // the underlying UniquenessProviders idempotent. + val conflicts = tx.inputs.filterIndexed { i, stateRef -> + val consumingTx = e.error.stateHistory[stateRef] + consumingTx != null && consumingTx != UniquenessProvider.ConsumingTx(tx.id, i, otherSide) + } + if (conflicts.isNotEmpty()) { + // TODO: Create a new UniquenessException that only contains the conflicts filtered above. + val conflictData = e.error.serialize() + val signedConflict = SignedData(conflictData, sign(conflictData.bytes)) + throw NotaryException(NotaryError.Conflict(tx, signedConflict)) + } } } diff --git a/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt b/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt index 474402e172..92e3542e71 100644 --- a/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt @@ -85,7 +85,7 @@ class NotaryServiceTests { assertThat(ex.error).isInstanceOf(NotaryError.TimestampInvalid::class.java) } - @Test fun `should report conflict for a duplicate transaction`() { + @Test fun `should sign identical transaction multiple times (signing is idempotent)`() { val stx = run { val inputState = issueState(clientNode) val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState) @@ -93,8 +93,32 @@ class NotaryServiceTests { tx.toSignedTransaction(false) } + val firstAttempt = NotaryFlow.Client(stx) + val secondAttempt = NotaryFlow.Client(stx) + clientNode.services.startFlow(firstAttempt) + val future = clientNode.services.startFlow(secondAttempt) + + net.runNetwork() + + future.resultFuture.getOrThrow() + } + + @Test fun `should report conflict when inputs are reused accross transactions`() { + val inputState = issueState(clientNode) + val stx = run { + val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState) + tx.signWith(clientNode.keyPair!!) + tx.toSignedTransaction(false) + } + val stx2 = run { + val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState) + tx.addInputState(issueState(clientNode)) + tx.signWith(clientNode.keyPair!!) + tx.toSignedTransaction(false) + } + val firstSpend = NotaryFlow.Client(stx) - val secondSpend = NotaryFlow.Client(stx) + val secondSpend = NotaryFlow.Client(stx2) // Double spend the inputState in a second transaction. clientNode.services.startFlow(firstSpend) val future = clientNode.services.startFlow(secondSpend) @@ -102,11 +126,10 @@ class NotaryServiceTests { val ex = assertFailsWith(NotaryException::class) { future.resultFuture.getOrThrow() } val notaryError = ex.error as NotaryError.Conflict - assertEquals(notaryError.tx, stx.tx) + assertEquals(notaryError.tx, stx2.tx) notaryError.conflict.verified() } - private fun runNotaryClient(stx: SignedTransaction): ListenableFuture { val flow = NotaryFlow.Client(stx) val future = clientNode.services.startFlow(flow).resultFuture From bbc9c763e39164be192b7b32cc03173f4e76ef8f Mon Sep 17 00:00:00 2001 From: Thomas Schroeter Date: Wed, 4 Jan 2017 21:49:39 +0000 Subject: [PATCH 2/5] Detect duplicate inputs in NotaryFlow Throw NotaryException when duplicate inputs are detected. --- .../main/kotlin/net/corda/flows/NotaryFlow.kt | 32 ++++++++++++++++--- .../corda/node/services/NotaryServiceTests.kt | 27 +++++++++++++--- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt index bb0fe49505..24ca709e85 100644 --- a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt +++ b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt @@ -1,6 +1,7 @@ package net.corda.flows import co.paralleluniverse.fibers.Suspendable +import net.corda.core.contracts.StateRef import net.corda.core.crypto.* import net.corda.core.flows.FlowLogic import net.corda.core.node.services.TimestampChecker @@ -129,21 +130,36 @@ object NotaryFlow { open fun beforeCommit(stx: SignedTransaction) { } + /** Throw NotaryException if inputs are not unique. */ + private fun detectDuplicateInputs(tx: WireTransaction) { + var seenInputs = emptySet() + var conflicts = emptyMap() + 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 or input + * states are present multiple times within this transaction. + */ private fun commitInputStates(tx: WireTransaction) { + detectDuplicateInputs(tx) try { uniquenessProvider.commit(tx.inputs, tx.id, otherSide) } catch (e: UniquenessException) { - // Allow re-committing the transaction to make the NotaryFlow idempotent. Alternatively, we could make - // the underlying UniquenessProviders idempotent. val conflicts = tx.inputs.filterIndexed { i, stateRef -> val consumingTx = e.error.stateHistory[stateRef] consumingTx != null && consumingTx != UniquenessProvider.ConsumingTx(tx.id, i, otherSide) } if (conflicts.isNotEmpty()) { // TODO: Create a new UniquenessException that only contains the conflicts filtered above. - val conflictData = e.error.serialize() - val signedConflict = SignedData(conflictData, sign(conflictData.bytes)) - throw NotaryException(NotaryError.Conflict(tx, signedConflict)) + throw notaryException(tx, e) } } } @@ -152,6 +168,12 @@ object NotaryFlow { val mySigningKey = serviceHub.notaryIdentityKey return mySigningKey.signWithECDSA(bits) } + + private fun notaryException(tx: WireTransaction, e: UniquenessException): NotaryException { + val conflictData = e.error.serialize() + val signedConflict = SignedData(conflictData, sign(conflictData.bytes)) + return NotaryException(NotaryError.Conflict(tx, signedConflict)) + } } data class SignRequest(val tx: SignedTransaction) diff --git a/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt b/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt index 92e3542e71..40c695c7d7 100644 --- a/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/NotaryServiceTests.kt @@ -95,15 +95,34 @@ class NotaryServiceTests { val firstAttempt = NotaryFlow.Client(stx) val secondAttempt = NotaryFlow.Client(stx) - clientNode.services.startFlow(firstAttempt) - val future = clientNode.services.startFlow(secondAttempt) + val f1 = clientNode.services.startFlow(firstAttempt) + val f2 = clientNode.services.startFlow(secondAttempt) net.runNetwork() - future.resultFuture.getOrThrow() + assertEquals(f1.resultFuture.getOrThrow(), f2.resultFuture.getOrThrow()) } - @Test fun `should report conflict when inputs are reused accross transactions`() { + @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 { val tx = TransactionType.General.Builder(notaryNode.info.notaryIdentity).withItems(inputState) From 67e807b0755e8ddb73ce23e9e6d04f2a2b514cc9 Mon Sep 17 00:00:00 2001 From: Thomas Schroeter Date: Thu, 5 Jan 2017 08:46:56 +0000 Subject: [PATCH 3/5] Detect duplicate inputs ahead of calling beforeCommit --- core/src/main/kotlin/net/corda/flows/NotaryFlow.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt index 24ca709e85..9599a3c070 100644 --- a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt +++ b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt @@ -101,6 +101,7 @@ object NotaryFlow { val result = try { validateTimestamp(wtx) + detectDuplicateInputs(wtx) beforeCommit(stx) commitInputStates(wtx) val sig = sign(stx.id.bytes) @@ -145,11 +146,10 @@ object NotaryFlow { } } - /** A NotaryException is thrown if any of the states have been consumed by a different transaction or input - * states are present multiple times within this transaction. + /** 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. */ private fun commitInputStates(tx: WireTransaction) { - detectDuplicateInputs(tx) try { uniquenessProvider.commit(tx.inputs, tx.id, otherSide) } catch (e: UniquenessException) { From 0b43c5634ed82a48b35e72ef0f272ff61f87fbf2 Mon Sep 17 00:00:00 2001 From: Thomas Schroeter Date: Thu, 5 Jan 2017 19:33:46 +0000 Subject: [PATCH 4/5] Clean comment --- core/src/main/kotlin/net/corda/flows/NotaryFlow.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt index 9599a3c070..ec0f642dda 100644 --- a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt +++ b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt @@ -146,7 +146,8 @@ object NotaryFlow { } } - /** A NotaryException is thrown if any of the states have been consumed by a different transaction. Note that + /** + * 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. */ private fun commitInputStates(tx: WireTransaction) { From 0f43a88b19484840701d3d1be0c655bc04f157c3 Mon Sep 17 00:00:00 2001 From: Thomas Schroeter Date: Fri, 6 Jan 2017 09:09:27 +0000 Subject: [PATCH 5/5] Add TODO to move duplicate input detection to TransactionType.verify Resolving the TODO in a follow-up PR. --- core/src/main/kotlin/net/corda/flows/NotaryFlow.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt index ec0f642dda..52e6468215 100644 --- a/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt +++ b/core/src/main/kotlin/net/corda/flows/NotaryFlow.kt @@ -101,6 +101,7 @@ object NotaryFlow { val result = try { validateTimestamp(wtx) + // TODO: Move the duplicate input detection to `TransactionType.verify()`. detectDuplicateInputs(wtx) beforeCommit(stx) commitInputStates(wtx)