Fetch and store blocks between last grandpa and last beefy finalized #2202

ErakhtinB · 2024-09-02T03:47:38Z

Referenced issues

Closes #2193
Closes #1917

Description of the Change

Possible Drawbacks

Checklist Before Opening a PR

Before you open a Pull Request (PR), please make sure you've completed the following steps and confirm by answering 'Yes' to each item:

Code is formatted: Have you run your code through clang-format to ensure it adheres to the project's coding standards? [Yes|No]
Code is documented: Have you added comments and documentation to your code according to the guidelines in the project's contributing guidelines? [Yes|No]
Self-review: Have you reviewed your own code to ensure it is free of typos, syntax errors, logical errors, and unresolved TODOs or FIXME without linking to an issue? [Yes|No]
Zombienet Tests: Have you ensured that the zombienet tests are passing? Zombienet is a network simulation and testing tool used in this project. It's important to ensure that these tests pass to maintain the stability and reliability of the project. [Yes|No]

core/consensus/beefy/impl/beefy_impl.hpp

core/consensus/beefy/impl/beefy_impl.cpp

core/consensus/beefy/impl/beefy_impl.hpp

core/consensus/beefy/impl/beefy_impl.cpp

core/network/impl/synchronizer_impl.cpp

core/consensus/beefy/impl/beefy_impl.cpp

core/consensus/beefy/impl/beefy_impl.hpp

core/network/impl/synchronizer_impl.cpp

test/core/consensus/beefy_test.cpp

core/consensus/beefy/impl/beefy_impl.cpp

core/consensus/beefy/impl/beefy_impl.hpp

core/network/impl/synchronizer_impl.cpp

core/consensus/beefy/impl/beefy_impl.cpp

core/network/impl/synchronizer_impl.cpp

kamilsa

Add docs and clang-format

kamilsa · 2024-09-11T09:54:02Z

core/consensus/beefy/impl/beefy_impl.hpp

@@ -161,6 +163,7 @@ namespace kagome::network {
    primitives::events::ChainSub chain_sub_;

    std::optional<primitives::BlockNumber> beefy_genesis_;
+    std::optional<primitives::BlockInfo> fetching_headers_;


not clear why field with plural name has optional type. Probably worth leaving a comment explaining the meaning

kamilsa · 2024-09-11T09:56:24Z

core/consensus/beefy/impl/beefy_impl.hpp

@@ -132,6 +134,7 @@ namespace kagome::network {
    outcome::result<void> apply(
        consensus::beefy::SignedCommitment justification, bool broadcast);
    outcome::result<void> update();
+    void fetchHeaders();


Function does not seem to be trivial inside. Please leave a comment explaining what it is doing and how

kamilsa · 2024-09-11T10:11:46Z

core/network/impl/synchronizer_impl.hpp

@@ -153,6 +156,9 @@ namespace kagome::network {
    bool fetchJustificationRange(primitives::BlockNumber min,
                                 FetchJustificationRangeCb cb) override;

+    bool fetchHeadersBack(const primitives::BlockInfo& max, primitives::BlockNumber min, bool isFinalized,


document function

…ync observer

…e reinvented

ErakhtinB requested a review from turuslan September 2, 2024 03:47

ErakhtinB linked an issue Sep 2, 2024 that may be closed by this pull request

[Feature Request]: Sync missing beefy block headers #2193

Closed

ErakhtinB force-pushed the 2193-feature-request-sync-missing-beefy-block-headers branch from c11e8ad to 64076a8 Compare September 2, 2024 04:33

turuslan reviewed Sep 2, 2024

View reviewed changes

turuslan reviewed Sep 3, 2024

View reviewed changes

core/consensus/beefy/impl/beefy_impl.cpp Outdated Show resolved Hide resolved

turuslan reviewed Sep 3, 2024

View reviewed changes

core/network/impl/synchronizer_impl.cpp Outdated Show resolved Hide resolved

ErakhtinB force-pushed the 2193-feature-request-sync-missing-beefy-block-headers branch from 7b05c57 to d854bed Compare September 3, 2024 11:38

turuslan reviewed Sep 4, 2024

View reviewed changes

core/consensus/beefy/impl/beefy_impl.cpp Outdated Show resolved Hide resolved

turuslan reviewed Sep 6, 2024

View reviewed changes

ErakhtinB force-pushed the 2193-feature-request-sync-missing-beefy-block-headers branch from aa6237c to 8c729bb Compare September 10, 2024 06:11

turuslan reviewed Sep 10, 2024

View reviewed changes

turuslan reviewed Sep 11, 2024

View reviewed changes

core/network/impl/synchronizer_impl.cpp Outdated Show resolved Hide resolved

turuslan approved these changes Sep 11, 2024

View reviewed changes

kamilsa reviewed Sep 11, 2024

View reviewed changes

ErakhtinB added 16 commits September 12, 2024 10:39

Fetch and store blocks between last grandpa and last beefy finalized

e22176b

Unit tests are modified regarding to block storage usage in beefy impl

8a185df

Clang werrors fix

f5efafe

Logic fixes on PR

1370594

Block storing by syncronizer is ready, but injector fails on build

bff662d

Build fixed with lazyptr, false on assigning hash

01d406f

Block info assertion in synchronizer fails

67415cd

Block info assertion in synchronizer fails works with warnings from s…

c0476eb

…ync observer

Syncronizer is tested with mock block storage

a030dd0

fixes for clang tidy pipeline

861edc6

Cant check some blocks for finilization

32be2cb

Blocks to store are checked on finalization and parent hash

924f011

Not storing block if can not establish relation from finilized one

18413e3

Saving blocks only on justification to mention bottom border

95fb5cd

Correct fetch headers, finalization and parent hash checking has to b…

7300a63

…e reinvented

Parent info is used for fetching and checking blocks

b1d4cec

ErakhtinB added 6 commits September 12, 2024 10:39

clang tidy error fix

995b897

Synchronizer include moved to cpp file of beefy

1a2d96e

Fetching headers obj is simplified

ff05998

Fetch headers back improved

0bfd1b1

Fetching header by batches anyway

6cce7e3

Custom warp sync test enabled

2dfe877

ErakhtinB force-pushed the 2193-feature-request-sync-missing-beefy-block-headers branch from fa931df to 2dfe877 Compare September 12, 2024 02:56

ErakhtinB added 3 commits September 12, 2024 11:10

Fixes for clang tidy errors

d3d27e7

clang format

8bfd8a9

Comments

f7340ce

kamilsa approved these changes Sep 12, 2024

View reviewed changes

ErakhtinB merged commit e6d02f6 into master Sep 12, 2024
19 of 22 checks passed

ErakhtinB deleted the 2193-feature-request-sync-missing-beefy-block-headers branch September 12, 2024 06:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch and store blocks between last grandpa and last beefy finalized #2202

Fetch and store blocks between last grandpa and last beefy finalized #2202

ErakhtinB commented Sep 2, 2024 •

edited by kamilsa

Loading

kamilsa left a comment

kamilsa Sep 11, 2024

kamilsa Sep 11, 2024

kamilsa Sep 11, 2024

Fetch and store blocks between last grandpa and last beefy finalized #2202

Fetch and store blocks between last grandpa and last beefy finalized #2202

Conversation

ErakhtinB commented Sep 2, 2024 • edited by kamilsa Loading

Referenced issues

Description of the Change

Possible Drawbacks

Checklist Before Opening a PR

kamilsa left a comment

Choose a reason for hiding this comment

kamilsa Sep 11, 2024

Choose a reason for hiding this comment

kamilsa Sep 11, 2024

Choose a reason for hiding this comment

kamilsa Sep 11, 2024

Choose a reason for hiding this comment

ErakhtinB commented Sep 2, 2024 •

edited by kamilsa

Loading