Fix non-split attesting protection (#1804)

* address #1679 - att protection in non-split mode

* AttachedValidator public_key -> pubkey

* Always build with slashing protection

* Remove now unnecessary forwarding impl proc

Jenkinsfile

@@ -38,9 +38,9 @@ def runStages() {
 						sh """#!/bin/bash
 						set -e
 						make -j${env.NPROC} V=1
-						make -j${env.NPROC} V=1 LOG_LEVEL=TRACE NIMFLAGS='-d:UseSlashingProtection=true -d:testnet_servers_image' beacon_node
+						make -j${env.NPROC} V=1 LOG_LEVEL=TRACE NIMFLAGS='-d:testnet_servers_image' beacon_node
             # Miracl fallback
-            # make -j${env.NPROC} V=1 LOG_LEVEL=TRACE NIMFLAGS='-d:BLS_FORCE_BACKEND=miracl -d:UseSlashingProtection=true -d:testnet_servers_image' beacon_node
+            # make -j${env.NPROC} V=1 LOG_LEVEL=TRACE NIMFLAGS='-d:BLS_FORCE_BACKEND=miracl -d:testnet_servers_image' beacon_node
 						"""
 					}
 				},
@@ -52,7 +52,6 @@ def runStages() {
 						// EXECUTOR_NUMBER will be 0 or 1, since we have 2 executors per Jenkins node
 						sh """#!/bin/bash
 						set -e
-            export NIMFLAGS='-d:UseSlashingProtection=true'
 						./scripts/launch_local_testnet.sh --testnet 0 --nodes 4 --stop-at-epoch 5 --log-level DEBUG --disable-htop --enable-logtrace --data-dir local_testnet0_data --base-port \$(( 9000 + EXECUTOR_NUMBER * 100 )) --base-rpc-port \$(( 7000 + EXECUTOR_NUMBER * 100 )) --base-metrics-port \$(( 8008 + EXECUTOR_NUMBER * 100 )) -- --verify-finalization --discv5:no
 						./scripts/launch_local_testnet.sh --testnet 1 --nodes 4 --stop-at-epoch 5 --log-level DEBUG --disable-htop --enable-logtrace --data-dir local_testnet1_data --base-port \$(( 9000 + EXECUTOR_NUMBER * 100 )) --base-rpc-port \$(( 7000 + EXECUTOR_NUMBER * 100 )) --base-metrics-port \$(( 8008 + EXECUTOR_NUMBER * 100 )) -- --verify-finalization --discv5:no
 						"""

beacon_chain.nimble

@@ -73,7 +73,7 @@ task test, "Run all tests":
   # Mainnet config
   buildAndRunBinary "proto_array", "beacon_chain/fork_choice/", """-d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
   buildAndRunBinary "fork_choice", "beacon_chain/fork_choice/", """-d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
-  buildAndRunBinary "all_tests", "tests/", """-d:UseSlashingProtection=true -d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
+  buildAndRunBinary "all_tests", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:chronicles_sinks="json[file]""""
 
   # Check Miracl/Milagro fallback on select tests
   buildAndRunBinary "test_interop", "tests/", """-d:chronicles_log_level=TRACE -d:const_preset=mainnet -d:BLS_FORCE_BACKEND=miracl -d:chronicles_sinks="json[file]""""

beacon_chain/beacon_node.nim

@@ -269,10 +269,7 @@ proc init*(T: type BeaconNode,
   res.attachedValidators = ValidatorPool.init(
     SlashingProtectionDB.init(
       chainDag.headState.data.data.genesis_validators_root,
-      when UseSlashingProtection:
-        kvStore SqStoreRef.init(conf.validatorsDir(), "slashing_protection").tryGet()
-      else:
-        KvStoreRef()
+      kvStore SqStoreRef.init(conf.validatorsDir(), "slashing_protection").tryGet()
     )
   )
 

beacon_chain/validator_client.nim

@@ -299,12 +299,11 @@ programMain:
       vc.beaconGenesis = waitFor vc.client.get_v1_beacon_genesis()
       vc.beaconClock = BeaconClock.init(vc.beaconGenesis.genesis_time)
 
-    when UseSlashingProtection:
-      vc.attachedValidators.slashingProtection =
-        SlashingProtectionDB.init(
-          vc.beaconGenesis.genesis_validators_root,
-          kvStore SqStoreRef.init(config.validatorsDir(), "slashing_protection").tryGet()
-        )
+    vc.attachedValidators.slashingProtection =
+      SlashingProtectionDB.init(
+        vc.beaconGenesis.genesis_validators_root,
+        kvStore SqStoreRef.init(config.validatorsDir(), "slashing_protection").tryGet()
+      )
 
     let
       curSlot = vc.beaconClock.now().slotOrZero()

beacon_chain/validator_duties.nim

@@ -399,6 +399,18 @@ proc handleAttestations(node: BeaconNode, head: BlockRef, slot: Slot) =
                              a.data.target.epoch)
 
     if notSlashable.isOk():
+      # TODO signing_root is recomputed in produceAndSignAttestation/signAttestation just after
+      let signing_root = compute_attestation_root(
+            fork, genesis_validators_root, a.data)
+      node.attachedValidators
+          .slashingProtection
+          .registerAttestation(
+            a.validator.pubkey,
+            a.data.source.epoch,
+            a.data.target.epoch,
+            signing_root
+          )
+
       traceAsyncErrors createAndSendAttestation(
         node, fork, genesis_validators_root, a.validator, a.data,
         a.committeeLen, a.indexInCommittee, num_active_validators)

beacon_chain/validator_slashing_protection.nim

@@ -216,13 +216,6 @@ type
 logScope:
   topics = "antislash"
 
-const UseSlashingProtection* {.booldefine.} = true
-
-when UseSlashingProtection:
-  static: echo "  Built with slashing protection"
-else:
-  static: echo "  Built without slashing protection"
-
 func subkey(
        kind: static SlashingKeyKind,
        validator: ValID,
@@ -346,18 +339,16 @@ proc init*(
        T: type SlashingProtectionDB,
        genesis_validator_root: Eth2Digest,
        backend: KVStoreRef): SlashingProtectionDB =
-  when UseSlashingProtection:
-    result = T(backend: backend)
-    result.setGenesis(genesis_validator_root)
+  result = T(backend: backend)
+  result.setGenesis(genesis_validator_root)
 
 proc close*(db: SlashingProtectionDB) =
-  when UseSlashingProtection:
-    discard db.backend.close()
+  discard db.backend.close()
 
 # DB Queries
 # --------------------------------------------
 
-proc checkSlashableBlockProposalImpl(
+proc checkSlashableBlockProposal*(
        db: SlashingProtectionDB,
        validator: ValidatorPubKey,
        slot: Slot
@@ -378,26 +369,7 @@ proc checkSlashableBlockProposalImpl(
     return ok()
   return err(foundBlock.unsafeGet().block_root)
 
-proc checkSlashableBlockProposal*(
-       db: SlashingProtectionDB,
-       validator: ValidatorPubKey,
-       slot: Slot
-     ): Result[void, Eth2Digest] =
-  ## Returns an error if the specified validator
-  ## already proposed a block for the specified slot.
-  ## This would lead to slashing.
-  ## The error contains the blockroot that was already proposed
-  ##
-  ## Returns success otherwise
-  # TODO distinct type for the result block root
-  when UseSlashingProtection:
-    checkSlashableBlockProposalImpl(
-      db, validator, slot
-    )
-  else:
-    ok()
-
-proc checkSlashableAttestationImpl(
+proc checkSlashableAttestation*(
        db: SlashingProtectionDB,
        validator: ValidatorPubKey,
        source: Epoch,
@@ -532,26 +504,6 @@ proc checkSlashableAttestationImpl(
 
   doAssert false, "Unreachable"
 
-proc checkSlashableAttestation*(
-       db: SlashingProtectionDB,
-       validator: ValidatorPubKey,
-       source: Epoch,
-       target: Epoch
-     ): Result[void, BadVote] =
-  ## Returns an error if the specified validator
-  ## already proposed a block for the specified slot.
-  ## This would lead to slashing.
-  ## The error contains the blockroot that was already proposed
-  ##
-  ## Returns success otherwise
-  # TODO distinct type for the result attestation root
-  when UseSlashingProtection:
-    checkSlashableAttestationImpl(
-      db, validator, source, target
-    )
-  else:
-    ok()
-
 # DB update
 # --------------------------------------------
 
@@ -571,7 +523,7 @@ proc registerValidator(db: SlashingProtectionDB, validator: ValidatorPubKey) =
 
   db.put(subkey(kValidator, valIndex), validator)
 
-proc registerBlockImpl(
+proc registerBlock*(
        db: SlashingProtectionDB,
        validator: ValidatorPubKey,
        slot: Slot, block_root: Eth2Digest) =
@@ -707,21 +659,7 @@ proc registerBlockImpl(
       # ).expect("Consistent linked-list in DB")
     ).unsafeGet()
 
-proc registerBlock*(
-       db: SlashingProtectionDB,
-       validator: ValidatorPubKey,
-       slot: Slot, block_root: Eth2Digest) =
-  ## Add a block to the slashing protection DB
-  ## `checkSlashableBlockProposal` MUST be run
-  ## before to ensure no overwrite.
-  when UseSlashingProtection:
-    registerBlockImpl(
-      db, validator, slot, block_root
-    )
-  else:
-    discard
-
-proc registerAttestationImpl(
+proc registerAttestation*(
        db: SlashingProtectionDB,
        validator: ValidatorPubKey,
        source, target: Epoch,
@@ -870,21 +808,6 @@ proc registerAttestationImpl(
       # ).expect("Consistent linked-list in DB")
     ).unsafeGet()
 
-proc registerAttestation*(
-       db: SlashingProtectionDB,
-       validator: ValidatorPubKey,
-       source, target: Epoch,
-       attestation_root: Eth2Digest) =
-  ## Add an attestation to the slashing protection DB
-  ## `checkSlashableAttestation` MUST be run
-  ## before to ensure no overwrite.
-  when UseSlashingProtection:
-    registerAttestationImpl(
-      db, validator, source, target, attestation_root
-    )
-  else:
-    discard
-
 # Debug tools
 # --------------------------------------------
 

tests/slashing_protection/test_slashing_interchange.nim

@@ -20,8 +20,6 @@ import
   # Test utilies
   ../testutil
 
-static: doAssert UseSlashingProtection, "The test was compiled without slashing protection, pass -d:UseSlashingProtection=true"
-
 template wrappedTimedTest(name: string, body: untyped) =
   # `check` macro takes a copy of whatever it's checking, on the stack!
   block: # Symbol namespacing

tests/slashing_protection/test_slashing_protection_db.nim

@@ -19,8 +19,6 @@ import
   # Test utilies
   ../testutil
 
-static: doAssert UseSlashingProtection, "The test was compiled without slashing protection, pass -d:UseSlashingProtection=true"
-
 template wrappedTimedTest(name: string, body: untyped) =
   # `check` macro takes a copy of whatever it's checking, on the stack!
   block: # Symbol namespacing