Check sok message outside of verifier #18329
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sok message has to be checked against digest, but it's better to be done on a different level (not in verifier executed in parallel process). This commit only removes the check, it is reintroduced in the follow-up commit.
Ensure that sok message is checked against the digests in all of the contexts where it was validated previously. Check is not reintroduced into scan state checking, because follow-up commits will remove the sok message from scan state's merge nodes.
Member
Author
|
!ci-build-me |
georgeee
commented
Jan 15, 2026
| Deferred.Or_error.return | ||
| (Or_error.error_string | ||
| "Transaction_snark.verify: Mismatched sok_message" ) | ||
| Deferred.Or_error.return (Ok ()) |
Member
Author
There was a problem hiding this comment.
It's possible that some tests might be failing because I didn't attempt to preserve dummy's behavior
Member
Author
There was a problem hiding this comment.
And so they failed: https://buildkite.com/o-1-labs-2/mina-o-1-labs/builds/40027/steps/canvas?jid=019bc356-5a4c-42ca-a519-67fb0d56c896
Error: { \"string\": \"proof's sok message digest does not match the sok message\" }"
Member
|
!ci-build-me |
1 similar comment
Member
|
!ci-build-me |
martyall
force-pushed
the
georgeee/check-sok-message-digest-outside-verifier
branch
from
d25f8f3 to
d5ce79c
Compare
Member
|
!ci-build-me |
martyall
approved these changes
Jan 21, 2026
| (Ledger_proof.Cached.read_proof_from_disk p, msg) ) ) | ||
| ) ) | ||
| in | ||
| let check_sok_digests () = |
Member
Author
There was a problem hiding this comment.
I just followed the semantics used below with check (). I agree both check () and check_sok_digests () can be computed eagerly, with no () argument.
Feel free to make a follow-up PR after it's merged to develop @martyall !
Member
Author
|
!ci-bypass-changelog |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The sok message was previously passed to the verifier process solely to perform a lightweight check that's better handled in the main process. This forced the sok message to remain in the scan state structure despite having no other use there.
This PR moves the sok message digest check to the main process, removing it from the verifier interface and enabling future removal of the sok message field from scan state merge nodes.
This PR is split into two parts. First part removes the digest check completely:
Second part reintroduces the digest check in all places which call the transaction snark verifier:
The sok message digest check is not re-introduced into verification of scan state in anticipation of #18325 being merged shortly after this PR.
Explain how you tested your changes:
Checklist: