diff --git a/core/src/main/kotlin/net/corda/core/crypto/CompositeKey.kt b/core/src/main/kotlin/net/corda/core/crypto/CompositeKey.kt index 1fe4103d4f..98bac0dae0 100644 --- a/core/src/main/kotlin/net/corda/core/crypto/CompositeKey.kt +++ b/core/src/main/kotlin/net/corda/core/crypto/CompositeKey.kt @@ -1,6 +1,5 @@ package net.corda.core.crypto -import net.corda.core.crypto.CompositeKey.NodeAndWeight import net.corda.core.serialization.CordaSerializable import net.corda.core.utilities.exactAdd import net.corda.core.utilities.sequence @@ -12,7 +11,7 @@ import java.util.* /** * A tree data structure that enables the representation of composite public keys, which are used to represent - * the signing requirements for multisignature scenarios such as RAFT notary services. A composite key is a list + * the signing requirements for multi-signature scenarios such as RAFT notary services. A composite key is a list * of leaf keys and their contributing weight, and each leaf can be a conventional single key or a composite key. * Keys contribute their weight to the total if they are matched by the signature. * @@ -53,9 +52,19 @@ class CompositeKey private constructor(val threshold: Int, children: List({ -it.weight }, { it.node.encoded.sequence() }) } - val children: List = children.sorted() + /** + * Τhe order of the children may not be the same to what was provided in the builder. + */ + val children: List = children.sortedWith(descWeightComparator) init { // TODO: replace with the more extensive, but slower, checkValidity() test. @@ -103,9 +112,9 @@ class CompositeKey private constructor(val threshold: Int, children: List() visitedMap.put(this, true) cycleDetection(visitedMap) // Graph cycle testing on the root node. @@ -143,6 +152,7 @@ class CompositeKey private constructor(val threshold: Int, children: List): Boolean { - if (keysToCheck.any { it is CompositeKey }) return false - val totalWeight = children.map { (node, weight) -> + var totalWeight = 0 + children.forEach { (node, weight) -> if (node is CompositeKey) { - if (node.checkFulfilledBy(keysToCheck)) weight else 0 + if (node.checkFulfilledBy(keysToCheck)) totalWeight += weight } else { - if (keysToCheck.contains(node)) weight else 0 + if (node in keysToCheck) totalWeight += weight } - }.sum() - return totalWeight >= threshold + if (totalWeight >= threshold) return true + } + return false } /** @@ -201,8 +212,8 @@ class CompositeKey private constructor(val threshold: Int, children: List): Boolean { // We validate keys only when checking if they're matched, as this checks subkeys as a result. // Doing these checks at deserialization/construction time would result in duplicate checks. - if (!validated) - checkValidity() // TODO: remove when checkValidity() will be eventually invoked during/after deserialization. + checkValidity() + if (keysToCheck.any { it is CompositeKey }) return false return checkFulfilledBy(keysToCheck) } diff --git a/core/src/main/kotlin/net/corda/core/transactions/SignedTransaction.kt b/core/src/main/kotlin/net/corda/core/transactions/SignedTransaction.kt index 015e745987..f6068c71f9 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/SignedTransaction.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/SignedTransaction.kt @@ -68,15 +68,15 @@ data class SignedTransaction(val txBits: SerializedBytes, */ fun buildFilteredTransaction(filtering: Predicate) = tx.buildFilteredTransaction(filtering) - /** Helper to access the inputs of the contained transaction */ + /** Helper to access the inputs of the contained transaction. */ val inputs: List get() = transaction.inputs - /** Helper to access the notary of the contained transaction */ + /** Helper to access the notary of the contained transaction. */ val notary: Party? get() = transaction.notary override val requiredSigningKeys: Set get() = tx.requiredSigningKeys override fun getKeyDescriptions(keys: Set): ArrayList { - // TODO: We need a much better way of structuring this data + // TODO: We need a much better way of structuring this data. val descriptions = ArrayList() this.tx.commands.forEach { command -> if (command.signers.any { it in keys }) @@ -134,8 +134,18 @@ data class SignedTransaction(val txBits: SerializedBytes, @JvmOverloads @Throws(SignatureException::class, AttachmentResolutionException::class, TransactionResolutionException::class) fun toLedgerTransaction(services: ServiceHub, checkSufficientSignatures: Boolean = true): LedgerTransaction { - checkSignaturesAreValid() - if (checkSufficientSignatures) verifyRequiredSignatures() + // TODO: We could probably optimise the below by + // a) not throwing if threshold is eventually satisfied, but some of the rest of the signatures are failing. + // b) omit verifying signatures when threshold requirement is met. + // c) omit verifying signatures from keys not included in [requiredSigningKeys]. + // For the above to work, [checkSignaturesAreValid] should take the [requiredSigningKeys] as input + // and probably combine logic from signature validation and key-fulfilment + // in [TransactionWithSignatures.verifySignaturesExcept]. + if (checkSufficientSignatures) { + verifyRequiredSignatures() // It internally invokes checkSignaturesAreValid(). + } else { + checkSignaturesAreValid() + } return tx.toLedgerTransaction(services) } @@ -153,28 +163,25 @@ data class SignedTransaction(val txBits: SerializedBytes, @Throws(SignatureException::class, AttachmentResolutionException::class, TransactionResolutionException::class, TransactionVerificationException::class) fun verify(services: ServiceHub, checkSufficientSignatures: Boolean = true) { if (isNotaryChangeTransaction()) { - verifyNotaryChangeTransaction(checkSufficientSignatures, services) + verifyNotaryChangeTransaction(services, checkSufficientSignatures) } else { - verifyRegularTransaction(checkSufficientSignatures, services) + verifyRegularTransaction(services, checkSufficientSignatures) } } - /** - * TODO: Verify contract constraints here as well as in LedgerTransaction to ensure that anything being deserialised - * from the attachment is trusted. This will require some partial serialisation work to not load the ContractState - * objects from the TransactionState. - */ - private fun verifyRegularTransaction(checkSufficientSignatures: Boolean, services: ServiceHub) { - checkSignaturesAreValid() - if (checkSufficientSignatures) verifyRequiredSignatures() - val ltx = tx.toLedgerTransaction(services) - // TODO: allow non-blocking verification + // TODO: Verify contract constraints here as well as in LedgerTransaction to ensure that anything being deserialised + // from the attachment is trusted. This will require some partial serialisation work to not load the ContractState + // objects from the TransactionState. + private fun verifyRegularTransaction(services: ServiceHub, checkSufficientSignatures: Boolean) { + val ltx = toLedgerTransaction(services, checkSufficientSignatures) + // TODO: allow non-blocking verification. services.transactionVerifierService.verify(ltx).getOrThrow() } - private fun verifyNotaryChangeTransaction(checkSufficientSignatures: Boolean, services: ServiceHub) { + private fun verifyNotaryChangeTransaction(services: ServiceHub, checkSufficientSignatures: Boolean) { val ntx = resolveNotaryChangeTransaction(services) if (checkSufficientSignatures) ntx.verifyRequiredSignatures() + else checkSignaturesAreValid() } fun isNotaryChangeTransaction() = transaction is NotaryChangeWireTransaction diff --git a/core/src/main/kotlin/net/corda/core/transactions/TransactionWithSignatures.kt b/core/src/main/kotlin/net/corda/core/transactions/TransactionWithSignatures.kt index 8999a62056..77ceb3840b 100644 --- a/core/src/main/kotlin/net/corda/core/transactions/TransactionWithSignatures.kt +++ b/core/src/main/kotlin/net/corda/core/transactions/TransactionWithSignatures.kt @@ -11,7 +11,7 @@ import java.security.PublicKey import java.security.SignatureException import java.util.* -/** An interface for transactions containing signatures, with logic for signature verification */ +/** An interface for transactions containing signatures, with logic for signature verification. */ @DoNotImplement interface TransactionWithSignatures : NamedByHash { /** @@ -21,7 +21,7 @@ interface TransactionWithSignatures : NamedByHash { */ val sigs: List - /** Specifies all the public keys that require signatures for the transaction to be valid */ + /** Specifies all the public keys that require signatures for the transaction to be valid. */ val requiredSigningKeys: Set /** @@ -65,11 +65,10 @@ interface TransactionWithSignatures : NamedByHash { */ @Throws(SignatureException::class) fun verifySignaturesExcept(allowedToBeMissing: Collection) { - checkSignaturesAreValid() - val needed = getMissingSigners() - allowedToBeMissing if (needed.isNotEmpty()) throw SignaturesMissingException(needed.toNonEmptySet(), getKeyDescriptions(needed), id) + checkSignaturesAreValid() } /**