search

NethermindEth / juno

Starknet client implementation.
https://juno.nethermind.io
Apache License 2.0
371 stars 157 forks source link

Spec update #1858

Open kirugan opened 1 month ago

kirugan commented 1 month ago

Issue #1688

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 75.40%. Comparing base (4e6dcd7) to head (8670ea7).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1858 +/- ## ========================================== - Coverage 75.43% 75.40% -0.03% ========================================== Files 97 97 Lines 8342 8340 -2 ========================================== - Hits 6293 6289 -4 - Misses 1519 1520 +1 - Partials 530 531 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

kirugan commented 1 week ago

There is some todo I think we can work but we can complete them in a separate PR. Also, there is some unused code which should be removed (such as randHeight and onBlockBodiesRequest.

In general, the PR looks good, I have left some questions and suggestions.

Here are some of my thoughts on the sync process:

* Good first step towards using the new conventions.

* To fully utilise the Stream convention we should request more than 1 block parts, however, this would complicate the sync process a lot.

* We should try to benchmark the current sync process so that it can be compared to any new ones in the future. It may be the case that the simple one is more efficient.

Totally agree