From 9955dcd6af1958074179f91489380e8f6335d4b8 Mon Sep 17 00:00:00 2001 From: Shams Asari Date: Thu, 21 Mar 2024 15:04:26 +0000 Subject: [PATCH] ENT-11448: Better error message if transaction has missing legacy attachments Especially if the transaction has multiple contracts and one of them doesn't have a legacy attachment whilst the others do. --- .../TransactionBuilderDriverTest.kt | 121 +++++++++++++----- .../core/transactions/TransactionBuilder.kt | 14 +- 2 files changed, 98 insertions(+), 37 deletions(-) diff --git a/core-tests/src/integration-test/kotlin/net/corda/coretests/transactions/TransactionBuilderDriverTest.kt b/core-tests/src/integration-test/kotlin/net/corda/coretests/transactions/TransactionBuilderDriverTest.kt index 818b511919..265bc70a6e 100644 --- a/core-tests/src/integration-test/kotlin/net/corda/coretests/transactions/TransactionBuilderDriverTest.kt +++ b/core-tests/src/integration-test/kotlin/net/corda/coretests/transactions/TransactionBuilderDriverTest.kt @@ -1,17 +1,26 @@ package net.corda.coretests.transactions +import co.paralleluniverse.fibers.Suspendable +import net.corda.core.contracts.TransactionState +import net.corda.core.flows.FlowLogic +import net.corda.core.flows.StartableByRPC import net.corda.core.internal.copyToDirectory import net.corda.core.internal.hash +import net.corda.core.internal.mapToSet import net.corda.core.internal.toPath import net.corda.core.messaging.startFlow import net.corda.core.transactions.SignedTransaction +import net.corda.core.transactions.TransactionBuilder import net.corda.core.utilities.OpaqueBytes import net.corda.core.utilities.getOrThrow import net.corda.coretesting.internal.delete import net.corda.coretesting.internal.modifyJarManifest import net.corda.coretesting.internal.useZipFile import net.corda.finance.DOLLARS +import net.corda.finance.contracts.CommercialPaper +import net.corda.finance.contracts.asset.Cash import net.corda.finance.flows.CashIssueAndPaymentFlow +import net.corda.finance.issuedBy import net.corda.testing.common.internal.testNetworkParameters import net.corda.testing.core.ALICE_NAME import net.corda.testing.core.internal.JarSignatureTestUtils.generateKey @@ -21,6 +30,7 @@ import net.corda.testing.driver.NodeHandle import net.corda.testing.driver.NodeParameters import net.corda.testing.node.internal.DriverDSLImpl import net.corda.testing.node.internal.FINANCE_WORKFLOWS_CORDAPP +import net.corda.testing.node.internal.enclosedCordapp import net.corda.testing.node.internal.internalDriver import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy @@ -28,14 +38,18 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder +import java.nio.file.Files import java.nio.file.Path +import java.time.Duration +import java.time.Instant import kotlin.io.path.Path import kotlin.io.path.absolutePathString import kotlin.io.path.copyTo import kotlin.io.path.createDirectories -import kotlin.io.path.deleteExisting import kotlin.io.path.div import kotlin.io.path.inputStream +import kotlin.io.path.isRegularFile +import kotlin.io.path.moveTo class TransactionBuilderDriverTest { companion object { @@ -58,8 +72,9 @@ class TransactionBuilderDriverTest { @Test(timeout=300_000) fun `adds CorDapp dependencies`() { - val (cordapp, dependency) = splitFinanceContractCordapp(currentFinanceContractsJar) internalDriver(cordappsForAllNodes = listOf(FINANCE_WORKFLOWS_CORDAPP), startNodesInProcess = false) { + val (cordapp, dependency) = splitFinanceContractCordapp(currentFinanceContractsJar) + cordapp.inputStream().use(defaultNotaryNode.getOrThrow().rpc::uploadAttachment) dependency.inputStream().use(defaultNotaryNode.getOrThrow().rpc::uploadAttachment) @@ -70,7 +85,7 @@ class TransactionBuilderDriverTest { // First make sure the missing dependency causes an issue assertThatThrownBy { createTransaction(node) - }.hasMessageContaining("java.lang.NoClassDefFoundError: net/corda/finance/contracts/asset") + }.hasMessageContaining("Transaction being built has a missing attachment for class net/corda/finance/contracts/asset/") // Upload the missing dependency dependency.inputStream().use(node.rpc::uploadAttachment) @@ -82,18 +97,17 @@ class TransactionBuilderDriverTest { @Test(timeout=300_000) fun `adds legacy contracts CorDapp dependencies`() { - val (legacyContracts, legacyDependency) = splitFinanceContractCordapp(legacyFinanceContractsJar) - - // Re-sign the current finance contracts CorDapp with the same key as the split legacy CorDapp - val currentContracts = currentFinanceContractsJar.copyTo(Path("${currentFinanceContractsJar.toString().substringBeforeLast(".")}-RESIGNED.jar"), overwrite = true) - currentContracts.unsignJar() - signJar(currentContracts) - internalDriver( cordappsForAllNodes = listOf(FINANCE_WORKFLOWS_CORDAPP), startNodesInProcess = false, networkParameters = testNetworkParameters(minimumPlatformVersion = 4) ) { + val (legacyContracts, legacyDependency) = splitFinanceContractCordapp(legacyFinanceContractsJar) + // Re-sign the current finance contracts CorDapp with the same key as the split legacy CorDapp + val currentContracts = currentFinanceContractsJar.copyTo(Path("${currentFinanceContractsJar.toString().substringBeforeLast(".")}-RESIGNED.jar"), overwrite = true) + currentContracts.unsignJar() + signJar(currentContracts) + currentContracts.inputStream().use(defaultNotaryNode.getOrThrow().rpc::uploadAttachment) // Start the node with the legacy CorDapp but without the dependency @@ -104,7 +118,7 @@ class TransactionBuilderDriverTest { // First make sure the missing dependency causes an issue assertThatThrownBy { createTransaction(node) - }.hasMessageContaining("java.lang.NoClassDefFoundError: net/corda/finance/contracts/asset") + }.hasMessageContaining("Transaction being built has a missing legacy attachment for class net/corda/finance/contracts/asset/") // Upload the missing dependency legacyDependency.inputStream().use(node.rpc::uploadAttachment) @@ -114,36 +128,63 @@ class TransactionBuilderDriverTest { } } + @Test(timeout=300_000) + fun `prevents transaction which is multi-contract but not backwards compatible because one of the contracts has missing legacy attachment`() { + internalDriver( + cordappsForAllNodes = listOf(FINANCE_WORKFLOWS_CORDAPP, enclosedCordapp()), + startNodesInProcess = false, + networkParameters = testNetworkParameters(minimumPlatformVersion = 4), + isDebug = true + ) { + val (currentCashContract, currentCpContract) = splitJar(currentFinanceContractsJar) { "CommercialPaper" in it.absolutePathString() } + val (legacyCashContract, _) = splitJar(legacyFinanceContractsJar) { "CommercialPaper" in it.absolutePathString() } + + currentCashContract.inputStream().use(defaultNotaryNode.getOrThrow().rpc::uploadAttachment) + currentCpContract.inputStream().use(defaultNotaryNode.getOrThrow().rpc::uploadAttachment) + + // The node has the legacy CommericalPaper contract missing + val cordappsDir = (baseDirectory(ALICE_NAME) / "cordapps").createDirectories() + currentCashContract.copyToDirectory(cordappsDir) + currentCpContract.copyToDirectory(cordappsDir) + legacyCashContract.copyToDirectory((baseDirectory(ALICE_NAME) / "legacy-contracts").createDirectories()) + + val node = startNode(NodeParameters(ALICE_NAME)).getOrThrow() + assertThatThrownBy { node.rpc.startFlow(::TwoContractTransactionFlow).returnValue.getOrThrow() } + .hasMessageContaining("Transaction being built has a missing legacy attachment") + .hasMessageContaining("CommercialPaper") + } + } + /** * Split the given finance contracts jar into two such that the second jar becomes a dependency to the first. */ - private fun splitFinanceContractCordapp(contractsJar: Path): Pair { - val cordapp = tempFolder.newFile("cordapp.jar").toPath() - val dependency = tempFolder.newFile("cordapp-dep.jar").toPath() + private fun DriverDSLImpl.splitFinanceContractCordapp(contractsJar: Path): Pair { + return splitJar(contractsJar) { it.absolutePathString() == "/net/corda/finance/contracts/asset/CashUtilities.class" } + } - // Split the CorDapp into two - contractsJar.copyTo(cordapp, overwrite = true) - cordapp.useZipFile { cordappZipFs -> - dependency.useZipFile { depZipFs -> - val targetDir = depZipFs.getPath("net/corda/finance/contracts/asset").createDirectories() - // CashUtilities happens to be a class that is only invoked in Cash.verify and so it's absence is only detected during - // verification - val clazz = cordappZipFs.getPath("net/corda/finance/contracts/asset/CashUtilities.class") - clazz.copyToDirectory(targetDir) - clazz.deleteExisting() + private fun DriverDSLImpl.splitJar(path: Path, move: (Path) -> Boolean): Pair { + val jar1 = Files.createTempFile(driverDirectory, "jar1-", ".jar") + val jar2 = Files.createTempFile(driverDirectory, "jar2-", ".jar") + + path.copyTo(jar1, overwrite = true) + jar1.useZipFile { zipFs1 -> + jar2.useZipFile { zipFs2 -> + Files.walk(zipFs1.getPath("/")).filter { it.isRegularFile() && move(it) }.forEach { file -> + val target = zipFs2.getPath(file.absolutePathString()) + target.parent?.createDirectories() + file.moveTo(target) + } } } - cordapp.modifyJarManifest { manifest -> + jar1.modifyJarManifest { manifest -> manifest.mainAttributes.delete("Sealed") } - cordapp.unsignJar() + jar1.unsignJar() - // Sign both current and legacy CorDapps with the same key - signJar(cordapp) - // The dependency needs to be signed as it contains a package from the main jar - signJar(dependency) + signJar(jar1) + signJar(jar2) - return Pair(cordapp, dependency) + return Pair(jar1, jar2) } private fun DriverDSLImpl.createTransaction(node: NodeHandle): SignedTransaction { @@ -156,4 +197,22 @@ class TransactionBuilderDriverTest { defaultNotaryIdentity ).returnValue.getOrThrow().stx } + + + @StartableByRPC + class TwoContractTransactionFlow : FlowLogic() { + @Suspendable + override fun call() { + val notary = serviceHub.networkMapCache.notaryIdentities[0] + val builder = TransactionBuilder(notary) + val issuer = ourIdentity.ref(OpaqueBytes.of(0x00)) + val amount = 1.DOLLARS.issuedBy(issuer) + val signers = Cash().generateIssue(builder, amount, ourIdentity, notary) + builder.addOutputState(TransactionState(CommercialPaper.State(issuer, ourIdentity, amount, Instant.MAX), notary = notary)) + builder.addCommand(CommercialPaper.Commands.Issue(), signers.first()) + builder.setTimeWindow(Instant.now(), Duration.ofMinutes(1)) + require(builder.outputStates().mapToSet { it.contract }.size > 1) + serviceHub.signInitialTransaction(builder, signers) + } + } } 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 b86fd6b0ee..63e9afb019 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt @@ -27,6 +27,7 @@ import net.corda.core.serialization.SerializationSchemeContext import net.corda.core.serialization.internal.CustomSerializationSchemeUtils.Companion.getCustomSerializationMagicFromSchemeId import net.corda.core.utilities.Try.Failure import net.corda.core.utilities.contextLogger +import net.corda.core.utilities.debug import java.security.PublicKey import java.time.Duration import java.time.Instant @@ -241,14 +242,17 @@ open class TransactionBuilder( * @return true if a new dependency was successfully added. */ private fun addMissingDependency(serviceHub: VerifyingServiceHub, wireTx: WireTransaction, tryCount: Int): Boolean { + log.debug { "Checking if there are any missing attachment dependencies for transaction ${wireTx.id}..." } val verificationResult = wireTx.tryVerify(serviceHub) // Check both legacy and non-legacy components are working, and try to add any missing dependencies if either are not. (verificationResult.inProcessResult as? Failure)?.let { (inProcessException) -> return addMissingDependency(inProcessException, wireTx, false, serviceHub, tryCount) } + log.debug("Non-legacy portion of transaction does not have any missing attachments, checking legacy portion...") (verificationResult.externalResult as? Failure)?.let { (externalException) -> return addMissingDependency(externalException, wireTx, true, serviceHub, tryCount) } + log.debug("Legacy portion of transaction also does not have any missing attachments") // The transaction verified successfully without needing any extra dependency. return false } @@ -256,7 +260,7 @@ open class TransactionBuilder( private fun addMissingDependency(e: Throwable, wireTx: WireTransaction, isLegacy: Boolean, serviceHub: VerifyingServiceHub, tryCount: Int): Boolean { val missingClass = extractMissingClass(e) if (log.isDebugEnabled) { - log.debug("Checking if transaction has missing attachment (missingClass=$missingClass) (legacy=$isLegacy) $wireTx", e) + log.debug("${if (isLegacy) "Legacy" else "Non-legacy"} portion of transaction has missing dependency (missingClass=$missingClass) $wireTx", e) } return when { missingClass != null -> { @@ -353,11 +357,9 @@ open class TransactionBuilder( } if (attachment == null) { - log.error("""The transaction currently built is missing an attachment for class: $missingClass. - Attempted to find a suitable attachment but could not find any in the storage. - Please contact the developer of the CorDapp for further instructions. - """.trimIndent()) - throw originalException + throw IllegalStateException("Transaction being built has a missing ${if (isLegacy) "legacy " else ""}attachment for class " + + "$missingClass. Could not find a suitable attachment from storage. Please contact the developer of the CorDapp for " + + "further instructions.", originalException) } log.warnOnce("""The transaction currently built is missing an attachment for class: $missingClass.