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

Make 3 unneeded PODTicket fields optional #2151

Closed
wants to merge 2 commits into from

Conversation

artwyman
Copy link
Member

As discussed on Telegram, this removes 3 fields from PODTickets in order to get the size of Devcon tickets down to 16 entries. This makes them fit in circuits with a Merkle Depth 5, which is what we optimized for when creating our circuit family for Devcon.

The relevant fields (isRevoked, ticketCategory, timestampConsumed) are unused for Devcon and foreseen use cases, but have been present in tickets for a long time. The approach here is to make them optional at the PODTicket level, and avoid setting them in PODBox pipelines (PretixPipeline, CSVPipeline). This should be relatively safe, but could still break assumptions we're not aware of.

I haven't directly confirmed the new size of Devcon tickets, and the circuits used for ticket proofs. We should test that on staging before going forward.

For this change to be effective we need to push it to Zupass and also to the Devcon PODBox. I'm on the fence about whether or not to merge this, because of the risks. Here are some thoughts on the tradeoffs:

Downside of not merging:

  • Ticket proofs will be slower than they should be (probably by 1-4s, and download a larger artifact than they need (roughly 25MB instead of 12MB).

Downside of merging:

  • Rob thinks some apps are using query code which looks for all
  • Unknown risks of things going wrong because of something we missed.

@artwyman artwyman requested review from robknight and rrrliu November 10, 2024 09:56
@artwyman artwyman self-assigned this Nov 10, 2024
@artwyman
Copy link
Member Author

I'll let @robknight and @rrrliu decide the timing of merging this, after the parcnet-js SDK is updated with various app dev teams as per proofcarryingdata/parcnet-client#30

@artwyman artwyman assigned artwyman and robknight and unassigned artwyman Nov 10, 2024
@artwyman
Copy link
Member Author

Note that this branch is live on staging-andrew.zupass.org, which also has test Devcon tickets set up on PODBox if you want to use it to test.

@artwyman
Copy link
Member Author

Call is to hold off on this for risk reasons for now. All the performance-sensitive apps have already switched to direct ticket access rather than ZK proofs.
I'm going to keep the PR around in case of need. Will clean up after Devcon.

@artwyman artwyman closed this Dec 10, 2024
@artwyman artwyman deleted the artwyman/optional-fields branch December 10, 2024 00:45
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