fix: select correct gossip type when publishing single attestation #7256

nflaig · 2024-11-28T09:41:33Z

Motivation

We are not selecting the correct type to serialize single attestation when publishing to gossip

Nov-28 09:00:03.241[api]             error: Error on submitPoolAttestations [0] slot=94, index=0 - Cannot read properties of undefined (reading 'bitLen')
TypeError: Cannot read properties of undefined (reading 'bitLen')
    at BitListType.value_serializedSize (/usr/app/node_modules/@chainsafe/ssz/src/type/bitList.ts:60:43)
    at ContainerType.value_serializedSize (/usr/app/node_modules/@chainsafe/ssz/src/type/container.ts:189:54)
    at ContainerType.serialize (/usr/app/node_modules/@chainsafe/ssz/src/type/abstract.ts:119:44)
    at Network.publishGossip (file:///usr/app/packages/beacon-node/src/network/network.ts:421:34)
    at Network.publishBeaconAttestation (file:///usr/app/packages/beacon-node/src/network/network.ts:333:17)
    at file:///usr/app/packages/beacon-node/src/api/impl/beacon/pool/index.ts:137:45
    at async Promise.all (index 0)
    at Object.submitPoolAttestationsV2 (file:///usr/app/packages/beacon-node/src/api/impl/beacon/pool/index.ts:99:7)
    at Object.<anonymous> (file:///usr/app/packages/api/src/utils/server/handler.ts:105:22)

Description

Select correct gossip type when publishing single attestation, instead of electra.Attestation, we need to use electra.SingleAttestation

twoeths · 2024-11-28T09:55:23Z

packages/beacon-node/src/network/gossip/topic.ts

@@ -89,7 +89,7 @@ export function getGossipSSZType(topic: GossipTopic) {
    case GossipType.beacon_aggregate_and_proof:
      return sszTypesFor(topic.fork).SignedAggregateAndProof;
    case GossipType.beacon_attestation:
-      return sszTypesFor(topic.fork).Attestation;
+      return isForkPostElectra(topic.fork) ? ssz.electra.SingleAttestation : ssz.phase0.Attestation;


could also be return sszTypesFor(topic.fork).SingleAttestation;?

For this to work we would have to define SingleAttestation in phase0 types

lodestar/packages/types/src/sszTypes.ts

Line 18 in 48dea55

[ForkName.phase0]: {...phase0},

Otherwise it does not exist in all forks

we can add it as an alias though

I think the alias makes sense as it matches with what we did in types

lodestar/packages/types/src/types.ts

Line 59 in 72d3448

SingleAttestation: phase0.Attestation;

codecov · 2024-11-28T09:58:37Z

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 48.44%. Comparing base (1a74884) to head (72d3448).
Report is 1 commits behind head on devnet-5.

Additional details and impacted files

@@            Coverage Diff            @@
##           devnet-5    #7256   +/-   ##
=========================================
  Coverage     48.43%   48.44%           
=========================================
  Files           602      602           
  Lines         40320    40321    +1     
  Branches       2057     2057           
=========================================
+ Hits          19531    19532    +1     
  Misses        20751    20751           
  Partials         38       38

twoeths

lgtm

…7256) * fix: select correct gossip type when publishing single attestation * Add SingleAttestation as alias to phase0 ssz types

fix: select correct gossip type when publishing single attestation

272c30b

nflaig requested a review from a team as a code owner November 28, 2024 09:41

twoeths approved these changes Nov 28, 2024

View reviewed changes

twoeths reviewed Nov 28, 2024

View reviewed changes

Add SingleAttestation as alias to phase0 ssz types

72d3448

twoeths approved these changes Nov 28, 2024

View reviewed changes

nflaig merged commit cfe6c89 into devnet-5 Nov 28, 2024
17 checks passed

nflaig deleted the nflaig/fix-gossip-publish branch November 28, 2024 10:54

nflaig added a commit that referenced this pull request Nov 29, 2024

fix: select correct gossip type when publishing single attestation (#…

4501e08

…7256) * fix: select correct gossip type when publishing single attestation * Add SingleAttestation as alias to phase0 ssz types

ensi321 pushed a commit that referenced this pull request Dec 14, 2024

fix: select correct gossip type when publishing single attestation (#…

c654d90

…7256) * fix: select correct gossip type when publishing single attestation * Add SingleAttestation as alias to phase0 ssz types

ensi321 pushed a commit that referenced this pull request Dec 14, 2024

fix: select correct gossip type when publishing single attestation (#…

2997fec

…7256) * fix: select correct gossip type when publishing single attestation * Add SingleAttestation as alias to phase0 ssz types

ensi321 pushed a commit that referenced this pull request Dec 17, 2024

fix: select correct gossip type when publishing single attestation (#…

3cd80bb

…7256) * fix: select correct gossip type when publishing single attestation * Add SingleAttestation as alias to phase0 ssz types

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: select correct gossip type when publishing single attestation #7256

fix: select correct gossip type when publishing single attestation #7256

nflaig commented Nov 28, 2024

twoeths Nov 28, 2024

nflaig Nov 28, 2024

nflaig Nov 28, 2024

nflaig Nov 28, 2024 •

edited

Loading

codecov bot commented Nov 28, 2024 •

edited

Loading

twoeths left a comment

fix: select correct gossip type when publishing single attestation #7256

fix: select correct gossip type when publishing single attestation #7256

Conversation

nflaig commented Nov 28, 2024

twoeths Nov 28, 2024

Choose a reason for hiding this comment

nflaig Nov 28, 2024

Choose a reason for hiding this comment

nflaig Nov 28, 2024

Choose a reason for hiding this comment

nflaig Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Nov 28, 2024 • edited Loading

Codecov Report

twoeths left a comment

Choose a reason for hiding this comment

nflaig Nov 28, 2024 •

edited

Loading

codecov bot commented Nov 28, 2024 •

edited

Loading