refactor(starknet_state_sync): refactor state sync to use p2p sync #2515
Conversation
|
This change is |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
noamsp-starkware
force-pushed
the
noam.s/refactor_state_sync_to_use_p2p_sync
branch
from
0104cc9 to
64882cb
Compare
noamsp-starkware
force-pushed
the
noam.s/refactor_state_sync_to_use_p2p_sync
branch
from
64882cb to
f1589c9
Compare
noamsp-starkware
changed the title
There was a problem hiding this comment.
Reviewed 1 of 5 files at r1, 2 of 3 files at r2, all commit messages.
Reviewable status: 3 of 5 files reviewed, 2 unresolved discussions (waiting on @noamsp-starkware)
crates/starknet_state_sync/src/runner/mod.rs line 47 at r2 (raw file):
} result = &mut self.p2p_sync_client_future => return result.map_err(|_| ComponentError::InternalComponentError), _ = &mut self.p2p_sync_server_future => {
change _ to () so that if sync server changes the return type you'll get a compiler error and will change the code here to return result.map_err
crates/starknet_state_sync/src/config.rs line 15 at r2 (raw file):
#[validate] pub storage_config: StorageConfig, // TODO(shahak): add validate to P2PSyncClientConfig, NetworkConfig and use them here.
NetworkConfig already has validate. Use it
There was a problem hiding this comment.
Reviewable status: 3 of 5 files reviewed, 3 unresolved discussions (waiting on @noamsp-starkware)
crates/starknet_state_sync/src/runner/test.rs line 1 at r2 (raw file):
// TODO: Refactor these to suit the change to state sync now using p2p sync.
Erase these instead of commenting them out
There was a problem hiding this comment.
Reviewed 1 of 5 files at r1, 1 of 3 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @noamsp-starkware)
crates/starknet_state_sync/src/runner/test.rs line 1 at r2 (raw file):
Previously, ShahakShama wrote…
Erase these instead of commenting them out
We have git to store the old code
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ShahakShama)
crates/starknet_state_sync/src/config.rs line 15 at r2 (raw file):
Previously, ShahakShama wrote…
NetworkConfig already has validate. Use it
Done.
crates/starknet_state_sync/src/runner/mod.rs line 47 at r2 (raw file):
Previously, ShahakShama wrote…
change _ to () so that if sync server changes the return type you'll get a compiler error and will change the code here to return result.map_err
Done.
crates/starknet_state_sync/src/runner/test.rs line 1 at r2 (raw file):
Previously, ShahakShama wrote…
We have git to store the old code
Alon's PR already took care of the file. I left this as so his PR doesn't have merge conflicts.
noamsp-starkware
force-pushed
the
noam.s/refactor_state_sync_to_use_p2p_sync
branch
from
f1589c9 to
5e44f1b
Compare
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @noamsp-starkware)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2515 +/- ##
===========================================
+ Coverage 40.10% 77.47% +37.36%
===========================================
Files 26 387 +361
Lines 1895 41434 +39539
Branches 1895 41434 +39539
===========================================
+ Hits 760 32101 +31341
- Misses 1100 7067 +5967
- Partials 35 2266 +2231 ☔ View full report in Codecov by Sentry. |
No description provided.