Add crate stwo_verify

[avi-starkware]

This change is 

[Yael-Starkware]

@Yael-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @gilbens-starkware, and @nitsan-starkware).

---

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

        args.program_hash_output,
        preprocessed_trace,
    )?;

this function should be implemented in stwo-cairo

Code quote:

    verify_proof(
        args.proof_path,
        args.proof_format,
        channel_hash,
        args.program_output,
        args.program_hash_output,
        preprocessed_trace,
    )?;

[Yael-Starkware]

@Yael-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @avi-starkware, @gilbens-starkware, and @nitsan-starkware).

---

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

this function should be implemented in stwo-cairo

@nitsan-starkware , this is the same logic that we moved from stwo_run_and_prove to stwo-cairo right?

[nitsan-starkware]

@nitsan-starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware and @Yael-Starkware).

---

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

@nitsan-starkware , this is the same logic that we moved from stwo_run_and_prove to stwo-cairo right?

Right, in function create_and_serialize_proof. And maybe the logic in parse_preprocessed_trace and parse_channel_hash should also move to stwo-cairo, so this binary will only be a wrapper.

[Yael-Starkware]

@Yael-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @anatgstarkware and @gilbens-starkware).

---

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

Previously, nitsan-starkware (Nitsan Davidovich) wrote…

Right, in function create_and_serialize_proof. And maybe the logic in parse_preprocessed_trace and parse_channel_hash should also move to stwo-cairo, so this binary will only be a wrapper.

@anatgstarkware wdyt?

[anatgstarkware]

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

@anatgstarkware wdyt?

I think these are small util functions relevant only to binaries, so if you have utils file they can go there, otherwise, they should stay here. Anyway, I don't think these should be in stwo_cairo. We have binary utils in stwo-cairo that are only used in proving utils afaik, so we will move them here at some point. For now, I think we shouldn't delay this pr.

[Yael-Starkware]

@Yael-Starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @anatgstarkware, @gilbens-starkware, and @nitsan-starkware).

---

crates/stwo_verify/src/main.rs line 95 at r2 (raw file):

Previously, anatgstarkware (anatg) wrote…

I think these are small util functions relevant only to binaries, so if you have utils file they can go there, otherwise, they should stay here. Anyway, I don't think these should be in stwo_cairo. We have binary utils in stwo-cairo that are only used in proving utils afaik, so we will move them here at some point. For now, I think we shouldn't delay this pr.

I think this code is duplicate of these utils.
But sure we can refactor later so we wouldn't delay this.

[nitsan-starkware]

@nitsan-starkware reviewed 3 files and all commit messages, and made 16 comments.
Reviewable status: 3 of 4 files reviewed, 17 unresolved discussions (waiting on @anatgstarkware, @avi-starkware, and @gilbens-starkware).

---

a discussion (no related file):
I didn't review the README file yet

---

crates/stwo_verify/src/main.rs line 23 at r2 (raw file):

/// Exit codes:
/// - 0: Proof is valid.
/// - 1: Proof is invalid or verification failed.

What do you mean by 'proof is invalid'?
From the previous line it seems that valid proof = verification succeeded.

Code quote:

Proof is invalid

---

crates/stwo_verify/src/main.rs line 36 at r2 (raw file):

        value_enum,
        default_value_t = ProofFormat::CairoSerde,
        help = "Proof format: Json, CairoSerde, or Binary."

help = "Proof format that was used to generate the proof: Json, CairoSerde, or Binary."

Code quote:

help = "Proof format: Json, CairoSerde, or Binary."

---

crates/stwo_verify/src/main.rs line 42 at r2 (raw file):

        long = "channel_hash",
        default_value = "blake2s",
        help = "Hash variant for the Merkle channel: blake2s or poseidon252."

help = "Hash variant of the Merkle channel to use for verifying the proof, should be the same as the one used to generate the proof: blake2s or poseidon252."

Code quote:

help = "Hash variant for the Merkle channel: blake2s or poseidon252."

---

crates/stwo_verify/src/main.rs line 54 at r2 (raw file):

        help = "Optional absolute path where the program hash will be saved."
    )]
    program_hash_output: Option<PathBuf>,

program_hash

Code quote:

program_hash_output

---

crates/stwo_verify/src/main.rs line 90 at r2 (raw file):

    verify_proof(
        args.proof_path,
        args.proof_format,

1. Can we make sure that this is indeed the format of the proof?
2. What failure will be returned if the user sends a wrong format? Maybe we should add a test for this case?

Code quote:

args.proof_format,

---

crates/stwo_verify/src/main.rs line 91 at r2 (raw file):

        args.proof_path,
        args.proof_format,
        channel_hash,

Same

Code quote:

channel_hash,

---

crates/stwo_verify/src/main.rs line 94 at r2 (raw file):

        args.program_output,
        args.program_hash_output,
        preprocessed_trace,

Same

Code quote:

preprocessed_trace,

---

crates/stwo_verify/src/main.rs line 104 at r2 (raw file):

    match preprocessed_trace {
        "canonical" => PreProcessedTraceVariant::Canonical,
        "canonical_without_pedersen" | "no_pedersen" => {

Why should we also accept this variant? If we need it, please also add it to the error message below.

Code quote:

"no_pedersen"

---

crates/stwo_verify/src/main.rs line 107 at r2 (raw file):

            PreProcessedTraceVariant::CanonicalWithoutPedersen
        }
        _ => panic!(

Please return an error instead of panicking.

Code quote:

panic!

---

crates/stwo_verify/src/main.rs line 118 at r2 (raw file):

        "blake2s" => Ok(ChannelHash::Blake2s),
        "poseidon252" => Ok(ChannelHash::Poseidon252),
        _ => Err(StwoVerifyError::Anyhow(anyhow::anyhow!(

Please create an explicit error instead of using anyhow.

Code quote:

Anyhow

---

crates/stwo_verify/src/main.rs line 174 at r2 (raw file):

        program_output_path,
        program_hash_path,
    )?;

In case this fails, I think that we should start with verify_cairo.

---

crates/stwo_verify/src/main.rs line 199 at r2 (raw file):

        program_output_path,
        program_hash_path,
    )?;

Same

---

crates/stwo_verify/src/main.rs line 207 at r2 (raw file):

}

fn write_verification_output(

write_program_output_and_program_hash

Code quote:

write_verification_output

---

crates/stwo_verify/src/main.rs line 212 at r2 (raw file):

    program_hash_path: Option<PathBuf>,
) -> Result<(), StwoVerifyError> {
    let verification_output = get_verification_output(public_memory);

It is redundant to call it if we don't get an output_path or a hash_path. Consider checking this first, or doing it before calling this function.

---

crates/stwo_verify/src/main.rs line 234 at r2 (raw file):

    Ok(())
}

Please add tests