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

[AUDIT][Med/Low Severity] Fix race condition in state update for block building #3508

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

jparr721
Copy link
Contributor

Closes #<ISSUE_NUMBER>

This PR:

Hotshot initiates a block build request when a node is the leader of the next view. This process is handled via the wait_for_block function call. However, a race condition exists concerning the timing of reads and writes to the validated_state_map.

When validate_proposal_safety_and_liveness is called, it can update the validated_state_map after the ViewChange from the update_view function is processed. This results in wait_for_block reading stale state as the parent during the while loop lookup.

This PR fixes the issue by moving the update_view call to happen after the validated state update has occurred and it also adds another call site to the liveness branch.

This PR does not:

Key places to review:

@jparr721 jparr721 requested a review from bfish713 as a code owner July 29, 2024 15:33
@@ -95,6 +94,7 @@ async fn test_quorum_proposal_recv_task() {
proposals[1].data.clone(),
leaves[0].clone(),
)),
exact(ViewChange(ViewNumber::new(2))),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a serial output, so this test changed but the liveness test did not need to.

@jparr721 jparr721 merged commit 34421ca into main Jul 30, 2024
36 checks passed
@jparr721 jparr721 deleted the jp/TIST-2 branch July 30, 2024 13:53
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.

3 participants