Attester slashing - split process_attester_slashing/processAttesterSlashings + tests (#424)

* attester slashing tests - pending #415

* split process_attester_slashing/processAttesterSlashings

* Add logs to attester_slashing

* deactivate bls tests for now (https://github.com/status-im/nim-beacon-chain/issues/429) and cherry-pick from 60f2437dd2

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,

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

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],

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

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)
+