Skip to content

Commit 6a4efe2

Browse files
Fix bug in ecrecover (our k256 was not being used)
1 parent 13fecc2 commit 6a4efe2

File tree

4 files changed

+10
-6
lines changed

4 files changed

+10
-6
lines changed

benchmarks/guest/ecrecover/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ edition = "2021"
66

77
[dependencies]
88
k256 = { version = "0.13.3", default-features = false, features = ["ecdsa"] }
9+
ecdsa = { version = "0.16.9", default-features = false }
910
openvm = { path = "../../../crates/toolchain/openvm", features = ["std"] }
1011
openvm-platform = { path = "../../../crates/toolchain/platform", default-features = false }
1112
openvm-algebra-guest = { path = "../../../extensions/algebra/guest", default-features = false }

benchmarks/guest/ecrecover/src/main.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
use alloy_primitives::{Bytes, B256, B512};
2-
use k256::ecdsa::{Error, RecoveryId, Signature, VerifyingKey};
2+
// Be careful not to import k256::ecdsa::{Signature, VerifyingKey}
3+
// because those are type aliases that their (non-zkvm) implementations
4+
use ecdsa::{Signature, VerifyingKey};
5+
use k256::ecdsa::{Error, RecoveryId};
36
use openvm::io::read_vec;
4-
use openvm_k256::Secp256k1Point;
7+
use openvm_k256::{Secp256k1, Secp256k1Point};
58
#[allow(unused_imports, clippy::single_component_path_imports)]
69
use openvm_keccak256::keccak256;
710
// export native keccak
@@ -29,7 +32,7 @@ fn ecrecover(sig: &B512, mut recid: u8, msg: &B256) -> Result<B256, Error> {
2932
}
3033
let recid = RecoveryId::from_byte(recid).expect("recovery ID is valid");
3134

32-
let recovered_key = VerifyingKey::recover_from_prehash(&msg[..], &sig, recid)?;
35+
let recovered_key = VerifyingKey::<Secp256k1>::recover_from_prehash(&msg[..], &sig, recid)?;
3336
let mut hash = keccak256(
3437
&recovered_key
3538
.to_encoded_point(/* compress = */ false)

guest-libs/k256/guest/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ impl Curve for Secp256k1 {
3535
/// 32-byte serialized field elements.
3636
type FieldBytesSize = U32;
3737

38-
// I don't think any arithmetic is done on Curve::Uint, so it's fine to use the U256 type
39-
// instead of the OpenVM ruint impl
38+
// Perf: Use the U256 type from openvm_ruint here
4039
type Uint = U256;
4140

4241
/// Curve order.

guest-libs/k256/guest/src/scalar.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,8 @@ impl Reduce<U256> for Secp256k1Scalar {
155155

156156
#[inline]
157157
fn reduce_bytes(bytes: &FieldBytes) -> Self {
158-
Self::reduce(U256::from_be_byte_array(*bytes))
158+
Self::from_le_bytes(bytes.as_slice())
159+
// Self::reduce(U256::from_be_byte_array(*bytes))
159160
}
160161
}
161162

0 commit comments

Comments
 (0)