ENT-2506 restore the attachment party signers (#4255)

* ENT-2506 restore the attachment party signers

* ENT-2506 restore the attachment party signers

* ENT-2506 restore the attachment party signers

* ENT-2675 Address code review changes.

* ENT-2675 Address code review changes.

.ci/api-current.txt

@@ -431,7 +431,9 @@ public static final class net.corda.core.contracts.AmountTransfer$Companion exte
 public interface net.corda.core.contracts.Attachment extends net.corda.core.contracts.NamedByHash
   public void extractFile(String, java.io.OutputStream)
   @NotNull
-  public abstract java.util.List<java.security.PublicKey> getSigners()
+  public abstract java.util.List<java.security.PublicKey> getSignerKeys()
+  @NotNull
+  public abstract java.util.List<net.corda.core.identity.Party> getSigners()
   public abstract int getSize()
   @NotNull
   public abstract java.io.InputStream open()
@@ -542,7 +544,7 @@ public final class net.corda.core.contracts.ContractAttachment extends java.lang
   @NotNull
   public net.corda.core.crypto.SecureHash getId()
   @NotNull
-  public java.util.List<java.security.PublicKey> getSigners()
+  public java.util.List<net.corda.core.identity.Party> getSigners()
   public int getSize()
   @Nullable
   public final String getUploader()

client/jackson/src/test/kotlin/net/corda/client/jackson/JacksonSupportTest.kt

@@ -234,7 +234,7 @@ class JacksonSupportTest(@Suppress("unused") private val name: String, factory:
         val attachment = rigorousMock<ContractAttachment>()
         doReturn(attachment).whenever(attachmentStorage).openAttachment(attachmentId)
         doReturn(attachmentId).whenever(attachment).id
-        doReturn(emptyList<Party>()).whenever(attachment).signers
+        doReturn(emptyList<Party>()).whenever(attachment).signerKeys
         doReturn(setOf(DummyContract.PROGRAM_ID)).whenever(attachment).allContracts
         doReturn("app").whenever(attachment).uploader
 

core-deterministic/testing/src/test/kotlin/net/corda/deterministic/contracts/AttachmentTest.kt

@@ -40,10 +40,12 @@ class AttachmentTest {
     @Before
     fun setup() {
         attachment = object : Attachment {
+            override val signerKeys: List<PublicKey>
+                get() = listOf(ALICE_KEY)
             override val id: SecureHash
                 get() = SecureHash.allOnesHash
-            override val signers: List<PublicKey>
+            override val signers: List<Party>
-                get() = listOf(ALICE_KEY)
+                get() = listOf(ALICE)
             override val size: Int
                 get() = jarData.size
 

core-deterministic/testing/verifier/src/main/kotlin/net/corda/deterministic/verifier/MockContractAttachment.kt

@@ -3,13 +3,19 @@ package net.corda.deterministic.verifier
 import net.corda.core.contracts.Attachment
 import net.corda.core.contracts.ContractClassName
 import net.corda.core.crypto.SecureHash
+import net.corda.core.identity.Party
 import net.corda.core.serialization.CordaSerializable
 import java.io.ByteArrayInputStream
 import java.io.InputStream
 import java.security.PublicKey
 
 @CordaSerializable
-class MockContractAttachment(override val id: SecureHash = SecureHash.zeroHash, val contract: ContractClassName, override val signers: List<PublicKey> = emptyList()) : Attachment {
+class MockContractAttachment(
+        override val id: SecureHash = SecureHash.zeroHash,
+        val contract: ContractClassName,
+        override val signerKeys: List<PublicKey> = emptyList(),
+        override val signers: List<Party> = emptyList()
+) : Attachment {
     override fun open(): InputStream = ByteArrayInputStream(id.bytes)
     override val size = id.size
 }

core/src/main/kotlin/net/corda/core/contracts/Attachment.kt

@@ -1,6 +1,8 @@
 package net.corda.core.contracts
 
+import net.corda.core.DoNotImplement
 import net.corda.core.KeepForDJVM
+import net.corda.core.identity.Party
 import net.corda.core.internal.extractFile
 import net.corda.core.serialization.CordaSerializable
 import java.io.FileNotFoundException
@@ -31,6 +33,7 @@ import java.util.jar.JarInputStream
  */
 @KeepForDJVM
 @CordaSerializable
+@DoNotImplement
 interface Attachment : NamedByHash {
     fun open(): InputStream
 
@@ -51,11 +54,20 @@ interface Attachment : NamedByHash {
     @JvmDefault
     fun extractFile(path: String, outputTo: OutputStream) = openAsJAR().use { it.extractFile(path, outputTo) }
 
+    /**
+     * The parties that have correctly signed the whole attachment.
+     * Even though this returns a list of party objects, it is not required that these parties exist on the network, but rather they are a mapping from the signing key to the X.500 name.
+     *
+     * Note: Anyone can sign attachments, not only Corda parties. It's recommended to use [signerKeys].
+     */
+    @Deprecated("Use signerKeys. There is no requirement that attachment signers are Corda parties.")
+    val signers: List<Party>
+
     /**
      * The keys that have correctly signed the whole attachment.
      * Can be empty, for example non-contract attachments won't be necessarily be signed.
      */
-    val signers: List<PublicKey>
+    val signerKeys: List<PublicKey>
 
     /**
      * Attachment size in bytes.

core/src/main/kotlin/net/corda/core/contracts/AttachmentConstraint.kt

@@ -72,7 +72,7 @@ interface AttachmentConstraint {
 
             // You can transition from the WhitelistConstraint to the SignatureConstraint only if all signers of the JAR are required to sign in the future.
             input is WhitelistedByZoneAttachmentConstraint && output is SignatureAttachmentConstraint ->
-                attachment.signers.isNotEmpty() && output.key.keys.containsAll(attachment.signers)
+                attachment.signerKeys.isNotEmpty() && output.key.keys.containsAll(attachment.signerKeys)
 
             else -> false
         }
@@ -180,5 +180,5 @@ data class SignatureAttachmentConstraint(
         val key: PublicKey
 ) : AttachmentConstraint {
     override fun isSatisfiedBy(attachment: Attachment): Boolean =
-        key.isFulfilledBy(attachment.signers.map { it })
+        key.isFulfilledBy(attachment.signerKeys.map { it })
 }

core/src/main/kotlin/net/corda/core/contracts/ContractAttachment.kt

@@ -18,7 +18,7 @@ class ContractAttachment @JvmOverloads constructor(
         val contract: ContractClassName,
         val additionalContracts: Set<ContractClassName> = emptySet(),
         val uploader: String? = null,
-        override val signers: List<PublicKey> = emptyList()) : Attachment by attachment {
+        override val signerKeys: List<PublicKey> = emptyList()) : Attachment by attachment {
 
     val allContracts: Set<ContractClassName> get() = additionalContracts + contract
 

core/src/main/kotlin/net/corda/core/internal/AbstractAttachment.kt

@@ -6,6 +6,7 @@ import net.corda.core.DeleteForDJVM
 import net.corda.core.KeepForDJVM
 import net.corda.core.contracts.Attachment
 import net.corda.core.crypto.SecureHash
+import net.corda.core.identity.Party
 import net.corda.core.serialization.MissingAttachmentsException
 import net.corda.core.serialization.SerializeAsTokenContext
 import java.io.FileNotFoundException
@@ -47,10 +48,15 @@ abstract class AbstractAttachment(dataLoader: () -> ByteArray) : Attachment {
     override val size: Int get() = attachmentData.size
 
     override fun open(): InputStream = attachmentData.inputStream()
-    override val signers: List<PublicKey> by lazy {
+
+    override val signerKeys: List<PublicKey> by lazy {
         openAsJAR().use(JarSignatureCollector::collectSigners)
     }
 
+    override val signers: List<Party> by lazy {
+        openAsJAR().use(JarSignatureCollector::collectSigningParties)
+    }
+
     override fun equals(other: Any?) = other === this || other is Attachment && other.id == this.id
     override fun hashCode() = id.hashCode()
     override fun toString() = "${javaClass.simpleName}(id=$id)"

core/src/main/kotlin/net/corda/core/transactions/ContractUpgradeTransactions.kt

@@ -252,7 +252,7 @@ data class ContractUpgradeLedgerTransaction(
     private fun verifyConstraints() {
         val attachmentForConstraintVerification = AttachmentWithContext(
                 legacyContractAttachment as? ContractAttachment
-                        ?: ContractAttachment(legacyContractAttachment, legacyContractClassName, signers = legacyContractAttachment.signers),
+                        ?: ContractAttachment(legacyContractAttachment, legacyContractClassName, signerKeys = legacyContractAttachment.signerKeys),
                 upgradedContract.legacyContract,
                 networkParameters.whitelistedContractImplementations
         )

core/src/main/kotlin/net/corda/core/transactions/LedgerTransaction.kt

@@ -166,7 +166,7 @@ data class LedgerTransaction private constructor(
 
         contractsAndOwners.forEach { contract, owner ->
             val attachment = contractAttachmentsByContract[contract]!!
-            if (!owner.isFulfilledBy(attachment.signers)) {
+            if (!owner.isFulfilledBy(attachment.signerKeys)) {
                 throw TransactionVerificationException.ContractAttachmentNotSignedByPackageOwnerException(this.id, id, contract)
             }
         }

core/src/main/kotlin/net/corda/core/transactions/TransactionBuilder.kt

@@ -19,7 +19,6 @@ import net.corda.core.node.services.AttachmentId
 import net.corda.core.node.services.KeyManagementService
 import net.corda.core.serialization.SerializationContext
 import net.corda.core.serialization.SerializationFactory
-import net.corda.core.utilities.loggerFor
 import net.corda.core.utilities.contextLogger
 import net.corda.core.utilities.warnOnce
 import java.security.PublicKey
@@ -310,7 +309,7 @@ open class TransactionBuilder @JvmOverloads constructor(
             attachmentToUse: ContractAttachment,
             services: ServicesForResolution): AttachmentConstraint = when {
         inputStates != null -> attachmentConstraintsTransition(inputStates.groupBy { it.constraint }.keys, attachmentToUse)
-        attachmentToUse.signers.isNotEmpty() && services.networkParameters.minimumPlatformVersion < 4 -> {
+        attachmentToUse.signerKeys.isNotEmpty() && services.networkParameters.minimumPlatformVersion < 4 -> {
             log.warnOnce("Signature constraints not available on network requiring a minimum platform version of 4. Current is: ${services.networkParameters.minimumPlatformVersion}.")
             if (useWhitelistedByZoneAttachmentConstraint(contractClassName, services.networkParameters)) {
                 log.warnOnce("Reverting back to using whitelisted zone constraints for contract $contractClassName")
@@ -320,7 +319,7 @@ open class TransactionBuilder @JvmOverloads constructor(
                 HashAttachmentConstraint(attachmentToUse.id)
             }
         }
-        attachmentToUse.signers.isNotEmpty() -> makeSignatureAttachmentConstraint(attachmentToUse.signers)
+        attachmentToUse.signerKeys.isNotEmpty() -> makeSignatureAttachmentConstraint(attachmentToUse.signerKeys)
         useWhitelistedByZoneAttachmentConstraint(contractClassName, services.networkParameters) -> WhitelistedByZoneAttachmentConstraint
         else -> HashAttachmentConstraint(attachmentToUse.id)
     }
@@ -361,8 +360,8 @@ open class TransactionBuilder @JvmOverloads constructor(
         constraints.any { it is SignatureAttachmentConstraint } && constraints.any { it is WhitelistedByZoneAttachmentConstraint } -> {
             val signatureConstraint = constraints.mapNotNull { it as? SignatureAttachmentConstraint }.single()
             when {
-                attachmentToUse.signers.isEmpty() -> throw IllegalArgumentException("Cannot mix a state with the WhitelistedByZoneAttachmentConstraint and a state with the SignatureAttachmentConstraint, when the latest attachment is not signed. Please contact your Zone operator.")
+                attachmentToUse.signerKeys.isEmpty() -> throw IllegalArgumentException("Cannot mix a state with the WhitelistedByZoneAttachmentConstraint and a state with the SignatureAttachmentConstraint, when the latest attachment is not signed. Please contact your Zone operator.")
-                signatureConstraint.key.keys.containsAll(attachmentToUse.signers) -> signatureConstraint
+                signatureConstraint.key.keys.containsAll(attachmentToUse.signerKeys) -> signatureConstraint
                 else -> throw IllegalArgumentException("Attempting to transition a WhitelistedByZoneAttachmentConstraint state backed by an attachment signed by multiple parties to a weaker SignatureConstraint that does not require all those signatures. Please contact your Zone operator.")
             }
         }

core/src/test/kotlin/net/corda/core/contracts/ConstraintsPropagationTests.kt

@@ -7,7 +7,6 @@ import net.corda.core.crypto.SecureHash
 import net.corda.core.identity.AbstractParty
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.transactions.LedgerTransaction
-import net.corda.core.transactions.MissingContractAttachments
 import net.corda.finance.POUNDS
 import net.corda.finance.`issued by`
 import net.corda.finance.contracts.asset.Cash
@@ -223,7 +222,7 @@ class ConstraintsPropagationTests {
     fun `Attachment canBeTransitionedFrom behaves as expected`() {
 
         val attachment = mock<ContractAttachment>()
-        whenever(attachment.signers).thenReturn(listOf(ALICE_PARTY.owningKey))
+        whenever(attachment.signerKeys).thenReturn(listOf(ALICE_PARTY.owningKey))
 
         // Exhaustive positive check
         assertTrue(HashAttachmentConstraint(SecureHash.randomSHA256()).canBeTransitionedFrom(SignatureAttachmentConstraint(ALICE_PUBKEY), attachment))

core/src/test/kotlin/net/corda/core/contracts/StructuresTests.kt

@@ -3,6 +3,7 @@ package net.corda.core.contracts
 import com.nhaarman.mockito_kotlin.doAnswer
 import com.nhaarman.mockito_kotlin.spy
 import com.nhaarman.mockito_kotlin.whenever
+import net.corda.core.identity.Party
 import org.junit.Test
 import java.io.ByteArrayOutputStream
 import java.io.IOException
@@ -30,7 +31,8 @@ class AttachmentTest {
         val attachment = object : Attachment {
             override val id get() = throw UnsupportedOperationException()
             override fun open() = inputStream
-            override val signers get() = throw UnsupportedOperationException()
+            override val signerKeys get() = throw UnsupportedOperationException()
+            override val signers: List<Party> get() = throw UnsupportedOperationException()
             override val size: Int = 512
         }
         try {

core/src/test/kotlin/net/corda/core/serialization/AttachmentSerializationTest.kt

@@ -111,7 +111,8 @@ class AttachmentSerializationTest {
 
     private class CustomAttachment(override val id: SecureHash, internal val customContent: String) : Attachment {
         override fun open() = throw UnsupportedOperationException("Not implemented.")
-        override val signers get() = throw UnsupportedOperationException()
+        override val signerKeys get() = throw UnsupportedOperationException()
+        override val signers: List<Party> get() = throw UnsupportedOperationException()
         override val size get() = throw UnsupportedOperationException()
     }
 

core/src/test/kotlin/net/corda/core/transactions/TransactionBuilderTest.kt

@@ -50,7 +50,7 @@ class TransactionBuilderTest {
         doReturn(contractAttachmentId).whenever(attachment).id
         doReturn(setOf(DummyContract.PROGRAM_ID)).whenever(attachment).allContracts
         doReturn("app").whenever(attachment).uploader
-        doReturn(emptyList<Party>()).whenever(attachment).signers
+        doReturn(emptyList<Party>()).whenever(attachment).signerKeys
     }
 
     @Test
@@ -128,12 +128,12 @@ class TransactionBuilderTest {
     private val unsignedAttachment = ContractAttachment(object : AbstractAttachment({ byteArrayOf() }) {
         override val id: SecureHash get() = throw UnsupportedOperationException()
 
-        override val signers: List<PublicKey> get() = emptyList()
+        override val signerKeys: List<PublicKey> get() = emptyList()
     }, DummyContract.PROGRAM_ID)
 
     private fun signedAttachment(vararg parties: Party) = ContractAttachment(object : AbstractAttachment({ byteArrayOf() }) {
         override val id: SecureHash get() = throw UnsupportedOperationException()
 
-        override val signers: List<PublicKey> get() = parties.map { it.owningKey }
+        override val signerKeys: List<PublicKey> get() = parties.map { it.owningKey }
-    }, DummyContract.PROGRAM_ID, signers = parties.map { it.owningKey })
+    }, DummyContract.PROGRAM_ID, signerKeys = parties.map { it.owningKey })
 }

docs/source/changelog.rst

@@ -7,8 +7,11 @@ release, see :doc:`upgrade-notes`.
 Unreleased
 ----------
 
+* Marked the `Attachment` interface as `@DoNotImplement` because it is not meant to be extended by CorDapp developers. If you have already done so,
+please get in contact on the usual communication channels.
+
 * Added auto-acceptance of network parameters for network updates. This behaviour is available for a subset of the network parameters
-  and is configurable via the node config. See :doc:`network-map` for more information.
+and is configurable via the node config. See :doc:`network-map` for more information.
 
 * Deprecated `SerializationContext.withAttachmentsClassLoader`. This functionality has always been disabled by flags
 and there is no reason for a CorDapp developer to use it. It is just an internal implementation detail of Corda.

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

@@ -87,7 +87,7 @@ class AttachmentsClassLoaderStaticContractTests {
         doReturn(it.cordappProvider.getContractAttachmentID(AttachmentDummyContract.ATTACHMENT_PROGRAM_ID)).whenever(attachment).id
         doReturn(setOf(AttachmentDummyContract.ATTACHMENT_PROGRAM_ID)).whenever(attachment).allContracts
         doReturn("app").whenever(attachment).uploader
-        doReturn(emptyList<Party>()).whenever(attachment).signers
+        doReturn(emptyList<Party>()).whenever(attachment).signerKeys
     }
 
     @Test

node/src/main/kotlin/net/corda/node/serialization/kryo/DefaultKryoCustomizer.kt

@@ -211,7 +211,7 @@ object DefaultKryoCustomizer {
             output.writeString(obj.contract)
             kryo.writeClassAndObject(output, obj.additionalContracts)
             output.writeString(obj.uploader)
-            kryo.writeClassAndObject(output, obj.signers)
+            kryo.writeClassAndObject(output, obj.signerKeys)
         }
 
         @Suppress("UNCHECKED_CAST")

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

@@ -91,7 +91,7 @@ class NodeAttachmentServiceTest {
             val signedJar = jarAndSigner.first
             signedJar.inputStream().use { jarStream ->
                 val attachmentId = storage.importAttachment(jarStream, "test", null)
-                assertEquals(listOf(jarAndSigner.second.hash), storage.openAttachment(attachmentId)!!.signers.map { it.hash })
+                assertEquals(listOf(jarAndSigner.second.hash), storage.openAttachment(attachmentId)!!.signerKeys.map { it.hash })
             }
         }
     }
@@ -102,7 +102,7 @@ class NodeAttachmentServiceTest {
             val jarName = makeTestContractJar(it.path, "com.example.MyContract")
             it.path.resolve(jarName).inputStream().use { jarStream ->
                 val attachmentId = storage.importAttachment(jarStream, "test", null)
-                assertEquals(0, storage.openAttachment(attachmentId)!!.signers.size)
+                assertEquals(0, storage.openAttachment(attachmentId)!!.signerKeys.size)
             }
         }
     }

serialization/src/main/kotlin/net/corda/serialization/internal/amqp/custom/ContractAttachmentSerializer.kt

@@ -24,7 +24,7 @@ class ContractAttachmentSerializer(factory: SerializerFactory) : CustomSerialize
         } catch (e: Exception) {
             throw MissingAttachmentsException(listOf(obj.id))
         }
-        return ContractAttachmentProxy(GeneratedAttachment(bytes), obj.contract, obj.additionalContracts, obj.uploader, obj.signers)
+        return ContractAttachmentProxy(GeneratedAttachment(bytes), obj.contract, obj.additionalContracts, obj.uploader, obj.signerKeys)
     }
 
     override fun fromProxy(proxy: ContractAttachmentProxy): ContractAttachment {

testing/test-utils/src/main/kotlin/net/corda/testing/services/MockAttachmentStorage.kt

@@ -61,7 +61,7 @@ class MockAttachmentStorage : AttachmentStorage, SingletonSerializeAsToken() {
 
     fun getAttachmentIdAndBytes(jar: InputStream): Pair<AttachmentId, ByteArray> = jar.readFully().let { bytes -> Pair(bytes.sha256(), bytes) }
 
-    private class MockAttachment(dataLoader: () -> ByteArray, override val id: SecureHash, override val signers: List<PublicKey>) : AbstractAttachment(dataLoader)
+    private class MockAttachment(dataLoader: () -> ByteArray, override val id: SecureHash, override val signerKeys: List<PublicKey>) : AbstractAttachment(dataLoader)
 
     private fun importAttachmentInternal(jar: InputStream, uploader: String, contractClassNames: List<ContractClassName>? = null, attachmentId: AttachmentId? = null, signers: List<PublicKey> = emptyList()): AttachmentId {
         // JIS makes read()/readBytes() return bytes of the current file, but we want to hash the entire container here.