Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add documentation for the Indexed Merkle Map API #1088
base: main
Are you sure you want to change the base?
Add documentation for the Indexed Merkle Map API #1088
Changes from all commits
872e59d
aa74239
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I would rather write new examples using zkProgram since we have more confidence that it will not be changed soon.
SmartContract
is more likely to be modified by the new API.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.
Regardless of documentation maintainability, I think that demonstrating a
SmartContract
example for theIndexed Merkle Map
API provides greater benefits for developers and serves as more effective documentation.When someone wants to use the Indexed Merkle Map API, they will likely integrate it directly within a smart contract, as it works seamlessly in provable code.
In contrast, a
ZkProgram
example might lead readers to mistakenly believe that the API should be used with aZkProgram
. This misunderstanding could imply that verifying a zkProgram proof for Indexed Merkle Map updates is straightforward, when in fact it is only practical in advanced scenarios such as recursive reducers.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.
Maybe I don't understand this myself. What's wrong with this example:
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.
There's nothing wrong with the example but I think whoever wants to use the
Indexed MM
would use it directly within a zkApp as it's more practical and for that, I think aSmartContract
example would be a better guide for the reader and less confusing.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.
I'm not sold on this. Do you actually believe indexed merkle map is a usable solution for zkapps today? To me, your example is misleading because it teases the existence of a
map
API that works on Mina, but doesn't address the same old concurrent update or state storage issues that exist for every zkapp. In production, this example would not work with multiple users accessing it at once, and I want to stop creating new examples that break down at scale.I know that this is a touchy issue in the community right now as protokit devs are looking for L1 alternatives to the
StateMap
so that they can deploy on mainnet: (discord example). IMO there is absolutely no alternative that currently works, and I don't want to beat around the bush with that fact and pretend this is a viable alternative. Do you disagree with this?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.
Fair enough, I don't disagree!
I see your point here! But that's also applicable to many primitives in the documentation. I was trying to keep it consistent with the current o1js docs as I was documenting the
Indexed Merkle Map
API.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.
My goal with example code in the docs is for developers to grasp the concept and then take it to build one step further. My problem with the pattern we have in most of our existing documentation is that it only works to a certain point, and that's what we show. When someone tries to take the next step, they hit a wall.
What I like about
ZkProgram
is that the final product can be a Mina ZkApp, or it could live on protokit or zeko. And before deploying it to any network, a developer can change different variables, combine it with otherZkProgram
s, and extend it in all kinds of ways locally.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.
I feel strongly that this should not be included here. This definitely belongs in the Typedoc reference, so we should work on generating that correctly instead of manually adding auto-generated type declarations here.