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: read success value from hook #7

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ecdsa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ cfg-if = "1.0"
hex-literal = "0.4"

[target.'cfg(all(target_os = "zkvm", target_vendor = "succinct"))'.dependencies]
sp1-lib = "3"
sp1-lib = "3.3"
anyhow = "1.0"

[dev-dependencies]
Expand Down
6 changes: 5 additions & 1 deletion ecdsa/src/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,11 @@ where

// Note: An additional check can be added to ensure the field bytes size is 32 bytes, but this is not neceessary.
if C::ORDER == <C as Curve>::Uint::decode_field_bytes(GenericArray::from_slice(&SECP256K1_ORDER)) && prehash.len() == 32 {
return Self::recover_from_prehash_secp256k1(prehash, signature, recovery_id);
// If we get an error here this means the executor couldnt witness the recovery
// so lets continue normally so we can constrain the failure.
if let Ok(s) = Self::recover_from_prehash_secp256k1(prehash, signature, recovery_id) {
return Ok(s);
}
}
}
}
Expand Down
19 changes: 12 additions & 7 deletions ecdsa/src/sp1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use elliptic_curve::{
};

use elliptic_curve::Field;
use sp1_lib::io::{self, FD_ECRECOVER_HOOK};
use sp1_lib::io::{self, FD_ECRECOVER_HOOK_2};
use sp1_lib::unconstrained;
use sp1_lib::{
secp256k1::Secp256k1Point, syscall_secp256k1_decompress, utils::AffinePoint as Sp1AffinePoint,
Expand Down Expand Up @@ -39,7 +39,7 @@ where
sig_bytes[..64].copy_from_slice(&signature.to_bytes());
sig_bytes[64] = recovery_id.to_byte();
let (compressed_pubkey, s_inv) =
recover_ecdsa_unconstrained(&sig_bytes, prehash.try_into().unwrap());
recover_ecdsa_unconstrained(&sig_bytes, prehash.try_into().unwrap())?;

// Convert the s_inverse bytes to a scalar.
let s_inverse = Scalar::<C>::from_repr(bits2field::<C>(&s_inv).unwrap()).unwrap();
Expand Down Expand Up @@ -149,7 +149,7 @@ fn be_bytes_to_le_bits(be_bytes: &[u8; 32]) -> [bool; 256] {
/// WARNING: The values are read from outside of the VM and are not constrained to be correct. Use
/// [`VerifyingKey::recover_from_prehash_secp256k1`] to securely recover the public key associated with
/// a signature and message hash.
fn recover_ecdsa_unconstrained(sig: &[u8; 65], msg_hash: &[u8; 32]) -> ([u8; 33], [u8; 32]) {
fn recover_ecdsa_unconstrained(sig: &[u8; 65], msg_hash: &[u8; 32]) -> Result<([u8; 33], [u8; 32])> {
// The `unconstrained!` wrapper is used to not include the cycles used to get the "hint" for the compressed
// public key and s_inverse values from a non-zkVM context, because the values will be constrained
// in the VM.
Expand All @@ -158,13 +158,18 @@ fn recover_ecdsa_unconstrained(sig: &[u8; 65], msg_hash: &[u8; 32]) -> ([u8; 33]
let (buf_sig, buf_msg_hash) = buf.split_at_mut(sig.len());
buf_sig.copy_from_slice(sig);
buf_msg_hash.copy_from_slice(msg_hash);
io::write(FD_ECRECOVER_HOOK, &buf);
io::write(FD_ECRECOVER_HOOK_2, &buf);
}

let success = io::read_vec();
if success.first().expect("A status flag from the executor, this is a bug") == &0 {
return Err(Error::new());
}

let recovered_compressed_pubkey: [u8; 33] = io::read_vec().try_into().unwrap();
let s_inv_bytes_le: [u8; 32] = io::read_vec().try_into().unwrap();
let recovered_compressed_pubkey: [u8; 33] = io::read_vec().try_into().expect("A compressed public key from the executor, this is a bug");
let s_inv_bytes_le: [u8; 32] = io::read_vec().try_into().expect("An s inverse value from the executor, this is a bug");

(recovered_compressed_pubkey, s_inv_bytes_le)
Ok((recovered_compressed_pubkey, s_inv_bytes_le))
}

/// Takes in a compressed public key and decompresses it using the SP1 syscall `syscall_secp256k1_decompress`.
Expand Down