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

[EpochSync] Implement EpochSyncInfo validation #10031

Open
2 tasks
Tracked by #9581
posvyatokum opened this issue Oct 30, 2023 · 1 comment
Open
2 tasks
Tracked by #9581

[EpochSync] Implement EpochSyncInfo validation #10031

posvyatokum opened this issue Oct 30, 2023 · 1 comment
Assignees
@posvyatokum
Labels
Near Core Node Node team

Comments

@posvyatokum
Copy link
Member

Tasks
Beta Give feedback
@posvyatokum posvyatokum self-assigned this Oct 30, 2023
@posvyatokum posvyatokum mentioned this issue Oct 30, 2023
Open
@gmilescu gmilescu changed the title Implement EpochSyncInfo validation [EpochSync] Implement EpochSyncInfo validation Oct 30, 2023
@posvyatokum
Copy link
Member Author

EpochSyncInfo should be written on epoch FINALISATION, which means that every block of an epoch should be finalised. Right now we write epoch_sync_data_hash like that (which means that it is actually in the NEXT epoch), but write EpochSyncInfo on block processing, which means that it may be rewritten with a fork.
Also, to validate EpochSyncInfo via epoch_sync_data_hash, we need a header from NEXT epoch. Also, we want to include header from PREV epoch to validate epochs first header. This finally gives a reason for garbage collection minimum threshold to be set to 3, as EpochSyncInfo data spans headers of 3 consecutive epochs.

@posvyatokum posvyatokum mentioned this issue Nov 6, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 7, 2023
@posvyatokum
fix(epoch-sync): write EpochSyncInfo after full epoch finalisation (#…
984c935 
…10109)

#10031
#10031 (comment)

1. Header potentially needed for `EpochInfo` validation in
`EpochSyncInfo` via `epoch_sync_data_hash` is actually in the next
epoch. This header should be recorded in `EpochSyncInfo` too.
2. All data in `EpochSyncInfo` should be finalised.
3. `test_continuous_epoch_sync_info_population` is changed to reflect
that `EpochSyncInfo` is only written after everything is finalised.

P.S. I don't think it is the last change to `EpochSyncInfo` (this week).
@posvyatokum posvyatokum mentioned this issue Nov 7, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 8, 2023
@posvyatokum
feat(epoch-sync): calculate legacy epoch_sync_data_hash from EpochSyn…
d8ada79 
…cInfo (#10120)

First part of #10031
Adding support for `EpochInfo` validation through `epoch_sync_data_hash`
in new epoch sync.

Included in PR:
- reconstruction of `BlockInfo` headers
- calculation of `epoch_sync_data_hash` from `EpochSyncInfo`
- bad attempt at good errors in all of that
- test for both `BlockInfo` and `epoch_sync_data_hash` reconstruction

Coming next: epoch sync testing tool, specifically command to test that
reconstructed `epoch_sync_data_hash` matches recorded
`epoch_sync_data_hash` on all real data in testnet/mainnet.
@github-actions github-actions bot mentioned this issue Nov 17, 2023
Open
@posvyatokum posvyatokum mentioned this issue Nov 21, 2023
Merged
github-merge-queue bot pushed a commit that referenced this issue Nov 21, 2023
@posvyatokum
feat(epoch-sync): EpochSyncInfo processing (#10226)
2318c3a 
#10031 
This PR is a step towards `EpochSyncInfo` validation that doesn't
actually include any validation.
I want to combine creation of necessary `StoreUpdate` with validation,
because to validate `EpochSyncInfo` we need to rebuild `BlockMerkleTree`
for every block in the epoch, but also not commit it if anything goes
bad, and that is supported in `ChainStoreUpdate`.
Creation of `StoreUpdate` is already a big change, so I want to do a
separate PR for it.
To test that we process `EpochSyncInfo` correctly I create a VERY hacky
test, that should not be included in CI ever. Right now it works as
intended, but if we change implementation of any sync, we will have to
rewrite this test.
I hope I will add epoch sync into `run_sync_step` flow soon enough for
us to replace this test with something stable before we need to rewrite
unstable test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@posvyatokum posvyatokum

Labels
Near Core Node Node team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@posvyatokum