Skip to content

Commit

Permalink
Fix incorrect timestamp handling (#191)
Browse files Browse the repository at this point in the history
We were passing in farcaster time to on chain events, which caused the
block number to be incorrect (derived from timestamp) and the revoke to
be rejected.
  • Loading branch information
sanjayprabhu authored Jan 2, 2025
1 parent c4bb337 commit 5c61849
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 11 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/verify.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
name: Verify

on:
push:
branches: [main]
pull_request:
branches: [main]
workflow_call:
Expand Down
2 changes: 1 addition & 1 deletion src/core/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::proto::{Block, FullProposal, ShardChunk};
pub use proto::Height;
pub use proto::ShardHash;

pub const FARCASTER_EPOCH: u64 = 1609459200; // January 1, 2021 UTC
pub const FARCASTER_EPOCH: u64 = 1609459200000; // January 1, 2021 UTC

// Fid must be a 32 bit unsigned integer for storage in RocksDB and the trie.
// However, protobuf uses 64 bit unsigned integers. So, map to the fid at the lowest level
Expand Down
12 changes: 11 additions & 1 deletion src/core/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,11 @@ mod tests {
.unwrap()
.as_millis() as u64;
assert!(time <= now);
assert_eq!(time, now / 1000 - FARCASTER_EPOCH / 1000);
}

#[test]
fn to_farcaster_time_test() {
fn test_to_farcaster_time() {
// It is an error to pass a time before the farcaster epoch
let time = to_farcaster_time(0);
assert!(time.is_err());
Expand All @@ -70,4 +71,13 @@ mod tests {
let time = to_farcaster_time(FARCASTER_EPOCH + 1000).unwrap();
assert_eq!(time, 1);
}

#[test]
fn test_from_farcaster_time() {
let time = from_farcaster_time(0);
assert_eq!(time, FARCASTER_EPOCH);

let time = from_farcaster_time(1);
assert_eq!(time, FARCASTER_EPOCH + 1000);
}
}
6 changes: 3 additions & 3 deletions src/storage/store/engine_tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#[cfg(test)]
mod tests {
use crate::core::util::calculate_message_hash;
use crate::core::util::{calculate_message_hash, from_farcaster_time};
use crate::proto::ShardChunk;
use crate::proto::{self, ReactionType};
use crate::proto::{HubEvent, ValidatorMessage};
Expand Down Expand Up @@ -1013,7 +1013,6 @@ mod tests {
}

#[tokio::test]
#[ignore]
async fn test_revoking_a_signer_deletes_all_messages_from_that_signer() {
let (mut engine, _tmpdir) = test_helper::new_engine();
let signer = SigningKey::generate(&mut rand::rngs::OsRng);
Expand Down Expand Up @@ -1078,11 +1077,12 @@ mod tests {
assert_eq!(1, messages.messages.len());

// Revoke a single signer
let revoke_timestamp = (from_farcaster_time((timestamp + 3) as u64) / 1000) as u32;
let revoke_event = events_factory::create_signer_event(
FID_FOR_TEST,
signer.clone(),
proto::SignerEventType::Remove,
Some(timestamp + 3),
Some(revoke_timestamp),
);
test_helper::commit_event(&mut engine, &revoke_event).await;
assert_onchain_hub_event(&event_rx.try_recv().unwrap(), &revoke_event);
Expand Down
15 changes: 9 additions & 6 deletions src/utils/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ pub mod time {
use super::*;

pub fn farcaster_time() -> u32 {
(current_timestamp() as u64 - FARCASTER_EPOCH) as u32
current_timestamp() - (FARCASTER_EPOCH / 1000) as u32
}

pub fn farcaster_time_with_offset(offset: i32) -> u32 {
(farcaster_time() as i32 + offset) as u32
}

// Returns the current timestamp in seconds since the unix epoch
pub fn current_timestamp() -> u32 {
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
Expand Down Expand Up @@ -114,6 +115,9 @@ pub mod events_factory {
event_type: proto::SignerEventType,
timestamp: Option<u32>,
) -> OnChainEvent {
if timestamp.is_some() && !(timestamp.unwrap() > (FARCASTER_EPOCH / 1000) as u32) {
panic!("Block timestamps must be unix epoch in seconds");
}
let signer_event_body = proto::SignerEventBody {
key: signer.verifying_key().as_bytes().to_vec(),
event_type: event_type as i32,
Expand Down Expand Up @@ -148,6 +152,9 @@ pub mod events_factory {
custody_address: Vec<u8>,
timestamp: Option<u32>,
) -> OnChainEvent {
if timestamp.is_some() && !(timestamp.unwrap() > (FARCASTER_EPOCH / 1000) as u32) {
panic!("Block timestamps must be unix epoch in seconds");
}
let id_register_event_body = proto::IdRegisterEventBody {
to: custody_address,
event_type: event_type as i32,
Expand Down Expand Up @@ -181,11 +188,7 @@ pub mod messages_factory {
use crate::core::util::calculate_message_hash;

pub fn farcaster_time() -> u32 {
(std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs()
- FARCASTER_EPOCH) as u32
time::farcaster_time()
}

pub fn create_message_with_data(
Expand Down

0 comments on commit 5c61849

Please sign in to comment.