Skip to content

Commit a73abb4

Browse files
pedromiguelmirandapmmiranda
andauthored
Use ordered table for attestation packing (#7053)
* added chain aggregates cache tests * in order to pack electra chained attestations, nimbus scores and aggregates in an ordered fashion to ease the packing of these attestations. However, after the scored and aggregate, a nimbus table (not ordered) was being used, and such, not producing optimal chain aggregates. Solution arise from the use of an ordered table. * review fixes --------- Co-authored-by: pmmiranda <[email protected]>
1 parent eec3f17 commit a73abb4

File tree

3 files changed

+66
-8
lines changed

3 files changed

+66
-8
lines changed

AllTests-mainnet.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ AllTests-mainnet
99
+ Aggregated attestations with disjoint comittee bits into a single on-chain aggregate [Pres OK
1010
+ Aggregating across committees [Preset: mainnet] OK
1111
+ Attestations with disjoint comittee bits and equal data into single on-chain aggregate [Pr OK
12+
+ Cache coherence on chain aggregates [Preset: mainnet] OK
1213
+ Can add and retrieve simple electra attestations [Preset: mainnet] OK
14+
+ Simple add and get with electra nonzero committee [Preset: mainnet] OK
1315
+ Working with electra aggregates [Preset: mainnet] OK
14-
+ simple add and get with electra nonzero committee [Preset: mainnet] OK
1516
```
1617
## Attestation pool processing [Preset: mainnet]
1718
```diff

beacon_chain/consensus_object_pools/attestation_pool.nim

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ type
102102
onPhase0AttestationAdded: OnPhase0AttestationCallback
103103
onSingleAttestationAdded: OnSingleAttestationCallback
104104

105+
CandidateKey = tuple
106+
## Search key for selecting the final candidates for
107+
## electra chain aggregates.
108+
hash: Eth2Digest
109+
slot: Slot
110+
105111
logScope: topics = "attpool"
106112

107113
declareGauge attestation_pool_block_attestation_packing_time,
@@ -1005,7 +1011,7 @@ proc getElectraAttestationsForBlock*(
10051011
# For each round, we'll look for the best attestation and add it to the result
10061012
# then re-score the other candidates.
10071013
var
1008-
candidatesPerBlock: Table[(Eth2Digest, Slot), seq[electra.Attestation]]
1014+
candidatesPerBlock: OrderedTable[CandidateKey, seq[electra.Attestation]]
10091015

10101016
let totalCandidates = candidates.len()
10111017
while candidates.len > 0 and candidatesPerBlock.lenu64() <
@@ -1027,13 +1033,10 @@ proc getElectraAttestationsForBlock*(
10271033
var e2 = entry.data
10281034
e2.index = 0
10291035
e2
1030-
key = (hash_tree_root(entry2), entry.data.slot)
1036+
key = (hash_tree_root(entry2), entry.data.slot.Slot)
10311037
newAtt = entry[].toElectraAttestation(entry[].aggregates[j])
10321038

1033-
candidatesPerBlock.withValue(key, candidate):
1034-
candidate[].add newAtt
1035-
do:
1036-
candidatesPerBlock[key] = @[newAtt]
1039+
candidatesPerBlock.mGetOrPut(key, @[]).add(newAtt)
10371040

10381041
# Update cache so that the new votes are taken into account when updating
10391042
# the score below

tests/test_attestation_pool.nim

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1186,7 +1186,7 @@ suite "Attestation pool electra processing" & preset():
11861186
pool[].verifyAttestationSignature(state, cache, attestations[0])
11871187
pool[].verifyAttestationSignature(state, cache, attestations[1])
11881188

1189-
test "simple add and get with electra nonzero committee" & preset():
1189+
test "Simple add and get with electra nonzero committee" & preset():
11901190
let
11911191
bc0 = get_beacon_committee(
11921192
state[], getStateField(state[], slot), 0.CommitteeIndex, cache)
@@ -1219,3 +1219,57 @@ suite "Attestation pool electra processing" & preset():
12191219
0.CommitteeIndex).isOk
12201220
pool[].getElectraAggregatedAttestation(1.Slot, hash_tree_root(attestation_2.data),
12211221
1.CommitteeIndex).isOk
1222+
1223+
test "Cache coherence on chain aggregates" & preset():
1224+
# Add attestation from different committee
1225+
var maxSlot = 0.Slot
1226+
1227+
for i in 0 ..< 4:
1228+
let
1229+
bc = get_beacon_committee(
1230+
state[], getStateField(state[], slot), i.CommitteeIndex, cache)
1231+
att = makeElectraAttestation(
1232+
state[], state[].latest_block_root, bc[0], cache)
1233+
var att2 = makeElectraAttestation(
1234+
state[], state[].latest_block_root, bc[1], cache)
1235+
1236+
pool[].addAttestation(
1237+
att, @[bc[0]], att.aggregation_bits.len, att.loadSig,
1238+
att.data.slot.start_beacon_time)
1239+
1240+
if att.data.slot < 2:
1241+
pool[].addAttestation(
1242+
att2, @[bc[1]], att2.aggregation_bits.len, att2.loadSig,
1243+
att2.data.slot.start_beacon_time)
1244+
1245+
if att.data.slot > maxSlot:
1246+
maxSlot = att.data.slot
1247+
1248+
check process_slots(
1249+
defaultRuntimeConfig, state[],
1250+
maxSlot + MIN_ATTESTATION_INCLUSION_DELAY, cache,
1251+
info, {}).isOk()
1252+
1253+
let attestations = pool[].getElectraAttestationsForBlock(state[], cache)
1254+
check:
1255+
## Considering that all structures in getElectraAttestationsForBlock
1256+
## are sorted, the most relevant should be at sequence head.
1257+
## Given the attestations added, the most "scored" is on
1258+
## slot 1
1259+
attestations.len() == 2
1260+
1261+
attestations[0].aggregation_bits.countOnes() == 4
1262+
attestations[0].committee_bits.countOnes() == 2
1263+
attestations[0].data.slot == 1.Slot
1264+
1265+
1266+
attestations[1].aggregation_bits.countOnes() == 2
1267+
attestations[1].committee_bits.countOnes() == 2
1268+
attestations[1].data.slot == 2.Slot
1269+
1270+
check_attestation(
1271+
state[].electraData.data, attestations[0], {}, cache, true).isOk
1272+
check_attestation(
1273+
state[].electraData.data, attestations[1], {}, cache, true).isOk
1274+
pool[].verifyAttestationSignature(state, cache, attestations[0])
1275+
pool[].verifyAttestationSignature(state, cache, attestations[1])

0 commit comments

Comments
 (0)