Slashing prot interchange tests v5.2.1 (#3277)

* initial support for minification and new interchange tests. Removal of v1 and v1 migration.

* Synthetic attestations: SQLite3 requires one statement/query per prepared statement

* Fix DB import interrupted if no attestation was found

* Skip test relying on undocumented test behavior (https://github.com/eth-clients/slashing-protection-interchange-tests/pull/12#issuecomment-1011158701)

* Skip test relying on unclear minification behavior:
creating an invalid minified attestation with source > target or setting target = max(source, target)

* remove DB v1 and update submodule

* Apply suggestions from code review

Co-authored-by: Jacek Sieka <jacek@status.im>

Co-authored-by: Jacek Sieka <jacek@status.im>

beacon_chain/validators/slashing_protection.nim

@@ -17,7 +17,6 @@ import
   # Internal
   ../spec/datatypes/base,
   ./slashing_protection_common,
-  ./slashing_protection_v1,
   ./slashing_protection_v2
 
 export slashing_protection_common, kvstore, kvstore_sqlite3
@@ -90,36 +89,13 @@ proc init*(
   result.db_v2 = db
 
   if requiresMigration:
-    var db_v1: SlashingProtectionDB_v1
+    fatal "The slashing database predates Altair hardfork from October 2021." &
-    let rawdb = kvstore result.db_v2.getRawDBHandle().openKvStore().get()
+      " You can migrate to the new DB format using Nimbus 1.6.0" & 
-    if not rawdb.checkOrPutGenesis_DbV1(genesis_validators_root):
+      " for a few minutes at https://github.com/status-im/nimbus-eth2/releases/tag/v1.6.0" &
-      fatal "The slashing database refers to another chain/mainnet/testnet",
+      " until the messages \"Migrating local validators slashing DB from v1 to v2\"" &
-        path = basePath/dbname,
+      " and \"Slashing DB migration successful.\""
-        genesis_validators_root = genesis_validators_root
-      quit 1
-    db_v1.fromRawDB(rawdb)
 
-    info "Migrating local validators slashing DB from v1 to v2"
+    quit 1
-    let spdir = try: db_v1.toSPDIR_lowWatermark()
-    except IOError as exc:
-      fatal "Cannot migrate v1 database", err = exc.msg
-      quit 1
-
-    let status = try: result.db_v2.inclSPDIR(spdir)
-    except CatchableError as exc:
-      fatal "Writing DB v2 failed", err = exc.msg
-      quit 1
-
-    case status
-    of siSuccess:
-      info "Slashing DB migration successful."
-    of siPartial:
-      warn "Slashing DB migration is a partial success."
-    of siFailure:
-      fatal "Slashing DB migration failure. Aborting to protect validators."
-      quit 1
-
-    db_v1.close()
 
 proc init*(
        T: type SlashingProtectionDB,
@@ -276,6 +252,9 @@ proc pruneAfterFinalization*(
     debug.logTime "Pruning slashing DB":
       db.db_v2.pruneAfterFinalization(finalizedEpoch)
 
+# Interchange
+# --------------------------------------------
+
 # The high-level import/export functions are
 # - importSlashingInterchange
 # - exportSlashingInterchange
@@ -284,6 +263,11 @@ proc pruneAfterFinalization*(
 # That builds on a DB backend inclSPDIR and toSPDIR
 # SPDIR being a common Intermediate Representation
 
+proc registerSyntheticAttestation*(db: SlashingProtectionDB,
+       validator: ValidatorPubKey,
+       source, target: Epoch) =
+  db.db_v2.registerSyntheticAttestation(validator, source, target)
+
 proc inclSPDIR*(db: SlashingProtectionDB, spdir: SPDIR): SlashingImportStatus
              {.raises: [SerializationError, IOError, Defect].} =
   db.db_v2.inclSPDIR(spdir)

beacon_chain/validators/slashing_protection_common.nim

@@ -375,7 +375,38 @@ proc importInterchangeV5Impl*(
             status.error.existingAttestation == A.signing_root.Eth2Digest:
           warn "Attestation already exists in the DB",
             pubkey = spdir.data[v].pubkey.PubKeyBytes.toHex(),
-            candidateAttestation = A
+            candidateAttestation = A,
+            conflict = status.error()
+          continue
+
+        elif status.error.kind == SurroundVote:
+          doAssert A.source_epoch.Epoch == status.error.sourceSlashable
+          doAssert A.target_epoch.Epoch == status.error.targetSlashable
+
+          # Formal proof of correctness: https://github.com/michaelsproul/slashing-proofs
+          let synth = SPDIR_SignedAttestation(
+            source_epoch: EpochString max(status.error.sourceSlashable, status.error.sourceExisting),
+            target_epoch: EpochString max(status.error.targetSlashable, status.error.targetExisting)
+          )
+
+          warn "Slashable surround vote. Constructing a synthetic attestation to reconcile DB and import",
+            pubkey = spdir.data[v].pubkey.PubKeyBytes.toHex(),
+            candidateAttestation = A,
+            conflict = status.error(),
+            syntheticAttestation = synth
+          
+          db.registerSyntheticAttestation(
+            parsedKey,
+            synth.source_epoch.Epoch,
+            synth.target_epoch.Epoch
+          )
+
+          if synth.source_epoch.int > maxValidSourceEpochSeen:
+            maxValidSourceEpochSeen = synth.source_epoch.int
+          if synth.target_epoch.int > maxValidTargetEpochSeen:
+            maxValidTargetEpochSeen = synth.target_epoch.int
+
+          result = siPartial
           continue
         else:
           error "Slashable vote. Skipping its import.",
@@ -394,6 +425,7 @@ proc importInterchangeV5Impl*(
     # this interchange file max slot
     if maxValidSourceEpochSeen < 0 or maxValidTargetEpochSeen < 0:
       doAssert maxValidSourceEpochSeen == -1 and maxValidTargetEpochSeen == -1
-      notice "No attestation found in slashing interchange file"
+      notice "No attestation found in slashing interchange file for validator",
-      return
+        pubkey = spdir.data[v].pubkey.PubKeyBytes.toHex()
+      continue
     db.pruneAttestations(parsedKey, maxValidSourceEpochSeen, maxValidTargetEpochSeen)

beacon_chain/validators/slashing_protection_v2.nim

@@ -208,6 +208,10 @@ type
     sqlPruneValidatorAttestations: SqliteStmt[(ValidatorInternalID, int64, int64), void]
     sqlPruneAfterFinalizationBlocks: SqliteStmt[int64, void]
     sqlPruneAfterFinalizationAttestations: SqliteStmt[(int64, int64), void]
+    # Synthetic attestations
+    sqlBeginTransaction: SqliteStmt[NoParams, void]
+    sqlDeleteValidatorAtt: SqliteStmt[ValidatorInternalID, void]
+    sqlCommitTransaction: SqliteStmt[NoParams, void]
     # Cached queries - read
     sqlGetValidatorInternalID: SqliteStmt[PubKeyBytes, ValidatorInternalID]
     sqlAttForSameTargetEpoch: SqliteStmt[(ValidatorInternalID, int64), Hash32]
@@ -568,6 +572,24 @@ proc setupCachedQueries(db: SlashingProtectionDB_v2) =
      """, (int64, int64), void
   ).get()
 
+  # Synthetic attestation
+  # --------------------------------------------------------
+
+  # Assuming pruning, we can:
+  # - keep 1 attestation
+  # - 2 attestations, with max source epoch and different target epoch
+  #   for example 10->15 and 10->20 (unique constraint on target but not source epoch)
+  # - many attestations post-finalization epochs
+  # Creating or updating a source/target epoch synthetic attestation
+  # might introduce duplicates or run afoul of slashing conditions
+  # so it's easier to cleanup and introduce a max source/target epoch synthetic attestation
+  db.sqlBeginTransaction = db.backend.prepareStmt("""BEGIN TRANSACTION;""", NoParams, void).get()
+  db.sqlDeleteValidatorAtt = db.backend.prepareStmt("""
+    DELETE FROM signed_attestations
+    where validator_id = ?;
+  """, ValidatorInternalID, void).get()
+  db.sqlCommitTransaction = db.backend.prepareStmt("""COMMIT TRANSACTION;""", NoParams, void).get()
+
 # DB Multiversioning
 # -------------------------------------------------------------
 
@@ -1131,6 +1153,7 @@ proc registerAttestation*(
        attestation_root: Eth2Digest): Result[void, BadVote] =
   registerAttestation(
     db, none(ValidatorIndex), validator, source, target, attestation_root)
+
 # DB maintenance
 # --------------------------------------------
 proc pruneBlocks*(
@@ -1212,6 +1235,40 @@ proc pruneAfterFinalization*(
 # Interchange
 # --------------------------------------------
 
+proc registerSyntheticAttestation*(
+       db: SlashingProtectionDB_v2,
+       validator: ValidatorPubKey,
+       source, target: Epoch) =
+  ## Add a synthetic attestation to the slashing protection DB
+  doAssert source < target
+
+  let valID = db.getOrRegisterValidator(none(ValidatorIndex), validator)
+
+  # Overflows in 14 trillion years (minimal) or 112 trillion years (mainnet)
+  doAssert source <= high(int64).uint64
+  doAssert target <= high(int64).uint64
+
+  template checkStatus: untyped =
+    doAssert status.isOk(),
+      "SQLite error when synthesizing an attestation: " & $status.error & "\n" &
+      "for validatorID " & $valID & " (0x" & $validator & ")\n" &
+      "sourceEpoch: " & $source & ", targetEpoch:" & $target & '\n'
+
+  block:
+    let status = db.sqlBeginTransaction.exec()
+    checkStatus()
+  block:
+    let status = db.sqlDeleteValidatorAtt.exec(valID)
+    checkStatus()
+  block:
+    let status = db.sqlInsertAtt.exec(
+      (valID, int64 source, int64 target,
+      Eth2Digest().data))
+    checkStatus()
+  block:
+    let status = db.sqlCommitTransaction.exec()
+    checkStatus()
+
 proc toSPDIR*(db: SlashingProtectionDB_v2): SPDIR
              {.raises: [IOError, Defect].} =
   ## Export the full slashing protection database

tests/all_tests.nim

@@ -40,8 +40,7 @@ import # Unit test
   ./consensus_spec/all_tests as consensus_all_tests,
   ./slashing_protection/test_fixtures,
   ./slashing_protection/test_slashing_interchange,
-  ./slashing_protection/test_slashing_protection_db,
+  ./slashing_protection/test_slashing_protection_db
-  ./slashing_protection/test_migration
 
 import # Refactor state transition unit tests
   # In mainnet these take 2 minutes and are empty TODOs

tests/slashing_protection/test_fixtures.nim

@@ -33,9 +33,18 @@ type
   TestStep = object
     should_succeed: bool
       ## Is "interchange" given a valid import
-    allow_partial_import: bool
+    contains_slashable_data: bool
       ## Does "interchange" contain slashable data either as standalone
       ## or with regards to previous steps
+      ## If contains_slashable_data is false, then the given interchange must be imported
+      ## successfully, and the given block/attestation checks must pass.
+      ## If contains_slashable_data is true, then implementations have the option to do one of two
+      ## things:
+      ##     - Import the interchange successfully, working around the slashable data by minification
+      ##       or some other mechanism. If the import succeeds, all checks must pass and the test
+      ##       should continue to the next step.
+      ##     - Reject the interchange (or partially import it), in which case the block/attestation
+      ##       checks and all future steps should be ignored.
     interchange: SPDIR
     blocks: seq[CandidateBlock]
       ## Blocks to try as proposer after DB is imported
@@ -79,7 +88,7 @@ proc sqlite3db_delete(basepath, dbname: string) =
   removeFile(basepath / dbname&".sqlite3-wal")
   removeFile(basepath / dbname&".sqlite3")
 
-const InterchangeTestsDir = FixturesDir / "tests-slashing-v5.0.0" / "tests" / "generated"
+const InterchangeTestsDir = FixturesDir / "tests-slashing-v5.2.1" / "tests" / "generated"
 const TestDir = ""
 const TestDbPrefix = "test_slashprot_"
 
@@ -88,7 +97,7 @@ proc statusOkOrDuplicateOrMinSlotViolation(
   # 1. We might be importing a duplicate which EIP-3076 allows
   #    there is no reason during normal operation to integrate
   #    a duplicate so checkSlashableBlockProposal would have rejected it.
-  # 2. The last test "multiple_interchanges_single_validator_single_message_gap"
+  # 2. The test "multiple_interchanges_single_validator_single_message_gap"
   #    requires implementing pruning in-between import to keep the
   #    MinSlotViolation check relevant.
   #    That check prevents duplicate because it doesn't keep history.
@@ -106,7 +115,7 @@ proc statusOkOrDuplicateOrMinSlotViolation(
     # Note: we tested the codepath without pruning.
     # Furthermore it's better to be to eager on MinSlotViolation
     # than allow slashing (unless the MinSlot is too far in the future)
-    warn "Block violates low watermark requirement. It's likely a duplicate though.",
+    warn "Block violates low watermark requirement. It might be an already pruned block.",
       candidateBlock = candidate,
       error = status.error
     return true
@@ -130,7 +139,7 @@ proc statusOkOrDuplicateOrMinEpochViolation(
     # Note: we tested the codepath without pruning.
     # Furthermore it's better to be to eager on MinSlotViolation
     # than allow slashing (unless the MinSlot is too far in the future)
-    warn "Attestation violates low watermark requirement. It's likely a duplicate though.",
+    warn "Attestation violates low watermark requirement. It might be an already pruned attestation.",
       candidateAttestation = candidate,
       error = status.error
     return true
@@ -141,76 +150,87 @@ proc runTest(identifier: string) =
   # The tests produce a lot of log noise
   # echo "\n\n===========================================\n\n"
 
-  test "Slashing test: " & identifier:
+  let t = parseTest(InterchangeTestsDir/identifier, Json, TestInterchange)
-    let t = parseTest(InterchangeTestsDir/identifier, Json, TestInterchange)
 
-    # Create a test specific DB
+  # Create a test specific DB
-    let dbname = TestDbPrefix & identifier.changeFileExt("")
+  let dbname = TestDbPrefix & identifier.changeFileExt("")
 
-    # Delete existing db in case of previous test failure
+  # Delete existing db in case of previous test failure
-    sqlite3db_delete(TestDir, dbname)
+  sqlite3db_delete(TestDir, dbname)
 
-    let db = SlashingProtectionDB.init(
+  let db = SlashingProtectionDB.init(
-      Eth2Digest t.genesis_validators_root,
+    Eth2Digest t.genesis_validators_root,
-      TestDir,
+    TestDir,
-      dbname
+    dbname
-    )
+  )
-    # We don't use defer to auto-close+delete the DB
+  # We don't use defer to auto-close+delete the DB
-    # as in case of issue we want to keep the DB around for investigation.
+  # as in case of issue we want to keep the DB around for investigation.
 
-    for step in t.steps:
+  for step in t.steps:
-      let status = db.inclSPDIR(step.interchange)
+    let status = db.inclSPDIR(step.interchange)
-      if not step.should_succeed:
+    if not step.should_succeed:
-        doAssert siFailure == status,
+      doAssert siFailure == status,
+        "Unexpected error:\n" &
+        "    " & $status & "\n"
+    elif step.contains_slashable_data:
+      doAssert siPartial == status,
+        "Unexpected error:\n" &
+        "    " & $status & "\n"
+    else:
+      doAssert siSuccess == status,
+        "Unexpected error:\n" &
+        "    " & $status & "\n"
+
+    for blck in step.blocks:
+      let status = db.db_v2.checkSlashableBlockProposal(none(ValidatorIndex),
+        ValidatorPubKey.fromRaw(blck.pubkey.PubKeyBytes).get(),
+        Slot blck.slot
+      )
+      if blck.should_succeed:
+        doAssert status.statusOkOrDuplicateOrMinSlotViolation(blck),
           "Unexpected error:\n" &
-          "    " & $status & "\n"
+          "    " & $status & "\n" &
-      elif step.allow_partial_import:
+          "    for " & $toHexLogs(blck)
-        doAssert siPartial == status,
-          "Unexpected error:\n" &
-          "    " & $status & "\n"
       else:
-        doAssert siSuccess == status,
+        doAssert status.isErr(),
+          "Unexpected success:\n" &
+          "    status: " & $status & "\n" &
+          "    for " & $toHexLogs(blck)
+
+    for att in step.attestations:
+      let status = db.db_v2.checkSlashableAttestation(none(ValidatorIndex),
+        ValidatorPubKey.fromRaw(att.pubkey.PubKeyBytes).get(),
+        Epoch att.source_epoch,
+        Epoch att.target_epoch
+      )
+      if att.should_succeed:
+        doAssert status.statusOkOrDuplicateOrMinEpochViolation(att),
           "Unexpected error:\n" &
-          "    " & $status & "\n"
+          "    " & $status & "\n" &
+          "    for " & $toHexLogs(att)
+      else:
+        doAssert status.isErr(),
+          "Unexpected success:\n" &
+          "    " & $status & "\n" &
+          "    for " & $toHexLogs(att)
 
-      for blck in step.blocks:
+  # Now close and delete resources.
-        let status = db.db_v2.checkSlashableBlockProposal(none(ValidatorIndex),
+  db.close()
-          ValidatorPubKey.fromRaw(blck.pubkey.PubKeyBytes).get(),
+  sqlite3db_delete(TestDir, dbname)
-          Slot blck.slot
-        )
-        if blck.should_succeed:
-          doAssert status.statusOkOrDuplicateOrMinSlotViolation(blck),
-            "Unexpected error:\n" &
-            "    " & $status & "\n" &
-            "    for " & $toHexLogs(blck)
-        else:
-          doAssert status.isErr(),
-            "Unexpected success:\n" &
-            "    " & $status & "\n" &
-            "    for " & $toHexLogs(blck)
-
-      for att in step.attestations:
-        let status = db.db_v2.checkSlashableAttestation(none(ValidatorIndex),
-          ValidatorPubKey.fromRaw(att.pubkey.PubKeyBytes).get(),
-          Epoch att.source_epoch,
-          Epoch att.target_epoch
-        )
-        if att.should_succeed:
-          doAssert status.statusOkOrDuplicateOrMinEpochViolation(att),
-            "Unexpected error:\n" &
-            "    " & $status & "\n" &
-            "    for " & $toHexLogs(att)
-        else:
-          doAssert status.isErr(),
-            "Unexpected success:\n" &
-            "    " & $status & "\n" &
-            "    for " & $toHexLogs(att)
-
-    # Now close and delete resources.
-    db.close()
-    sqlite3db_delete(TestDir, dbname)
 
 suite "Slashing Interchange tests " & preset():
   for kind, path in walkDir(
       InterchangeTestsDir, relative = true, checkDir = true):
-    runTest(path)
+    test "Slashing test: " & path:
+
+      if path == "multiple_interchanges_single_validator_multiple_blocks_out_of_order.json":
+        # TODO: test relying on undocumented behavior (if signing a test block is possible import it)
+        #       https://github.com/eth-clients/slashing-protection-interchange-tests/pull/12#issuecomment-1011158701
+        skip()
+      elif path == "single_validator_source_greater_than_target_surrounding.json":
+        # TODO: test relying on unclear minification behavior:
+        #       creating an invalid minified attestation with source > target
+        #       or setting target = max(source, target)
+        skip()
+      else:
+        runTest(path)

tests/slashing_protection/test_migration.nim

@@ -1,95 +0,0 @@
-# Nimbus
-# Copyright (c) 2018-2021 Status Research & Development GmbH
-# Licensed under either of
-#  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or https://www.apache.org/licenses/LICENSE-2.0)
-#  * MIT license ([LICENSE-MIT](LICENSE-MIT) or https://opensource.org/licenses/MIT)
-# at your option. This file may not be copied, modified, or distributed except according to those terms.
-
-{.used.}
-
-import
-  # Standard library
-  std/[os],
-  # Status lib
-  eth/db/[kvstore, kvstore_sqlite3],
-  stew/results,
-  nimcrypto/utils,
-  serialization,
-  json_serialization,
-  # Internal
-  ../../beacon_chain/validators/[
-    slashing_protection,
-    slashing_protection_v1
-  ],
-  ../../beacon_chain/spec/datatypes/base,
-  # Test utilies
-  ../testutil
-
-func hexToDigest(hex: string): Eth2Digest =
-  Eth2Digest.fromHex(hex)
-
-proc sqlite3db_delete(basepath, dbname: string) =
-  removeFile(basepath / dbname&".sqlite3-shm")
-  removeFile(basepath / dbname&".sqlite3-wal")
-  removeFile(basepath / dbname&".sqlite3")
-
-const TestDir = ""
-const TestDbName = "t_slashprot_migration"
-
-suite "Slashing Protection DB - v1 and v2 migration" & preset():
-  # https://eips.ethereum.org/EIPS/eip-3076
-  sqlite3db_delete(TestDir, TestDbName)
-
-  test "Minimal format migration" & preset():
-    let genesis_validators_root = hexToDigest"0x04700007fabc8282644aed6d1c7c9e21d38a03a0c4ba193f3afe428824b3a673"
-    block: # export from a v1 DB
-      let db = SlashingProtectionDB_v1.init(
-                genesis_validators_root,
-                TestDir,
-                TestDbName
-              )
-
-      let pubkey = ValidatorPubKey
-                    .fromHex"0xb845089a1457f811bfc000588fbb4e713669be8ce060ea6be3c6ece09afc3794106c91ca73acda5e5457122d58723bed"
-                    .get()
-      check:
-        db.registerBlock(
-          pubkey,
-          Slot 81952,
-          Eth2Digest()
-        ).isOk()
-
-        db.registerAttestation(
-          pubkey,
-          source = Epoch 2290,
-          target = Epoch 3007,
-          Eth2Digest()
-        ).isOk()
-
-      let spdir = db.toSPDIR_lowWatermark()
-      Json.saveFile(
-        currentSourcePath.parentDir/"t_migration_slashing_protection_v1.json",
-        spdir,
-        pretty = true
-      )
-
-      db.close()
-
-    block: # Reopen as the new version
-      let db = SlashingProtectionDB.init(
-                genesis_validators_root,
-                TestDir,
-                TestDbName
-              )
-
-      # Check that v2 as been initialized (private field :/)
-      # doAssert: db.db_v2.getMetadataTable_DbV2().get() == genesis_validators_root
-
-      db.exportSlashingInterchange(
-        currentSourcePath.parentDir/"t_migration_slashing_protection_migrated.json"
-      )
-
-      doAssert sameFileContent(
-        currentSourcePath.parentDir/"t_migration_slashing_protection_v1.json",
-        currentSourcePath.parentDir/"t_migration_slashing_protection_migrated.json"
-      )

vendor/nim-eth2-scenarios

@@ -1 +1 @@
-Subproject commit 1074fab55d882b6a2179c878f3f5daa24e1add94
+Subproject commit 00b5327e268c41d922c1dd992c9d821755cf47a5