CORDA-2526 Allow for duplicate signed attachments in devMode (#4691)

* Allow for duplicate (contract class, version) signed attachments in devMode.

* Code clean-up.

* Fix compilation error in test code.

* Additional gating/warning and added Unit test for development mode behaviour.

node-api/src/test/kotlin/net/corda/nodeapi/internal/AttachmentsClassLoaderStaticContractTests.kt

@@ -76,7 +76,7 @@ class AttachmentsClassLoaderStaticContractTests {
 
     private val serviceHub get() = rigorousMock<ServicesForResolution>().also {
         val cordappProviderImpl = CordappProviderImpl(cordappLoaderForPackages(listOf("net.corda.nodeapi.internal")), MockCordappConfigProvider(), MockAttachmentStorage())
-        cordappProviderImpl.start(testNetworkParameters().whitelistedContractImplementations)
+        cordappProviderImpl.start()
         doReturn(cordappProviderImpl).whenever(it).cordappProvider
         doReturn(networkParametersService).whenever(it).networkParametersService
         doReturn(networkParameters).whenever(it).networkParameters

node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt

@@ -170,7 +170,7 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
     @Suppress("LeakingThis")
     val transactionStorage = makeTransactionStorage(configuration.transactionCacheSizeBytes).tokenize()
     val networkMapClient: NetworkMapClient? = configuration.networkServices?.let { NetworkMapClient(it.networkMapURL, versionInfo) }
-    val attachments = NodeAttachmentService(metricRegistry, cacheFactory, database).tokenize()
+    val attachments = NodeAttachmentService(metricRegistry, cacheFactory, database, configuration.devMode).tokenize()
     val cryptoService = configuration.makeCryptoService()
     @Suppress("LeakingThis")
     val networkParametersStorage = makeNetworkParametersStorage()
@@ -371,7 +371,7 @@ abstract class AbstractNode<S>(val configuration: NodeConfiguration,
             networkParametersStorage.setCurrentParameters(signedNetParams, trustRoot)
             identityService.loadIdentities(nodeInfo.legalIdentitiesAndCerts)
             attachments.start()
-            cordappProvider.start(netParams.whitelistedContractImplementations)
+            cordappProvider.start()
             nodeProperties.start()
             // Place the long term identity key in the KMS. Eventually, this is likely going to be separated again because
             // the KMS is meant for derived temporary keys used in transactions, and we're not supposed to sign things with

node/src/main/kotlin/net/corda/node/internal/cordapp/CordappProviderImpl.kt

@@ -35,7 +35,7 @@ open class CordappProviderImpl(val cordappLoader: CordappLoader,
      */
     override val cordapps: List<CordappImpl> get() = cordappLoader.cordapps
 
-    fun start(whitelistedContractImplementations: Map<String, List<AttachmentId>>) {
+    fun start() {
         cordappAttachments.putAll(loadContractsIntoAttachmentStore())
         verifyInstalledCordapps()
     }

node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt

@@ -52,8 +52,13 @@ import javax.persistence.*
 class NodeAttachmentService(
         metrics: MetricRegistry,
         cacheFactory: NamedCacheFactory,
-        private val database: CordaPersistence
+        private val database: CordaPersistence,
+        val devMode: Boolean
 ) : AttachmentStorageInternal, SingletonSerializeAsToken() {
+    constructor(metrics: MetricRegistry,
+                cacheFactory: NamedCacheFactory,
+                database: CordaPersistence) : this(metrics, cacheFactory, database, false)
+
     // This is to break the circular dependency.
     lateinit var servicesForResolution: ServicesForResolution
 
@@ -356,9 +361,8 @@ class NodeAttachmentService(
                     val jarSigners = getSigners(bytes)
                     val contractVersion = increaseDefaultVersionIfWhitelistedAttachment(contractClassNames, getVersion(bytes), id)
                     val session = currentDBSession()
-
+                    if (!devMode)
-                    verifyVersionUniquenessForSignedAttachments(contractClassNames, contractVersion, jarSigners)
+                        verifyVersionUniquenessForSignedAttachments(contractClassNames, contractVersion, jarSigners)
-
                     val attachment = NodeAttachmentService.DBAttachment(
                             attId = id.toString(),
                             content = bytes,
@@ -379,7 +383,8 @@ class NodeAttachmentService(
                     val attachment = session.get(NodeAttachmentService.DBAttachment::class.java, id.toString())
                     // update the `uploader` field (as the existing attachment may have been resolved from a peer)
                     if (attachment.uploader != uploader) {
-                        verifyVersionUniquenessForSignedAttachments(contractClassNames, attachment.version, attachment.signers)
+                        if (!devMode)
+                            verifyVersionUniquenessForSignedAttachments(contractClassNames, attachment.version, attachment.signers)
                         attachment.uploader = uploader
                         log.info("Updated attachment $id with uploader $uploader")
                         contractClassNames.forEach { contractsCache.invalidate(it) }
@@ -489,11 +494,14 @@ class NodeAttachmentService(
 
     private fun makeAttachmentIds(it: Map.Entry<Int, List<DBAttachment>>, contractClassName: String): Pair<Version, AttachmentIds> {
         val signed = it.value.filter { it.signers?.isNotEmpty() ?: false }.map { AttachmentId.parse(it.attId) }
-        check (signed.size <= 1) //sanity check
+        if (!devMode)
+            check (signed.size <= 1) //sanity check
+        else
+            log.warn("(Dev Mode) Multiple signed attachments ${signed.map { it.toString() }} for contract $contractClassName version '${it.key}'.")
         val unsigned = it.value.filter { it.signers?.isEmpty() ?: true }.map { AttachmentId.parse(it.attId) }
         if (unsigned.size > 1)
             log.warn("Selecting attachment ${unsigned.first()} from duplicated, unsigned attachments ${unsigned.map { it.toString() }} for contract $contractClassName version '${it.key}'.")
-        return it.key to AttachmentIds(signed.singleOrNull(), unsigned.firstOrNull())
+        return it.key to AttachmentIds(signed.firstOrNull(), unsigned.firstOrNull())
     }
 
     override fun getLatestContractAttachments(contractClassName: String, minContractVersion: Int): List<AttachmentId> {

node/src/test/kotlin/net/corda/node/internal/cordapp/CordappProviderImplTests.kt

@@ -77,7 +77,7 @@ class CordappProviderImplTests {
         val configProvider = MockCordappConfigProvider()
         configProvider.cordappConfigs[isolatedCordappName] = validConfig
         val loader = JarScanningCordappLoader.fromJarUrls(listOf(isolatedJAR), VersionInfo.UNKNOWN)
-        val provider = CordappProviderImpl(loader, configProvider, attachmentStore).apply { start(whitelistedContractImplementations) }
+        val provider = CordappProviderImpl(loader, configProvider, attachmentStore).apply { start() }
 
         val expected = provider.getAppContext(provider.cordapps.first()).config
 
@@ -86,6 +86,6 @@ class CordappProviderImplTests {
 
     private fun newCordappProvider(vararg urls: URL): CordappProviderImpl {
         val loader = JarScanningCordappLoader.fromJarUrls(urls.toList(), VersionInfo.UNKNOWN)
-        return CordappProviderImpl(loader, stubConfigProvider, attachmentStore).apply { start(whitelistedContractImplementations) }
+        return CordappProviderImpl(loader, stubConfigProvider, attachmentStore).apply { start() }
     }
 }

node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt

@@ -44,6 +44,7 @@ import java.nio.charset.StandardCharsets
 import java.nio.file.FileAlreadyExistsException
 import java.nio.file.FileSystem
 import java.nio.file.Path
+import java.util.*
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 import kotlin.test.assertNotEquals
@@ -55,6 +56,7 @@ class NodeAttachmentServiceTest {
     private lateinit var fs: FileSystem
     private lateinit var database: CordaPersistence
     private lateinit var storage: NodeAttachmentService
+    private lateinit var devModeStorage: NodeAttachmentService
     private val services = rigorousMock<ServicesForResolution>().also {
         doReturn(testNetworkParameters()).whenever(it).networkParameters
     }
@@ -73,6 +75,12 @@ class NodeAttachmentServiceTest {
             }
         }
         storage.servicesForResolution = services
+        devModeStorage = NodeAttachmentService(MetricRegistry(), TestingNamedCacheFactory(), database, true).also {
+            database.transaction {
+                it.start()
+            }
+        }
+        devModeStorage.servicesForResolution = services
     }
 
     @After
@@ -688,6 +696,32 @@ class NodeAttachmentServiceTest {
         }
     }
 
+    @Test
+    fun `development mode - retrieve latest versions of signed contracts - multiple versions of same version id exist in store`() {
+        SelfCleaningDir().use { file ->
+            val (signedContractJar, publicKey) = makeTestSignedContractJar(file.path, "com.example.MyContract")
+            val (signedContractJarSameVersion, _) = makeTestSignedContractJar(file.path,"com.example.MyContract", versionSeed = Random().nextInt())
+
+            signedContractJar.read { devModeStorage.privilegedImportAttachment(it, "app", "contract-signed.jar") }
+            var attachmentIdSameVersionLatest: AttachmentId? = null
+            signedContractJarSameVersion.read { attachmentIdSameVersionLatest = devModeStorage.privilegedImportAttachment(it, "app", "contract-signed-same-version.jar") }
+
+            // this assertion is only true in development mode
+            assertEquals(
+                    2,
+                    devModeStorage.queryAttachments(AttachmentsQueryCriteria(
+                            contractClassNamesCondition = Builder.equal(listOf("com.example.MyContract")),
+                            versionCondition = Builder.equal(1),
+                            isSignedCondition = Builder.equal(true))).size
+            )
+
+            val latestAttachments = devModeStorage.getLatestContractAttachments("com.example.MyContract")
+            assertEquals(1, latestAttachments.size)
+            // should return latest version given by insertion date
+            assertEquals(attachmentIdSameVersionLatest, latestAttachments[0])
+        }
+    }
+
     // Not the real FetchAttachmentsFlow!
     private class FetchAttachmentsFlow : FlowLogic<Unit>() {
         @Suspendable

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockServices.kt

@@ -309,7 +309,7 @@ open class MockServices private constructor(
         }
     override val transactionVerifierService: TransactionVerifierService get() = InMemoryTransactionVerifierService(2)
     private val mockCordappProvider: MockCordappProvider = MockCordappProvider(cordappLoader, attachments).also {
-        it.start(initialNetworkParameters.whitelistedContractImplementations)
+        it.start()
     }
     override val cordappProvider: CordappProvider get() = mockCordappProvider
     override val networkParametersService: NetworkParametersService = MockNetworkParametersStorage(initialNetworkParameters)

testing/test-utils/src/main/kotlin/net/corda/testing/core/internal/ContractJarTestUtils.kt

@@ -55,18 +55,18 @@ object ContractJarTestUtils {
     }
 
     @JvmOverloads
-    fun makeTestSignedContractJar(workingDir: Path, contractName: String, version: Int = 1): Pair<Path, PublicKey> {
+    fun makeTestSignedContractJar(workingDir: Path, contractName: String, version: Int = 1, versionSeed: Int = 0): Pair<Path, PublicKey> {
-        val jarName = makeTestContractJar(workingDir, contractName, true, version)
+        val jarName = makeTestContractJar(workingDir, contractName, true, version, versionSeed)
         val signer = workingDir.signWithDummyKey(jarName)
         return workingDir.resolve(jarName) to signer
     }
 
     @JvmOverloads
-    fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1): Path {
+    fun makeTestContractJar(workingDir: Path, contractName: String, signed: Boolean = false, version: Int = 1, versionSeed: Int = 0): Path {
         val packages = contractName.split(".")
-        val jarName = "attachment-${packages.last()}-$version-${(if (signed) "signed" else "")}.jar"
+        val jarName = "attachment-${packages.last()}-$version-$versionSeed-${(if (signed) "signed" else "")}.jar"
         val className = packages.last()
-        createTestClass(workingDir, className, packages.subList(0, packages.size - 1))
+        createTestClass(workingDir, className, packages.subList(0, packages.size - 1), versionSeed)
         workingDir.createJar(jarName, "${contractName.replace(".", "/")}.class")
         workingDir.addManifest(jarName, Pair(Attributes.Name(CORDAPP_CONTRACT_VERSION), version.toString()))
         return workingDir.resolve(jarName)
@@ -81,7 +81,7 @@ object ContractJarTestUtils {
         }
         val packages = contractNames.first().split(".")
         val jarName = jarFileName ?: "attachment-${packages.last()}-$version-${(if (signed) "signed" else "")}.jar"
-        workingDir.createJar(jarName, *contractNames.map{ "${it.replace(".", "/")}.class" }.toTypedArray() )
+        workingDir.createJar(jarName, *contractNames.map{ "${it.replace(".", "/")}.class" }.toTypedArray())
         if (generateManifest)
             workingDir.addManifest(jarName, Pair(Attributes.Name(CORDAPP_CONTRACT_VERSION), version.toString()))
         if (signed)
@@ -89,12 +89,13 @@ object ContractJarTestUtils {
         return workingDir.resolve(jarName)
     }
 
-    private fun createTestClass(workingDir: Path, className: String, packages: List<String>): Path {
+    private fun createTestClass(workingDir: Path, className: String, packages: List<String>, versionSeed: Int = 0): Path {
         val newClass = """package ${packages.joinToString(".")};
                 import net.corda.core.contracts.*;
                 import net.corda.core.transactions.*;
 
                 public class $className implements Contract {
+                    private int seed = $versionSeed;
                     @Override
                     public void verify(LedgerTransaction tx) throws IllegalArgumentException {
                     }