From 9396ee95512a51e7def962121c58d40432c157d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Lepr=C3=AAtre?= Date: Sat, 25 Apr 2020 22:50:09 +0400 Subject: [PATCH 1/2] Make TransactionBuilder's input references deep copied on copy. --- .../transactions/TransactionBuilderTest.kt | 100 ++++++++++++++++++ .../core/transactions/TransactionBuilder.kt | 12 +-- 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt b/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt index 55fb43f003..af20bbba0f 100644 --- a/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt +++ b/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt @@ -29,6 +29,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import java.security.PublicKey +import java.time.Instant class TransactionBuilderTest { @Rule @@ -150,4 +151,103 @@ class TransactionBuilderTest { override val signerKeys: List get() = parties.map { it.owningKey } }, DummyContract.PROGRAM_ID, signerKeys = parties.map { it.owningKey }) + + @Test(timeout=300_000) + fun `list accessors are mutable copies`() { + val inputState1 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val inputStateRef1 = StateRef(SecureHash.randomSHA256(), 0) + val referenceState1 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val referenceStateRef1 = StateRef(SecureHash.randomSHA256(), 1) + val timeWindow = TimeWindow.untilOnly(Instant.now()) + val builder = TransactionBuilder(notary) + .addInputState(StateAndRef(inputState1, inputStateRef1)) + .addAttachment(SecureHash.allOnesHash) + .addOutputState(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) + .addCommand(DummyCommandData, notary.owningKey) + .addReferenceState(StateAndRef(referenceState1, referenceStateRef1).referenced()) + val inputState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val inputStateRef2 = StateRef(SecureHash.randomSHA256(), 0) + val referenceState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val referenceStateRef2 = StateRef(SecureHash.randomSHA256(), 1) + + // List accessors are mutable. + (builder.inputStates() as ArrayList).add(inputStateRef2) + (builder.attachments() as ArrayList).add(SecureHash.zeroHash) + (builder.outputStates() as ArrayList).add(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) + (builder.commands() as ArrayList).add(Command(DummyCommandData, notary.owningKey)) + (builder.referenceStates() as ArrayList).add(referenceStateRef2) + + // List accessors are copies. + assertThat(builder.inputStates()).hasSize(1) + assertThat(builder.attachments()).hasSize(1) + assertThat(builder.outputStates()).hasSize(1) + assertThat(builder.commands()).hasSize(1) + assertThat(builder.referenceStates()).hasSize(1) + } + + @Test(timeout=300_000) + fun `copy makes copy except lockId`() { + val inputState = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val inputStateRef = StateRef(SecureHash.randomSHA256(), 0) + val referenceState = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val referenceStateRef = StateRef(SecureHash.randomSHA256(), 1) + val timeWindow = TimeWindow.untilOnly(Instant.now()) + val builder = TransactionBuilder(notary) + .addInputState(StateAndRef(inputState, inputStateRef)) + .addAttachment(SecureHash.allOnesHash) + .addOutputState(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) + .addCommand(DummyCommandData, notary.owningKey) + .setTimeWindow(timeWindow) + .setPrivacySalt(PrivacySalt()) + .addReferenceState(StateAndRef(referenceState, referenceStateRef).referenced()) + val copy = builder.copy() + + assertThat(builder.notary).isEqualTo(copy.notary) + assertThat(builder.lockId).isNotEqualTo(copy.lockId) + assertThat(builder.inputStates()).isEqualTo(copy.inputStates()) + assertThat(builder.attachments()).isEqualTo(copy.attachments()) + assertThat(builder.outputStates()).isEqualTo(copy.outputStates()) + assertThat(builder.commands()).isEqualTo(copy.commands()) +// assertThat(builder.timeWindow()).isEqualTo(copy.timeWindow()) +// assertThat(builder.privacySalt()).isEqualTo(copy.privacySalt()) + assertThat(builder.referenceStates()).isEqualTo(copy.referenceStates()) + } + + @Test(timeout=300_000) + fun `copy makes deep copy of lists`() { + val inputState1 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val inputStateRef1 = StateRef(SecureHash.randomSHA256(), 0) + val referenceState1 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val referenceStateRef1 = StateRef(SecureHash.randomSHA256(), 1) + val builder = TransactionBuilder(notary) + .addInputState(StateAndRef(inputState1, inputStateRef1)) + .addAttachment(SecureHash.allOnesHash) + .addOutputState(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) + .addCommand(DummyCommandData, notary.owningKey) + .addReferenceState(StateAndRef(referenceState1, referenceStateRef1).referenced()) + val inputState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val inputStateRef2 = StateRef(SecureHash.randomSHA256(), 0) + val referenceState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) + val referenceStateRef2 = StateRef(SecureHash.randomSHA256(), 1) + val copy = builder.copy() + .addInputState(StateAndRef(inputState2, inputStateRef2)) + .addAttachment(SecureHash.zeroHash) + .addOutputState(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) + .addCommand(DummyCommandData, notary.owningKey) + .addReferenceState(StateAndRef(referenceState2, referenceStateRef2).referenced()) + + // Lists on the copy are longer + assertThat(copy.inputStates()).hasSize(2) + assertThat(copy.attachments()).hasSize(2) + assertThat(copy.outputStates()).hasSize(2) + assertThat(copy.commands()).hasSize(2) + assertThat(copy.referenceStates()).hasSize(2) + + // Lists on the original are unchanged + assertThat(builder.inputStates()).hasSize(1) + assertThat(builder.attachments()).hasSize(1) + assertThat(builder.outputStates()).hasSize(1) + assertThat(builder.commands()).hasSize(1) + assertThat(builder.referenceStates()).hasSize(1) + } } diff --git a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt index d7d95db642..0be1c1acdf 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt @@ -100,7 +100,7 @@ open class TransactionBuilder( commands = ArrayList(commands), window = window, privacySalt = privacySalt, - references = references, + references = ArrayList(references), serviceHub = serviceHub ) t.inputsWithTransactionState.addAll(this.inputsWithTransactionState) @@ -813,19 +813,19 @@ open class TransactionBuilder( this.privacySalt = privacySalt } - /** Returns an immutable list of input [StateRef]s. */ + /** Returns a mutable copy of the list of input [StateRef]s. */ fun inputStates(): List = ArrayList(inputs) - /** Returns an immutable list of reference input [StateRef]s. */ + /** Returns a mutable copy of the list of reference input [StateRef]s. */ fun referenceStates(): List = ArrayList(references) - /** Returns an immutable list of attachment hashes. */ + /** Returns a mutable copy of the list of attachment hashes. */ fun attachments(): List = ArrayList(attachments) - /** Returns an immutable list of output [TransactionState]s. */ + /** Returns a mutable copy of the list of output [TransactionState]s. */ fun outputStates(): List> = ArrayList(outputs) - /** Returns an immutable list of [Command]s. */ + /** Returns a mutable copy of the list of [Command]s. */ fun commands(): List> = ArrayList(commands) /** From c4fd63ced62df208bb5957bdd47141fd11980cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Xavier=20Lepr=C3=AAtre?= Date: Thu, 30 Apr 2020 18:46:14 +0400 Subject: [PATCH 2/2] Reverted comment change about immutable accessors. --- .../transactions/TransactionBuilderTest.kt | 13 +++++-------- .../corda/core/transactions/TransactionBuilder.kt | 10 +++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt b/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt index af20bbba0f..083d91753f 100644 --- a/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt +++ b/core-tests/src/test/kotlin/net/corda/coretests/transactions/TransactionBuilderTest.kt @@ -158,24 +158,21 @@ class TransactionBuilderTest { val inputStateRef1 = StateRef(SecureHash.randomSHA256(), 0) val referenceState1 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) val referenceStateRef1 = StateRef(SecureHash.randomSHA256(), 1) - val timeWindow = TimeWindow.untilOnly(Instant.now()) val builder = TransactionBuilder(notary) .addInputState(StateAndRef(inputState1, inputStateRef1)) .addAttachment(SecureHash.allOnesHash) .addOutputState(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) .addCommand(DummyCommandData, notary.owningKey) .addReferenceState(StateAndRef(referenceState1, referenceStateRef1).referenced()) - val inputState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) val inputStateRef2 = StateRef(SecureHash.randomSHA256(), 0) - val referenceState2 = TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary) val referenceStateRef2 = StateRef(SecureHash.randomSHA256(), 1) // List accessors are mutable. - (builder.inputStates() as ArrayList).add(inputStateRef2) - (builder.attachments() as ArrayList).add(SecureHash.zeroHash) - (builder.outputStates() as ArrayList).add(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) - (builder.commands() as ArrayList).add(Command(DummyCommandData, notary.owningKey)) - (builder.referenceStates() as ArrayList).add(referenceStateRef2) + assertThat((builder.inputStates() as ArrayList).also { it.add(inputStateRef2) }).hasSize(2) + assertThat((builder.attachments() as ArrayList).also { it.add(SecureHash.zeroHash) }).hasSize(2) + assertThat((builder.outputStates() as ArrayList).also { it.add(TransactionState(DummyState(), DummyContract.PROGRAM_ID, notary)) }).hasSize(2) + assertThat((builder.commands() as ArrayList).also { it.add(Command(DummyCommandData, notary.owningKey)) }).hasSize(2) + assertThat((builder.referenceStates() as ArrayList).also { it.add(referenceStateRef2) }).hasSize(2) // List accessors are copies. assertThat(builder.inputStates()).hasSize(1) diff --git a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt index 0be1c1acdf..6d41464edf 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt @@ -813,19 +813,19 @@ open class TransactionBuilder( this.privacySalt = privacySalt } - /** Returns a mutable copy of the list of input [StateRef]s. */ + /** Returns an immutable list of input [StateRef]s. */ fun inputStates(): List = ArrayList(inputs) - /** Returns a mutable copy of the list of reference input [StateRef]s. */ + /** Returns an immutable list of reference input [StateRef]s. */ fun referenceStates(): List = ArrayList(references) - /** Returns a mutable copy of the list of attachment hashes. */ + /** Returns an immutable list of attachment hashes. */ fun attachments(): List = ArrayList(attachments) - /** Returns a mutable copy of the list of output [TransactionState]s. */ + /** Returns an immutable list of output [TransactionState]s. */ fun outputStates(): List> = ArrayList(outputs) - /** Returns a mutable copy of the list of [Command]s. */ + /** Returns an immutable list of [Command]s. */ fun commands(): List> = ArrayList(commands) /**