Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bls v0.10.1 #780

Merged
merged 13 commits into from
Mar 13, 2020
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ TOOLS := \
beacon_node \
inspector \
logtrace \
bench_bls_sig_agggregation \
deposit_contract \
ncli_hash_tree_root \
ncli_pretty \
ncli_transition \
process_dashboard
# bench_bls_sig_agggregation TODO reenable after bls v0.10.1 changes
TOOLS_DIRS := \
beacon_chain \
benchmarks \
Expand Down Expand Up @@ -153,4 +153,3 @@ libnfuzz.a: | build deps
rm -f build/$@ && \
$(ENV_SCRIPT) nim c -d:release --app:staticlib --noMain --nimcache:nimcache/libnfuzz_static -o:build/$@ $(NIM_PARAMS) nfuzz/libnfuzz.nim && \
[[ -e "$@" ]] && mv "$@" build/ # workaround for https://github.com/nim-lang/Nim/issues/12745

4 changes: 2 additions & 2 deletions beacon_chain/attestation_pool.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ proc combine*(tgt: var Attestation, src: Attestation, flags: UpdateFlags) =
tgt.aggregation_bits.combine(src.aggregation_bits)

if skipBlsValidation notin flags:
tgt.signature.combine(src.signature)
tgt.signature.aggregate(src.signature)
else:
trace "Ignoring overlapping attestations"

Expand Down Expand Up @@ -341,7 +341,7 @@ proc getAttestationsForBlock*(
# one new attestation in there
if not attestation.aggregation_bits.overlaps(v.aggregation_bits):
attestation.aggregation_bits.combine(v.aggregation_bits)
attestation.signature.combine(v.aggregate_signature)
attestation.signature.aggregate(v.aggregate_signature)

result.add(attestation)

Expand Down
1 change: 0 additions & 1 deletion beacon_chain/conf.nim
Original file line number Diff line number Diff line change
Expand Up @@ -293,4 +293,3 @@ iterator validatorKeys*(conf: BeaconNodeConf): ValidatorPrivKey =
template writeValue*(writer: var JsonWriter,
value: TypedInputFile|InputFile|InputDir|OutPath|OutDir|OutFile) =
writer.writeValue(string value)

35 changes: 15 additions & 20 deletions beacon_chain/interop.nim
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,19 @@ func get_eth1data_stub*(deposit_count: uint64, current_epoch: Epoch): Eth1Data =
block_hash: hash_tree_root(hash_tree_root(voting_period).data),
)

when ValidatorPrivKey is BlsValue:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is related to the local private key storage, this might be an instance of what @zah discussed as excessive specialization on a design which should be regardless decoupled. If so, maybe note as such.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was when we were hesitating between using the raw private keys and being able to handle fake blobs, especially due to testing faking the BLS stuff.

func makeInteropPrivKey*(i: int): ValidatorPrivKey =
discard
{.fatal: "todo/unused?".}
else:
func makeInteropPrivKey*(i: int): ValidatorPrivKey =
var bytes: array[32, byte]
bytes[0..7] = uint64(i).toBytesLE()
func makeInteropPrivKey*(i: int): ValidatorPrivKey =
var bytes: array[32, byte]
bytes[0..7] = uint64(i).toBytesLE()

let
# BLS381-12 curve order - same as milagro but formatted different
curveOrder =
"52435875175126190479447740508185965837690552500527637822603658699938581184513".parse(UInt256)
let
# BLS381-12 curve order - same as milagro but formatted different
curveOrder =
"52435875175126190479447740508185965837690552500527637822603658699938581184513".parse(UInt256)

privkeyBytes = eth2hash(bytes)
key = (UInt256.fromBytesLE(privkeyBytes.data) mod curveOrder).toBytesBE()
privkeyBytes = eth2hash(bytes)
key = (UInt256.fromBytesLE(privkeyBytes.data) mod curveOrder).toBytesBE()

ValidatorPrivKey.init(key)
result.initFromBytes(key)

const eth1BlockHash* = block:
var x: Eth2Digest
Expand All @@ -56,10 +51,10 @@ func makeDeposit*(
pubkey: pubkey,
withdrawal_credentials: makeWithdrawalCredentials(pubkey)))

if skipBlsValidation notin flags:
ret.data.signature =
bls_sign(
privkey, hash_tree_root(ret.getDepositMessage).data,
compute_domain(DOMAIN_DEPOSIT))
if skipBLSValidation notin flags:
let domain = compute_domain(DOMAIN_DEPOSIT)
let signing_root = compute_signing_root(ret.getDepositMessage, domain)

ret.data.signature = bls_sign(privkey, signing_root.data)

ret
26 changes: 14 additions & 12 deletions beacon_chain/spec/beaconstate.nim
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ proc process_deposit*(

if index == -1:
# Verify the deposit signature (proof of possession)
if skipBlsValidation notin flags and not bls_verify(
pubkey, hash_tree_root(deposit.getDepositMessage).data,
deposit.data.signature, compute_domain(DOMAIN_DEPOSIT)):
let domain = compute_domain(DOMAIN_DEPOSIT)
let signing_root = compute_signing_root(deposit.getDepositMessage, domain)
if skipBLSValidation notin flags and not bls_verify(
pubkey, signing_root.data,
deposit.data.signature):
return false

# Add validator and balance entries
Expand Down Expand Up @@ -214,7 +216,7 @@ func initialize_beacon_state_from_eth1*(
latest_block_header:
BeaconBlockHeader(
body_root: hash_tree_root(BeaconBlockBody(
randao_reveal: BlsValue[Signature](kind: OpaqueBlob)
randao_reveal: ValidatorSig(kind: OpaqueBlob)
))
)
)
Expand Down Expand Up @@ -263,7 +265,7 @@ func get_initial_beacon_block*(state: BeaconState): SignedBeaconBlock =
state_root: hash_tree_root(state),
body: BeaconBlockBody(
# TODO: This shouldn't be necessary if OpaqueBlob is the default
randao_reveal: BlsValue[Signature](kind: OpaqueBlob))))
randao_reveal: ValidatorSig(kind: OpaqueBlob))))
# parent_root, randao_reveal, eth1_data, signature, and body automatically
# initialized to default values.

Expand Down Expand Up @@ -381,13 +383,13 @@ proc is_valid_indexed_attestation*(
return false

# Verify aggregate signature
if skipBlsValidation notin flags and not bls_verify(
bls_aggregate_pubkeys(mapIt(indices, state.validators[it.int].pubkey)),
hash_tree_root(indexed_attestation.data).data,
indexed_attestation.signature,
get_domain(
state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
):
let pubkeys = mapIt(indices, state.validators[it.int].pubkey) # TODO: fuse loops with blsFastAggregateVerify
let domain = state.get_domain(DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)
let signing_root = compute_signing_root(indexed_attestation.data, domain)
if skipBLSValidation notin flags and
not blsFastAggregateVerify(
pubkeys, signing_root.data, indexed_attestation.signature
):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the possible apparent cause of a couple of EF test vector failures. Hopefully, it improves things.

notice "indexed attestation: signature verification failure"
return false

Expand Down
Loading