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.

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

refactor(sync): move non fatal errors from P2PSyncClientError to BadPeerError #1822

Conversation

noamsp-starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@noamsp-starkware noamsp-starkware changed the title refactor(p2p_sync): Move non fatal errors from P2PSyncClientError to BadPeerError refactor(sync): Move non fatal errors from P2PSyncClientError to BadPeerError Nov 5, 2024
@noamsp-starkware noamsp-starkware force-pushed the noam.s/light_verifications_inside_sync_2 branch from 07bdb1d to cfe3d0d Compare November 5, 2024 11:15
@noamsp-starkware noamsp-starkware force-pushed the noam.s/light_verifications_inside_sync_2 branch from cfe3d0d to 7d0efad Compare November 5, 2024 11:17
@noamsp-starkware noamsp-starkware changed the title refactor(sync): Move non fatal errors from P2PSyncClientError to BadPeerError refactor(sync): move non fatal errors from P2PSyncClientError to BadPeerError Nov 5, 2024
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 18.63%. Comparing base (7a23b58) to head (22b2d31).
Report is 2 commits behind head on noam.s/light_verifications_inside_sync_1.

Files with missing lines Patch % Lines
crates/papyrus_p2p_sync/src/client/transaction.rs 0.00% 5 Missing ⚠️
crates/papyrus_p2p_sync/src/client/header.rs 0.00% 2 Missing ⚠️
...ates/papyrus_p2p_sync/src/client/stream_builder.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                            Coverage Diff                            @@
##           noam.s/light_verifications_inside_sync_1    #1822   +/-   ##
=========================================================================
  Coverage                                     18.63%   18.63%           
=========================================================================
  Files                                           100      100           
  Lines                                         12220    12216    -4     
  Branches                                      12220    12216    -4     
=========================================================================
  Hits                                           2277     2277           
+ Misses                                         9516     9509    -7     
- Partials                                        427      430    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)

moves non-fatal state diff related errors to BadPeerError to allow
retrying the query with a different peer. previously state diff related
tests checked these non-fatal errors as assertions to certain scenarios.
we now just check that these scenarios cause peer reporting.
@noamsp-starkware noamsp-starkware merged commit da56660 into noam.s/light_verifications_inside_sync_1 Nov 10, 2024
14 of 15 checks passed
@noamsp-starkware noamsp-starkware deleted the noam.s/light_verifications_inside_sync_2 branch November 10, 2024 12:04
@github-actions github-actions bot locked and limited conversation to collaborators Nov 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants