Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: resolve the errors in GetLightClientUpdatesByRange function #14171

Closed
wants to merge 4 commits into from
Closed

fix: resolve the errors in GetLightClientUpdatesByRange function #14171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df1a801
fix: resolve the errors in ```GetLightClientUpdatesByRange``` function
rupam-04 Jul 2, 2024
52c4d71
added missing returns and replaced ```HandleError``` with continue wh…
rupam-04 Jul 2, 2024
9bf7b4b
feat: added unit tests for ```GetLightClientUpdatesByRange```
rupam-04 Jul 2, 2024
f6db4a3
fix: fixed areas where ``err.Error()`` was called when ``err`` is empty
rupam-04 Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 40 additions & 19 deletions beacon-chain/rpc/eth/light-client/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,40 +130,50 @@ func (s *Server) GetLightClientUpdatesByRange(w http.ResponseWriter, req *http.R
}
firstSlotInPeriod := period * slotsPerPeriod

// Let's not use the first slot in the period, otherwise the attested header will be in previous period
firstSlotInPeriod++

var state state.BeaconState
var block interfaces.ReadOnlySignedBeaconBlock
for slot := lastSlotInPeriod; slot >= firstSlotInPeriod; slot-- {
state, err = s.Stater.StateBySlot(ctx, types.Slot(slot))
if err != nil {
continue
httputil.HandleError(w, "Could not get state: "+err.Error(), http.StatusInternalServerError)
return
}

// Get the block
latestBlockHeader := state.LatestBlockHeader()
latestStateRoot, err := state.HashTreeRoot(ctx)
if err != nil {
continue
httputil.HandleError(w, "Could not get state root: "+err.Error(), http.StatusInternalServerError)
return
}
latestBlockHeader.StateRoot = latestStateRoot[:]
blockRoot, err := latestBlockHeader.HashTreeRoot()
if err != nil {
continue
httputil.HandleError(w, "Could not get block root: "+err.Error(), http.StatusInternalServerError)
return
}

block, err = s.Blocker.Block(ctx, blockRoot[:])
if err != nil || block == nil {
continue
if err != nil {
httputil.HandleError(w, "Could not get block: "+err.Error(), http.StatusInternalServerError)
return
}
if block == nil {
httputil.HandleError(w, "Could not get block", http.StatusInternalServerError)
return
}

syncAggregate, err := block.Block().Body().SyncAggregate()
if err != nil || syncAggregate == nil {
continue
if err != nil {
httputil.HandleError(w, "Could not get sync aggregate: "+err.Error(), http.StatusInternalServerError)
return
}
if syncAggregate == nil {
httputil.HandleError(w, "Could not get sync aggregate", http.StatusInternalServerError)
return
}

if syncAggregate.SyncCommitteeBits.Count()*3 < config.SyncCommitteeSize*2 {
if syncAggregate.SyncCommitteeBits.Count() < 1 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we even should filter out the blocks with zero SyncComitteeBits. since the light clients themselves check that.
But according to the specs, this will not count as a valid update anyway, so maybe we could save them the trouble of computation and just don't send them invalid updates.
Unless there is another use for this function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full node spec does not require to send only updates with 1 or more participants, but since it's the sync committee that provides an update, having 0 participants essentially means that the sync committee did not give an update.

// Not enough votes
continue
}
Expand All @@ -173,20 +183,27 @@ func (s *Server) GetLightClientUpdatesByRange(w http.ResponseWriter, req *http.R

if block == nil {
// No valid block found for the period
continue
httputil.HandleError(w, "No valid block found for the period", http.StatusNotFound)
rupam-04 marked this conversation as resolved.
Show resolved Hide resolved
return
}

// Get attested state
attestedRoot := block.Block().ParentRoot()
attestedBlock, err := s.Blocker.Block(ctx, attestedRoot[:])
if err != nil || attestedBlock == nil {
continue
if err != nil {
httputil.HandleError(w, "Could not get attested block: "+err.Error(), http.StatusInternalServerError)
rupam-04 marked this conversation as resolved.
Show resolved Hide resolved
return
}
if attestedBlock == nil {
httputil.HandleError(w, "Could not get attested block", http.StatusInternalServerError)
return
}

attestedSlot := attestedBlock.Block().Slot()
attestedState, err := s.Stater.StateBySlot(ctx, attestedSlot)
if err != nil {
continue
httputil.HandleError(w, "Could not get attested state: "+err.Error(), http.StatusInternalServerError)
rupam-04 marked this conversation as resolved.
Show resolved Hide resolved
return
}

// Get finalized block
Expand Down Expand Up @@ -241,22 +258,26 @@ func (s *Server) GetLightClientFinalityUpdate(w http.ResponseWriter, req *http.R

state, err := s.Stater.StateBySlot(ctx, block.Block().Slot())
if err != nil {
httputil.HandleError(w, "could not get state: "+err.Error(), http.StatusInternalServerError)
httputil.HandleError(w, "Could not get state: "+err.Error(), http.StatusInternalServerError)
return
}

// Get attested state
attestedRoot := block.Block().ParentRoot()
attestedBlock, err := s.Blocker.Block(ctx, attestedRoot[:])
if err != nil || attestedBlock == nil {
httputil.HandleError(w, "could not get attested block: "+err.Error(), http.StatusInternalServerError)
if err != nil {
httputil.HandleError(w, "Could not get attested block: "+err.Error(), http.StatusInternalServerError)
return
}
if attestedBlock == nil {
httputil.HandleError(w, "Could not get attested block", http.StatusInternalServerError)
return
}

attestedSlot := attestedBlock.Block().Slot()
attestedState, err := s.Stater.StateBySlot(ctx, attestedSlot)
if err != nil {
httputil.HandleError(w, "could not get attested state: "+err.Error(), http.StatusInternalServerError)
httputil.HandleError(w, "Could not get attested state: "+err.Error(), http.StatusInternalServerError)
return
}

Expand Down
200 changes: 199 additions & 1 deletion beacon-chain/rpc/eth/light-client/handlers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func TestLightClientHandler_GetLightClientBootstrap(t *testing.T) {
require.NotNil(t, resp.Data)
}

func TestLightClientHandler_GetLightClientUpdatesByRange(t *testing.T) {
func TestLightClientHandler_GetLightClientUpdatesByRange_MultipleSyncCommitteeBits(t *testing.T) {
helpers.ClearCache()
ctx := context.Background()
config := params.BeaconConfig()
Expand Down Expand Up @@ -180,6 +180,204 @@ func TestLightClientHandler_GetLightClientUpdatesByRange(t *testing.T) {
require.NotNil(t, resp)
}

func TestLightClientHandler_GetLightClientUpdatesByRange_ZeroSyncCommitteeBits(t *testing.T) {
helpers.ClearCache()
ctx := context.Background()
config := params.BeaconConfig()
slot := primitives.Slot(config.AltairForkEpoch * primitives.Epoch(config.SlotsPerEpoch)).Add(1)

attestedState, err := util.NewBeaconStateCapella()
require.NoError(t, err)
err = attestedState.SetSlot(slot.Sub(1))
require.NoError(t, err)

parent := util.NewBeaconBlockCapella()
parent.Block.Slot = slot.Sub(1)

signedParent, err := blocks.NewSignedBeaconBlock(parent)
require.NoError(t, err)

parentHeader, err := signedParent.Header()
require.NoError(t, err)
attestedHeader := parentHeader.Header

err = attestedState.SetLatestBlockHeader(attestedHeader)
require.NoError(t, err)
attestedStateRoot, err := attestedState.HashTreeRoot(ctx)
require.NoError(t, err)

// get a new signed block so the root is updated with the new state root
parent.Block.StateRoot = attestedStateRoot[:]
signedParent, err = blocks.NewSignedBeaconBlock(parent)
require.NoError(t, err)

st, err := util.NewBeaconStateCapella()
require.NoError(t, err)
err = st.SetSlot(slot)
require.NoError(t, err)

parentRoot, err := signedParent.Block().HashTreeRoot()
require.NoError(t, err)

block := util.NewBeaconBlockCapella()
block.Block.Slot = slot
block.Block.ParentRoot = parentRoot[:]

signedBlock, err := blocks.NewSignedBeaconBlock(block)
require.NoError(t, err)

h, err := signedBlock.Header()
require.NoError(t, err)

err = st.SetLatestBlockHeader(h.Header)
require.NoError(t, err)
stateRoot, err := st.HashTreeRoot(ctx)
require.NoError(t, err)

// get a new signed block so the root is updated with the new state root
block.Block.StateRoot = stateRoot[:]
signedBlock, err = blocks.NewSignedBeaconBlock(block)
require.NoError(t, err)

root, err := block.Block.HashTreeRoot()
require.NoError(t, err)

mockBlocker := &testutil.MockBlocker{
RootBlockMap: map[[32]byte]interfaces.ReadOnlySignedBeaconBlock{
parentRoot: signedParent,
root: signedBlock,
},
SlotBlockMap: map[primitives.Slot]interfaces.ReadOnlySignedBeaconBlock{
slot.Sub(1): signedParent,
slot: signedBlock,
},
}
mockChainService := &mock.ChainService{Optimistic: true, Slot: &slot, State: st}
s := &Server{
Stater: &testutil.MockStater{StatesBySlot: map[primitives.Slot]state.BeaconState{
slot.Sub(1): attestedState,
slot: st,
}},
Blocker: mockBlocker,
HeadFetcher: mockChainService,
}
startPeriod := slot.Div(uint64(config.EpochsPerSyncCommitteePeriod)).Div(uint64(config.SlotsPerEpoch))
url := fmt.Sprintf("http://foo.com/?count=1&start_period=%d", startPeriod)
request := httptest.NewRequest("GET", url, nil)
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}

s.GetLightClientUpdatesByRange(writer, request)

require.Equal(t, http.StatusOK, writer.Code)
var resp []structs.LightClientUpdateWithVersion
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), &resp))
require.Equal(t, 1, len(resp))
require.Equal(t, "capella", resp[0].Version)
require.Equal(t, hexutil.Encode(attestedHeader.BodyRoot), resp[0].Data.AttestedHeader.BodyRoot)
require.NotNil(t, resp)
}

func TestLightClientHandler_GetLightClientUpdatesByRange_OneSyncCommitteeBit(t *testing.T) {
helpers.ClearCache()
ctx := context.Background()
config := params.BeaconConfig()
slot := primitives.Slot(config.AltairForkEpoch * primitives.Epoch(config.SlotsPerEpoch)).Add(1)

attestedState, err := util.NewBeaconStateCapella()
require.NoError(t, err)
err = attestedState.SetSlot(slot.Sub(1))
require.NoError(t, err)

parent := util.NewBeaconBlockCapella()
parent.Block.Slot = slot.Sub(1)

signedParent, err := blocks.NewSignedBeaconBlock(parent)
require.NoError(t, err)

parentHeader, err := signedParent.Header()
require.NoError(t, err)
attestedHeader := parentHeader.Header

err = attestedState.SetLatestBlockHeader(attestedHeader)
require.NoError(t, err)
attestedStateRoot, err := attestedState.HashTreeRoot(ctx)
require.NoError(t, err)

// get a new signed block so the root is updated with the new state root
parent.Block.StateRoot = attestedStateRoot[:]
signedParent, err = blocks.NewSignedBeaconBlock(parent)
require.NoError(t, err)

st, err := util.NewBeaconStateCapella()
require.NoError(t, err)
err = st.SetSlot(slot)
require.NoError(t, err)

parentRoot, err := signedParent.Block().HashTreeRoot()
require.NoError(t, err)

block := util.NewBeaconBlockCapella()
block.Block.Slot = slot
block.Block.ParentRoot = parentRoot[:]

block.Block.Body.SyncAggregate.SyncCommitteeBits.SetBitAt(0, true)

signedBlock, err := blocks.NewSignedBeaconBlock(block)
require.NoError(t, err)

h, err := signedBlock.Header()
require.NoError(t, err)

err = st.SetLatestBlockHeader(h.Header)
require.NoError(t, err)
stateRoot, err := st.HashTreeRoot(ctx)
require.NoError(t, err)

// get a new signed block so the root is updated with the new state root
block.Block.StateRoot = stateRoot[:]
signedBlock, err = blocks.NewSignedBeaconBlock(block)
require.NoError(t, err)

root, err := block.Block.HashTreeRoot()
require.NoError(t, err)

mockBlocker := &testutil.MockBlocker{
RootBlockMap: map[[32]byte]interfaces.ReadOnlySignedBeaconBlock{
parentRoot: signedParent,
root: signedBlock,
},
SlotBlockMap: map[primitives.Slot]interfaces.ReadOnlySignedBeaconBlock{
slot.Sub(1): signedParent,
slot: signedBlock,
},
}
mockChainService := &mock.ChainService{Optimistic: true, Slot: &slot, State: st}
s := &Server{
Stater: &testutil.MockStater{StatesBySlot: map[primitives.Slot]state.BeaconState{
slot.Sub(1): attestedState,
slot: st,
}},
Blocker: mockBlocker,
HeadFetcher: mockChainService,
}
startPeriod := slot.Div(uint64(config.EpochsPerSyncCommitteePeriod)).Div(uint64(config.SlotsPerEpoch))
url := fmt.Sprintf("http://foo.com/?count=1&start_period=%d", startPeriod)
request := httptest.NewRequest("GET", url, nil)
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}

s.GetLightClientUpdatesByRange(writer, request)

require.Equal(t, http.StatusOK, writer.Code)
var resp []structs.LightClientUpdateWithVersion
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), &resp))
require.Equal(t, 1, len(resp))
require.Equal(t, "capella", resp[0].Version)
require.Equal(t, hexutil.Encode(attestedHeader.BodyRoot), resp[0].Data.AttestedHeader.BodyRoot)
require.NotNil(t, resp)
}

func TestLightClientHandler_GetLightClientUpdatesByRange_TooBigInputCount(t *testing.T) {
helpers.ClearCache()
ctx := context.Background()
Expand Down