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

Fix deadlock for garble-vm #197

Merged
merged 3 commits into from
Dec 13, 2024
Merged

Fix deadlock for garble-vm #197

merged 3 commits into from
Dec 13, 2024

Conversation

th4s
Copy link
Member

@th4s th4s commented Dec 12, 2024

This PR fixes the VM hanging, when circuit output ranges are used as input ranges for a new circuit. The change fixes the curently hanging tests of the prf and cipher crates of tlsnotary.

When a circuit is executed the input range corresponding to the output range of the circuit output is not marked as complete. This leads to is_committed returning false and the circuit never being executed.

Questions

  1. Do we also need to touch assigned or all of InputView? Anything else which needs to be adapted for consistency?
  2. Is this change compatible with the overall picture, because I get the idea that so far inputs and outputs are treated entirely different.

@th4s th4s requested a review from sinui0 December 12, 2024 11:09
@sinui0
Copy link
Collaborator

sinui0 commented Dec 12, 2024

This isn't the right solution, as we do need to distinguish between input and output values. The right approach is likely to modify is_committed so that it considers completed outputs as committed.

@th4s th4s mentioned this pull request Dec 13, 2024
@th4s
Copy link
Member Author

th4s commented Dec 13, 2024

Adapted is_committed now instead of mixing input and output ranges. I also included Dan's suggestion to fix the preprocessing in this PR.

@sinui0 sinui0 merged commit 0eab0be into alpha.1 Dec 13, 2024
@sinui0 sinui0 deleted the fix-deadlock-vm branch December 13, 2024 17:51
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.

2 participants