fix for get_ptc iterator

Author: Tomi-3-0

AllTests-mainnet.md

@@ -836,6 +836,15 @@ AllTests-mainnet
 ```diff
 + pre-1.1.0                                                                                  OK
 ```
+## Payload attestation pool [Preset: mainnet]
+```diff
++ Can add and retrieve payload attestations [Preset: mainnet]                                OK
++ Can get payload attestations for block production [Preset: mainnet]                        OK
++ Different payload presence values [Preset: mainnet]                                        OK
++ Duplicate validator in PTC - multiple signatures [Preset: mainnet]                         OK
++ Multiple validators in PTC can attest [Preset: mainnet]                                    OK
++ Payload attestations get pruned [Preset: mainnet]                                          OK
+```
 ## PeerPool testing suite
 ```diff
 + Access peers by key test                                                                   OK

beacon_chain/consensus_object_pools/payload_attestation_pool.nim

@@ -16,7 +16,7 @@ import
   "."/[spec_cache, blockchain_dag],
   ../beacon_clock
 
-from ../spec/beaconstate import get_ptc
+from ../spec/beaconstate import get_ptc_list
 
 logScope: topics = "payattpool"
 
@@ -33,7 +33,7 @@ type
     dag*: ChainDAGRef
     attestations: Table[Slot, Table[Eth2Digest, PayloadAttestationEntry]]
 
-proc init*(T: type PayloadAttestationPool, dag: ChainDAGRef): T =
+func init*(T: type PayloadAttestationPool, dag: ChainDAGRef): T =
   T(dag: dag)
 
 func pruneOldEntries(pool: var PayloadAttestationPool, wallTime: BeaconTime) =
@@ -77,47 +77,29 @@ proc addPayloadAttestation*(
 
   true
 
-func findAllPtcPositions(
-    ptc: seq[ValidatorIndex], validator_index: ValidatorIndex
-): seq[int] =
-  # Find all positions where validator appears in the PTC
-  var positions: seq[int]
-  for i, ptc_member in ptc:
-    if ptc_member == validator_index:
-      positions.add(i)
-  positions
-
 proc aggregateMessages(
     pool: PayloadAttestationPool, slot: Slot,
-    entry: ptr PayloadAttestationEntry, cache: var StateCache
+    entry: var PayloadAttestationEntry, cache: var StateCache
 ): Opt[PayloadAttestation] =
-  ## Aggregate individual messages into a single PayloadAttestation
 
-  if entry[].messages.len == 0:
+  if entry.messages.len == 0:
     return Opt.none(PayloadAttestation)
 
   withState(pool.dag.headState):
     when consensusFork >= ConsensusFork.Gloas:
-      var ptc = newSeqOfCap[ValidatorIndex](PTC_SIZE)
-      for validator_index in get_ptc(forkyState.data, slot, cache):
-        ptc.add(validator_index)
-
       var
-        aggregation_bits = BitArray[int(PTC_SIZE)].init()
+        aggregation_bits: BitArray[int(PTC_SIZE)]
         signatures: seq[CookedSig]
+        ptc_index = 0
 
-      for validator_index, message in entry.messages:
-        # Find all positions where this validator appears in PTC,
-        # a single member might appear multiple times in a committee
-        let ptc_positions = findAllPtcPositions(ptc, validator_index)
-
-        let cookedSig = message.signature.load().valueOr:
-          continue
-
-        # set the aggregation bits and add the signature for each position
-        for ptc_index in ptc_positions:
+      let ptc_list = get_ptc_list(forkyState.data, slot, cache)
+      for ptc_validator_index in ptc_list:
+        entry.messages.withValue(ptc_validator_index, message):
+          let cookedSig = message[].signature.load().valueOr:
+            continue
           aggregation_bits[ptc_index] = true
           signatures.add(cookedSig)
+        ptc_index += 1
 
       if signatures.len == 0:
         return Opt.none(PayloadAttestation)
@@ -126,13 +108,13 @@ proc aggregateMessages(
       for i in 1..<signatures.len:
         aggregated_signature.aggregate(signatures[i])
 
-      return Opt.some(PayloadAttestation(
+      Opt.some(PayloadAttestation(
         aggregation_bits: aggregation_bits,
         data: entry.data,
         signature: aggregated_signature.finish().toValidatorSig()
       ))
     else:
-      return Opt.none(PayloadAttestation)
+      Opt.none(PayloadAttestation)
 
 proc getAggregatedPayloadAttestation*(
     pool: var PayloadAttestationPool, slot: Slot,
@@ -143,7 +125,7 @@ proc getAggregatedPayloadAttestation*(
   pool.attestations.withValue(slot, slotEntries):
     slotEntries[].withValue(beacon_block_root, entry):
       if entry[].aggregated.isNone():
-        entry[].aggregated = pool.aggregateMessages(slot, entry, cache)
+        entry[].aggregated = pool.aggregateMessages(slot, entry[], cache)
       return entry[].aggregated
 
   Opt.none(PayloadAttestation)

beacon_chain/spec/beaconstate.nim

@@ -2761,7 +2761,7 @@ func can_advance_slots*(
   withState(state): forkyState.can_advance_slots(block_root, target_slot)
 
 # https://github.com/ethereum/consensus-specs/blob/v1.6.0-alpha.6/specs/gloas/beacon-chain.md#new-get_ptc
-iterator get_ptc*(state: gloas.BeaconState, slot: Slot, cache: var StateCache):
+iterator get_ptc(state: gloas.BeaconState, slot: Slot, cache: var StateCache):
     ValidatorIndex =
   ## Get the payload timeliness committee for the given ``slot``
   let epoch = slot.epoch()
@@ -2782,6 +2782,12 @@ iterator get_ptc*(state: gloas.BeaconState, slot: Slot, cache: var StateCache):
       state, indices, seed, size=PTC_SIZE, shuffle_indices=false):
     yield candidate_index
 
+func get_ptc_list*(state: gloas.BeaconState, slot: Slot,
+    cache: var StateCache): seq[ValidatorIndex] =
+  # workaround for https://github.com/nim-lang/Nim/issues/25287
+  # TODO: Remove if issue gets fixed and use iterator directly
+  toSeq(get_ptc(state, slot, cache))
+
 # https://github.com/ethereum/consensus-specs/blob/v1.6.0-alpha.6/specs/gloas/beacon-chain.md#new-get_indexed_payload_attestation
 func get_indexed_payload_attestation*(
     state: gloas.BeaconState, slot: Slot,

tests/test_payload_attestation_pool.nim

@@ -16,12 +16,12 @@ import
   ../beacon_chain/consensus_object_pools/[
     blockchain_dag, payload_attestation_pool, spec_cache],
   ../beacon_chain/spec/[
-    datatypes/gloas, forks, helpers, signatures, state_transition],
+    forks, helpers, signatures, state_transition],
   ../beacon_chain/beacon_clock,
   # Test utilities
-  ./testutil, ./testdbutil, ./testblockutil
+  ./testutil, ./testdbutil, ./testblockutil, ./consensus_spec/fixtures_utils
 
-from ../beacon_chain/spec/beaconstate import get_ptc
+from ../beacon_chain/spec/beaconstate import get_ptc_list
 
 proc makePayloadAttestationMessage(
     state: gloas.HashedBeaconState,
@@ -30,7 +30,8 @@ proc makePayloadAttestationMessage(
     privkey: ValidatorPrivKey,
     cache: var StateCache,
     payload_present: bool = true,
-    blob_data_available: bool = true): PayloadAttestationMessage =
+    blob_data_available: bool = true
+  ): PayloadAttestationMessage =
 
   let
     slot = state.data.slot
@@ -58,22 +59,29 @@ proc makePayloadAttestationMessage(
 
 suite "Payload attestation pool" & preset():
   setup:
+    # Genesis state that results in 512 members in a committee
     let rng = HmacDrbgContext.new()
+    const TOTAL_COMMITTEES = 1
     var
-      cfg = defaultRuntimeConfig
+      cfg = genesisTestRuntimeConfig(ConsensusFork.Gloas)
       validatorMonitor = newClone(ValidatorMonitor.init(cfg.timeParams))
       dag = init(
         ChainDAGRef, cfg,
-        cfg.makeTestDB(128),
+        cfg.makeTestDB(
+          TOTAL_COMMITTEES * PTC_SIZE),
         validatorMonitor, {})
-      pool = PayloadAttestationPool.init(dag)
+      pool = newClone(PayloadAttestationPool.init(dag))
       state = newClone(dag.headState)
       cache = StateCache()
       info = ForkedEpochInfo()
-
     check:
-      process_slots(dag.cfg, state[], getStateField(state[], slot) + 2, cache,
-        info, {}).isOk()
+      process_slots(
+        dag.cfg,
+        state[],
+        getStateField(state[], slot) + 1,
+        cache,
+        info,
+        {}).isOk()
 
   test "Can add and retrieve payload attestations" & preset():
     let
@@ -85,10 +93,10 @@ suite "Payload attestation pool" & preset():
       when consensusFork >= ConsensusFork.Gloas:
         var ptc_member: ValidatorIndex
         var found = false
-        for validator_index in get_ptc(forkyState.data, slot, cache):
-          ptc_member = validator_index
-          found = true
-          break
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
+            ptc_member = validator_index
+            found = true
+            break
 
         check found
 
@@ -97,13 +105,14 @@ suite "Payload attestation pool" & preset():
           message = makePayloadAttestationMessage(
             forkyState, beacon_block_root, ptc_member, privkey, cache)
 
-        check pool.addPayloadAttestation(message, wallTime)
+        check pool[].addPayloadAttestation(message, wallTime)
 
         # Should not be able to add the same attestation twice
-        check not pool.addPayloadAttestation(message, wallTime)
+        check not pool[].addPayloadAttestation(message, wallTime)
+
+        let aggregated = pool[].getAggregatedPayloadAttestation(
+            slot, beacon_block_root, cache)
 
-        let aggregated = pool.getAggregatedPayloadAttestation(
-          slot, beacon_block_root, cache)
         check aggregated.isSome()
         check aggregated.get().data == message.data
         check aggregated.get().aggregation_bits.countOnes() > 0
@@ -119,8 +128,7 @@ suite "Payload attestation pool" & preset():
         var messages: seq[PayloadAttestationMessage]
         var ptc_members: seq[ValidatorIndex]
 
-        # Get multiple validators from PTC
-        for validator_index in get_ptc(forkyState.data, slot, cache):
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
           if ptc_members.len >= 3:
             break
           ptc_members.add(validator_index)
@@ -133,9 +141,9 @@ suite "Payload attestation pool" & preset():
             message = makePayloadAttestationMessage(
               forkyState, beacon_block_root, ptc_member, privkey, cache)
           messages.add(message)
-          check pool.addPayloadAttestation(message, wallTime)
+          check pool[].addPayloadAttestation(message, wallTime)
 
-        let aggregated = pool.getAggregatedPayloadAttestation(
+        let aggregated = pool[].getAggregatedPayloadAttestation(
           slot, beacon_block_root, cache)
         check aggregated.isSome()
         check aggregated.get().aggregation_bits.countOnes() >= ptc_members.len
@@ -148,18 +156,20 @@ suite "Payload attestation pool" & preset():
 
     withState(state[]):
       when consensusFork >= ConsensusFork.Gloas:
-        var validator_positions: Table[ValidatorIndex, seq[int]]
-        var ptc_index = 0
+        var
+          validator_positions: Table[ValidatorIndex, seq[int]]
+          ptc_index = 0
 
-        for validator_index in get_ptc(forkyState.data, slot, cache):
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
           if validator_index notin validator_positions:
             validator_positions[validator_index] = @[]
           validator_positions[validator_index].add(ptc_index)
           ptc_index += 1
 
-        var multi_position_validator: ValidatorIndex
-        var positions: seq[int]
-        var found = false
+        var
+          multi_position_validator: ValidatorIndex
+          positions: seq[int]
+          found = false
 
         for validator_index, position_list in validator_positions:
           if position_list.len > 1:
@@ -174,9 +184,9 @@ suite "Payload attestation pool" & preset():
             message = makePayloadAttestationMessage(
               forkyState, beacon_block_root, multi_position_validator, privkey, cache)
 
-          check pool.addPayloadAttestation(message, wallTime)
+          check pool[].addPayloadAttestation(message, wallTime)
 
-          let aggregated = pool.getAggregatedPayloadAttestation(
+          let aggregated = pool[].getAggregatedPayloadAttestation(
             slot, beacon_block_root, cache)
           check aggregated.isSome()
 
@@ -194,17 +204,17 @@ suite "Payload attestation pool" & preset():
     withState(state[]):
       when consensusFork >= ConsensusFork.Gloas:
         var added_count = 0
-        for validator_index in get_ptc(forkyState.data, slot, cache):
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
           if added_count >= 2:
             break
           let
             privkey = MockPrivKeys[validator_index]
             message = makePayloadAttestationMessage(
               forkyState, beacon_block_root, validator_index, privkey, cache)
-          check pool.addPayloadAttestation(message, wallTime)
+          check pool[].addPayloadAttestation(message, wallTime)
           added_count += 1
 
-        let attestations = pool.getPayloadAttestationsForBlock(target_slot, cache)
+        let attestations = pool[].getPayloadAttestationsForBlock(target_slot, cache)
         check attestations.len > 0
         check attestations[0].data.slot == slot
 
@@ -213,12 +223,12 @@ suite "Payload attestation pool" & preset():
       slot = getStateField(state[], slot)
       beacon_block_root = withState(state[]): hash_tree_root(forkyState.data.latest_block_header)
       wallTime = slot.start_beacon_time(dag.cfg.timeParams)
-      future_time = (slot + 5).start_beacon_time(dag.cfg.timeParams) # Much later
+      future_time = (slot + 5).start_beacon_time(dag.cfg.timeParams)
 
     withState(state[]):
       when consensusFork >= ConsensusFork.Gloas:
         var ptc_member: ValidatorIndex
-        for validator_index in get_ptc(forkyState.data, slot, cache):
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
           ptc_member = validator_index
           break
 
@@ -228,13 +238,13 @@ suite "Payload attestation pool" & preset():
             forkyState, beacon_block_root, ptc_member, privkey, cache)
 
         # Add attestation
-        check pool.addPayloadAttestation(message, wallTime)
+        check pool[].addPayloadAttestation(message, wallTime)
 
-        # Add another attestation at a future time (should automatically prune old ones)
-        check pool.addPayloadAttestation(message, future_time)
+        # Add another attestation at a future time (should trigger pruning)
+        check pool[].addPayloadAttestation(message, future_time)
 
         # Old attestation should no longer be retrievable
-        let attestations = pool.getPayloadAttestationsForBlock(slot + 6, cache)
+        let attestations = pool[].getPayloadAttestationsForBlock(slot + 6, cache)
         check attestations.len == 0
 
   test "Different payload presence values" & preset():
@@ -246,7 +256,7 @@ suite "Payload attestation pool" & preset():
     withState(state[]):
       when consensusFork >= ConsensusFork.Gloas:
         var ptc_members: seq[ValidatorIndex]
-        for validator_index in get_ptc(forkyState.data, slot, cache):
+        for validator_index in get_ptc_list(forkyState.data, slot, cache):
           if ptc_members.len >= 2:
             break
           ptc_members.add(validator_index)
@@ -263,10 +273,9 @@ suite "Payload attestation pool" & preset():
             MockPrivKeys[ptc_members[1]], cache,
             payload_present = false, blob_data_available = false)
 
-        check pool.addPayloadAttestation(message1, wallTime)
-        check pool.addPayloadAttestation(message2, wallTime)
+        check pool[].addPayloadAttestation(message1, wallTime)
+        check pool[].addPayloadAttestation(message2, wallTime)
 
         let
-          agg1 = pool.getAggregatedPayloadAttestation(slot, beacon_block_root, cache)
-
+          agg1 = pool[].getAggregatedPayloadAttestation(slot, beacon_block_root, cache)
         check agg1.isSome()