apollo_central_sync: use starknet version as single source of truth for non BC classes#13312
Conversation
…or non BC classes
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Artifacts upload workflows: |
ShahakShama
left a comment
There was a problem hiding this comment.
@ShahakShama reviewed 9 files and all commit messages, and made 11 comments.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on noamsp-starkware).
crates/apollo_central_sync/src/lib.rs line 469 at r1 (raw file):
.expect("Block header must be present before state diff is processed."); let is_bc_compatible_block = block_starknet_version >= STARKNET_VERSION_TO_COMPILE_FROM;
bc = backward compatibility. You're saying compatible twice
I personally prefer bc -> backward
crates/apollo_central_sync/src/lib.rs line 473 at r1 (raw file):
// fails we retry the same block. if let Some(class_manager_client) = &self.class_manager_client { // Non-BC blocks (starknet_version < STARKNET_VERSION_TO_COMPILE_FROM) must be added
In the documentation at least you should have enough space to not use acronym (BC -> backward compatible)
crates/apollo_central_sync/src/lib.rs line 493 at r1 (raw file):
} for (class_hash, deprecated_class) in &deprecated_classes {
Add a comment above here that deprecated classes are always backward compatible
crates/apollo_central_sync/src/lib.rs line 517 at r1 (raw file):
if !is_bc_compatible_block { debug!( "Storing Sierra classes {:?} in storage (non-BC block {block_number})",
Also in logs avoid using acronyms
crates/apollo_central_sync/src/lib.rs line 525 at r1 (raw file):
} } else { debug!("Block {} contains non backward compatible classes.", block_number);
Restore this debug log
crates/apollo_central_sync/src/lib.rs line 558 at r1 (raw file):
) -> StateSyncResult { // The compiled class stream pauses when it reaches a BC block, so this function should // only be called for non-BC blocks.
same here (BC -> backward compatible)
crates/apollo_central_sync/src/lib.rs line 566 at r1 (raw file):
.map(|v| v < STARKNET_VERSION_TO_COMPILE_FROM) .unwrap_or(true), "store_compiled_class called for BC block {block_number}"
same here (BC -> backward compatible)
crates/apollo_central_sync/src/lib.rs line 859 at r1 (raw file):
while from < state_marker { // The block header is guaranteed to be in storage for any block below state_marker. let starknet_version_at_from = txn.get_starknet_version(from)?.expect("Block header must exist for block below state_marker.");
Rename this variable. What is at_from?
crates/apollo_central_sync/src/lib.rs line 879 at r1 (raw file):
); pending::<()>().await; continue;
Do you really need this continue statement?
crates/apollo_central_sync/src/lib.rs line 894 at r1 (raw file):
// Since starknet_version is monotonically non-decreasing, the non-BC range is always // a contiguous prefix. let mut up_to = min(state_marker, BlockNumber(from.0 + u64::from(max_stream_size)));
Consider extracting to a helper function
crates/apollo_central_sync/src/lib.rs line 990 at r1 (raw file):
let is_casm_stuck = casm_marker == new_casm_marker && new_casm_marker < new_state_marker && reader
Move the read to the read that's done above
|
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. |

Note
Medium Risk
Changes the state sync/class-manager integration and compiled-class streaming behavior based on stored
starknet_version, which can affect how/when classes and CASMs are persisted and sent to the class manager during sync.Overview
Switches non-backward-compatible (non-BC) class handling to use the block header’s
starknet_versionas the single source of truth, removing the previous reliance on thecompiler_backward_compatibility_markerand thestore_sierras_and_casms_block_thresholdconfig.State diff processing now sends Sierra classes directly to the class manager only for BC blocks, and persists Sierra classes to storage only for non-BC blocks so the compiled-class stream can later attach CASM and call
add_class_and_executable_unsafe. The compiled-class stream now scans headers to stop/pause at the first BC block and batches downloads to include only non-BC ranges; the sync “stuck” detector (check_sync_progress) is updated accordingly.Removes
store_sierras_and_casms_block_thresholdfrom config/schema/deployment overlays and stops updating the compiler-compatibility marker during header writes (marker remains in storage with a TODO to delete).Written by Cursor Bugbot for commit 8c6ec29. This will update automatically on new commits. Configure here.