From 712b8b7719aa256eb1e7d17593b3f1589b76da9b Mon Sep 17 00:00:00 2001 From: Tudor Malene Date: Wed, 9 Oct 2019 11:43:06 +0100 Subject: [PATCH] CORDA-2965 Make Tx verification exceptions serializable. (#5560) --- ...VerificationExceptionSerialisationTests.kt | 133 +++++++++++++++++- .../TransactionVerificationException.kt | 85 +++++++---- detekt-baseline.xml | 4 +- detekt-config.yml | 25 +++- 4 files changed, 211 insertions(+), 36 deletions(-) diff --git a/core-tests/src/test/kotlin/net/corda/coretests/contracts/TransactionVerificationExceptionSerialisationTests.kt b/core-tests/src/test/kotlin/net/corda/coretests/contracts/TransactionVerificationExceptionSerialisationTests.kt index 03e2f5e731..e8bf2cf807 100644 --- a/core-tests/src/test/kotlin/net/corda/coretests/contracts/TransactionVerificationExceptionSerialisationTests.kt +++ b/core-tests/src/test/kotlin/net/corda/coretests/contracts/TransactionVerificationExceptionSerialisationTests.kt @@ -1,8 +1,13 @@ package net.corda.coretests.contracts +import net.corda.core.contracts.AlwaysAcceptAttachmentConstraint import net.corda.core.contracts.Contract +import net.corda.core.contracts.StateRef +import net.corda.core.contracts.TransactionState import net.corda.core.contracts.TransactionVerificationException import net.corda.core.crypto.SecureHash +import net.corda.core.crypto.generateKeyPair +import net.corda.core.identity.Party import net.corda.core.internal.createContractCreationError import net.corda.core.internal.createContractRejection import net.corda.core.transactions.LedgerTransaction @@ -13,6 +18,9 @@ import net.corda.serialization.internal.amqp.SerializationOutput import net.corda.serialization.internal.amqp.SerializerFactoryBuilder import net.corda.serialization.internal.amqp.custom.PublicKeySerializer import net.corda.serialization.internal.amqp.custom.ThrowableSerializer +import net.corda.testing.common.internal.testNetworkParameters +import net.corda.testing.core.ALICE_NAME +import net.corda.testing.core.BOB_NAME import net.corda.testing.core.DUMMY_BANK_A_NAME import net.corda.testing.core.DUMMY_NOTARY_NAME import net.corda.testing.core.TestIdentity @@ -23,7 +31,9 @@ class TransactionVerificationExceptionSerialisationTests { private fun defaultFactory() = SerializerFactoryBuilder.build( AllWhitelist, ClassLoader.getSystemClassLoader() - ).apply { register(ThrowableSerializer(this)) } + ).apply { + register(ThrowableSerializer(this)) + } private val context get() = AMQP_RPC_CLIENT_CONTEXT @@ -179,4 +189,125 @@ class TransactionVerificationExceptionSerialisationTests { assertEquals(exc.message, exc2.message) } + + @Test + fun transactionNetworkParameterOrderingExceptionTest() { + val exception = TransactionVerificationException.TransactionNetworkParameterOrderingException( + txid, + StateRef(SecureHash.zeroHash, 1), + testNetworkParameters(), + testNetworkParameters()) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun missingNetworkParametersExceptionTest() { + val exception = TransactionVerificationException.MissingNetworkParametersException(txid, SecureHash.zeroHash) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun constraintPropagationRejectionTest() { + val exception = TransactionVerificationException.ConstraintPropagationRejection(txid, "com.test.Contract", + AlwaysAcceptAttachmentConstraint, AlwaysAcceptAttachmentConstraint) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + assertEquals("com.test.Contract", exception2.contractClass) + } + + @Test + fun transactionDuplicateEncumbranceExceptionTest() { + val exception = TransactionVerificationException.TransactionDuplicateEncumbranceException(txid, 1) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun transactionNonMatchingEncumbranceExceptionTest() { + val exception = TransactionVerificationException.TransactionNonMatchingEncumbranceException(txid, listOf(1, 2, 3)) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun transactionNotaryMismatchEncumbranceExceptionTest() { + val exception = TransactionVerificationException.TransactionNotaryMismatchEncumbranceException( + txid, 1, 2, Party(ALICE_NAME, generateKeyPair().public), Party(BOB_NAME, generateKeyPair().public)) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun transactionContractConflictExceptionTest() { + val exception = TransactionVerificationException.TransactionContractConflictException( + txid, TransactionState(DummyContractState(), notary = Party(BOB_NAME, generateKeyPair().public)), "aa") + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } + + @Test + fun transactionRequiredContractUnspecifiedExceptionTest() { + val exception = TransactionVerificationException.TransactionRequiredContractUnspecifiedException( + txid, TransactionState(DummyContractState(), notary = Party(BOB_NAME, generateKeyPair().public))) + val exception2 = DeserializationInput(factory) + .deserialize( + SerializationOutput(factory) + .serialize(exception, context), + context) + + assertEquals(exception.message, exception2.message) + assertEquals(exception.cause?.message, exception2.cause?.message) + assertEquals(exception.txId, exception2.txId) + } } \ No newline at end of file diff --git a/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt b/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt index 84429f19bf..826a7d9eb4 100644 --- a/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt +++ b/core/src/main/kotlin/net/corda/core/contracts/TransactionVerificationException.kt @@ -70,8 +70,17 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S * @property outputConstraint The constraint of the outputs state. */ @KeepForDJVM - class ConstraintPropagationRejection(txId: SecureHash, val contractClass: String, inputConstraint: AttachmentConstraint, outputConstraint: AttachmentConstraint) - : TransactionVerificationException(txId, "Contract constraints for $contractClass are not propagated correctly. The outputConstraint: $outputConstraint is not a valid transition from the input constraint: $inputConstraint.", null) + class ConstraintPropagationRejection(txId: SecureHash, message: String) : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, + contractClass: String, + inputConstraint: AttachmentConstraint, + outputConstraint: AttachmentConstraint) : + this(txId, "Contract constraints for $contractClass are not propagated correctly. " + + "The outputConstraint: $outputConstraint is not a valid transition from the input constraint: $inputConstraint.") + + // This is only required for backwards compatibility. In case the message format changes, update the index. + val contractClass: String = message.split(" ")[3] + } /** * The transaction attachment that contains the [contractClass] class didn't meet the constraints specified by @@ -153,19 +162,24 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S * be satisfied. */ @KeepForDJVM - class TransactionDuplicateEncumbranceException(txId: SecureHash, index: Int) - : TransactionVerificationException(txId, "The bi-directionality property of encumbered output states " + - "is not satisfied. Index $index is referenced more than once", null) + class TransactionDuplicateEncumbranceException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, index: Int) : this(txId, "The bi-directionality property of encumbered output states " + + "is not satisfied. Index $index is referenced more than once") + } /** * An encumbered state should also be referenced as the encumbrance of another state in order to satisfy the * bi-directionality property (a full cycle should be present). */ @KeepForDJVM - class TransactionNonMatchingEncumbranceException(txId: SecureHash, nonMatching: Collection) - : TransactionVerificationException(txId, "The bi-directionality property of encumbered output states " + - "is not satisfied. Encumbered states should also be referenced as an encumbrance of another state to form " + - "a full cycle. Offending indices $nonMatching", null) + class TransactionNonMatchingEncumbranceException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, nonMatching: Collection) : this(txId, + "The bi-directionality property of encumbered output states " + + "is not satisfied. Encumbered states should also be referenced as an encumbrance of another state to form " + + "a full cycle. Offending indices $nonMatching") + } /** * All encumbered states should be assigned to the same notary. This is due to the fact that multi-notary @@ -173,9 +187,13 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S * in the same transaction. */ @KeepForDJVM - class TransactionNotaryMismatchEncumbranceException(txId: SecureHash, encumberedIndex: Int, encumbranceIndex: Int, encumberedNotary: Party, encumbranceNotary: Party) - : TransactionVerificationException(txId, "Encumbered output states assigned to different notaries found. " + - "Output state with index $encumberedIndex is assigned to notary [$encumberedNotary], while its encumbrance with index $encumbranceIndex is assigned to notary [$encumbranceNotary]", null) + class TransactionNotaryMismatchEncumbranceException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, encumberedIndex: Int, encumbranceIndex: Int, encumberedNotary: Party, encumbranceNotary: Party) : + this(txId, "Encumbered output states assigned to different notaries found. " + + "Output state with index $encumberedIndex is assigned to notary [$encumberedNotary], " + + "while its encumbrance with index $encumbranceIndex is assigned to notary [$encumbranceNotary]") + } /** * If a state is identified as belonging to a contract, either because the state class is defined as an inner class @@ -186,35 +204,44 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S * @param requiredContractClassName The class name of the contract to which the state belongs. */ @KeepForDJVM - class TransactionContractConflictException(txId: SecureHash, state: TransactionState, requiredContractClassName: String) - : TransactionVerificationException(txId, - """ - State of class ${state.data::class.java.typeName} belongs to contract $requiredContractClassName, but + class TransactionContractConflictException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, state: TransactionState, requiredContractClassName: String): this(txId, + """ + State of class ${state.data ::class.java.typeName} belongs to contract $requiredContractClassName, but is bundled in TransactionState with ${state.contract}. For details see: https://docs.corda.net/api-contract-constraints.html#contract-state-agreement - """.trimIndent().replace('\n', ' '), null) + """.trimIndent().replace('\n', ' ')) + } // TODO: add reference to documentation @KeepForDJVM - class TransactionRequiredContractUnspecifiedException(txId: SecureHash, state: TransactionState) - : TransactionVerificationException(txId, - """ + class TransactionRequiredContractUnspecifiedException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, state: TransactionState) : this(txId, + """ State of class ${state.data::class.java.typeName} does not have a specified owning contract. Add the @BelongsToContract annotation to this class to ensure that it can only be bundled in a TransactionState with the correct contract. For details see: https://docs.corda.net/api-contract-constraints.html#contract-state-agreement - """.trimIndent(), null) - + """.trimIndent()) + } /** * If the network parameters associated with an input or reference state in a transaction are more recent than the network parameters of the new transaction itself. */ @KeepForDJVM - class TransactionNetworkParameterOrderingException(txId: SecureHash, inputStateRef: StateRef, txnNetworkParameters: NetworkParameters, inputNetworkParameters: NetworkParameters) - : TransactionVerificationException(txId, "The network parameters epoch (${txnNetworkParameters.epoch}) of this transaction " + - "is older than the epoch (${inputNetworkParameters.epoch}) of input state: $inputStateRef", null) + class TransactionNetworkParameterOrderingException(txId: SecureHash, message: String) : + TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, + inputStateRef: StateRef, + txnNetworkParameters: NetworkParameters, + inputNetworkParameters: NetworkParameters) + : this(txId, "The network parameters epoch (${txnNetworkParameters.epoch}) of this transaction " + + "is older than the epoch (${inputNetworkParameters.epoch}) of input state: $inputStateRef") + } /** * Thrown when the network parameters with hash: missingNetworkParametersHash is not available at this node. Usually all the parameters @@ -224,9 +251,11 @@ abstract class TransactionVerificationException(val txId: SecureHash, message: S * @param missingNetworkParametersHash Missing hash of the network parameters associated to this transaction */ @KeepForDJVM - class MissingNetworkParametersException(txId: SecureHash, missingNetworkParametersHash: SecureHash) - : TransactionVerificationException(txId, "Couldn't find network parameters with hash: $missingNetworkParametersHash related to this transaction: $txId", null) - + class MissingNetworkParametersException(txId: SecureHash, message: String) + : TransactionVerificationException(txId, message, null) { + constructor(txId: SecureHash, missingNetworkParametersHash: SecureHash) : + this(txId, "Couldn't find network parameters with hash: $missingNetworkParametersHash related to this transaction: $txId") + } /** Whether the inputs or outputs list contains an encumbrance issue, see [TransactionMissingEncumbranceException]. */ @CordaSerializable diff --git a/detekt-baseline.xml b/detekt-baseline.xml index 740f8e1f80..5c90a6e66d 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -1226,6 +1226,7 @@ MagicNumber:TransactionBuilder.kt$TransactionBuilder$4 MagicNumber:TransactionDSLInterpreter.kt$TransactionDSL$30 MagicNumber:TransactionUtils.kt$4 + MagicNumber:TransactionVerificationException.kt$TransactionVerificationException.ConstraintPropagationRejection$3 MagicNumber:TransactionVerifierServiceInternal.kt$Verifier$4 MagicNumber:TransactionViewer.kt$TransactionViewer$15.0 MagicNumber:TransactionViewer.kt$TransactionViewer$20.0 @@ -3581,17 +3582,14 @@ MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$ContractCreationError : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$ContractRejection : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$InvalidAttachmentException : TransactionVerificationException - MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$MissingNetworkParametersException : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$NotaryChangeInWrongTransactionType : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$OverlappingAttachmentsException : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$PackageOwnershipException : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$SignersMissing : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$TransactionNetworkParameterOrderingException : TransactionVerificationException - MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException$TransactionNotaryMismatchEncumbranceException : TransactionVerificationException MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException.ContractCreationError$internal constructor(txId: SecureHash, contractClass: String, cause: Throwable) : this(txId, contractClass, cause, cause.message ?: "") MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException.ContractRejection$internal constructor(txId: SecureHash, contract: Contract, cause: Throwable) : this(txId, contract.javaClass.name, cause, cause.message ?: "") MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException.PackageOwnershipException$"""The attachment JAR: $attachmentHash containing the class: $invalidClassName is not signed by the owner of package $packageName specified in the network parameters. Please check the source of this attachment and if it is malicious contact your zone operator to report this incident. For details see: https://docs.corda.net/network-map.html#network-parameters""" - MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException.TransactionNotaryMismatchEncumbranceException$"Output state with index $encumberedIndex is assigned to notary [$encumberedNotary], while its encumbrance with index $encumbranceIndex is assigned to notary [$encumbranceNotary]" MaxLineLength:TransactionVerificationException.kt$TransactionVerificationException.UntrustedAttachmentsException$"Please follow the operational steps outlined in https://docs.corda.net/cordapp-build-systems.html#cordapp-contract-attachments to learn more and continue." MaxLineLength:TransactionVerificationException.kt$net.corda.core.contracts.TransactionVerificationException.kt MaxLineLength:TransactionVerificationRequest.kt$TransactionVerificationRequest$@Suppress("MemberVisibilityCanBePrivate") //TODO the use of deprecated toLedgerTransaction need to be revisited as resolveContractAttachment requires attachments of the transactions which created input states... //TODO ...to check contract version non downgrade rule, curretly dummy Attachment if not fund is used which sets contract version to '1' @CordaSerializable diff --git a/detekt-config.yml b/detekt-config.yml index 033fbdca4e..65bb613bd7 100644 --- a/detekt-config.yml +++ b/detekt-config.yml @@ -10,29 +10,33 @@ complexity: active: true ComplexCondition: active: true + excludes: "**/buildSrc/**" threshold: 4 ComplexMethod: active: true + excludes: "**/buildSrc/**" threshold: 10 ignoreSingleWhenExpression: true LargeClass: active: true - excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt" + excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt,**/buildSrc/**" threshold: 600 LongMethod: active: true - excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt" + excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt,**/buildSrc/**" threshold: 120 LongParameterList: active: true + excludes: "**/buildSrc/**" threshold: 6 ignoreDefaultParameters: false NestedBlockDepth: active: true + excludes: "**/buildSrc/**" threshold: 4 TooManyFunctions: active: true - excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt" + excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt,**/buildSrc/**" thresholdInFiles: 15 thresholdInClasses: 15 thresholdInInterfaces: 15 @@ -41,6 +45,7 @@ complexity: empty-blocks: active: true + excludes: "**/buildSrc/**" EmptyCatchBlock: active: true allowedExceptionNameRegex: "^(_|(ignore|expected).*)" @@ -71,6 +76,7 @@ empty-blocks: exceptions: active: true + excludes: "**/buildSrc/**" TooGenericExceptionCaught: active: true exceptionNames: @@ -92,6 +98,7 @@ exceptions: naming: active: true + excludes: "**/buildSrc/**" ClassNaming: active: true classPattern: '[A-Z$][a-zA-Z0-9$]*' @@ -127,6 +134,7 @@ naming: performance: active: true + excludes: "**/buildSrc/**" ForEachOnRange: active: true SpreadOperator: @@ -136,6 +144,7 @@ performance: potential-bugs: active: true + excludes: "**/buildSrc/**" DuplicateCaseInWhenExpression: active: true EqualsWithHashCodeExist: @@ -149,10 +158,11 @@ style: active: true ForbiddenComment: active: true + excludes: "**/buildSrc/**" values: 'TODO:,FIXME:,STOPSHIP:' MagicNumber: active: true - excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt" + excludes: "**/test/**,**/integration-test/**,**/integration-test-slow/**,**/*Test.kt,**/*Tests.kt,**/buildSrc/**" ignoreNumbers: '-1,0,1,2' ignoreHashCodeFunction: true ignorePropertyDeclaration: false @@ -163,23 +173,30 @@ style: ignoreEnums: false MaxLineLength: active: true + excludes: "**/buildSrc/**" maxLineLength: 140 excludePackageStatements: true excludeImportStatements: true ModifierOrder: active: true + excludes: "**/buildSrc/**" OptionalAbstractKeyword: active: true + excludes: "**/buildSrc/**" ReturnCount: active: true + excludes: "**/buildSrc/**" max: 2 excludedFunctions: "equals" excludeReturnFromLambda: true SafeCast: active: true + excludes: "**/buildSrc/**" ThrowsCount: active: true + excludes: "**/buildSrc/**" max: 2 WildcardImport: active: true + excludes: "**/buildSrc/**" excludeImports: 'java.util.*,kotlinx.android.synthetic.*' \ No newline at end of file