diff --git a/packages/common/src/crypto.rs b/packages/common/src/crypto.rs index adaa837e..e81c9714 100644 --- a/packages/common/src/crypto.rs +++ b/packages/common/src/crypto.rs @@ -1,21 +1,28 @@ use k256::ecdsa::{RecoveryId, Signature, VerifyingKey}; use sha3::{Digest, Keccak256}; -use crate::types::{Secpk256k1PublicKey, Signature as Sig}; +use crate::{ + error::ContractError, + types::{Secpk256k1PublicKey, Signature as Sig}, +}; -pub fn recover_pubkey(msg_hash: [u8; 32], signature: Sig) -> Secpk256k1PublicKey { +pub fn recover_pubkey( + msg_hash: [u8; 32], + signature: Sig, +) -> Result { let rs = signature.0[0..64].into(); let id = match signature.0[64] { 0 => RecoveryId::new(false, false), 1 => RecoveryId::new(true, false), - _ => todo!("ERROR"), + _ => return Err(ContractError::InvalidSignatureRecoveryId), }; - let sig = Signature::from_bytes(rs).expect("TODO"); + let sig = Signature::from_bytes(rs).map_err(|_| ContractError::InvalidSignature)?; // Recover - let pubkey = VerifyingKey::recover_from_msg(&msg_hash, &sig, id).expect("TODO"); - pubkey.to_sec1_bytes().to_vec() + let pubkey = VerifyingKey::recover_from_msg(&msg_hash, &sig, id) + .map_err(|_| ContractError::InvalidSignature)?; + Ok(pubkey.to_sec1_bytes().to_vec()) } pub fn hash<'a, I>(iter: I) -> [u8; 32] diff --git a/packages/common/src/error.rs b/packages/common/src/error.rs index 4f856169..f2542822 100644 --- a/packages/common/src/error.rs +++ b/packages/common/src/error.rs @@ -23,6 +23,8 @@ pub enum ContractError { NotOnAllowlist, #[error("InvalidSignature: Invalid signature")] InvalidSignature, + #[error("InvalidSignatureRecoveryId: Invalid signature recovery ID")] + InvalidSignatureRecoveryId, // DR contract errors #[error("InsufficientFunds: Insufficient funds. Required: {0}, available: {1}")] diff --git a/packages/data-requests/src/data_request_result.rs b/packages/data-requests/src/data_request_result.rs index 1b1cfae6..0815a5d9 100644 --- a/packages/data-requests/src/data_request_result.rs +++ b/packages/data-requests/src/data_request_result.rs @@ -43,8 +43,6 @@ pub mod data_request_results { ) -> Result { let sender = validate_sender(&deps, info.sender, sender)?; - // TODO: verify proof is signed by public key - if !check_eligibility(&deps, public_key.clone())? { return Err(IneligibleExecutor); } @@ -89,16 +87,14 @@ pub mod data_request_results { ) -> Result { let sender = validate_sender(&deps, info.sender, sender)?; - // TODO: verify proof is signed by public key, then check eligibility - // if !check_eligibility(&deps, public_key)? { - // return Err(IneligibleExecutor); - // } - // compute hash of reveal body let reveal_body_hash = compute_hash(reveal_body.clone()); // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(reveal_body_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(reveal_body_hash, signature)?; + if !check_eligibility(&deps, public_key.clone())? { + return Err(IneligibleExecutor); + } // find the data request from the committed pool (if it exists, otherwise error) let mut dr = DATA_REQUESTS_POOL.load(deps.storage, dr_id)?; diff --git a/packages/data-requests/src/utils.rs b/packages/data-requests/src/utils.rs index 76d09412..eba8ce8a 100644 --- a/packages/data-requests/src/utils.rs +++ b/packages/data-requests/src/utils.rs @@ -61,25 +61,22 @@ pub fn hash_data_result( ) -> Hash { // hash non-fixed-length inputs let mut results_hasher = Keccak256::new(); - results_hasher.update(result); // TODO check this + results_hasher.update(result); let results_hash = results_hasher.finalize(); - let mut payback_address_hasher = Keccak256::new(); - payback_address_hasher.update(&dr.payback_address); - let payback_address_hash = payback_address_hasher.finalize(); - let mut seda_payload_hasher = Keccak256::new(); seda_payload_hasher.update(&dr.seda_payload); let seda_payload_hash = seda_payload_hasher.finalize(); // hash data result let mut dr_hasher = Keccak256::new(); + dr_hasher.update(dr.version.to_string().as_bytes()); dr_hasher.update(dr.id); dr_hasher.update(block_height.to_be_bytes()); dr_hasher.update(exit_code.to_be_bytes()); dr_hasher.update(results_hash); dr_hasher.update(gas_used.to_be_bytes()); - dr_hasher.update(payback_address_hash); + dr_hasher.update(&dr.payback_address); dr_hasher.update(seda_payload_hash); dr_hasher.finalize().into() } diff --git a/packages/integration-tests/src/data_result.rs b/packages/integration-tests/src/data_result.rs index e21a3593..4f5ed28d 100644 --- a/packages/integration-tests/src/data_result.rs +++ b/packages/integration-tests/src/data_result.rs @@ -487,7 +487,7 @@ fn pop_and_swap_in_pool() { .unwrap(); let fetched_drs = res.value; assert_eq!(fetched_drs.len(), 1); - // assert_eq!(fetched_drs[0].dr_id, dr_id_3); // TODO + assert_eq!(fetched_drs[0].id, dr_id_3); // `GetDataRequestsFromPool` with position = 2 or 3 should return empty array let msg = ProxyQueryMsg::GetDataRequestsFromPool { diff --git a/packages/integration-tests/src/sign.rs b/packages/integration-tests/src/sign.rs index 484c73bf..b9a7c0fa 100644 --- a/packages/integration-tests/src/sign.rs +++ b/packages/integration-tests/src/sign.rs @@ -12,7 +12,7 @@ pub fn recover_pub_key_from_sig() { let sig = executor.sign(["hello world".as_bytes().to_vec()]); - let pk = recover_pubkey(hash.into(), sig); + let pk = recover_pubkey(hash.into(), sig).unwrap(); assert_eq!(pk, executor.public_key); } diff --git a/packages/staking/src/executors_registry.rs b/packages/staking/src/executors_registry.rs index 3d35faf3..89145703 100644 --- a/packages/staking/src/executors_registry.rs +++ b/packages/staking/src/executors_registry.rs @@ -1,6 +1,5 @@ #[cfg(not(feature = "library"))] use cosmwasm_std::{Deps, DepsMut, MessageInfo, Response, StdResult}; -// use cosmwasm_crypto::secp256k1_verify; // TODO use crate::state::{CONFIG, DATA_REQUEST_EXECUTORS, TOKEN}; use crate::utils::{get_attached_funds, validate_sender}; @@ -45,13 +44,6 @@ pub mod data_request_executors { } } - // TODO: do we even need to verify signature here, if we already check the caller can sign using the public key in every other call? - // let expected_message_hash = sender.as_bytes(); // TODO hash this - // let is_signature_valid = secp256k1_verify(&expected_message, &signature, &public_key).unwrap(); - // if !is_signature_valid { - // return Err(ContractError::InvalidSignature); - // } - // compute message hash let message_hash = if let Some(m) = memo.as_ref() { hash([ @@ -67,7 +59,7 @@ pub mod data_request_executors { }; // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature)?; // require token deposit let token = TOKEN.load(deps.storage)?; @@ -127,7 +119,7 @@ pub mod data_request_executors { ]); // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature)?; // require that the executor has no staked or tokens pending withdrawal let executor = DATA_REQUEST_EXECUTORS.load(deps.storage, public_key.clone())?; diff --git a/packages/staking/src/staking.rs b/packages/staking/src/staking.rs index 2198ccc2..ed407eb8 100644 --- a/packages/staking/src/staking.rs +++ b/packages/staking/src/staking.rs @@ -46,7 +46,7 @@ pub mod staking { let message_hash = hash(["deposit_and_stake".as_bytes(), sender.as_bytes()]); // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature)?; // update staked tokens for executor let mut executor = DATA_REQUEST_EXECUTORS.load(deps.storage, public_key.clone())?; @@ -104,7 +104,7 @@ pub mod staking { ]); // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature)?; // error if amount is greater than staked tokens let mut executor = DATA_REQUEST_EXECUTORS.load(deps.storage, public_key.clone())?; @@ -172,7 +172,7 @@ pub mod staking { ]); // recover public key from signature - let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature); + let public_key: Secpk256k1PublicKey = recover_pubkey(message_hash, signature)?; // TODO: add delay after calling unstake let token = TOKEN.load(deps.storage)?;