feat: verify shard transitions + fixes #482
Conversation
ctian1
changed the base branch from
main
to
kevjue/shard_transition_public_values
ctian1
force-pushed
the
chris/full-verify
branch
2 times, most recently
from
a0f2549 to
2165e2a
Compare
ctian1
force-pushed
the
chris/full-verify
branch
from
2165e2a to
288b940
Compare
ctian1
changed the title
prover/src/lib.rs
Outdated
| @@ -527,6 +527,7 @@ mod tests { | |||
| // reduce steps to prove that the witnessed challenger was correct. | |||
| let mut sp1_challenger = sp1_machine.config().challenger(); | |||
| sp1_challenger.observe(vk.commit); | |||
There was a problem hiding this comment.
should we just have a method that gets all the vkey fields? this feels a bit unsafe?
There was a problem hiding this comment.
also I feel lke you have to do this everywhere where you observe all the fields and manually add it. what about sp1_challenger.observe(vk.as_elements()) or something liek that
core/src/stark/machine.rs
Outdated
| @@ -279,13 +291,97 @@ impl<SC: StarkGenericConfig, A: MachineAir<Val<SC>>> MachineStark<SC, A> { | |||
|
|
|||
| // Verify the segment proofs. | |||
There was a problem hiding this comment.
nit: "Verify the shard proofs"
| )); | ||
| } | ||
| // Digests and exit code should be the same in all shards. | ||
| if public_values.committed_value_digest |
There was a problem hiding this comment.
wait I feel like don't we just need to check all this stuff in the last shard? or is there an edge case because it could be one of the last 2 shards? either way it feels a bit weird to me that we have to check it in here? cc @tamirhemo as well
There was a problem hiding this comment.
The digests are committed with a syscall that does one word at a time so it has to be committed in multiple shards
| @@ -574,17 +574,6 @@ impl CpuChip { | |||
| builder.index_word_array(&commit_digest, &ecall_columns.index_bitmap); | |||
|
|
|||
| let digest_word = local.op_c_access.prev_value(); | |||
| // Verify b and c do not change during commit syscall. | |||
There was a problem hiding this comment.
Why can we just delete this? Doesn't we need this?
There was a problem hiding this comment.
I think it's read cols so there's no point even checking it? Also we actually check it in other places in the same file as well lol
(PublicValuesDigest, DeferredDigest)pc_starttoVerifyingKeyProgramMemoryChipso it can't be ignored in favor of normal MemoryInit