Skip to content

Commit

Permalink
polygon/sync: block downloader to not re-insert blocks behind start (#…
Browse files Browse the repository at this point in the history
…11929)

Run into an issue since we started pruning total difficulty.
```
EROR[09-09|10:58:03.057] [2/6 PolygonSync] stopping node          err="parent's total difficulty not found with hash 9334099de5d77c0d56afefde9985d44f8b4416db99dfe926908d5501fa8dbd9e and height 11736178: <nil>
```

It happened for checkpoint
[9703](https://heimdall-api-amoy.polygon.technology/checkpoints/9703).
Our start block was in the middle of the checkpoint range which meant we
have to fetch all the 8k blocks in this checkpoint to verify the
checkpoint root hash when receiving blocks from the peer.

The current logic will attempt to insert all these 8k blocks and it will
fail with a missing parent td error because we only keep the last 1000
parent td records.

This PR fixes this by enhancing the block downloader to not re-insert
blocks behind the `start` block. This solves the parent td error and
also is saving some unnecessary inserts on the first waypoint processing
on startup.
  • Loading branch information
taratorio committed Sep 9, 2024
1 parent 4a992a3 commit 6d15c01
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 7 deletions.
14 changes: 9 additions & 5 deletions polygon/sync/block_downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (d *blockDownloader) DownloadBlocksUsingCheckpoints(ctx context.Context, st
return nil, err
}

return d.downloadBlocksUsingWaypoints(ctx, waypoints, d.checkpointVerifier)
return d.downloadBlocksUsingWaypoints(ctx, waypoints, d.checkpointVerifier, start)
}

func (d *blockDownloader) DownloadBlocksUsingMilestones(ctx context.Context, start uint64) (*types.Header, error) {
Expand All @@ -126,13 +126,14 @@ func (d *blockDownloader) DownloadBlocksUsingMilestones(ctx context.Context, sta
return nil, err
}

return d.downloadBlocksUsingWaypoints(ctx, waypoints, d.milestoneVerifier)
return d.downloadBlocksUsingWaypoints(ctx, waypoints, d.milestoneVerifier, start)
}

func (d *blockDownloader) downloadBlocksUsingWaypoints(
ctx context.Context,
waypoints heimdall.Waypoints,
verifier WaypointHeadersVerifier,
startBlockNum uint64,
) (*types.Header, error) {
if len(waypoints) == 0 {
return nil, nil
Expand Down Expand Up @@ -267,9 +268,12 @@ func (d *blockDownloader) downloadBlocksUsingWaypoints(
break
}

if blockBatch[0].Number().Uint64() == 0 {
// we do not want to insert block 0 (genesis)
blockBatch = blockBatch[1:]
batchStart := blockBatch[0].Number().Uint64()
batchEnd := blockBatch[len(blockBatch)-1].Number().Uint64()
if batchStart <= startBlockNum && startBlockNum <= batchEnd {
// we do not want to re-insert blocks of the first waypoint if the start block
// falls in the middle of the waypoint range
blockBatch = blockBatch[startBlockNum-batchStart:]
}

blocks = append(blocks, blockBatch...)
Expand Down
38 changes: 36 additions & 2 deletions polygon/sync/block_downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ import (
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

"github.com/erigontech/erigon-lib/log/v3"

"github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/log/v3"
"github.com/erigontech/erigon/core/types"
"github.com/erigontech/erigon/polygon/heimdall"
"github.com/erigontech/erigon/polygon/p2p"
Expand Down Expand Up @@ -309,6 +308,41 @@ func TestBlockDownloaderDownloadBlocksUsingCheckpoints(t *testing.T) {
require.Equal(t, blocks[len(blocks)-1].Header(), tip)
}

func TestBlockDownloaderDownloadBlocksUsingCheckpointsWhenStartIsInMiddleOfCheckpointRange(t *testing.T) {
test := newBlockDownloaderTest(t)
test.waypointReader.EXPECT().
CheckpointsFromBlock(gomock.Any(), gomock.Any()).
Return(test.fakeCheckpoints(2), nil).
Times(1)
test.p2pService.EXPECT().
ListPeersMayHaveBlockNum(gomock.Any()).
Return(test.fakePeers(2)).
Times(1)
test.p2pService.EXPECT().
FetchHeaders(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(test.defaultFetchHeadersMock()).
Times(2)
test.p2pService.EXPECT().
FetchBodies(gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(test.defaultFetchBodiesMock()).
Times(2)
var blocks []*types.Block
test.store.EXPECT().
InsertBlocks(gomock.Any(), gomock.Any()).
DoAndReturn(test.defaultInsertBlocksMock(&blocks)).
Times(1)

tip, err := test.blockDownloader.DownloadBlocksUsingCheckpoints(context.Background(), 513)
require.NoError(t, err)
require.Len(t, blocks, 1536) // [513,1024] = 512 blocks + 1024 blocks from 2nd checkpoint
// check blocks are written in order
require.Equal(t, uint64(513), blocks[0].Header().Number.Uint64())
require.Equal(t, uint64(1024), blocks[511].Header().Number.Uint64())
require.Equal(t, uint64(1025), blocks[512].Header().Number.Uint64())
require.Equal(t, uint64(2048), blocks[1535].Header().Number.Uint64())
require.Equal(t, blocks[len(blocks)-1].Header(), tip)
}

func TestBlockDownloaderDownloadBlocksWhenInvalidHeadersThenPenalizePeerAndReDownload(t *testing.T) {
var firstTimeInvalidReturned bool
firstTimeInvalidReturnedPtr := &firstTimeInvalidReturned
Expand Down

0 comments on commit 6d15c01

Please sign in to comment.