Skip to content

Commit

Permalink
Fix: get_active_signer was returning removed signers
Browse files Browse the repository at this point in the history
  • Loading branch information
sanjayprabhu committed Dec 19, 2024
1 parent 976b4ac commit 5e54df3
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 9 deletions.
8 changes: 7 additions & 1 deletion src/perf/gen_multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@ impl MessageGenerator for MultiUser {
fid,
proto::IdRegisterEventType::Register,
vec![],
None,
),
));
messages.push(NextMessage::OnChainEvent(
events_factory::create_signer_event(fid, private_key, proto::SignerEventType::Add),
events_factory::create_signer_event(
fid,
private_key,
proto::SignerEventType::Add,
None,
),
));

self.initialized_fids.insert(fid);
Expand Down
2 changes: 2 additions & 0 deletions src/perf/gen_single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ impl MessageGenerator for SingleUser {
fid,
proto::IdRegisterEventType::Register,
vec![],
None,
),
));
messages.push(NextMessage::OnChainEvent(
events_factory::create_signer_event(
fid,
self.private_key.clone(),
proto::SignerEventType::Add,
None,
),
));
}
Expand Down
17 changes: 16 additions & 1 deletion src/storage/store/account/onchain_event_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ static PAGE_SIZE: usize = 1000;

const LEGACY_STORAGE_UNIT_CUTOFF_TIMESTAMP: u32 = 1724889600;
const ONE_YEAR_IN_SECONDS: u32 = 365 * 24 * 60 * 60;
const SUPPORTED_SIGNER_KEY_TYPE: u32 = 1;

#[derive(Error, Debug)]
pub enum OnchainEventStorageError {
Expand Down Expand Up @@ -436,7 +437,21 @@ impl OnchainEventStore {
signer: Vec<u8>,
) -> Result<Option<OnChainEvent>, OnchainEventStorageError> {
let signer_key = make_signer_onchain_event_by_signer_key(fid, signer);
get_event_by_secondary_key(&self.db, signer_key)
let signer = get_event_by_secondary_key(&self.db, signer_key)
.map_err(|e| OnchainEventStorageError::from(e))?;
if let Some(signer) = signer {
if let Some(body) = &signer.body {
if let on_chain_event::Body::SignerEventBody(signer_event_body) = body {
// Only return the signer if it's active (not removed) and the key type is supported
if signer_event_body.event_type() == SignerEventType::Add
&& signer_event_body.key_type == SUPPORTED_SIGNER_KEY_TYPE
{
return Ok(Some(signer));
}
}
}
}
Ok(None)
}

pub fn get_storage_slot_for_fid(
Expand Down
15 changes: 15 additions & 0 deletions src/storage/store/engine_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,7 @@ mod tests {
FID_FOR_TEST,
another_signer.clone(),
proto::SignerEventType::Add,
None,
);
test_helper::commit_event(&mut engine, &another_signer_event).await;
test_helper::register_user(
Expand Down Expand Up @@ -1080,6 +1081,7 @@ mod tests {
FID_FOR_TEST,
signer.clone(),
proto::SignerEventType::Remove,
Some(timestamp + 3),
);
test_helper::commit_event(&mut engine, &revoke_event).await;
assert_onchain_hub_event(&event_rx.try_recv().unwrap(), &revoke_event);
Expand All @@ -1095,6 +1097,15 @@ mod tests {
// Different Fid with the same signer is unaffected
let messages = engine.get_casts_by_fid(FID_FOR_TEST + 1).unwrap();
assert_eq!(1, messages.messages.len());

// Submitting a message from the revoked signer should fail
let post_revoke_message = messages_factory::casts::create_cast_add(
FID_FOR_TEST,
"after revoke",
Some(timestamp + 5),
Some(&signer),
);
assert_commit_fails(&mut engine, &post_revoke_message).await;
}

#[tokio::test]
Expand Down Expand Up @@ -1223,6 +1234,7 @@ mod tests {
FID_FOR_TEST,
test_helper::default_signer(),
proto::SignerEventType::Add,
None,
),
)
.await;
Expand All @@ -1233,6 +1245,7 @@ mod tests {
FID_FOR_TEST,
proto::IdRegisterEventType::Register,
vec![],
None,
);
test_helper::commit_event(&mut engine, &id_register).await;
commit_message(&mut engine, &default_message("msg1")).await;
Expand All @@ -1254,6 +1267,7 @@ mod tests {
FID_FOR_TEST,
proto::IdRegisterEventType::Register,
vec![],
None,
),
)
.await;
Expand All @@ -1266,6 +1280,7 @@ mod tests {
FID_FOR_TEST,
test_helper::default_signer(),
proto::SignerEventType::Add,
None,
),
)
.await;
Expand Down
3 changes: 2 additions & 1 deletion src/storage/store/test_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,11 @@ pub async fn register_user(
fid,
proto::IdRegisterEventType::Register,
custody_address,
None,
);
commit_event(engine, &id_register_event).await;
let signer_event =
events_factory::create_signer_event(fid, signer, proto::SignerEventType::Add);
events_factory::create_signer_event(fid, signer, proto::SignerEventType::Add, None);
commit_event(engine, &signer_event).await;
}

Expand Down
25 changes: 19 additions & 6 deletions src/utils/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,13 @@ pub mod events_factory {
units: rent_units,
payer: rand::random::<[u8; 32]>().to_vec(),
};
let random_number_under_1000 = rand::random::<u32>() % 1000;
// Ensure higher timestamp always has higher block number by left shifting the timestamp by 10 bits (1024)
let block_number = timestamp.checked_shl(10).unwrap() + random_number_under_1000;
OnChainEvent {
r#type: OnChainEventType::EventTypeStorageRent as i32,
chain_id: 10,
block_number: rand::random::<u32>(),
block_number,
block_hash: vec![],
block_timestamp: timestamp as u64,
transaction_hash: rand::random::<[u8; 32]>().to_vec(),
Expand All @@ -109,6 +112,7 @@ pub mod events_factory {
fid: u64,
signer: SigningKey,
event_type: proto::SignerEventType,
timestamp: Option<u32>,
) -> OnChainEvent {
let signer_event_body = proto::SignerEventBody {
key: signer.verifying_key().as_bytes().to_vec(),
Expand All @@ -117,12 +121,16 @@ pub mod events_factory {
key_type: 1,
metadata_type: 1,
};
let block_timestamp = timestamp.unwrap_or_else(|| time::current_timestamp_with_offset(-10));
let random_number_under_1000 = rand::random::<u32>() % 1000;
// Ensure higher timestamp always has higher block number by left shifting the timestamp by 10 bits (1024)
let block_number = block_timestamp.checked_shl(10).unwrap() + random_number_under_1000;
OnChainEvent {
r#type: OnChainEventType::EventTypeSigner as i32,
chain_id: 10,
block_number: rand::random::<u32>(),
block_number,
block_hash: vec![],
block_timestamp: time::current_timestamp_with_offset(-10) as u64,
block_timestamp: block_timestamp as u64,
transaction_hash: rand::random::<[u8; 32]>().to_vec(),
log_index: 0,
fid,
Expand All @@ -138,19 +146,24 @@ pub mod events_factory {
fid: u64,
event_type: proto::IdRegisterEventType,
custody_address: Vec<u8>,
timestamp: Option<u32>,
) -> OnChainEvent {
let id_register_event_body = proto::IdRegisterEventBody {
to: custody_address,
event_type: event_type as i32,
from: vec![],
recovery_address: vec![],
};
let block_timestamp = timestamp.unwrap_or_else(|| time::current_timestamp_with_offset(-10));
let random_number_under_1000 = rand::random::<u32>() % 1000;
// Ensure higher timestamp always has higher block number by left shifting the timestamp by 10 bits (1024)
let block_number = block_timestamp.checked_shl(10).unwrap() + random_number_under_1000;
OnChainEvent {
r#type: OnChainEventType::EventTypeSigner as i32,
r#type: OnChainEventType::EventTypeIdRegister as i32,
chain_id: 10,
block_number: rand::random::<u32>(),
block_number,
block_hash: vec![],
block_timestamp: time::current_timestamp_with_offset(-10) as u64,
block_timestamp: block_timestamp as u64,
transaction_hash: rand::random::<[u8; 32]>().to_vec(),
log_index: 0,
fid,
Expand Down

0 comments on commit 5e54df3

Please sign in to comment.