Skip to content

Verification of zkapp proofs prior to block creation #16809

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Apr 3, 2025
Merged

Verification of zkapp proofs prior to block creation #16809

merged 20 commits into from
Apr 3, 2025

Conversation

glyh
Copy link
Member

@glyh glyh commented Mar 31, 2025 •
edited
Loading

This is a refactor of #16753, should be merged after #16796

This PR allow staged_ledger to have access to txn pool, so txns already in the pool could be verified faster utilizing the fact any txn in pool must already pass some other checks.

@glyh glyh force-pushed the corvo/verification-of-zkapp-proofs-prior-to-block-creation-based-on-georgee branch 5 times, most recently from 0e4bb14 to bc361b2 Compare March 31, 2025 11:05
@glyh
Copy link
Member Author

glyh commented Mar 31, 2025

!ci-build-me

@glyh
Copy link
Member Author

glyh commented Mar 31, 2025

Did a bunch of force-pushes to make commit message readable 😅

@glyh glyh marked this pull request as ready for review March 31, 2025 11:11
@glyh glyh requested a review from a team as a code owner March 31, 2025 11:11
@glyh glyh assigned glyh and unassigned glyh Mar 31, 2025
@glyh glyh requested a review from georgeee March 31, 2025 11:12
@glyh
Copy link
Member Author

glyh commented Mar 31, 2025

!ci-build-me

@georgeee georgeee changed the base branch from compatible to georgeee/refactor-verification-functions March 31, 2025 11:22
georgeee
georgeee previously requested changes Mar 31, 2025
View reviewed changes
Copy link
Member

@georgeee georgeee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main comment: collect_vk_assumptions does all the necessary checks inside, no need to even consider what it returns.

@georgeee georgeee mentioned this pull request Mar 31, 2025
Merged
7 tasks
@glyh
Copy link
Member Author

glyh commented Mar 31, 2025

done

@glyh glyh requested a review from georgeee March 31, 2025 13:16
@glyh
Copy link
Member Author

glyh commented Mar 31, 2025

!ci-build-me

@glyh glyh force-pushed the corvo/verification-of-zkapp-proofs-prior-to-block-creation-based-on-georgee branch from ab92b4d to efdc495 Compare April 1, 2025 02:32
@glyh
Copy link
Member Author

glyh commented Apr 1, 2025

!ci-build-me

@glyh glyh dismissed georgeee’s stale review April 1, 2025 02:47

Modified as requested

@glyh glyh mentioned this pull request Apr 1, 2025
Open
georgeee
georgeee requested changes Apr 1, 2025
View reviewed changes
@glyh glyh force-pushed the corvo/verification-of-zkapp-proofs-prior-to-block-creation-based-on-georgee branch from fb27a21 to 6e7035f Compare April 1, 2025 12:57
@glyh
Copy link
Member Author

glyh commented Apr 1, 2025

!ci-build-me

@georgeee
Copy link
Member

georgeee commented Apr 1, 2025 via email

@glyh
Copy link
Member Author

glyh commented Apr 1, 2025 •
edited
Loading

But could we make it true, I wonder? To me it seems like we could by not specifying the default argument outside of staged_ledger.ml.
…
On Tue, Apr 1, 2025, 2:04 PM glyh @.> wrote: @.* commented on this pull request. ------------------------------ In src/lib/staged_ledger/staged_ledger.mli <#16809 (comment)>: > @@ -343,9 +351,12 @@ val all_work_pairs : (** Statements of all the pending work in t*) val all_work_statements_exn : t -> Transaction_snark_work.Statement.t list +val dummy_transaction_pool_proxy : transaction_pool_proxy image.png (view on web) <user-attachments/assets/edf25680-30bf-41b8-a13f-72b6f80d3792> — Reply to this email directly, view it on GitHub <#16809 (comment)>, or unsubscribe <github.com/notifications/unsubscribe-auth/AAJ4IISVZISIUKR746NNEBD2XJ6EXAVCNFSM6AAAAAB2DZK4MGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDOMZSGQ3DAMJQHE> . You are receiving this because your review was requested.Message ID: @.***>

Could wrap it in an option I guess, need to check code to see if there's fancier ways.

@glyh glyh requested a review from a team as a code owner April 3, 2025 05:01
@glyh
Copy link
Member Author

glyh commented Apr 3, 2025

!ci-build-me

@glyh
Copy link
Member Author

glyh commented Apr 3, 2025 •
edited
Loading

A merge from compatible is needed so I can pass CI check

@glyh glyh changed the base branch from georgeee/refactor-verification-functions to compatible April 3, 2025 05:17
glyh added 20 commits April 3, 2025 08:09
@glyh @georgeee
Introduce a transaction_pool_proxy that can pass access to find_by_…
868b03b 
…hash in a transaction_pool down to Staged_ledger.check_commands
@glyh @georgeee
Implement function Staged_ledger.verify_command_against_pool, a fas…
7143c0c 
…ter alternative of `Verifier.verify_commands` that utilizing the txn pool
@glyh @georgeee
implement process_separately, a helper we\'ll use shortly for `Stag…
c55223d 
…ed_ledger.check_commands`
@glyh @georgeee
Replace batching verify commands on a process_separately call
4e98be4
@glyh @georgeee
add changelog
892f64c
@glyh @georgeee
format verifier/verifier.mli to pass CI
245a738
@glyh @georgeee
remove redundant comment
e11fa78
@glyh @georgeee
remove check for verificatoin keys in zkapp cmd case as it's already …
c3745b5 
…been taken care of; simplify `verify_command_against_pool` signature
@glyh @georgeee
Refactor portion of verify_command_with_transaction_pool_proxy(name…
278f6cf 
…d `verify_command_against_pool` earlier), and put it into `Verifier.Common`
@glyh @georgeee
update changelog
dbcfb09
@glyh @georgeee
add comments and some style changes
6e2fa45
@glyh @georgeee
revert: make Transaction_hash.User_command_with_valid_signature.t o…
76d0c37 
…paque again
@glyh @georgeee
refactor: remove check commands related function from `Staged_ledger.…
d9d1cb8 
…Stage_ledger` to `Staged_ledger.Check_commands`
@glyh @georgeee
refactor Check_commands.process_separately so it's monad free, credit
8914579 
@georgeee
@glyh @georgeee
remove use of dummy proxy in breadcrumb.ml
b0f9078
@glyh @georgeee
refactor: only expose transaction_pool_proxy instead of a whole `Ch…
5784f87 
…eck_commands` module in staged_ledger
@glyh @georgeee
put pool proxy definition next to Staged_ledger.apply
4112711
@glyh @georgeee
add staged_ledger/check_commands.mli
c135058
@glyh @georgeee
mark in verify_command_with_transaction_pool_proxy that `hash_comma…
d71cf75 
…nd` is slow, we'll come back to this later
@glyh @georgeee
update changelog accordingly
5827ed6
@georgeee georgeee force-pushed the corvo/verification-of-zkapp-proofs-prior-to-block-creation-based-on-georgee branch from 76aa258 to 5827ed6 Compare April 3, 2025 08:10
@georgeee
Copy link
Member

georgeee commented Apr 3, 2025

!ci-build-me

1 similar comment
@glyh
Copy link
Member Author

glyh commented Apr 3, 2025

!ci-build-me

@georgeee georgeee merged commit 4c1ce28 into compatible Apr 3, 2025
47 checks passed
@glyh glyh deleted the corvo/verification-of-zkapp-proofs-prior-to-block-creation-based-on-georgee branch April 3, 2025 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@georgeee georgeee georgeee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@glyh @georgeee