properly use Electra attestations in Electra attestation tests (#7551)

Author: tersec

beacon_chain/consensus_object_pools/attestation_pool.nim

@@ -680,9 +680,10 @@ iterator electraAttestations*(
 
 type
   AttestationCacheKey = (Slot, uint64)
-  AttestationCache[CVBType] = Table[AttestationCacheKey, CVBType] ##\
-    ## Cache for quick lookup during beacon block construction of attestations
-    ## which have already been included, and therefore should be skipped.
+  AttestationCache =
+    Table[AttestationCacheKey, ElectraCommitteeValidatorsBits] ##\
+      ## Cache for quick lookup during beacon block construction of attestations
+      ## which have already been included, and therefore should be skipped.
 
 func getAttestationCacheKey(ad: AttestationData): AttestationCacheKey =
   # The committee is unique per slot and committee index which means we can use
@@ -691,7 +692,7 @@ func getAttestationCacheKey(ad: AttestationData): AttestationCacheKey =
 
 func add(
     attCache: var AttestationCache, data: AttestationData,
-    aggregation_bits: CommitteeValidatorsBits | ElectraCommitteeValidatorsBits) =
+    aggregation_bits: ElectraCommitteeValidatorsBits) =
   let key = data.getAttestationCacheKey()
   attCache.withValue(key, v) do:
     v[].incl(aggregation_bits)
@@ -754,8 +755,8 @@ func score(
 func check_attestation_compatible*(
     dag: ChainDAGRef,
     state: ForkyHashedBeaconState,
-    attestation: SomeAttestation | electra.Attestation |
-                 electra.TrustedAttestation): Result[void, cstring] =
+    attestation: electra.Attestation | electra.TrustedAttestation):
+    Result[void, cstring] =
   let
     targetEpoch = attestation.data.target.epoch
     compatibleRoot = state.dependent_root(targetEpoch.get_previous_epoch)
@@ -806,7 +807,7 @@ proc getAttestationsForBlock*(
     candidates: seq[tuple[
       score: int, slot: Slot, entry: ptr ElectraAttestationEntry,
       validation: int]]
-    attCache = AttestationCache[ElectraCommitteeValidatorsBits].init(state, cache)
+    attCache = AttestationCache.init(state, cache)
 
   for i in 0..<ATTESTATION_LOOKBACK:
     if i > maxAttestationSlot: # Around genesis..
@@ -868,8 +869,7 @@ proc getAttestationsForBlock*(
   #
   # For each round, we'll look for the best attestation and add it to the result
   # then re-score the other candidates.
-  var
-    candidatesPerBlock: OrderedTable[CandidateKey, seq[electra.Attestation]]
+  var candidatesPerBlock: OrderedTable[CandidateKey, seq[electra.Attestation]]
 
   let totalCandidates = candidates.len()
   while candidates.len > 0 and candidatesPerBlock.lenu64() <
@@ -974,9 +974,7 @@ func getElectraAggregatedAttestation*(
     pool: var AttestationPool, slot: Slot,
     attestationDataRoot: Eth2Digest, committeeIndex: CommitteeIndex):
     Opt[electra.Attestation] =
-
-  let
-    candidateIdx = pool.candidateIdx(slot, CandidateIdxType.electraIdx)
+  let candidateIdx = pool.candidateIdx(slot, CandidateIdxType.electraIdx)
   if candidateIdx.isNone:
     return Opt.none(electra.Attestation)
 

tests/test_attestation_pool.nim

@@ -175,16 +175,6 @@ suite "Attestation pool processing" & preset():
         check_attestation(forkyState.data, att1, flags, cache2).isOk
         check_attestation(forkyState.data, att2, flags, cache2).isOk
 
-    # An additional compatibility check catches that (used in block production)
-    withState(state[]):
-      check:
-        check_attestation_compatible(dag, forkyState, att1).isOk
-        check_attestation_compatible(dag, forkyState, att2).isErr
-    withState(state2[]):
-      check:
-        check_attestation_compatible(dag, forkyState, att1).isErr
-        check_attestation_compatible(dag, forkyState, att2).isOk
-
   test "Can add and retrieve simple attestations" & preset():
     let
       # Create an attestation for slot 1!
@@ -468,8 +458,6 @@ suite "Attestation pool processing" & preset():
             # signature: ValidatorSig()
           )
 
-      cache = StateCache()
-
     # -------------------------------------------------------------
     # Prune
 
@@ -569,13 +557,13 @@ suite "Attestation pool electra processing" & preset():
           slot = getStateField(state[], slot)
           parent_root = withState(state[]): forkyState.latest_block_root
           committee = get_beacon_committee(state[], slot, cIndex, cache)
-        makeAttestation(state[], parent_root, committee[0], cache)
+        makeElectraAttestation(state[], parent_root, committee[0], cache)
       att2 = block:
         let
           slot = getStateField(state2[], slot)
           parent_root = withState(state2[]): forkyState.latest_block_root
           committee = get_beacon_committee(state2[], slot, cIndex, cache2)
-        makeAttestation(state2[], parent_root, committee[0], cache2)
+        makeElectraAttestation(state2[], parent_root, committee[0], cache2)
       maxSlot = max(att1.data.slot, att2.data.slot)
 
     # Advance time so attestations become valid
@@ -589,24 +577,28 @@ suite "Attestation pool electra processing" & preset():
 
     # They should remain valid only within a compatible state
     withState(state[]):
-      check:
-        check_attestation(forkyState.data, att1, {}, cache).isOk
-        check_attestation(forkyState.data, att2, {}, cache).isErr
+      when consensusFork >= ConsensusFork.Electra:
+        check:
+          check_attestation(forkyState.data, att1, {}, cache, true).isOk
+          check_attestation(forkyState.data, att2, {}, cache, true).isErr
     withState(state2[]):
-      check:
-        check_attestation(forkyState.data, att1, {}, cache2).isErr
-        check_attestation(forkyState.data, att2, {}, cache2).isOk
+      when consensusFork >= ConsensusFork.Electra:
+        check:
+          check_attestation(forkyState.data, att1, {}, cache2, true).isErr
+          check_attestation(forkyState.data, att2, {}, cache2, true).isOk
 
     # If signature checks are skipped, state incompatibility is not detected
-    let flags = {skipBlsValidation}
+    const flags = {skipBlsValidation}
     withState(state[]):
-      check:
-        check_attestation(forkyState.data, att1, flags, cache).isOk
-        check_attestation(forkyState.data, att2, flags, cache).isOk
+      when consensusFork >= ConsensusFork.Electra:
+        check:
+          check_attestation(forkyState.data, att1, flags, cache, true).isOk
+          check_attestation(forkyState.data, att2, flags, cache, true).isOk
     withState(state2[]):
-      check:
-        check_attestation(forkyState.data, att1, flags, cache2).isOk
-        check_attestation(forkyState.data, att2, flags, cache2).isOk
+      when consensusFork >= ConsensusFork.Electra:
+        check:
+          check_attestation(forkyState.data, att1, flags, cache2, true).isOk
+          check_attestation(forkyState.data, att2, flags, cache2, true).isOk
 
     # An additional compatibility check catches that (used in block production)
     withState(state[]):