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

fix: inputs for RSA example #271

Merged
merged 4 commits into from
Apr 18, 2024
Merged

fix: inputs for RSA example #271

merged 4 commits into from
Apr 18, 2024

Conversation

brozorec
Copy link
Contributor

The RSA example's keys and signature are hardcoded in the program:

let private_key = RsaPrivateKey::from_pkcs8_der(RSA_2048_PRIV_DER).unwrap();
let public_key = RsaPublicKey::from_public_key_der(RSA_2048_PUB_DER).unwrap();
println!("{:?}{:?}", private_key, public_key);
let message = b"Hello world!";
let mut hasher = Sha256::new();
hasher.update(message);
let hashed_msg = hasher.finalize();
let signature = vec![

That makes this example program look "rigid" by generating an elf file for a specific key and message.

It seems more natural to have those elements in the script as inputs. That allows us to generalize the program and reuse the generated elf file for the verification of any key-message pair.

// Read an input to the program.
//
// Behind the scenes, this compiles down to a custom system call which handles reading inputs
let pk_der = sp1_zkvm::io::read::<Vec<u8>>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this program actually work? I don't think this should work because the io reader doesn't know how many bytes to actually read, if I'm not mistaken. You can either use read_slice and pass in a mutable slice with a particular length or you can just read a RsaPublicKey if that implements Serde, Deserde

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it does when running in dev mode with SP1Prover::execute(REGEX_IO_ELF, stdin) but when trying with SP1Prover::prove(REGEX_IO_ELF, stdin) it prints the message from the program and gets killed:
image

I thought it was something related to my machine (Ubuntu 22) because it gets killed too when running the initial RSA example.

because the io reader doesn't know how many bytes to actually read

This was my understanding as well but then it works when String is passed along and String boils down to Vec<u8> 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tested it on a macOS, and I confirm the program works.

pkcs8::{DecodePrivateKey, DecodePublicKey},
RsaPrivateKey, RsaPublicKey,
};
use std::vec;

/// The ELF we want to execute inside the zkVM.
const REGEX_IO_ELF: &[u8] = include_bytes!("../../program/elf/riscv32im-succinct-zkvm-elf");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I know this isn't your fault, but do you mind changing this to "RSA_ELF"


let message = b"Hello world!".to_vec();

let signature: Vec<u8> = vec![
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps it would be better instead of hardcoding to have the signature generated here using the rsa library?

// Instead of generating and verifying the proof each time while developing,
// execute the program with the RISC-V runtime and read stdout.
//
// let mut stdout = SP1Prover::execute(REGEX_IO_ELF, stdin).expect("proving failed");
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can keep this commented-in because it shows how to use the execution mode as well?

@jtguibas jtguibas merged commit 66d2688 into succinctlabs:main Apr 18, 2024
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