`SingleAttestation` implementation #6488

eserilev · 2024-10-15T04:31:32Z

Issue Addressed

Closes #6380

Proposed Changes

Spec: ethereum/consensus-specs#3900

Convert single attestation as early as possible to reduce code diff

Conga-lined off:

Electra spec changes for v1.5.0-beta.0 #6731

eserilev · 2024-10-15T19:27:10Z

@dapplion

eserilev · 2024-11-28T05:43:00Z

I've ran some kurtosis tests interoping against prysm and we are attesting correctly

EDIT: we are NOT interoping correctly at the moment. I am working on debugging the issue

…ngle_attestation

…estation

michaelsproul

Did a quick re-review of about half of this. Will come back to it ASAP tomorrow

beacon_node/beacon_chain/src/test_utils.rs

beacon_node/http_api/src/publish_attestations.rs

validator_client/validator_services/src/attestation_service.rs

…ngle_attestation

consensus/types/src/subnet_id.rs

validator_client/validator_services/src/attestation_service.rs

beacon_node/lighthouse_network/src/types/pubsub.rs

beacon_node/http_api/tests/fork_tests.rs

beacon_node/http_api/src/publish_attestations.rs

beacon_node/network/src/network_beacon_processor/mod.rs

consensus/types/src/attestation.rs

dapplion · 2025-01-15T05:32:00Z

consensus/types/src/attestation.rs

+        };
+
+        if committee.index != self.committee_index as u64 {
+            return Err(Error::InvalidCommittee);


Suggested change

return Err(Error::InvalidCommittee);

return Err(Error::UnexpectedCommittee(committee.index, self.committee_index));

I've updated the args for this method. Instead of passing in the full BeaconCommittee obj, I'm passing in the committee slice. Upstream from this method call I make a check for the existence of the specific committee for the slot and committee_index and return either a Error::NoCommitteeForSlotAndIndex or log a warn message if the committee isn't found

consensus/types/src/attestation.rs

michaelsproul

Approved modulo Lion's small tweaks

beacon_node/beacon_chain/src/attestation_verification.rs

dapplion · 2025-01-15T05:56:50Z

beacon_node/beacon_chain/src/events.rs

@@ -90,6 +90,10 @@ impl<E: EthSpec> ServerSentEventHandler<E> {
                .attestation_tx
                .send(kind)
                .map(|count| log_count("attestation", count)),
+            EventKind::SingleAttestation(_) => self
+                .attestation_tx


This is not correct I believe. There's a new event name single_attestation, so you should have a new (tx,rx) and broadcast there. https://github.com/ethereum/beacon-APIs/blob/aa1be259e2d8a64f451dd304913e0310779f6f80/apis/eventstream/index.yaml#L70

Yeah otherwise calls into the event stream with error with unknown topic

I believe the attestation_tx is just for SSE which is for our own internal infra? We still emit a SingleAttestation event with the new single_attestation topic name, which I think is spec compliant. I could be wrong though

ok maybe I am wrong, ill double check, thanks

lighthouse/common/eth2/src/types.rs

Line 1267 in 0d90135

pub enum EventTopic {

this enum must include a SingleAttestation variant

Yep you were right, heres the fix
54844b3

michaelsproul · 2025-01-15T08:55:50Z

Looks like one of the SSE tests is failing (get_events)

eserilev · 2025-01-15T09:38:41Z

I believe i've fixed the test failure but I think I need to add some test coverage for the new single_attestation event topic

eserilev · 2025-01-16T04:58:48Z

i've added test coverage for the new single_attestation event topic. This PR should hopefully be good to go

dapplion · 2025-01-16T10:04:33Z

beacon_node/beacon_chain/src/beacon_chain.rs

+                        let current_fork = self
+                            .spec
+                            .fork_name_at_slot::<T::EthSpec>(v.attestation().data().slot);
+                        if current_fork.electra_enabled() {


Is it necessary to check the fork now? Can we have single attestations before electra?

dapplion

PR is good to go!

pawanjay176 added 10 commits August 29, 2024 16:11

First pass

25feedf

Add restrictions to RuntimeVariableList api

4dc6e65

Use empty_uninitialized and fix warnings

a9cb329

Fix some todos

60100fc

Merge branch 'unstable' into max-blobs-preset

13f9bba

Fix take impl on RuntimeFixedList

e71020e

cleanup

52bb581

Fix test compilations

d37733b

Fix some more tests

12c6ef1

Fix test from unstable

2fcb293

eserilev changed the title ~~Single attestation~~ SingleAttestation implementation Oct 15, 2024

eserilev added the work-in-progress PR is a work-in-progress label Oct 15, 2024

eserilev added the electra Required for the Electra/Prague fork label Oct 15, 2024

Merge branch 'unstable' into max-blobs-preset

21ecb58

eserilev added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Oct 24, 2024

This was referenced Nov 27, 2024

SingleAttestation full implementation #6616

Open

Separate type for unaggregated network attestations #6380

Open

eserilev added blocked and removed ready-for-review The code is ready for review labels Dec 2, 2024

michaelsproul added the v6.1.0 New release c. Q1 2025 label Dec 3, 2024

SingleAttestation

366bed3

eserilev force-pushed the single_attestation branch from ba06501 to 366bed3 Compare December 4, 2024 09:29

eserilev added 2 commits December 13, 2024 21:51

Add post attestation v2 endpoint logic to attestation service

68292ad

Merge branch 'unstable' of https://github.com/sigp/lighthouse into si…

5a4cd3b

…ngle_attestation

michaelsproul self-requested a review December 19, 2024 06:23

michaelsproul added the electra-alpha10 Electra release for devnet 5 label Dec 19, 2024

Implement "Bugfix and more withdrawal tests"

9d5a3af

Merge remote-tracking branch 'origin/electra-alpha10' into single_att…

11c1949

…estation

michaelsproul changed the base branch from electra-alpha10 to unstable January 14, 2025 01:13

michaelsproul added 2 commits January 14, 2025 12:14

Merge remote-tracking branch 'origin/unstable' into single_attestation

e76322e

Merge remote-tracking branch 'origin/unstable' into single_attestation

d8f32de

jimmygchen mentioned this pull request Jan 14, 2025

Implement PeerDAS Fulu fork activation #6795

Open

michaelsproul reviewed Jan 14, 2025

View reviewed changes

beacon_node/beacon_chain/src/test_utils.rs Outdated Show resolved Hide resolved

beacon_node/http_api/src/publish_attestations.rs Outdated Show resolved Hide resolved

validator_client/validator_services/src/attestation_service.rs Outdated Show resolved Hide resolved

eserilev added 3 commits January 14, 2025 13:56

Merge branch 'unstable' of https://github.com/sigp/lighthouse into si…

fb5e378

…ngle_attestation

publish_attestations should accept Either<Attestation,SingleAttestation>

2d61b65

log an error when failing to convert to SingleAttestation

b6d80eb

pawanjay176 reviewed Jan 14, 2025

View reviewed changes

michaelsproul added 2 commits January 15, 2025 16:02

Use Cow to avoid clone

86ce323

Avoid reconverting to SingleAttestation

8cc43c1

michaelsproul mentioned this pull request Jan 15, 2025

Ensure SingleAttestation conversion happens on the BeaconProcessor #6805

Open

michaelsproul and others added 4 commits January 15, 2025 16:28

Tweak VC error message

e900bb8

update comments

72da039

update comments

b9b9b99

pass in single attestation as ref to subnetid calculation method

4f2182e

dapplion reviewed Jan 15, 2025

View reviewed changes

michaelsproul approved these changes Jan 15, 2025

View reviewed changes

dapplion reviewed Jan 15, 2025

View reviewed changes

beacon_node/beacon_chain/src/attestation_verification.rs Show resolved Hide resolved

dapplion reviewed Jan 15, 2025

View reviewed changes

eserilev added 2 commits January 15, 2025 14:26

Improved API, new error variants and other minor tweaks

aa66b06

Fix single_attestation event topic boilerplate

54844b3

fix sse event failure

f7a0b37

Add single_attestation event topic test coverage

42c01c6

dapplion reviewed Jan 16, 2025

View reviewed changes

dapplion approved these changes Jan 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`SingleAttestation` implementation #6488

`SingleAttestation` implementation #6488

eserilev commented Oct 15, 2024 •

edited by michaelsproul

Loading

eserilev commented Oct 15, 2024

eserilev commented Nov 28, 2024 •

edited

Loading

michaelsproul left a comment

dapplion Jan 15, 2025

eserilev Jan 15, 2025 •

edited

Loading

michaelsproul left a comment

dapplion Jan 15, 2025

dapplion Jan 15, 2025

eserilev Jan 15, 2025 •

edited

Loading

eserilev Jan 15, 2025

dapplion Jan 15, 2025

eserilev Jan 15, 2025

michaelsproul commented Jan 15, 2025

eserilev commented Jan 15, 2025

eserilev commented Jan 16, 2025

dapplion Jan 16, 2025

dapplion left a comment

	return Err(Error::InvalidCommittee);
	return Err(Error::UnexpectedCommittee(committee.index, self.committee_index));

SingleAttestation implementation #6488

Are you sure you want to change the base?

SingleAttestation implementation #6488

Conversation

eserilev commented Oct 15, 2024 • edited by michaelsproul Loading

Issue Addressed

Proposed Changes

eserilev commented Oct 15, 2024

eserilev commented Nov 28, 2024 • edited Loading

michaelsproul left a comment

Choose a reason for hiding this comment

dapplion Jan 15, 2025

Choose a reason for hiding this comment

eserilev Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

michaelsproul left a comment

Choose a reason for hiding this comment

dapplion Jan 15, 2025

Choose a reason for hiding this comment

dapplion Jan 15, 2025

Choose a reason for hiding this comment

eserilev Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

eserilev Jan 15, 2025

Choose a reason for hiding this comment

dapplion Jan 15, 2025

Choose a reason for hiding this comment

eserilev Jan 15, 2025

Choose a reason for hiding this comment

michaelsproul commented Jan 15, 2025

eserilev commented Jan 15, 2025

eserilev commented Jan 16, 2025

dapplion Jan 16, 2025

Choose a reason for hiding this comment

dapplion left a comment

Choose a reason for hiding this comment

`SingleAttestation` implementation #6488

`SingleAttestation` implementation #6488

eserilev commented Oct 15, 2024 •

edited by michaelsproul

Loading

eserilev commented Nov 28, 2024 •

edited

Loading

eserilev Jan 15, 2025 •

edited

Loading

eserilev Jan 15, 2025 •

edited

Loading