From 511fb96b0a6bcc0dfcbb7b37e62a9f61ee111847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Fri, 24 Jul 2020 16:22:30 +0200 Subject: [PATCH] Lazy loading of crypto objects --- beacon_chain/spec/beaconstate.nim | 6 +-- beacon_chain/spec/crypto.nim | 70 +++++++++++++++++++++---------- tests/helpers/debug_state.nim | 2 + tests/test_zero_signature.nim | 2 +- 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 58c07218de..c3cdd28741 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -238,7 +238,7 @@ proc initialize_beacon_state_from_eth1*( # This differs from the spec intentionally. # We must specify the default value for `ValidatorSig` # in order to get a correct `hash_tree_root`. - randao_reveal: ValidatorSig(kind: OpaqueBlob) + randao_reveal: ValidatorSig(kind: ToBeChecked) )) ) ) @@ -301,8 +301,8 @@ func get_initial_beacon_block*(state: BeaconState): SignedBeaconBlock = slot: GENESIS_SLOT, state_root: hash_tree_root(state), body: BeaconBlockBody( - # TODO: This shouldn't be necessary if OpaqueBlob is the default - randao_reveal: ValidatorSig(kind: OpaqueBlob))) + # TODO: This shouldn't be necessary if ToBeChecked is the default + randao_reveal: ValidatorSig(kind: ToBeChecked))) # parent_root, randao_reveal, eth1_data, signature, and body automatically # initialized to default values. SignedBeaconBlock(message: message, root: hash_tree_root(message)) diff --git a/beacon_chain/spec/crypto.nim b/beacon_chain/spec/crypto.nim index 021e4246f0..b00ecd0ad4 100644 --- a/beacon_chain/spec/crypto.nim +++ b/beacon_chain/spec/crypto.nim @@ -45,17 +45,23 @@ const RawPrivKeySize* = 48 type - BlsValueType* = enum + BlsValueKind* = enum + ToBeChecked Real - OpaqueBlob + InvalidBLS + OpaqueBlob # For SSZ testing only BlsValue*[N: static int, T] = object - # TODO This is a temporary type needed until we sort out the - # issues with invalid BLS values appearing in the SSZ test suites. - case kind*: BlsValueType + ## This is a lazily initiated wrapper for the underlying cryptographic type + ## Note, since 0.20 case object transition are very restrictive + ## and do not allow to preserve content (https://github.com/nim-lang/RFCs/issues/56) + ## Fortunately, the content is transformed anyway if the object is valid + ## but we might want to keep the invalid content at least for logging before discarding it. + ## Our usage requires "-d:nimOldCaseObjects" + case kind*: BlsValueKind of Real: blsValue*: T - of OpaqueBlob: + of ToBeChecked, InvalidBLS, OpaqueBlob: blob*: array[N, byte] ValidatorPubKey* = BlsValue[RawPubKeySize, blscurve.PublicKey] @@ -75,7 +81,30 @@ type SomeSig* = TrustedSig | ValidatorSig +func unsafePromote*[N, T](a: ptr BlsValue[N, T]) = + ## Try promoting an opaque blob to its corresponding + ## BLS value. + ## + ## ⚠️ Warning - unsafe. + ## At a low-level we mutate the input but all API like + ## bls_sign, bls_verify assume that their inputs are immutable + if a.kind != ToBeChecked: + return + + # Try if valid BLS value + var buffer: T + let success = buffer.fromBytes(a.blob) + + # Unsafe hidden mutation of the input + if true: + a.kind = Real # Requires "-d:nimOldCaseObjects" + a.blsValue = buffer + else: + a.kind = InvalidBLS + func `==`*(a, b: BlsValue): bool = + unsafePromote(a.unsafeAddr) + unsafePromote(b.unsafeAddr) if a.kind != b.kind: return false if a.kind == Real: return a.blsValue == b.blsValue @@ -96,12 +125,7 @@ func toPubKey*(privkey: ValidatorPrivKey): ValidatorPubKey = ## Create a private key from a public key # Un-specced in either hash-to-curve or Eth2 # TODO: Test suite should use `keyGen` instead - when ValidatorPubKey is BlsValue: - ValidatorPubKey(kind: Real, blsValue: SecretKey(privkey).privToPub()) - elif ValidatorPubKey is array: - privkey.getKey.getBytes - else: - privkey.getKey + ValidatorPubKey(kind: Real, blsValue: SecretKey(privkey).privToPub()) func aggregate*(x: var ValidatorSig, other: ValidatorSig) = ## Aggregate 2 Validator Signatures @@ -119,8 +143,11 @@ func blsVerify*( ## The proof-of-possession MUST be verified before calling this function. ## It is recommended to use the overload that accepts a proof-of-possession ## to enforce correct usage. + unsafePromote(pubkey.unsafeAddr) + unsafePromote(signature.unsafeAddr) + if signature.kind != Real: - # Invalid signatures are possible in deposits (discussed with Danny) + # InvalidBLS signatures are possible in deposits (discussed with Danny) return false if pubkey.kind != Real: # TODO: chronicles warning @@ -164,13 +191,15 @@ func blsFastAggregateVerify*( # in blscurve which already exists internally # - or at network/databases/serialization boundaries we do not # allow invalid BLS objects to pollute consensus routines + unsafePromote(signature.unsafeAddr) if signature.kind != Real: return false var unwrapped: seq[PublicKey] - for pubkey in publicKeys: - if pubkey.kind != Real: + for i in 0 ..< publicKeys.len: + unsafePromote(publicKeys[i].unsafeAddr) + if publicKeys[i].kind != Real: return false - unwrapped.add pubkey.blsValue + unwrapped.add publicKeys[i].blsValue fastAggregateVerify(unwrapped, message, signature.blsValue) @@ -223,12 +252,9 @@ func fromRaw*[N, T](BT: type BlsValue[N, T], bytes: openArray[byte]): BlsResult[ # Only for SSZ parsing tests, everything is an opaque blob ok BT(kind: OpaqueBlob, blob: toArray(N, bytes)) else: - # Try if valid BLS value - var val: T - if fromBytes(val, bytes): - ok BT(kind: Real, blsValue: val) - else: - ok BT(kind: OpaqueBlob, blob: toArray(N, bytes)) + # Lazily instantiate the value, it will be checked only on use + # TODO BlsResult is now unnecessary + ok BT(kind: ToBeChecked, blob: toArray(N, bytes)) func fromHex*(T: type BlsCurveType, hexStr: string): BlsResult[T] {.inline.} = ## Initialize a BLSValue from its hex representation diff --git a/tests/helpers/debug_state.nim b/tests/helpers/debug_state.nim index f603571bf5..a2c137f4fb 100644 --- a/tests/helpers/debug_state.nim +++ b/tests/helpers/debug_state.nim @@ -20,6 +20,8 @@ proc `==`*(a, b: BlsValue): bool = ## We sometimes need to compare real BlsValue ## from parsed opaque blobs that are not really on the BLS curve ## and full of zeros + unsafePromote(a.unsafeAddr) + unsafePromote(b.unsafeAddr) if a.kind == Real: if b.kind == Real: a.blsvalue == b.blsValue diff --git a/tests/test_zero_signature.nim b/tests/test_zero_signature.nim index 391a5615db..968096b6f1 100644 --- a/tests/test_zero_signature.nim +++ b/tests/test_zero_signature.nim @@ -39,7 +39,7 @@ suiteReport "Zero signature sanity checks": timedTest "SSZ serialization roundtrip of SignedBeaconBlockHeader": let defaultBlockHeader = SignedBeaconBlockHeader( - signature: ValidatorSig(kind: OpaqueBlob) + signature: ValidatorSig(kind: ToBeChecked) ) check: