diff --git a/core/src/main/kotlin/net/corda/core/flows/CollectSignaturesFlow.kt b/core/src/main/kotlin/net/corda/core/flows/CollectSignaturesFlow.kt index b6c2b2e83d..609c710f35 100644 --- a/core/src/main/kotlin/net/corda/core/flows/CollectSignaturesFlow.kt +++ b/core/src/main/kotlin/net/corda/core/flows/CollectSignaturesFlow.kt @@ -8,6 +8,7 @@ import net.corda.core.identity.AbstractParty import net.corda.core.identity.AnonymousParty import net.corda.core.identity.Party import net.corda.core.identity.groupPublicKeysByWellKnownParty +import net.corda.core.internal.mapToSet import net.corda.core.node.ServiceHub import net.corda.core.transactions.SignedTransaction import net.corda.core.transactions.WireTransaction @@ -90,7 +91,7 @@ class CollectSignaturesFlow @JvmOverloads constructor(val partiallySignedTx: Sig // Check the signatures which have already been provided and that the transaction is valid. // Usually just the Initiator and possibly an oracle would have signed at this point. val myKeys: Iterable<PublicKey> = myOptionalKeys ?: listOf(ourIdentity.owningKey) - val signed = partiallySignedTx.sigs.map { it.by } + val signed = partiallySignedTx.sigs.mapToSet { it.by } val notSigned = partiallySignedTx.tx.requiredSigningKeys - signed // One of the signatures collected so far MUST be from the initiator of this flow. @@ -99,9 +100,7 @@ class CollectSignaturesFlow @JvmOverloads constructor(val partiallySignedTx: Sig } // The signatures must be valid and the transaction must be valid. - partiallySignedTx.verifySignaturesExcept(notSigned) - // TODO Should this be calling SignedTransaction.verify directly? https://r3-cev.atlassian.net/browse/ENT-11458 - partiallySignedTx.tx.toLedgerTransaction(serviceHub).verify() + partiallySignedTx.verify(serviceHub, checkSufficientSignatures = false) // Determine who still needs to sign. progressTracker.currentStep = COLLECTING @@ -286,10 +285,7 @@ abstract class SignTransactionFlow @JvmOverloads constructor(val otherSideSessio progressTracker.currentStep = VERIFYING // Check that the Responder actually needs to sign. checkMySignaturesRequired(stx, signingKeys) - // Check the signatures which have already been provided. Usually the Initiators and possibly an Oracle's. - checkSignatures(stx) - // TODO Should this be calling SignedTransaction.verify directly? https://r3-cev.atlassian.net/browse/ENT-11458 - stx.tx.toLedgerTransaction(serviceHub).verify() + stx.verify(serviceHub, checkSufficientSignatures = false) // Perform some custom verification over the transaction. try { checkTransaction(stx) @@ -310,14 +306,6 @@ abstract class SignTransactionFlow @JvmOverloads constructor(val otherSideSessio return stx + mySignatures } - @Suspendable - private fun checkSignatures(stx: SignedTransaction) { - val signed = stx.sigs.map { it.by } - val allSigners = stx.tx.requiredSigningKeys - val notSigned = allSigners - signed - stx.verifySignaturesExcept(notSigned) - } - /** * The [checkTransaction] method allows the caller of this flow to provide some additional checks over the proposed * transaction received from the counterparty. For example: diff --git a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt index ca38124ca9..d10b6b962d 100644 --- a/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt +++ b/core/src/main/kotlin/net/corda/core/serialization/internal/AttachmentsClassLoader.kt @@ -183,7 +183,10 @@ class AttachmentsClassLoader(attachments: List<Attachment>, @Suppress("ThrowsCount", "ComplexMethod", "NestedBlockDepth") private fun checkAttachments(attachments: List<Attachment>) { - require(attachments.isNotEmpty()) { "attachments list is empty" } + require(attachments.isNotEmpty()) { + "Transaction attachments list is empty. This can happen if verifying a legacy transaction (4.11 or older) with " + + "LedgerTransaction.verify(). Try using SignedTransaction.verify() instead." + } // Here is where we enforce the no-overlap and package ownership rules. // diff --git a/core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt b/core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt index 306e0f98b7..3a866562e4 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt @@ -240,6 +240,10 @@ private constructor( * The reason for this is that classes (contract states) deserialized in this classloader would actually be a different type from what * the calling code would expect. * + * If receiving [SignedTransaction]s over the wire from other nodes, then it is recommended the verification be done via + * [SignedTransaction.verify] directly rather than via [SignedTransaction.toLedgerTransaction]. If transaction is from a legacy node + * (4.11 or older) then the later solution will not work. + * * @throws TransactionVerificationException if anything goes wrong. */ @Throws(TransactionVerificationException::class) 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 63e9afb019..237c8c39d2 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt @@ -693,8 +693,8 @@ open class TransactionBuilder( @Throws(AttachmentResolutionException::class, TransactionResolutionException::class, TransactionVerificationException::class) fun verify(services: ServiceHub) { - // TODO ENT-11445: Need to verify via SignedTransaction to ensure legacy components also work - toLedgerTransaction(services).verify() + // Make sure the external verifier is involved if the transaction has a legacy component. + toWireTransaction(services).tryVerify(services.toVerifyingServiceHub()).enforceSuccess() } private fun checkNotary(stateAndRef: StateAndRef<*>) { diff --git a/core/src/main/kotlin/net/corda/core/transactions/WireTransaction.kt b/core/src/main/kotlin/net/corda/core/transactions/WireTransaction.kt index c561ea6ecb..79765cb6c6 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/WireTransaction.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/WireTransaction.kt @@ -375,6 +375,12 @@ class WireTransaction(componentGroups: List<ComponentGroup>, val privacySalt: Pr sig.verify(id) } + /** + * Perform verification of this [WireTransaction] and return the result as a [VerificationResult]. Depending on what types of attachments + * this transaction has, verification may have been done in-process by the node, or via the external verifier, or both. + * + * It's important that [VerificationResult.enforceSuccess] be called to make sure the verification was successful or its value analysed. + */ @CordaInternal @JvmSynthetic internal fun tryVerify(verificationSupport: NodeVerificationSupport): VerificationResult {