CORDA-986 and CORDA-985 CompositeKey and Signature verification performance fixes (#2467)

This commit is contained in:
Konstantinos Chalkias 2018-02-08 15:49:33 +00:00 committed by GitHub
parent 4a68145f23
commit ff972508bd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 35 deletions

View File

@ -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<NodeAn
}
return builder.build(threshold)
}
// Required for sorting [children] list. To ensure a deterministic way of adding children required for equality
// checking, [children] list is sorted during construction. A DESC ordering in the [NodeAndWeight.weight] field
// will improve efficiency, because keys with bigger "weights" are the first to be checked and thus the
// threshold requirement might be met earlier without requiring a full [children] scan.
// TODO: node.encoded.sequence() might be expensive, consider a faster deterministic compareTo implementation
// for public keys in general.
private val descWeightComparator = compareBy<NodeAndWeight>({ -it.weight }, { it.node.encoded.sequence() })
}
val children: List<NodeAndWeight> = children.sorted()
/**
* Τhe order of the children may not be the same to what was provided in the builder.
*/
val children: List<NodeAndWeight> = 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<NodeAn
* requirements are met, while it tests for aggregated-weight integer overflow.
* In practice, this method should be always invoked on the root [CompositeKey], as it inherently
* validates the child nodes (all the way till the leaves).
* TODO: Always call this method when deserialising [CompositeKey]s.
*/
fun checkValidity() {
if (validated) return
val visitedMap = IdentityHashMap<CompositeKey, Boolean>()
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<NodeAn
override fun compareTo(other: NodeAndWeight): Int {
return if (weight == other.weight)
// TODO: this might be expensive, consider a faster deterministic compareTo implementation when weights are equal.
node.encoded.sequence().compareTo(other.node.encoded.sequence())
else
weight.compareTo(other.weight)
@ -180,17 +190,18 @@ class CompositeKey private constructor(val threshold: Int, children: List<NodeAn
override fun getFormat() = ASN1Encoding.DER
// Extracted method from isFulfilledBy.
// Return true when and if the threshold requirement is met.
private fun checkFulfilledBy(keysToCheck: Iterable<PublicKey>): 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<NodeAn
fun isFulfilledBy(keysToCheck: Iterable<PublicKey>): 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)
}

View File

@ -68,15 +68,15 @@ data class SignedTransaction(val txBits: SerializedBytes<CoreTransaction>,
*/
fun buildFilteredTransaction(filtering: Predicate<Any>) = tx.buildFilteredTransaction(filtering)
/** Helper to access the inputs of the contained transaction */
/** Helper to access the inputs of the contained transaction. */
val inputs: List<StateRef> 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<PublicKey> get() = tx.requiredSigningKeys
override fun getKeyDescriptions(keys: Set<PublicKey>): ArrayList<String> {
// 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<String>()
this.tx.commands.forEach { command ->
if (command.signers.any { it in keys })
@ -134,8 +134,18 @@ data class SignedTransaction(val txBits: SerializedBytes<CoreTransaction>,
@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<CoreTransaction>,
@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

View File

@ -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<TransactionSignature>
/** 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<PublicKey>
/**
@ -65,11 +65,10 @@ interface TransactionWithSignatures : NamedByHash {
*/
@Throws(SignatureException::class)
fun verifySignaturesExcept(allowedToBeMissing: Collection<PublicKey>) {
checkSignaturesAreValid()
val needed = getMissingSigners() - allowedToBeMissing
if (needed.isNotEmpty())
throw SignaturesMissingException(needed.toNonEmptySet(), getKeyDescriptions(needed), id)
checkSignaturesAreValid()
}
/**