diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 5e2112a48..32abbedb3 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -169,6 +169,8 @@ func slash_validator*(state: var BeaconState, slashed_index: ValidatorIndex, (validator.effective_balance div WHISTLEBLOWER_REWARD_QUOTIENT).Gwei proposer_reward = whistleblowing_reward div PROPOSER_REWARD_QUOTIENT increase_balance(state, proposer_index, proposer_reward) + # TODO: evaluate if spec bug / underflow can be triggered + doAssert(whistleblowing_reward >= proposer_reward, "Spec bug: underflow in slash_validator") increase_balance( state, whistleblower_index, whistleblowing_reward - proposer_reward) @@ -368,9 +370,11 @@ func process_registry_updates*(state: var BeaconState) = compute_activation_exit_epoch(get_current_epoch(state)) # https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/core/0_beacon-chain.md#is_valid_indexed_attestation -func is_valid_indexed_attestation*( +proc is_valid_indexed_attestation*( state: BeaconState, indexed_attestation: IndexedAttestation): bool = - # Check if ``indexed_attestation`` has valid indices and signature. + ## Check if ``indexed_attestation`` has valid indices and signature. + # TODO: this is noSideEffect besides logging + # https://github.com/status-im/nim-chronicles/issues/62 let bit_0_indices = indexed_attestation.custody_bit_0_indices.asSeq @@ -378,45 +382,53 @@ func is_valid_indexed_attestation*( # Verify no index has custody bit equal to 1 [to be removed in phase 1] if len(bit_1_indices) != 0: + notice "indexed attestation: custody_bit equal to 1" return false # Verify max number of indices let combined_len = len(bit_0_indices) + len(bit_1_indices) if not (combined_len <= MAX_VALIDATORS_PER_COMMITTEE): + notice "indexed attestation: validator index beyond max validators per committee" return false # Verify index sets are disjoint if len(intersection(bit_0_indices.toSet, bit_1_indices.toSet)) != 0: + notice "indexed attestation: indices set not disjoint" return false # Verify indices are sorted if bit_0_indices != sorted(bit_0_indices, system.cmp): + notice "indexed attestation: indices 0 not sorted" return false if bit_1_indices != sorted(bit_1_indices, system.cmp): + notice "indexed attestation: indices 0 not sorted" return false # Verify aggregate signature - bls_verify_multiple( - @[ - bls_aggregate_pubkeys( - mapIt(bit_0_indices, state.validators[it.int].pubkey)), - bls_aggregate_pubkeys( - mapIt(bit_1_indices, state.validators[it.int].pubkey)), - ], - @[ - hash_tree_root(AttestationDataAndCustodyBit( - data: indexed_attestation.data, custody_bit: false)), - hash_tree_root(AttestationDataAndCustodyBit( - data: indexed_attestation.data, custody_bit: true)), - ], + let + pubkeys = @[ # TODO, bls_verify_multiple should accept openarray + bls_aggregate_pubkeys(mapIt(bit_0_indices, state.validators[it.int].pubkey)), + bls_aggregate_pubkeys(mapIt(bit_1_indices, state.validators[it.int].pubkey)), + ] + msg1 = AttestationDataAndCustodyBit( + data: indexed_attestation.data, custody_bit: false) + msg2 = AttestationDataAndCustodyBit( + data: indexed_attestation.data, custody_bit: true) + message_hashes = [ + hash_tree_root(msg1), + hash_tree_root(msg2), + ] + domain = get_domain(state, DOMAIN_ATTESTATION, indexed_attestation.data.target.epoch) + + result = bls_verify_multiple( + pubkeys, + message_hashes, indexed_attestation.signature, - get_domain( - state, - DOMAIN_ATTESTATION, - indexed_attestation.data.target.epoch - ), + domain, ) + if not result: + notice "indexed attestation: signature verification failure" # https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/core/0_beacon-chain.md#get_attesting_indices func get_attesting_indices*(state: BeaconState, diff --git a/beacon_chain/spec/crypto.nim b/beacon_chain/spec/crypto.nim index 25ca34bb9..c0ab89b61 100644 --- a/beacon_chain/spec/crypto.nim +++ b/beacon_chain/spec/crypto.nim @@ -48,7 +48,8 @@ import sequtils, stew/objects, hashes, nimcrypto/utils, blscurve, json_serialization, - ../version, digest + ../version, digest, + chronicles export json_serialization @@ -145,12 +146,17 @@ func pubKey*(pk: ValidatorPrivKey): ValidatorPubKey = else: pk.getKey -proc combine*[T](a: openarray[BlsValue[T]]): T = - doAssert a.len > 0 and a[0].kind == Real - result = a[0].blsValue - for i in 1 ..< a.len: - doAssert a[i].kind == Real - result.combine a[i].blsValue +proc init(T: type VerKey): VerKey = + result.point.inf() + +proc init(T: type SigKey): SigKey = + result.point.inf() + +proc combine*[T](values: openarray[BlsValue[T]]): BlsValue[T] = + result = BlsValue[T](kind: Real, blsValue: T.init()) + + for value in values: + result.blsValue.combine(value.blsValue) proc combine*[T](x: var BlsValue[T], other: BlsValue[T]) = doAssert x.kind == Real and other.kind == Real @@ -158,13 +164,7 @@ proc combine*[T](x: var BlsValue[T], other: BlsValue[T]) = # https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/bls_signature.md#bls_aggregate_pubkeys func bls_aggregate_pubkeys*(keys: openArray[ValidatorPubKey]): ValidatorPubKey = - var empty = true - for key in keys: - if empty: - result = key - empty = false - else: - result.combine(key) + keys.combine() # https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/bls_signature.md#bls_verify func bls_verify*( @@ -180,13 +180,14 @@ func bls_verify*( sig.verify(msg, domain, pubkey) # https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/bls_signature.md#bls_verify_multiple -func bls_verify_multiple*( +proc bls_verify_multiple*( pubkeys: seq[ValidatorPubKey], message_hashes: openArray[Eth2Digest], sig: ValidatorSig, domain: uint64): bool = + # {.noSideEffect.} - https://github.com/status-im/nim-chronicles/issues/62 let L = len(pubkeys) doAssert L == len(message_hashes) if sig.kind != Real: - # TODO: chronicles warning + warn "Raw bytes do not match with a BLS signature." return false # TODO optimize using multiPairing @@ -195,8 +196,10 @@ func bls_verify_multiple*( doAssert pubkey.kind == Real # TODO spec doesn't say to handle this specially, but it's silly to # validate without any actual public keys. - if pubkey.blsValue != VerKey() and - not sig.blsValue.verify(message_hash.data, domain, pubkey.blsValue): + if pubkey.blsValue == VerKey(): + trace "Received empty public key, skipping verification." + continue + if not sig.blsValue.verify(message_hash.data, domain, pubkey.blsValue): return false true diff --git a/beacon_chain/spec/state_transition_block.nim b/beacon_chain/spec/state_transition_block.nim index c369e4c43..b2b15b84f 100644 --- a/beacon_chain/spec/state_transition_block.nim +++ b/beacon_chain/spec/state_transition_block.nim @@ -206,49 +206,58 @@ func is_slashable_attestation_data( (data_1.source.epoch < data_2.source.epoch and data_2.target.epoch < data_1.target.epoch) -# https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#attester-slashings -proc processAttesterSlashings(state: var BeaconState, blck: BeaconBlock, - stateCache: var StateCache): bool = - # Process ``AttesterSlashing`` operation. - if len(blck.body.attester_slashings) > MAX_ATTESTER_SLASHINGS: - notice "CaspSlash: too many!" - return false - - result = true - for attester_slashing in blck.body.attester_slashings: +# https://github.com/ethereum/eth2.0-specs/blob/v0.8.3/specs/core/0_beacon-chain.md#attester-slashings +proc process_attester_slashing*( + state: var BeaconState, + attester_slashing: AttesterSlashing, + stateCache: var StateCache + ): bool = let attestation_1 = attester_slashing.attestation_1 attestation_2 = attester_slashing.attestation_2 if not is_slashable_attestation_data( attestation_1.data, attestation_2.data): - notice "CaspSlash: surround or double vote check failed" + notice "Attester slashing: surround or double vote check failed" return false if not is_valid_indexed_attestation(state, attestation_1): - notice "CaspSlash: invalid votes 1" + notice "Attester slashing: invalid attestation 1" return false if not is_valid_indexed_attestation(state, attestation_2): - notice "CaspSlash: invalid votes 2" + notice "Attester slashing: invalid attestation 2" return false - var slashed_any = false + var slashed_any = false # Detect if trying to slash twice ## TODO there's a lot of sorting/set construction here and ## verify_indexed_attestation, but go by spec unless there ## is compelling perf evidence otherwise. - let attesting_indices_1 = - attestation_1.custody_bit_0_indices & attestation_1.custody_bit_1_indices - let attesting_indices_2 = - attestation_2.custody_bit_0_indices & attestation_2.custody_bit_1_indices + let + attesting_indices_1 = attestation_1.custody_bit_0_indices & attestation_1.custody_bit_1_indices + attesting_indices_2 = attestation_2.custody_bit_0_indices & attestation_2.custody_bit_1_indices for index in sorted(toSeq(intersection(toSet(attesting_indices_1), toSet(attesting_indices_2)).items), system.cmp): - if is_slashable_validator(state.validators[index.int], - get_current_epoch(state)): + if is_slashable_validator(state.validators[index.int], get_current_epoch(state)): slash_validator(state, index.ValidatorIndex, stateCache) slashed_any = true - result = result and slashed_any + if not slashed_any: + notice "Attester slashing: Trying to slash participant(s) twice" + return false + return true + +proc processAttesterSlashings(state: var BeaconState, blck: BeaconBlock, + stateCache: var StateCache): bool = + # Process ``AttesterSlashing`` operation. + if len(blck.body.attester_slashings) > MAX_ATTESTER_SLASHINGS: + notice "Attester slashing: too many!" + return false + + for attester_slashing in blck.body.attester_slashings: + if not process_attester_slashing(state, attester_slashing, stateCache): + return false + return true func get_attesting_indices( state: BeaconState, attestations: openarray[PendingAttestation], diff --git a/tests/official/all_fixtures_require_ssz.nim b/tests/official/all_fixtures_require_ssz.nim index 293541b7e..fa6a29a7a 100644 --- a/tests/official/all_fixtures_require_ssz.nim +++ b/tests/official/all_fixtures_require_ssz.nim @@ -15,4 +15,5 @@ import ./test_fixture_state_transition_epoch, ./test_fixture_operations_attestations, ./test_fixture_operations_block_header, - ./test_fixture_operations_voluntary_exit + ./test_fixture_operations_voluntary_exit, + ./test_fixture_operations_attester_slashings diff --git a/tests/official/test_fixture_operations_attester_slashings.nim b/tests/official/test_fixture_operations_attester_slashings.nim new file mode 100644 index 000000000..59341dfc7 --- /dev/null +++ b/tests/official/test_fixture_operations_attester_slashings.nim @@ -0,0 +1,86 @@ +# beacon_chain +# Copyright (c) 2018-Present Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at http://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at http://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import + # Standard library + os, unittest, strutils, + # Beacon chain internals + ../../beacon_chain/spec/[datatypes, state_transition_block, validator], + ../../beacon_chain/[ssz, extras], + # Test utilities + ../testutil, + ./fixtures_utils, + ../helpers/debug_state + +const OpAttSlashingDir = SszTestsDir/const_preset/"phase0"/"operations"/"attester_slashing"/"pyspec_tests" + +template runTest(identifier: untyped) = + # We wrap the tests in a proc to avoid running out of globals + # in the future: Nim supports up to 3500 globals + # but unittest with the macro/templates put everything as globals + # https://github.com/nim-lang/Nim/issues/12084#issue-486866402 + + const testDir = OpAttSlashingDir / astToStr(identifier) + + proc `testImpl _ operations_attester_slashing _ identifier`() = + + var flags: UpdateFlags + var prefix: string + if not existsFile(testDir/"meta.yaml"): + flags.incl skipValidation + if existsFile(testDir/"post.ssz"): + prefix = "[Valid] " + else: + prefix = "[Invalid] " + + test prefix & astToStr(identifier): + var stateRef, postRef: ref BeaconState + var attesterSlashingRef: ref AttesterSlashing + new attesterSlashingRef + new stateRef + + var cache = get_empty_per_epoch_cache() + + attesterSlashingRef[] = parseTest(testDir/"attester_slashing.ssz", SSZ, AttesterSlashing) + stateRef[] = parseTest(testDir/"pre.ssz", SSZ, BeaconState) + + if existsFile(testDir/"post.ssz"): + new postRef + postRef[] = parseTest(testDir/"post.ssz", SSZ, BeaconState) + + if postRef.isNil: + let done = process_attester_slashing(stateRef[], attesterSlashingRef[], cache) + doAssert done == false, "We didn't expect this invalid attester slashing to be processed." + else: + let done = process_attester_slashing(stateRef[], attesterSlashingRef[], cache) + doAssert done, "Valid attestater slashing not processed" + check: stateRef.hash_tree_root() == postRef.hash_tree_root() + reportDiff(stateRef, postRef) + + `testImpl _ operations_attester_slashing _ identifier`() + +suite "Official - Operations - Attester slashing " & preset(): + runTest(success_double) + runTest(success_surround) + runTest(success_already_exited_recent) + runTest(success_already_exited_long_ago) + runTest(invalid_sig_1) + when false: # TODO - https://github.com/status-im/nim-beacon-chain/issues/429 + runTest(invalid_sig_2) + runTest(invalid_sig_1_and_2) + runTest(same_data) + runTest(no_double_or_surround) + runTest(participants_already_slashed) + runTest(custody_bit_0_and_1_intersect) + when false: # TODO - https://github.com/status-im/nim-beacon-chain/issues/429 + runTest(att1_bad_extra_index) + runTest(att1_bad_replaced_index) + runTest(att2_bad_extra_index) + runTest(att2_bad_replaced_index) + runTest(unsorted_att_1_bit0) + runTest(unsorted_att_2_bit0) +