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

feat(consensus): update sync when decision_reached #2925

Merged
merged 1 commit into from
Dec 26, 2024

Conversation

asmaastarkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor

@matan-starkware matan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @asmaastarkware)


crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 346 at r1 (raw file):

            .decision_reached(DecisionReachedInput { proposal_id })
            .await
            .unwrap()

expect

Code quote:

            .unwrap()

crates/sequencing/papyrus_consensus_orchestrator/src/sequencer_consensus_context.rs line 365 at r1 (raw file):

                .await
                .expect("Failed to add new block.")
        });

Suggestion:

        let transaction_hashes =
            transactions.iter().map(|tx| tx.tx_hash()).collect::<Vec<TransactionHash>>();
        let sync_block =
            SyncBlock { block_number: BlockNumber(height), state_diff, transaction_hashes };
        let state_sync_client = self.state_sync_client.clone();
        // `add_new_block` returns immediately, it doesn't wait for sync to fully process the block.
            state_sync_client
                .add_new_block(BlockNumber(height), sync_block)
                .await
                .expect("Failed to add new block.");

@asmaastarkware asmaastarkware force-pushed the asmaa/decision_reached_updates_sync branch from 3567937 to bdf3383 Compare December 25, 2024 08:28
Copy link

Artifacts upload workflows:

@asmaastarkware asmaastarkware changed the base branch from asmaa/try_sync to asmaa/impl_try_sync December 25, 2024 08:29
@matan-starkware matan-starkware self-requested a review December 25, 2024 12:06
@asmaastarkware asmaastarkware force-pushed the asmaa/impl_try_sync branch 3 times, most recently from 39684f3 to 9477a68 Compare December 26, 2024 09:12
@asmaastarkware asmaastarkware force-pushed the asmaa/decision_reached_updates_sync branch 2 times, most recently from 4b5235f to 748c777 Compare December 26, 2024 09:53
@asmaastarkware asmaastarkware changed the base branch from asmaa/impl_try_sync to main December 26, 2024 09:53
@asmaastarkware asmaastarkware force-pushed the asmaa/decision_reached_updates_sync branch from 748c777 to f29b499 Compare December 26, 2024 10:03
@asmaastarkware asmaastarkware force-pushed the asmaa/decision_reached_updates_sync branch from f29b499 to 8f5e8e7 Compare December 26, 2024 10:06
Copy link
Contributor

@matan-starkware matan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @asmaastarkware)

@asmaastarkware asmaastarkware added this pull request to the merge queue Dec 26, 2024
Merged via the queue into main with commit 8a4662e Dec 26, 2024
26 checks passed
@asmaastarkware asmaastarkware deleted the asmaa/decision_reached_updates_sync branch December 26, 2024 12:47
@github-actions github-actions bot locked and limited conversation to collaborators Dec 28, 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