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

Replace timestamp with block number #866

Merged
merged 8 commits into from
May 31, 2024
Merged
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@ 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))

### 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

Expand Down
15 changes: 2 additions & 13 deletions crates/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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"))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah good that we no longer need to worry about the system clock on different platforms here

fn get_current_time() -> SystemTime {
SystemTime::now()
}
6 changes: 3 additions & 3 deletions crates/client/src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/shared/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion crates/threshold-signature-server/src/user/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down Expand Up @@ -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()))?;
Expand Down
41 changes: 21 additions & 20 deletions crates/threshold-signature-server/src/user/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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()],
Expand All @@ -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());
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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];
Expand Down Expand Up @@ -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()],
Expand All @@ -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 =
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand All @@ -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;
Expand Down Expand Up @@ -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(),
};
Expand Down
4 changes: 4 additions & 0 deletions crates/threshold-signature-server/src/validation/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
39 changes: 18 additions & 21 deletions crates/threshold-signature-server/src/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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(())
Expand All @@ -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());
}
}
Loading