Skip to content

Commit

Permalink
Use slot to determine fork version (#14653)
Browse files Browse the repository at this point in the history 
* Use slot to determine version

* gaz

* solve cyclic dependency

* Radek' review

* unit test

* gaz

* use require instead of assert

* fix test

* fix test

* fix TestGetAggregateAttestation

* fix ListAttestations test

* James' review

* Radek' review

* add extra checks to GetAttesterSlashingsV2

* fix matchingAtts

* improve tests + fix

* fix

* stop appending all non electra atts

* more tests

* changelog

* revert TestProduceSyncCommitteeContribution changes

---------

Co-authored-by: Radosław Kapka <[email protected]>
Co-authored-by: rkapka <[email protected]>
  • Loading branch information
@saolyn @rkapka
3 people authored Nov 26, 2024
1 parent 8025a48 commit 74bb082
Show file tree
Hide file tree
Showing 9 changed files with 497 additions and 205 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
- Added `Eth-Consensus-Version` header to `ListAttestationsV2` and `GetAggregateAttestationV2` endpoints.
- Updated light client consensus types. [PR](https://github.com/prysmaticlabs/prysm/pull/14652)
- Fixed pending deposits processing on Electra.
- Modified `ListAttestationsV2`, `GetAttesterSlashingsV2` and `GetAggregateAttestationV2` endpoints to use slot to determine fork version.

### Deprecated

Expand Down
31 changes: 15 additions & 16 deletions beacon-chain/rpc/eth/beacon/handlers_pool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (s *Server) ListAttestations(w http.ResponseWriter, r *http.Request) {
// ListAttestationsV2 retrieves attestations known by the node but
// not necessarily incorporated into any block. Allows filtering by committee index or slot.
func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
ctx, span := trace.StartSpan(r.Context(), "beacon.ListAttestationsV2")
_, span := trace.StartSpan(r.Context(), "beacon.ListAttestationsV2")
defer span.End()

rawSlot, slot, ok := shared.UintFromQuery(w, r, "slot", false)
Expand All @@ -98,13 +98,10 @@ func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
if !ok {
return
}

headState, err := s.ChainInfoFetcher.HeadStateReadOnly(ctx)
if err != nil {
httputil.HandleError(w, "Could not get head state: "+err.Error(), http.StatusInternalServerError)
return
v := slots.ToForkVersion(primitives.Slot(slot))
if rawSlot == "" {
v = slots.ToForkVersion(s.TimeFetcher.CurrentSlot())
}

attestations := s.AttestationsPool.AggregatedAttestations()
unaggAtts, err := s.AttestationsPool.UnaggregatedAttestations()
if err != nil {
Expand All @@ -116,7 +113,7 @@ func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
filteredAtts := make([]interface{}, 0, len(attestations))
for _, att := range attestations {
var includeAttestation bool
if headState.Version() >= version.Electra {
if v >= version.Electra && att.Version() >= version.Electra {
attElectra, ok := att.(*eth.AttestationElectra)
if !ok {
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestation of type %T", att), http.StatusInternalServerError)
Expand All @@ -128,7 +125,7 @@ func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
attStruct := structs.AttElectraFromConsensus(attElectra)
filteredAtts = append(filteredAtts, attStruct)
}
} else {
} else if v < version.Electra && att.Version() < version.Electra {
attOld, ok := att.(*eth.Attestation)
if !ok {
httputil.HandleError(w, fmt.Sprintf("Unable to convert attestation of type %T", att), http.StatusInternalServerError)
Expand All @@ -149,9 +146,9 @@ func (s *Server) ListAttestationsV2(w http.ResponseWriter, r *http.Request) {
return
}

w.Header().Set(api.VersionHeader, version.String(headState.Version()))
w.Header().Set(api.VersionHeader, version.String(v))
httputil.WriteJson(w, &structs.ListAttestationsResponse{
Version: version.String(headState.Version()),
Version: version.String(v),
Data: attsData,
})
}
Expand Down Expand Up @@ -726,31 +723,33 @@ func (s *Server) GetAttesterSlashingsV2(w http.ResponseWriter, r *http.Request)
ctx, span := trace.StartSpan(r.Context(), "beacon.GetAttesterSlashingsV2")
defer span.End()

v := slots.ToForkVersion(s.TimeFetcher.CurrentSlot())
headState, err := s.ChainInfoFetcher.HeadStateReadOnly(ctx)
if err != nil {
httputil.HandleError(w, "Could not get head state: "+err.Error(), http.StatusInternalServerError)
return
}

var attStructs []interface{}
sourceSlashings := s.SlashingsPool.PendingAttesterSlashings(ctx, headState, true /* return unlimited slashings */)

for _, slashing := range sourceSlashings {
var attStruct interface{}
if headState.Version() >= version.Electra {
if v >= version.Electra && slashing.Version() >= version.Electra {
a, ok := slashing.(*eth.AttesterSlashingElectra)
if !ok {
httputil.HandleError(w, fmt.Sprintf("Unable to convert slashing of type %T to an Electra slashing", slashing), http.StatusInternalServerError)
return
}
attStruct = structs.AttesterSlashingElectraFromConsensus(a)
} else {
} else if v < version.Electra && slashing.Version() < version.Electra {
a, ok := slashing.(*eth.AttesterSlashing)
if !ok {
httputil.HandleError(w, fmt.Sprintf("Unable to convert slashing of type %T to a Phase0 slashing", slashing), http.StatusInternalServerError)
return
}
attStruct = structs.AttesterSlashingFromConsensus(a)
} else {
continue
}
attStructs = append(attStructs, attStruct)
}
Expand All @@ -762,10 +761,10 @@ func (s *Server) GetAttesterSlashingsV2(w http.ResponseWriter, r *http.Request)
}

resp := &structs.GetAttesterSlashingsResponse{
Version: version.String(headState.Version()),
Version: version.String(v),
Data: attBytes,
}
w.Header().Set(api.VersionHeader, version.String(headState.Version()))
w.Header().Set(api.VersionHeader, version.String(v))
httputil.WriteJson(w, resp)
}

Expand Down
103 changes: 92 additions & 11 deletions beacon-chain/rpc/eth/beacon/handlers_pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,16 @@ func TestListAttestations(t *testing.T) {
Signature: bytesutil.PadTo([]byte("signature4"), 96),
}
t.Run("V1", func(t *testing.T) {
bs, err := util.NewBeaconState()
require.NoError(t, err)

chainService := &blockchainmock.ChainService{State: bs}
s := &Server{
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
AttestationsPool: attestations.NewPool(),
}

require.NoError(t, s.AttestationsPool.SaveAggregatedAttestations([]ethpbv1alpha1.Att{att1, att2}))
require.NoError(t, s.AttestationsPool.SaveUnaggregatedAttestations([]ethpbv1alpha1.Att{att3, att4}))

Expand Down Expand Up @@ -204,10 +211,19 @@ func TestListAttestations(t *testing.T) {
t.Run("Pre-Electra", func(t *testing.T) {
bs, err := util.NewBeaconState()
require.NoError(t, err)

chainService := &blockchainmock.ChainService{State: bs}
s := &Server{
ChainInfoFetcher: &blockchainmock.ChainService{State: bs},
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
AttestationsPool: attestations.NewPool(),
}

params.SetupTestConfigCleanup(t)
config := params.BeaconConfig()
config.DenebForkEpoch = 0
params.OverrideBeaconConfig(config)

require.NoError(t, s.AttestationsPool.SaveAggregatedAttestations([]ethpbv1alpha1.Att{att1, att2}))
require.NoError(t, s.AttestationsPool.SaveUnaggregatedAttestations([]ethpbv1alpha1.Att{att3, att4}))
t.Run("empty request", func(t *testing.T) {
Expand All @@ -226,7 +242,7 @@ func TestListAttestations(t *testing.T) {
var atts []*structs.Attestation
require.NoError(t, json.Unmarshal(resp.Data, &atts))
assert.Equal(t, 4, len(atts))
assert.Equal(t, "phase0", resp.Version)
assert.Equal(t, "deneb", resp.Version)
})
t.Run("slot request", func(t *testing.T) {
url := "http://example.com?slot=2"
Expand All @@ -244,7 +260,7 @@ func TestListAttestations(t *testing.T) {
var atts []*structs.Attestation
require.NoError(t, json.Unmarshal(resp.Data, &atts))
assert.Equal(t, 2, len(atts))
assert.Equal(t, "phase0", resp.Version)
assert.Equal(t, "deneb", resp.Version)
for _, a := range atts {
assert.Equal(t, "2", a.Data.Slot)
}
Expand All @@ -265,7 +281,7 @@ func TestListAttestations(t *testing.T) {
var atts []*structs.Attestation
require.NoError(t, json.Unmarshal(resp.Data, &atts))
assert.Equal(t, 2, len(atts))
assert.Equal(t, "phase0", resp.Version)
assert.Equal(t, "deneb", resp.Version)
for _, a := range atts {
assert.Equal(t, "4", a.Data.CommitteeIndex)
}
Expand All @@ -286,7 +302,7 @@ func TestListAttestations(t *testing.T) {
var atts []*structs.Attestation
require.NoError(t, json.Unmarshal(resp.Data, &atts))
assert.Equal(t, 1, len(atts))
assert.Equal(t, "phase0", resp.Version)
assert.Equal(t, "deneb", resp.Version)
for _, a := range atts {
assert.Equal(t, "2", a.Data.Slot)
assert.Equal(t, "4", a.Data.CommitteeIndex)
Expand Down Expand Up @@ -370,12 +386,21 @@ func TestListAttestations(t *testing.T) {
}
bs, err := util.NewBeaconStateElectra()
require.NoError(t, err)

params.SetupTestConfigCleanup(t)
config := params.BeaconConfig()
config.ElectraForkEpoch = 0
params.OverrideBeaconConfig(config)

chainService := &blockchainmock.ChainService{State: bs}
s := &Server{
AttestationsPool: attestations.NewPool(),
ChainInfoFetcher: &blockchainmock.ChainService{State: bs},
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
}
require.NoError(t, s.AttestationsPool.SaveAggregatedAttestations([]ethpbv1alpha1.Att{attElectra1, attElectra2}))
require.NoError(t, s.AttestationsPool.SaveUnaggregatedAttestations([]ethpbv1alpha1.Att{attElectra3, attElectra4}))
// Added one pre electra attestation to ensure it is ignored.
require.NoError(t, s.AttestationsPool.SaveAggregatedAttestations([]ethpbv1alpha1.Att{attElectra1, attElectra2, att1}))
require.NoError(t, s.AttestationsPool.SaveUnaggregatedAttestations([]ethpbv1alpha1.Att{attElectra3, attElectra4, att3}))

t.Run("empty request", func(t *testing.T) {
url := "http://example.com"
Expand Down Expand Up @@ -1658,12 +1683,59 @@ func TestGetAttesterSlashings(t *testing.T) {
})
})
t.Run("V2", func(t *testing.T) {
t.Run("post-electra-ok-1-pre-slashing", func(t *testing.T) {
bs, err := util.NewBeaconStateElectra()
require.NoError(t, err)

params.SetupTestConfigCleanup(t)
config := params.BeaconConfig()
config.ElectraForkEpoch = 100
params.OverrideBeaconConfig(config)

chainService := &blockchainmock.ChainService{State: bs}

s := &Server{
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
SlashingsPool: &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashing1PostElectra, slashing2PostElectra, slashing1PreElectra}},
}

request := httptest.NewRequest(http.MethodGet, "http://example.com/eth/v2/beacon/pool/attester_slashings", nil)
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}

s.GetAttesterSlashingsV2(writer, request)
require.Equal(t, http.StatusOK, writer.Code)
resp := &structs.GetAttesterSlashingsResponse{}
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp))
require.NotNil(t, resp)
require.NotNil(t, resp.Data)
assert.Equal(t, "electra", resp.Version)

// Unmarshal resp.Data into a slice of slashings
var slashings []*structs.AttesterSlashingElectra
require.NoError(t, json.Unmarshal(resp.Data, &slashings))

ss, err := structs.AttesterSlashingsElectraToConsensus(slashings)
require.NoError(t, err)

require.DeepEqual(t, slashing1PostElectra, ss[0])
require.DeepEqual(t, slashing2PostElectra, ss[1])
})
t.Run("post-electra-ok", func(t *testing.T) {
bs, err := util.NewBeaconStateElectra()
require.NoError(t, err)

params.SetupTestConfigCleanup(t)
config := params.BeaconConfig()
config.ElectraForkEpoch = 100
params.OverrideBeaconConfig(config)

chainService := &blockchainmock.ChainService{State: bs}

s := &Server{
ChainInfoFetcher: &blockchainmock.ChainService{State: bs},
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
SlashingsPool: &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashing1PostElectra, slashing2PostElectra}},
}

Expand Down Expand Up @@ -1692,9 +1764,11 @@ func TestGetAttesterSlashings(t *testing.T) {
t.Run("pre-electra-ok", func(t *testing.T) {
bs, err := util.NewBeaconState()
require.NoError(t, err)
chainService := &blockchainmock.ChainService{State: bs}

s := &Server{
ChainInfoFetcher: &blockchainmock.ChainService{State: bs},
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
SlashingsPool: &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{slashing1PreElectra, slashing2PreElectra}},
}

Expand Down Expand Up @@ -1722,8 +1796,15 @@ func TestGetAttesterSlashings(t *testing.T) {
bs, err := util.NewBeaconStateElectra()
require.NoError(t, err)

params.SetupTestConfigCleanup(t)
config := params.BeaconConfig()
config.ElectraForkEpoch = 100
params.OverrideBeaconConfig(config)

chainService := &blockchainmock.ChainService{State: bs}
s := &Server{
ChainInfoFetcher: &blockchainmock.ChainService{State: bs},
ChainInfoFetcher: chainService,
TimeFetcher: chainService,
SlashingsPool: &slashingsmock.PoolMock{PendingAttSlashings: []ethpbv1alpha1.AttSlashing{}},
}

Expand Down
36 changes: 21 additions & 15 deletions beacon-chain/rpc/eth/validator/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (s *Server) GetAggregateAttestation(w http.ResponseWriter, r *http.Request)

// GetAggregateAttestationV2 aggregates all attestations matching the given attestation data root and slot, returning the aggregated result.
func (s *Server) GetAggregateAttestationV2(w http.ResponseWriter, r *http.Request) {
ctx, span := trace.StartSpan(r.Context(), "validator.GetAggregateAttestationV2")
_, span := trace.StartSpan(r.Context(), "validator.GetAggregateAttestationV2")
defer span.End()

_, attDataRoot, ok := shared.HexFromQuery(w, r, "attestation_data_root", fieldparams.RootLength, true)
Expand All @@ -91,14 +91,15 @@ func (s *Server) GetAggregateAttestationV2(w http.ResponseWriter, r *http.Reques
return
}

v := slots.ToForkVersion(primitives.Slot(slot))
agg := s.aggregatedAttestation(w, primitives.Slot(slot), attDataRoot, primitives.CommitteeIndex(index))
if agg == nil {
return
}
resp := &structs.AggregateAttestationResponse{
Version: version.String(agg.Version()),
Version: version.String(v),
}
if agg.Version() >= version.Electra {
if v >= version.Electra {
typedAgg, ok := agg.(*ethpbalpha.AttestationElectra)
if !ok {
httputil.HandleError(w, fmt.Sprintf("Attestation is not of type %T", &ethpbalpha.AttestationElectra{}), http.StatusInternalServerError)
Expand All @@ -123,12 +124,7 @@ func (s *Server) GetAggregateAttestationV2(w http.ResponseWriter, r *http.Reques
}
resp.Data = data
}
headState, err := s.ChainInfoFetcher.HeadStateReadOnly(ctx)
if err != nil {
httputil.HandleError(w, "Could not get head state: "+err.Error(), http.StatusInternalServerError)
return
}
w.Header().Set(api.VersionHeader, version.String(headState.Version()))
w.Header().Set(api.VersionHeader, version.String(v))
httputil.WriteJson(w, resp)
}

Expand Down Expand Up @@ -179,27 +175,37 @@ func matchingAtts(atts []ethpbalpha.Att, slot primitives.Slot, attDataRoot []byt
return []ethpbalpha.Att{}, nil
}

postElectra := atts[0].Version() >= version.Electra

postElectra := slots.ToForkVersion(slot) >= version.Electra
result := make([]ethpbalpha.Att, 0)
for _, att := range atts {
if att.GetData().Slot != slot {
continue
}

// We ignore the committee index from the request before Electra.
// This is because before Electra the committee index is part of the attestation data,
// meaning that comparing the data root is sufficient.
// Post-Electra the committee index in the data root is always 0, so we need to
// compare the committee index separately.
if postElectra {
ci, err := att.GetCommitteeIndex()
if err != nil {
return nil, err
if att.Version() >= version.Electra {
ci, err := att.GetCommitteeIndex()
if err != nil {
return nil, err
}
if ci != index {
continue
}
} else {
continue
}
if ci != index {
} else {
// If postElectra is false and att.Version >= version.Electra, ignore the attestation.
if att.Version() >= version.Electra {
continue
}
}

root, err := att.GetData().HashTreeRoot()
if err != nil {
return nil, errors.Wrap(err, "could not get attestation data root")
Expand Down
Loading

0 comments on commit 74bb082

Please sign in to comment.