-
Notifications
You must be signed in to change notification settings - Fork 18
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: Proving for the playground #50
Conversation
Since this will need to be rebased and updated after the repo structure PR is changed, let's keep it as a draft until then. I'm going to put it into draft mode and remove my review request. Once it's been rebased, please mark it as "ready for review" and then re-request review |
I think this is addressed by discussions below. As long as we have a way to provide/show the proof then it's fine for the output to be the same (imo). Is there something else you were thinking about here? Also, I think deeper considerations about UX should probably wait until we are actually focusing on design. At the moment, I think we should focus on functionality & a modular structure so we can update the design easily once we have someone working on it
We can use react-hot-toast to do this very easily: https://react-hot-toast.com/
I think we should create a new issue for this, but reference #19 inside that issue, as well as the other related open questions/issues you've noted
I think this is part of a larger conversation in the VM, and related to changes for debugging improvements as well (0xPolygonMiden/miden-vm#580). We should bring it up on the team sync call, but maybe this should be separated out from this set of playground chanages and become its own working group that looks at the 2 things together.
This seems to be related to the comment above about having a way to show the proof, so it seems to me that we should just create a new issue for showing the proof. |
5765eb5
to
8ffcac2
Compare
Thanks, I just rebased. |
Here is a new issue about getting and showing the proof. We can tackle this in the next milestone. #52 |
Sounds good! I think we should also create an issue about adding toast messages with https://react-hot-toast.com/ and put that in the "nice to have" for this milestone. wdyt? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you! I left a few very minor comments. Also, I want to be sure all inline TODOs are captured in issues. I think many/most are, but just to check
8ffcac2
to
83bed3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved! Thank you!
// TODO: We must catch all cases of inputs. Input can be not a valid json, | ||
// input can be empty, stack_init can be empty, advice_tape can be empty. And all combinations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I see - you're talking about cases of input errors. I would probably have said "all cases of input errors", but the message is now clear, and no need to update, especially since it'll be tackled in the next PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah indeed. I like it those nits
83bed3d
to
c24c692
Compare
Closes #19
This PR enables proving to the playground.
There are some open questions regarding the UX and the proving step code itself:
UX
Proving step
ProgramOutputs
orExecutionTrace
, so we can't get the numbers (maybe I am mistaken, but I didn't find it)