feat: finish todos

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<Secpk256k1PublicKey, ContractError> {
     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]

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}")]

packages/data-requests/src/data_request_result.rs

@@ -43,8 +43,6 @@ pub mod data_request_results {
     ) -> Result<Response, ContractError> {
         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<Response, ContractError> {
         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)?;

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()
 }

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 {

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);
 }

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())?;

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)?;