feat(sync): implement get_block in StateSync

[noamsp-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[noamsp-starkware]

• feat(sync): implement get_block in StateSync #2285 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

Attention: Patch coverage is 8.00000% with 23 lines in your changes missing coverage. Please review.

Project coverage is 77.03%. Comparing base (e3165c4) to head (e7dc16b).
Report is 601 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/starknet_state_sync/src/runner/mod.rs	9.09%	19 Missing and 1 partial ⚠️
crates/starknet_state_sync_types/src/errors.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2285       +/-   ##
===========================================
+ Coverage   40.10%   77.03%   +36.92%     
===========================================
  Files          26      386      +360     
  Lines        1895    40552    +38657     
  Branches     1895    40552    +38657     
===========================================
+ Hits          760    31239    +30479     
- Misses       1100     7009     +5909     
- Partials       35     2304     +2269     

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

[ShahakShama]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @noamsp-starkware)

---

crates/starknet_state_sync/src/runner/mod.rs line 52 at r1 (raw file):

        tokio::select! {
            result = &mut self.sync_future => result.map_err(|_| ComponentError::InternalComponentError),
            Some((request, sender)) = self.request_receiver.next() => {

Run this inside an async block that loops for requests. Right now it will only handle one request and stop

---

crates/starknet_state_sync/src/runner/mod.rs line 55 at r1 (raw file):

                let response = match request {
                    StateSyncRequest::GetBlock(block_number) => {
                        StateSyncResponse::GetBlock(self.get_block(block_number))

See comment below, and then change this to Ok(self.get_block....map_err(|_| ComponentError::Int...)?

---

crates/starknet_state_sync/src/runner/mod.rs line 110 at r1 (raw file):

    }

    fn get_block(&self, block_number: BlockNumber) -> StateSyncResult<Option<SyncBlock>> {

This wasn't the last thing we said today. We said it should be Result<Option, StorageError>, and all the unwraps should be ? instead

[noamsp-starkware]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ShahakShama)

---

crates/starknet_state_sync/src/runner/mod.rs line 52 at r1 (raw file):

Previously, ShahakShama wrote…

Run this inside an async block that loops for requests. Right now it will only handle one request and stop

Ack

---

crates/starknet_state_sync/src/runner/mod.rs line 55 at r1 (raw file):

Previously, ShahakShama wrote…

See comment below, and then change this to Ok(self.get_block....map_err(|_| ComponentError::Int...)?

Ack

---

crates/starknet_state_sync/src/runner/mod.rs line 110 at r1 (raw file):

Previously, ShahakShama wrote…

This wasn't the last thing we said today. We said it should be Result<Option, StorageError>, and all the unwraps should be ? instead

We also said that we should panic if encountered an error, so I thought it was more readable this way.

[ShahakShama]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @noamsp-starkware)

---

crates/starknet_state_sync/src/runner/mod.rs line 52 at r1 (raw file):

Previously, noamsp-starkware wrote…

Ack

This also works :)

---

crates/starknet_state_sync/src/runner/mod.rs line 108 at r2 (raw file):

    }

    fn get_block(&self, block_number: BlockNumber) -> Result<Option<SyncBlock>, StorageError> {

I thought about it this morning, and I think that we should return the error to the user instead of crashing. This means that you should return this back to StateSyncResult and add a StorageError variant to StateSyncError

sorry for the mess

[noamsp-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ShahakShama)

---

crates/starknet_state_sync/src/runner/mod.rs line 52 at r1 (raw file):

Previously, ShahakShama wrote…

This also works :)

It was really problematic (ownership-wise) to put the loop in the select. I figured this would have the same effect and be much less of a hassle.

---

crates/starknet_state_sync/src/runner/mod.rs line 108 at r2 (raw file):

Previously, ShahakShama wrote…

I thought about it this morning, and I think that we should return the error to the user instead of crashing. This means that you should return this back to StateSyncResult and add a StorageError variant to StateSyncError

sorry for the mess

On it

[github-actions]

Artifacts upload workflows:

• Started at 2024-11-27T09:47:23Z
• Started at 2024-11-27T11:41:57Z

[ShahakShama]

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