Replace timestamp with block number (#866)

* replace timestamp with blocknumber * tests * clean * changelog * lint * clean * Apply suggestions from code review Co-authored-by: Hernando Castano <[email protected]> * changelog --------- Co-authored-by: Hernando Castano <[email protected]>
entropyxyz · May 31, 2024 · e625559 · e625559
1 parent 6d77405
commit e625559
Show file tree

Hide file tree

Showing 11 changed files with 90 additions and 73 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,13 +9,17 @@ At the moment this project **does not** adhere to
 
 ## [[Unreleased]](https://github.com/entropyxyz/entropy-core/compare/release/v0.1.0...master)
 
+### Breaking Changes
+- In [#866](https://github.com/entropyxyz/entropy-core/pull/866) timestamp was removed from `UserSignatureRequest` and replaced with block_number. Thus check_stale now uses block_number for stale checks
+
 ### Added
 - Add a way to change program modification account  ([#843](https://github.com/entropyxyz/entropy-core/pull/843))
 - Add support for `--mnemonic-file` and `THRESHOLD_SERVER_MNEMONIC` ([#864](https://github.com/entropyxyz/entropy-core/pull/864))
 
 ### Changed
 - Move TSS mnemonic out of keystore [#853](https://github.com/entropyxyz/entropy-core/pull/853)
 - Prepare test CLI for use in Programs repo ([#856](https://github.com/entropyxyz/entropy-core/pull/856))
+- Replace timestamp with block number ([#866](https://github.com/entropyxyz/entropy-core/pull/866))
 
 ## [0.1.0](https://github.com/entropyxyz/entropy-core/compare/release/v0.0.12...release/v0.1.0) - 2024-05-20
 

diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs
@@ -47,7 +47,6 @@ use entropy_protocol::{
 use entropy_shared::HashingAlgorithm;
 use futures::{future, stream::StreamExt};
 use sp_core::{sr25519, Pair};
-use std::time::SystemTime;
 use subxt::{
     backend::legacy::LegacyRpcMethods,
     events::EventsClient,
@@ -171,12 +170,12 @@ pub async fn sign(
     let message_hash_hex = hex::encode(message_hash);
     let validators_info = get_current_subgroup_signers(api, rpc, &message_hash_hex).await?;
     tracing::debug!("Validators info {:?}", validators_info);
-
+    let block_number = rpc.chain_get_header(None).await?.ok_or(ClientError::BlockNumber)?.number;
     let signature_request = UserSignatureRequest {
         message: hex::encode(message),
         auxilary_data: Some(vec![auxilary_data.map(hex::encode)]),
         validators_info: validators_info.clone(),
-        timestamp: get_current_time(),
+        block_number,
         hash: HashingAlgorithm::Keccak,
         signature_verifying_key: signature_verifying_key.to_vec(),
     };
@@ -435,13 +434,3 @@ async fn select_validator_from_subgroup(
     };
     Ok(address.clone())
 }
-
-#[cfg(feature = "full-client-wasm")]
-fn get_current_time() -> SystemTime {
-    use std::time::{Duration, UNIX_EPOCH};
-    UNIX_EPOCH + Duration::from_secs(js_sys::Date::now() as u64)
-}
-#[cfg(not(feature = "full-client-wasm"))]
-fn get_current_time() -> SystemTime {
-    SystemTime::now()
-}
diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs
@@ -17,11 +17,11 @@ use crate::{
     chain_api::{entropy, EntropyConfig},
     substrate::query_chain,
 };
-use entropy_shared::{user::ValidatorInfo, HashingAlgorithm, SIGNING_PARTY_SIZE};
+use entropy_shared::{user::ValidatorInfo, BlockNumber, HashingAlgorithm, SIGNING_PARTY_SIZE};
 use futures::future::join_all;
 use num::{BigInt, Num, ToPrimitive};
 use serde::{Deserialize, Serialize};
-use std::{sync::Arc, time::SystemTime};
+use std::sync::Arc;
 use subxt::{backend::legacy::LegacyRpcMethods, OnlineClient};
 
 pub use crate::errors::SubgroupGetError;
@@ -36,7 +36,7 @@ pub struct UserSignatureRequest {
     /// Information from the validators in signing party
     pub validators_info: Vec<ValidatorInfo>,
     /// When the message was created and signed
-    pub timestamp: SystemTime,
+    pub block_number: BlockNumber,
     /// Hashing algorithm to be used for signing
     pub hash: HashingAlgorithm,
     /// The veryfying key for the signature requested

diff --git a/crates/shared/src/types.rs b/crates/shared/src/types.rs
@@ -31,7 +31,7 @@ pub type X25519PublicKey = [u8; 32];
 
 /// This should match the type found in `entropy-runtime`. We define it ourselves manually here
 /// since we don't want to pull that whole crate it just for a `u32`.
-type BlockNumber = u32;
+pub type BlockNumber = u32;
 
 /// Defines an application's accessibility
 /// Public -> User does not hold a keyshare

diff --git a/crates/threshold-signature-server/src/user/api.rs b/crates/threshold-signature-server/src/user/api.rs
@@ -135,7 +135,13 @@ pub async fn sign_tx(
     request_limit_check(&rpc, &app_state.kv_store, string_verifying_key.clone(), request_limit)
         .await?;
 
-    check_stale(user_sig_req.timestamp)?;
+    let block_number = rpc
+        .chain_get_header(None)
+        .await?
+        .ok_or_else(|| UserErr::OptionUnwrapError("Error Getting Block Number".to_string()))?
+        .number;
+
+    check_stale(user_sig_req.block_number, block_number).await?;
     let user_details =
         get_registered_details(&api, &rpc, user_sig_req.signature_verifying_key.clone()).await?;
     check_hash_pointer_out_of_bounds(&user_sig_req.hash, user_details.programs_data.0.len())?;
@@ -608,6 +614,7 @@ pub async fn recover_key(
         key_server_info,
         signer,
         x25519_secret,
+        rpc,
     )
     .await
     .map_err(|e| UserErr::ValidatorError(e.to_string()))?;

diff --git a/crates/threshold-signature-server/src/user/tests.rs b/crates/threshold-signature-server/src/user/tests.rs
@@ -185,7 +185,7 @@ async fn test_sign_tx_no_chain() {
     let (_validators_info, mut generic_msg, validator_ips_and_keys) =
         get_sign_tx_data(validator_ips, hex::encode(PREIMAGE_SHOULD_SUCCEED));
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     // test points to no program
     let test_no_program =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -206,7 +206,7 @@ async fn test_sign_tx_no_chain() {
     .await
     .unwrap();
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     let test_user_res =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
 
@@ -230,14 +230,15 @@ async fn test_sign_tx_no_chain() {
     let request_info: RequestLimitStorage =
         RequestLimitStorage::decode(&mut serialized_request_amount.as_ref()).unwrap();
     assert_eq!(request_info.request_amount, 1);
-    generic_msg.timestamp = SystemTime::now();
+
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.validators_info = generic_msg.validators_info.into_iter().rev().collect::<Vec<_>>();
     let test_user_res_order =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
 
     verify_signature(test_user_res_order, message_hash, keyshare_option.clone()).await;
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.signature_verifying_key = DEFAULT_VERIFYING_KEY_NOT_REGISTERED.to_vec();
     let test_user_res_not_registered =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), two).await;
@@ -284,7 +285,7 @@ async fn test_sign_tx_no_chain() {
         encrypted_connection.recv().await.is_err()
     });
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.signature_verifying_key = DAVE_VERIFYING_KEY.to_vec().to_vec();
     let test_user_bad_connection_res = submit_transaction_requests(
         vec![validator_ips_and_keys[1].clone()],
@@ -303,7 +304,7 @@ async fn test_sign_tx_no_chain() {
     assert!(connection_attempt_handle.await.unwrap());
 
     // Bad Account ID - an account ID is given which is not in the signing group
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     let mut generic_msg_bad_account_id = generic_msg.clone();
     generic_msg_bad_account_id.validators_info[0].tss_account =
         subxtAccountId32(AccountKeyring::Dave.into());
@@ -324,7 +325,7 @@ async fn test_sign_tx_no_chain() {
     // Now, test a signature request that should fail
     // The test program is written to fail when `auxilary_data` is `None`
     generic_msg.auxilary_data = None;
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
 
     let test_user_failed_programs_res =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -338,7 +339,7 @@ async fn test_sign_tx_no_chain() {
 
     // The test program is written to fail when `auxilary_data` is `None` but only on the second program
     generic_msg.auxilary_data = Some(vec![Some(hex::encode(AUXILARY_DATA_SHOULD_SUCCEED))]);
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
 
     let test_user_failed_aux_data =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -347,7 +348,7 @@ async fn test_sign_tx_no_chain() {
         assert_eq!(res.unwrap().text().await.unwrap(), "Auxilary data is mismatched");
     }
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.hash = HashingAlgorithm::Custom(3);
     let test_user_custom_hash_out_of_bounds =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), two).await;
@@ -519,7 +520,7 @@ async fn test_program_with_config() {
     .await
     .unwrap();
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     let test_user_res =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
 
@@ -1127,7 +1128,7 @@ async fn test_sign_tx_user_participates() {
     verify_signature(test_user_res, message_should_succeed_hash, users_keyshare_option.clone())
         .await;
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.signature_verifying_key = DEFAULT_VERIFYING_KEY_NOT_REGISTERED.to_vec();
 
     // test failing cases
@@ -1141,7 +1142,7 @@ async fn test_sign_tx_user_participates() {
         );
     }
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     generic_msg.signature_verifying_key = verifying_key;
     let mut generic_msg_bad_validators = generic_msg.clone();
     generic_msg_bad_validators.validators_info[0].x25519_public_key = [0; 32];
@@ -1203,7 +1204,7 @@ async fn test_sign_tx_user_participates() {
         // returns true if this part of the test passes
         encrypted_connection.recv().await.is_err()
     });
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
 
     let test_user_bad_connection_res = submit_transaction_requests(
         vec![validator_ips_and_keys[1].clone()],
@@ -1220,7 +1221,7 @@ async fn test_sign_tx_user_participates() {
     }
 
     assert!(connection_attempt_handle.await.unwrap());
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     // Bad Account ID - an account ID is given which is not in the signing group
     let mut generic_msg_bad_account_id = generic_msg.clone();
     generic_msg_bad_account_id.validators_info[0].tss_account =
@@ -1242,7 +1243,7 @@ async fn test_sign_tx_user_participates() {
     // Now, test a signature request that should fail
     // The test program is written to fail when `auxilary_data` is `None`
     generic_msg.auxilary_data = None;
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
 
     let test_user_failed_programs_res =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -1544,7 +1545,7 @@ async fn test_fail_infinite_program() {
             Some(hex::encode(AUXILARY_DATA_SHOULD_SUCCEED)),
         ]),
         validators_info,
-        timestamp: SystemTime::now(),
+        block_number: rpc.chain_get_header(None).await.unwrap().unwrap().number,
         hash: HashingAlgorithm::Keccak,
         signature_verifying_key: DAVE_VERIFYING_KEY.to_vec(),
     };
@@ -1554,7 +1555,7 @@ async fn test_fail_infinite_program() {
         (validator_ips[1].clone(), X25519_PUBLIC_KEYS[1]),
     ];
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
 
     let test_infinite_loop =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -1663,7 +1664,7 @@ async fn test_device_key_proxy() {
             &serde_json::to_string(&aux_data_json_sr25519.clone()).unwrap(),
         ))]),
         validators_info,
-        timestamp: SystemTime::now(),
+        block_number: rpc.chain_get_header(None).await.unwrap().unwrap().number,
         hash: HashingAlgorithm::Keccak,
         signature_verifying_key: DAVE_VERIFYING_KEY.to_vec(),
     };
@@ -1673,7 +1674,7 @@ async fn test_device_key_proxy() {
         (validator_ips[1].clone(), X25519_PUBLIC_KEYS[1]),
     ];
 
-    generic_msg.timestamp = SystemTime::now();
+    generic_msg.block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number;
     let message_hash = Hasher::keccak(PREIMAGE_SHOULD_SUCCEED);
     let test_user_res =
         submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
@@ -1858,7 +1859,7 @@ pub fn get_sign_tx_data(
             Some(hex::encode(AUXILARY_DATA_SHOULD_SUCCEED)),
         ]),
         validators_info: validators_info.clone(),
-        timestamp: SystemTime::now(),
+        block_number: 0,
         hash: HashingAlgorithm::Keccak,
         signature_verifying_key: DAVE_VERIFYING_KEY.to_vec(),
     };

diff --git a/crates/threshold-signature-server/src/validation/errors.rs b/crates/threshold-signature-server/src/validation/errors.rs
@@ -23,4 +23,8 @@ pub enum ValidationErr {
     StaleMessage,
     #[error("Time subtraction error: {0}")]
     SystemTime(#[from] std::time::SystemTimeError),
+    #[error("Error getting block number")]
+    BlockNumber,
+    #[error("Generic Substrate error: {0}")]
+    GenericSubstrate(#[from] subxt::error::Error),
 }
diff --git a/crates/threshold-signature-server/src/validation/mod.rs b/crates/threshold-signature-server/src/validation/mod.rs
@@ -13,20 +13,17 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-use std::time::{Duration, SystemTime};
-
 use bip39::Mnemonic;
 pub use entropy_protocol::sign_and_encrypt::{
     EncryptedSignedMessage, EncryptedSignedMessageErr, SignedMessage,
 };
+use entropy_shared::BlockNumber;
 use rand_core::{OsRng, RngCore};
 use subxt::ext::sp_core::{sr25519, Pair};
-
 pub mod errors;
-
 use errors::ValidationErr;
 
-pub const TIME_BUFFER: Duration = Duration::from_secs(25);
+pub const BLOCK_BUFFER: BlockNumber = 5u32;
 
 /// Derives a sr25519::Pair from a Mnemonic
 pub fn mnemonic_to_pair(m: &Mnemonic) -> Result<sr25519::Pair, ValidationErr> {
@@ -36,9 +33,12 @@ pub fn mnemonic_to_pair(m: &Mnemonic) -> Result<sr25519::Pair, ValidationErr> {
 }
 
 /// Checks if the message sent was within X amount of time
-pub fn check_stale(message_time: SystemTime) -> Result<(), ValidationErr> {
-    let time_difference = SystemTime::now().duration_since(message_time)?;
-    if time_difference > TIME_BUFFER {
+pub async fn check_stale(
+    user_block_number: BlockNumber,
+    chain_block_number: BlockNumber,
+) -> Result<(), ValidationErr> {
+    let block_difference = chain_block_number.abs_diff(user_block_number);
+    if block_difference > BLOCK_BUFFER {
         return Err(ValidationErr::StaleMessage);
     }
     Ok(())
@@ -55,21 +55,18 @@ pub fn new_mnemonic() -> Result<Mnemonic, bip39::Error> {
 mod tests {
     use super::*;
 
-    #[test]
-    fn test_stale_check() {
-        let result = check_stale(SystemTime::now());
+    #[tokio::test]
+    async fn test_stale_check() {
+        let result = check_stale(1, 1).await;
         assert!(result.is_ok());
 
-        let fail_time =
-            SystemTime::now().checked_sub(TIME_BUFFER).unwrap().checked_sub(TIME_BUFFER).unwrap();
-        let fail_stale = check_stale(fail_time).unwrap_err();
-        assert_eq!(fail_stale.to_string(), "Message is too old".to_string());
+        let result_server_larger = check_stale(1, 2).await;
+        assert!(result_server_larger.is_ok());
+
+        let result_user_larger = check_stale(2, 1).await;
+        assert!(result_user_larger.is_ok());
 
-        let future_time = SystemTime::now().checked_add(TIME_BUFFER).unwrap();
-        let fail_future = check_stale(future_time).unwrap_err();
-        assert_eq!(
-            fail_future.to_string(),
-            "Time subtraction error: second time provided was later than self".to_string()
-        );
+        let fail_stale = check_stale(1, 2 + BLOCK_BUFFER).await.unwrap_err();
+        assert_eq!(fail_stale.to_string(), "Message is too old".to_string());
     }
 }