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

LocalBlockchain vs Mina protocol handling of max event elements #757

Closed
maht0rz opened this issue Feb 28, 2023 · 4 comments · Fixed by #758
Closed

LocalBlockchain vs Mina protocol handling of max event elements #757

maht0rz opened this issue Feb 28, 2023 · 4 comments · Fixed by #758
Assignees

Comments

@maht0rz
Copy link
Contributor

maht0rz commented Feb 28, 2023

The LocalBlockchain enforces that the total amount of elements emitted across account updates, regardless of the contents of the event, should be 16. This behaviour is implemented here.

Based on my limited OCaml understanding, this protocol code implements the same logic, constraining the total number of events across all account updates, regardless of event contents.

However based on my discussion with @mitschabaude, it seems like:

  • The actual behaviour should be to constraint the amount of field elements within events to be 16. Which means 16 field elements total after summing up contents of all events and account updates.
  • Which then opens the question of what is the max allowed amount of events themselves?

Note: the limit of '16' is a protocol/genesis constant in the mina protocol.

I'd like to understand which of these described behaviours is correct (per protocol), and also depending on the outcome of this discussion, i'd like to propose an increase in the limits (e.g. 64 fields in total, or 64 events in total with no content constraints).

These event limits will become very noticable when trying to design an offchain storage ecosystem/library, it creates extra overhead when you write a rollup, since you might emit events inside the reduce itself, or outside on the already reduced state to decrease the amount of events you emit. All of the above is an extra overhead and potentially a hard limitation of the Mina's smart contract platform.

I believe that we can find a reasonable limit that would accommodate snarky rollups/reducers without having to worry too much about the number of emitted events

@mitschabaude
Copy link
Member

mitschabaude commented Feb 28, 2023

I'd like to understand which of these described behaviours is correct (per protocol)

In my reading of the code you linked, it checks for "total amount of field elements", not events! Specifically, this function used in that code counts the field elements in a list of events:
https://github.com/MinaProtocol/mina/blob/7d9e2f544680a3cbf9ad564ab17500a716e4b777/src/lib/mina_base/zkapp_command.ml#L1940-L1941

Which then opens the question of what is the max allowed amount of events themselves?

I don't understand why there should be a total max of events when we already have the max on field elements. The one implies the other: if your events have minimal size (1 field element per event), then the max # events is 16. When your events are bigger, the max # becomes less.

@mitschabaude
Copy link
Member

Tagging @deepthiskumar to comment on increasing the protocol limit

@maht0rz
Copy link
Contributor Author

maht0rz commented Feb 28, 2023

@mitschabaude fair enough on the max field elements vs max events, having max field elements covers all the relevant cases.

What about the LocalBlockchain behaviour? Assuming i tested and understood the local blockchain code correctly, it checks for the amount of events, not field elements.

@mitschabaude
Copy link
Member

What about the LocalBlockchain behaviour? Assuming i tested and understood the local blockchain code correctly, it checks for the amount of events, not field elements.

that's true, working on a fix

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 a pull request may close this issue.

2 participants