Borevent snapshot validation #9436

mh0lt · 2024-02-13T13:51:37Z

This adds integrity checking for borevents in the following places

Stage Bor Heimdall - check that the event occurs withing the expected time window
Dump Events to snapshots - check that the event ids are continuous in and between blocks (will add a time window tests)
Index event snapshots - check that the event ids are continuous in and between blocks

It also adds an external integrity checker which runs on snapshots checking for continuity and timeliness of events. This can be called using the following command:
erigon snapshots integrity --datadir=~/snapshots/bor-mainnet-patch-1 --from=45500000
(--to specifies an end block)

This also now fixes the long running issue with bor events causing unexpected fails in executions:

the problem event validation uncovered was a follows:

The kv.BorEventNums mapping table currently keeps the mapping first event id->block. The code which produces bor-event snapshots to determine which events to put into the snapshot.

however if no additional blocks have events by the time the block is stored in the snapshot, the snapshot creation code does not know which events to include - so drops them.

This causes problems in two places:

RPC queries & re-execution from snapshots can't find these dropped events
Depending on purge timing these events may erroneously get inserted into future blocks

The code change in this PR fixes that bug.

It has been tested by running:

erigon --datadir=~/chains/e3/amoy --chain=amoy --bor.heimdall=https://heimdall-api-amoy.polygon.technology --db.writemap=false --txpool.disable --no-downloader --bor.milestone=false

with validation in place and the confimed by running the following:

erigon snapshots rm-all-state-snapshots --datadir=~/chains/e3/amoy
rm ~/chains/e3/amoy/chaindata/
erigon --datadir=~/chains/e3/amoy --chain=amoy --bor-heimdall=https://heimdall-api-amoy.polygon.technology --db.writemap=false --no-downloader --bor.milestone=false

To recreate the chain from snapshots.

It has also been checked with:

erigon snapshots integrity --datadir=~/chains/e3/amoy --check=NoBorEventGaps --failFast=true"

polygon/bor/bor.go

polygon/bor/genesis_contracts.go

taratorio · 2024-02-13T19:43:11Z

eth/stagedsync/bor_heimdall_shared.go

@@ -220,19 +217,21 @@ func fetchAndWriteHeimdallStateSyncEvents(
 			continue
 		}

-		if lastStateSyncEventID+1 != eventRecord.ID || eventRecord.ChainID != chainID || !eventRecord.Time.Before(to) {
+		if lastStateSyncEventID+1 != eventRecord.ID || eventRecord.ChainID != chainID ||
+			!(eventRecord.Time.After(from) && eventRecord.Time.Before(to)) {


I'd suggest to add a new unit test for this error path - it should be fairly easy to add.

You can have a look at TestBorHeimdallForwardPersistsStateSyncEvents and stagedsynctest.Harness.mockHeimdallClient. For this you will need to pass in a new function to h.heimdallClient.EXPECT().FetchStateSyncEvents(...).DoAndReturn().

A clean way to do this may be to add an optional MockFetchStateSyncEventsCallback to the HarnessCfg and check if it is present use that inside the DoAndReturn otherwise use the current one.

ok I'll look at this tomorrow.

eth/integrity/snap_blocks_read.go

taratorio · 2024-02-13T19:49:01Z

eth/integrity/snap_blocks_read.go

-	for i := uint64(0); i < maxBlockNum; i += 10_000 {
+
+	if to != 0 && maxBlockNum > to {
+		maxBlockNum = 2


out of curiosity - why is this necessary? is it worth leaving a comment?

hmm - should say: maxBlockNum = to

@mh0lt this typo still needs fixing

eth/stagedsync/bor_heimdall_shared.go

AskAlexSharov · 2024-02-20T05:00:39Z

did try this "integrity" check, see:

EROR[02-20|04:59:03.985] [integrity] NoGapsInBorEvents: invalid event time block=25196544 event=1716410 time=2022-02-21T14:37:02+0000 expected="2022-02-21 14:38:38 +0000 UTC-2022-02-21 14:38:38 +0000 UTC"

for all events

…gerwatch/erigon into borevent_snapshot_validation

mh0lt · 2024-02-21T18:56:42Z

did try this "integrity" check, see:

EROR[02-20|04:59:03.985] [integrity] NoGapsInBorEvents: invalid event time block=25196544 event=1716410 time=2022-02-21T14:37:02+0000 expected="2022-02-21 14:38:38 +0000 UTC-2022-02-21 14:38:38 +0000 UTC"

for all events

I'm only just testing this now. I need to validate that the check is actually correctly configured - which I'm talking through with Polygon. As there is no spec for the start of a window its going to take some adjustment to get right.

polygon/bor/borabi/abi.go

taratorio · 2024-08-18T09:36:26Z

eth/integrity/no_gaps_bor_events.go

+					return err
+				}
+
+				fmt.Println("LAST Event", lastEventId, "BH", borHeimdallProgress, "B", bodyProgress)


looks like leftover print from debugging? should we remove and/or use a logger?

taratorio · 2024-08-18T09:39:32Z

polygon/heimdall/event_fetch_test.go

+	"github.com/erigontech/erigon-lib/log/v3"
+)
+
+func TestOver50EventBlockFetch(t *testing.T) {


this test should maybe be removed since it is calling an external service https://heimdall-api.polygon.technology/ and may fail our CI or alternatively we can add t.Skip with info why we skip it and why it is left behind in the code base

taratorio · 2024-08-18T09:41:55Z

polygon/heimdall/event_record.go

@@ -131,3 +131,29 @@ type StateSyncEventResponse struct {
 	Height string              `json:"height"`
 	Result EventRecordWithTime `json:"result"`
 }
+
+var methodId []byte = borabi.StateReceiverContractABI().Methods["commitState"].ID


[]byte type can be omitted in the declaration

taratorio · 2024-08-18T09:47:44Z

eth/integrity/no_gaps_bor_events.go

+
+				fmt.Println("LAST Event", lastEventId, "BH", borHeimdallProgress, "B", bodyProgress)
+
+				if bodyProgress > borHeimdallProgress {


this if and for loop don't seem to do anything?
probably the whole if db != nil if branch above on line 89 can be removed?

mh0lt added 9 commits February 10, 2024 21:14

decoupled block merge operations

5ff732e

fix retire loop

07cbcd6

file range selector fix

8c6155d

no events gap integration running

b211726

fix from check

96e5721

check events by block

4741fb1

log block

701ee4e

added time window checks

cf361e3

Added !failfast error logging

a84efd2

mh0lt added the polygon label Feb 13, 2024

taratorio reviewed Feb 13, 2024

View reviewed changes

polygon/bor/bor.go Outdated Show resolved Hide resolved

taratorio reviewed Feb 13, 2024

View reviewed changes

polygon/bor/genesis_contracts.go Outdated Show resolved Hide resolved

taratorio reviewed Feb 13, 2024

View reviewed changes

eth/integrity/snap_blocks_read.go Outdated Show resolved Hide resolved

taratorio reviewed Feb 13, 2024

View reviewed changes

eth/stagedsync/bor_heimdall_shared.go Outdated Show resolved Hide resolved

AskAlexSharov added 3 commits February 20, 2024 11:25

linter fix

f2d095a

Merge branch 'devel' into borevent_snapshot_validation

f74f9c5

linter fix

86b0731

mh0lt added 2 commits February 21, 2024 18:08

added muti event test

e29dace

Merge branch 'borevent_snapshot_validation' of https://github.com/led…

a395ed4

…gerwatch/erigon into borevent_snapshot_validation

mh0lt added 7 commits February 22, 2024 12:34

dec start number

9fc37a3

Merge branch 'devel' into borevent_snapshot_validation

eed2371

moved view to outer function

ccdce40

removed db dependency

4d9fb37

debug logging

dce4fa1

fix event window calc

d9bbe0b

add diff

70e7b4f

Mark Holt and others added 11 commits August 13, 2024 11:30

capture err

c152a70

Merge branch 'main' into borevent_snapshot_validation

66ea8a7

remove heimdall re-lookup

c6eb440

export events for last block

3481e66

check lastevent for processing

d9a0ac8

split out ValidateBorEvents

29f8cf4

exclude bor-mainnet from initial checks

6750739

Merge branch 'main' into borevent_snapshot_validation

894a5ad

compile

a2082f7

re add missing funcs

7b48cac

reinstate event getters

c6fdae5

AskAlexSharov approved these changes Aug 17, 2024

View reviewed changes

mh0lt added 2 commits August 17, 2024 10:44

don't validate test chains

06df5f4

fix typo

641d350

mh0lt added imp1 High importance and removed imp3 Low importance labels Aug 17, 2024

mh0lt added 3 commits August 18, 2024 10:15

times after initial event can be before window

5728507

add comment

6ab8d1f

update comment

904639e

taratorio reviewed Aug 18, 2024

View reviewed changes

polygon/bor/borabi/abi.go Outdated Show resolved Hide resolved

taratorio reviewed Aug 18, 2024

View reviewed changes

adjust integration validity check

63d0cc6

taratorio reviewed Aug 18, 2024

View reviewed changes

taratorio approved these changes Aug 18, 2024

View reviewed changes

fixed review issues

e063214

mh0lt merged commit e4eb9fc into main Aug 19, 2024
10 checks passed

mh0lt deleted the borevent_snapshot_validation branch August 19, 2024 08:23

mh0lt mentioned this pull request Aug 19, 2024

e3: bor.go has hack if len(events) == 50 to fallback to remote Heimdall #10310

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Borevent snapshot validation #9436

Borevent snapshot validation #9436

mh0lt commented Feb 13, 2024 •

edited

Loading

taratorio Feb 13, 2024 •

edited

Loading

mh0lt Feb 13, 2024

taratorio Feb 13, 2024

mh0lt Feb 13, 2024

taratorio Aug 18, 2024

AskAlexSharov commented Feb 20, 2024

mh0lt commented Feb 21, 2024

taratorio Aug 18, 2024

taratorio Aug 18, 2024 •

edited

Loading

taratorio Aug 18, 2024

taratorio Aug 18, 2024


		fmt.Println("LAST Event", lastEventId, "BH", borHeimdallProgress, "B", bodyProgress)

		if bodyProgress > borHeimdallProgress {

Borevent snapshot validation #9436

Borevent snapshot validation #9436

Conversation

mh0lt commented Feb 13, 2024 • edited Loading

taratorio Feb 13, 2024 • edited Loading

Choose a reason for hiding this comment

mh0lt Feb 13, 2024

Choose a reason for hiding this comment

taratorio Feb 13, 2024

Choose a reason for hiding this comment

mh0lt Feb 13, 2024

Choose a reason for hiding this comment

taratorio Aug 18, 2024

Choose a reason for hiding this comment

AskAlexSharov commented Feb 20, 2024

mh0lt commented Feb 21, 2024

taratorio Aug 18, 2024

Choose a reason for hiding this comment

taratorio Aug 18, 2024 • edited Loading

Choose a reason for hiding this comment

taratorio Aug 18, 2024

Choose a reason for hiding this comment

taratorio Aug 18, 2024

Choose a reason for hiding this comment

mh0lt commented Feb 13, 2024 •

edited

Loading

taratorio Feb 13, 2024 •

edited

Loading

taratorio Aug 18, 2024 •

edited

Loading