Slashing protection v2 would fail interchange tests under Nim 1.2.10

[tersec]

diff --git a/beacon_chain/validators/slashing_protection_v2.nim b/beacon_chain/validators/slashing_protection_v2.nim
index 74a50051..26599740 100644
--- a/beacon_chain/validators/slashing_protection_v2.nim
+++ b/beacon_chain/validators/slashing_protection_v2.nim
@@ -991,6 +991,10 @@ proc pruneAttestations*(
   ## Prune all blocks from a validator before the specified newMinSlot
   ## This is intended for interchange import.
   let valID = db.getOrRegisterValidator(validator)
+
+  doAssert newMinSourceEpoch <= high(int64).uint64
+  doAssert newMinTargetEpoch <= high(int64).uint64
+
   let status = db.sqlPruneValidatorAttestations.exec(
     (valID, int64 newMinSourceEpoch, int64 newMinTargetEpoch))
   doAssert status.isOk(),

interchange_test_failures.txt

This came out of #2392.

It's part of #2366 and blocks updating to Nim 1.2.10 because nim-lang/Nim#15210.

[mratsim]

It seems like the failure happens here on line 995

nimbus-eth2/beacon_chain/validators/slashing_protection_v2.nim

Lines 984 to 1005 in c0d8ecd

	proc registerAttestation*(
	db: SlashingProtectionDB_v2,
	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.
	let valID = db.getOrRegisterValidator(validator)
	# Overflows in 14 trillion years (minimal) or 112 trillion years (mainnet)
	doAssert source <= high(int64).uint64
	doAssert target <= high(int64).uint64
	let status = db.sqlInsertAtt.exec(
	(valID, int64 source, int64 target,
	attestation_root.data))
	doAssert status.isOk(),
	"SQLite error when registering attestation: " & $status.error & "\n" &
	"for validator: 0x" & validator.toHex() &
	", sourceEpoch: " & $source &
	", targetEpoch: " & $target

For example for the input [FAILED] Slashing test: single_validator_slashable_blocks_no_root.json

{
  "name": "single_validator_slashable_blocks_no_root",
  "genesis_validators_root": "0x0000000000000000000000000000000000000000000000000000000000000000",
  "steps": [
    {
      "should_succeed": true,
      "allow_partial_import": true,
      "interchange": {
        "metadata": {
          "interchange_format_version": "5",
          "genesis_validators_root": "0x0000000000000000000000000000000000000000000000000000000000000000"
        },
        "data": [
          {
            "pubkey": "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c",
            "signed_blocks": [
              {
                "slot": "10"
              },
              {
                "slot": "10"
              }
            ],
            "signed_attestations": []
          }
        ]
      },
      "blocks": [],
      "attestations": []
    }
  ]
}

This is called here when deserializing the interchange format

nimbus-eth2/beacon_chain/validators/slashing_protection_common.nim

Lines 399 to 445 in c0d8ecd

	# Attestations
	# ---------------------------------------------------
	# After import we need to prune the DB from everything
	# besides the last imported attestation source and target epochs.
	# This ensures that even if 2 slashing DB are imported in the wrong order
	# (the last before the earliest) the minEpochViolation check stays consistent.
	var maxValidSourceEpochSeen = -1
	var maxValidTargetEpochSeen = -1
	for a in 0 ..< spdir.data[v].signed_attestations.len:
	template A: untyped = spdir.data[v].signed_attestations[a]
	let status = db.checkSlashableAttestation(
	parsedKey,
	A.source_epoch.Epoch,
	A.target_epoch.Epoch
	)
	if status.isErr():
	# We might be importing a duplicate which EIP-3076 allows
	# there is no reason during normal operation to integrate
	# a duplicate so checkSlashableAttestation would have rejected it.
	# We special-case that for imports.
	if status.error.kind == DoubleVote and
	A.signing_root.Eth2Digest != ZeroDigest and
	status.error.existingAttestation == A.signing_root.Eth2Digest:
	warn "Attestation already exists in the DB",
	pubkey = spdir.data[v].pubkey.PubKeyBytes.toHex(),
	candidateAttestation = A
	continue
	else:
	error "Slashable vote. Skipping its import.",
	pubkey = spdir.data[v].pubkey.PubKeyBytes.toHex(),
	candidateAttestation = A,
	conflict = status.error()
	result = siPartial
	continue
	if A.source_epoch.int > maxValidSourceEpochSeen:
	maxValidSourceEpochSeen = A.source_epoch.int
	if A.target_epoch.int > maxValidTargetEpochSeen:
	maxValidTargetEpochSeen = A.target_epoch.int
	db.registerAttestation(
	parsedKey,
	A.source_epoch.Epoch,
	A.target_epoch.Epoch,
	A.signing_root.Eth2Digest
	)

but signed_attestation is empty so it shouldn't have been called at all

[mratsim]

Ah it seems like the line 995 in the attachment throw me off.

So the reason why there is a failure is because I use -1 as a special value here

 var maxValidSourceEpochSeen = -1 
 var maxValidTargetEpochSeen = -1 

And if there is no attestation, this special value ends up here

nimbus-eth2/beacon_chain/validators/slashing_protection_common.nim

Lines 447 to 449 in c0d8ecd

	# Now prune everything that predates
	# this interchange file max slot
	db.pruneAttestations(parsedKey, Epoch maxValidSourceEpochSeen, Epoch maxValidTargetEpochSeen)

[mratsim]

Closed in #2395