Skip to content

Conversation

@ShahakShama
Copy link
Collaborator

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

@ShahakShama ShahakShama force-pushed the shahak/fix_parent_hash_bug branch from 4eed5d0 to fdb5fa0 Compare October 30, 2025 15:30
@ShahakShama ShahakShama enabled auto-merge October 30, 2025 15:31
@ShahakShama ShahakShama force-pushed the shahak/fix_parent_hash_bug branch from fdb5fa0 to 1182b41 Compare October 30, 2025 15:34
Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion


crates/apollo_state_sync/src/lib.rs line 172 at r2 (raw file):

                // As a fallback, try to get the block hash through the feeder directly. This
                // method is faster than get_block which the sync runner uses.
                // TODO(shahak): Test this flow.

What about this?

Code quote:

 // TODO(shahak): Test this flow.

Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @ShahakShama)


crates/apollo_state_sync/src/lib.rs line 166 at r2 (raw file):

            )
        })
        .await??;

Please add comment explaining what's happening here.
Also, why do we need a spawn_blocking here?

Code quote:

        let block_hash_opt = tokio::task::spawn_blocking(move || {
            Ok::<_, StateSyncError>(
                storage_reader
                    .begin_ro_txn()?
                    .get_block_header(block_number)?
                    .map(|header| header.block_hash),
            )
        })
        .await??;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants