From 5c341f11a3b1f0501d1a92b8aeefe7c5696b640f Mon Sep 17 00:00:00 2001 From: Mshehu5 Date: Fri, 29 Aug 2025 18:51:20 +0100 Subject: [PATCH] Replace SystemTime with bitcoin::absolute::Time Convert timestamp handling throughout the v2 implementation to use bitcoin::absolute::Time instead of std::time::SystemTime for better consistency with Bitcoin ecosystem conventions. Add helper functions now() and now_as_unix_seconds() in uri::v2 module to centralize time operations. Update SessionContext, error types, and all related functionality to work with the new Time type. This change eliminates potential conversion issues between different time representations and provides better integration with Bitcoin's timestamp handling patterns used elsewhere in the codebase." --- payjoin/src/core/receive/v2/error.rs | 4 +- payjoin/src/core/receive/v2/mod.rs | 76 +++++++++++++++----------- payjoin/src/core/receive/v2/session.rs | 7 +-- payjoin/src/core/send/error.rs | 1 + payjoin/src/core/send/v2/error.rs | 4 +- payjoin/src/core/send/v2/mod.rs | 22 +++++--- payjoin/src/core/send/v2/session.rs | 11 +++- payjoin/src/core/uri/mod.rs | 1 + payjoin/src/core/uri/v2.rs | 46 +++++++++++----- 9 files changed, 111 insertions(+), 61 deletions(-) diff --git a/payjoin/src/core/receive/v2/error.rs b/payjoin/src/core/receive/v2/error.rs index 2a15f7161..185cf1f63 100644 --- a/payjoin/src/core/receive/v2/error.rs +++ b/payjoin/src/core/receive/v2/error.rs @@ -1,6 +1,8 @@ use core::fmt; use std::error; +use bitcoin::absolute::Time; + use super::Error::V2; use crate::hpke::HpkeError; use crate::ohttp::{DirectoryResponseError, OhttpEncapsulationError}; @@ -27,7 +29,7 @@ pub(crate) enum InternalSessionError { /// Url parsing failed ParseUrl(crate::into_url::Error), /// The session has expired - Expired(std::time::SystemTime), + Expired(Time), /// OHTTP Encapsulation failed OhttpEncapsulation(OhttpEncapsulationError), /// Hybrid Public Key Encryption failed diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index f6eb3ab84..51c34ca1f 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -25,8 +25,9 @@ //! but request reuse makes correlation trivial for the relay. use std::str::FromStr; -use std::time::{Duration, SystemTime}; +use std::time::Duration; +use bitcoin::absolute::Time; use bitcoin::hashes::{sha256, Hash}; use bitcoin::psbt::Psbt; use bitcoin::{Address, Amount, FeeRate, OutPoint, Script, TxOut}; @@ -70,7 +71,7 @@ pub struct SessionContext { directory: url::Url, mailbox: Option, ohttp_keys: OhttpKeys, - expiry: SystemTime, + expiry: Time, amount: Option, s: HpkeKeyPair, e: Option, @@ -236,7 +237,8 @@ fn extract_err_req( ohttp_relay: impl IntoUrl, session_context: &SessionContext, ) -> Result<(Request, ohttp::ClientResponse), SessionError> { - if SystemTime::now() > session_context.expiry { + let now = crate::uri::v2::now(); + if now >= session_context.expiry { return Err(InternalSessionError::Expired(session_context.expiry).into()); } let mailbox = mailbox_endpoint(&session_context.directory, &session_context.id()); @@ -278,12 +280,14 @@ impl ReceiverBuilder { ohttp_keys: OhttpKeys, ) -> Result { let directory = directory.into_url()?; + let now_seconds = crate::uri::v2::now_as_unix_seconds(); + let expiry_seconds = now_seconds + TWENTY_FOUR_HOURS_DEFAULT_EXPIRY.as_secs() as u32; let session_context = SessionContext { address, directory, ohttp_keys, s: HpkeKeyPair::gen_keypair(), - expiry: SystemTime::now() + TWENTY_FOUR_HOURS_DEFAULT_EXPIRY, + expiry: Time::from_consensus(expiry_seconds).expect("Valid timestamp"), amount: None, mailbox: None, e: None, @@ -292,7 +296,12 @@ impl ReceiverBuilder { } pub fn with_expiry(self, expiry: Duration) -> Self { - Self(SessionContext { expiry: SystemTime::now() + expiry, ..self.0 }) + let now_seconds = crate::uri::v2::now_as_unix_seconds(); + let expiry_seconds = now_seconds + expiry.as_secs() as u32; + Self(SessionContext { + expiry: Time::from_consensus(expiry_seconds).expect("Valid timestamp"), + ..self.0 + }) } pub fn with_amount(self, amount: Amount) -> Self { @@ -322,7 +331,8 @@ impl Receiver { &mut self, ohttp_relay: impl IntoUrl, ) -> Result<(Request, ohttp::ClientResponse), Error> { - if SystemTime::now() > self.context.expiry { + let now = crate::uri::v2::now(); + if self.state.context.expiry <= now { return Err(InternalSessionError::Expired(self.context.expiry).into()); } let (body, ohttp_ctx) = @@ -442,7 +452,8 @@ impl Receiver { event: Original, reply_key: Option, ) -> Result { - if self.state.context.expiry < SystemTime::now() { + let now = crate::uri::v2::now(); + if self.state.context.expiry <= now { // Session is expired, close the session return Err(InternalReplayError::SessionExpired(self.state.context.expiry)); } @@ -1102,19 +1113,23 @@ pub mod test { use crate::receive::{v2, ReplyableError}; use crate::ImplementationError; - pub(crate) static SHARED_CONTEXT: Lazy = Lazy::new(|| SessionContext { - address: Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4") - .expect("valid address") - .assume_checked(), - directory: EXAMPLE_URL.clone(), - mailbox: None, - ohttp_keys: OhttpKeys( - ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), - ), - expiry: SystemTime::now() + Duration::from_secs(60), - s: HpkeKeyPair::gen_keypair(), - e: None, - amount: None, + pub(crate) static SHARED_CONTEXT: Lazy = Lazy::new(|| { + let now_seconds = crate::uri::v2::now_as_unix_seconds(); + let expiry_seconds = now_seconds + 60; + SessionContext { + address: Address::from_str("tb1q6d3a2w975yny0asuvd9a67ner4nks58ff0q8g4") + .expect("valid address") + .assume_checked(), + directory: EXAMPLE_URL.clone(), + mailbox: None, + ohttp_keys: OhttpKeys( + ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), + ), + expiry: Time::from_consensus(expiry_seconds).expect("Valid timestamp"), + s: HpkeKeyPair::gen_keypair(), + e: None, + amount: None, + } }); pub(crate) fn unchecked_proposal_v2_from_test_vector() -> UncheckedProposal { @@ -1313,7 +1328,7 @@ pub mod test { #[test] fn test_extract_err_req_expiry() -> Result<(), BoxError> { - let now = SystemTime::now(); + let now = crate::uri::v2::now(); let noop_persister = NoopSessionPersister::default(); let context = SessionContext { expiry: now, ..SHARED_CONTEXT.clone() }; let receiver = Receiver { @@ -1348,7 +1363,7 @@ pub mod test { #[test] fn default_expiry() { - let now = SystemTime::now(); + let now_seconds = crate::uri::v2::now_as_unix_seconds(); let noop_persister = NoopSessionPersister::default(); let session = ReceiverBuilder::new( @@ -1360,17 +1375,16 @@ pub mod test { .build() .save(&noop_persister) .expect("Noop persister shouldn't fail"); - let session_expiry = session.context.expiry.duration_since(now).unwrap().as_secs(); + let session_expiry_seconds = session.context.expiry.to_consensus_u32(); let default_expiry = Duration::from_secs(86400); - if let Some(expected_expiry) = now.checked_add(default_expiry) { - assert_eq!(TWENTY_FOUR_HOURS_DEFAULT_EXPIRY, default_expiry); - assert_eq!(session_expiry, expected_expiry.duration_since(now).unwrap().as_secs()); - } + let expected_expiry_seconds = now_seconds + default_expiry.as_secs() as u32; + assert_eq!(TWENTY_FOUR_HOURS_DEFAULT_EXPIRY, default_expiry); + assert_eq!(session_expiry_seconds, expected_expiry_seconds); } #[test] fn build_receiver_with_non_default_expiry() { - let now = SystemTime::now(); + let now_seconds = crate::uri::v2::now_as_unix_seconds(); let expiry = Duration::from_secs(60); let noop_persister = NoopSessionPersister::default(); let receiver = ReceiverBuilder::new( @@ -1383,10 +1397,8 @@ pub mod test { .build() .save(&noop_persister) .expect("Noop persister shouldn't fail"); - assert_eq!( - receiver.context.expiry.duration_since(now).unwrap().as_secs(), - expiry.as_secs() - ); + let expected_expiry_seconds = now_seconds + expiry.as_secs() as u32; + assert_eq!(receiver.context.expiry.to_consensus_u32(), expected_expiry_seconds); } #[test] diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index d6d093127..59a7b3120 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -1,5 +1,4 @@ -use std::time::SystemTime; - +use bitcoin::absolute::Time; use serde::{Deserialize, Serialize}; use super::{ReceiveSession, SessionContext}; @@ -35,7 +34,7 @@ impl From for ReplayError { #[derive(Debug)] pub(crate) enum InternalReplayError { /// Session expired - SessionExpired(SystemTime), + SessionExpired(Time), /// Invalid combination of state and event InvalidStateAndEvent(Box, Box), /// Application storage error @@ -324,7 +323,7 @@ mod tests { #[test] fn test_replaying_unchecked_proposal_expiry() { - let now = SystemTime::now(); + let now = crate::uri::v2::now(); let context = SessionContext { expiry: now, ..SHARED_CONTEXT.clone() }; let original = original_from_test_vector(); diff --git a/payjoin/src/core/send/error.rs b/payjoin/src/core/send/error.rs index 13a02910d..e4cb2fd12 100644 --- a/payjoin/src/core/send/error.rs +++ b/payjoin/src/core/send/error.rs @@ -364,6 +364,7 @@ mod tests { use super::*; #[test] + #[cfg(feature = "v1")] fn test_parse_json() { let known_str_error = r#"{"errorCode":"version-unsupported", "message":"custom message here", "supported": [1, 2]}"#; match ResponseError::parse(known_str_error) { diff --git a/payjoin/src/core/send/v2/error.rs b/payjoin/src/core/send/v2/error.rs index efa10add2..4618dcb1e 100644 --- a/payjoin/src/core/send/v2/error.rs +++ b/payjoin/src/core/send/v2/error.rs @@ -1,5 +1,7 @@ use core::fmt; +use bitcoin::absolute::Time; + use crate::ohttp::DirectoryResponseError; /// Error returned when request could not be created. @@ -15,7 +17,7 @@ pub(crate) enum InternalCreateRequestError { Url(crate::into_url::Error), Hpke(crate::hpke::HpkeError), OhttpEncapsulation(crate::ohttp::OhttpEncapsulationError), - Expired(std::time::SystemTime), + Expired(Time), } impl fmt::Display for CreateRequestError { diff --git a/payjoin/src/core/send/v2/mod.rs b/payjoin/src/core/send/v2/mod.rs index 4a645ed8e..c6be81b63 100644 --- a/payjoin/src/core/send/v2/mod.rs +++ b/payjoin/src/core/send/v2/mod.rs @@ -269,7 +269,8 @@ impl Sender { &self, ohttp_relay: impl IntoUrl, ) -> Result<(Request, V2PostContext), CreateRequestError> { - if std::time::SystemTime::now() > self.pj_param.expiration() { + let now = crate::uri::v2::now(); + if now > self.pj_param.expiration() { return Err(InternalCreateRequestError::Expired(self.pj_param.expiration()).into()); } @@ -510,8 +511,8 @@ impl Sender { #[cfg(test)] mod test { use std::str::FromStr; - use std::time::{Duration, SystemTime}; + use bitcoin::absolute::Time; use bitcoin::hex::FromHex; use bitcoin::Address; use payjoin_test_utils::{BoxError, EXAMPLE_URL, KEM, KEY_ID, PARSED_ORIGINAL_PSBT, SYMMETRIC}; @@ -524,7 +525,7 @@ mod test { const SERIALIZED_BODY_V2: &str = "63484e696450384241484d43414141414159386e757447674a647959475857694245623435486f65396c5747626b78682f36624e694f4a6443447544414141414141442b2f2f2f2f41747956754155414141414146366b554865684a38476e536442554f4f7636756a584c72576d734a5244434867495165414141414141415871525233514a62627a30686e513849765130667074476e2b766f746e656f66544141414141414542494b6762317755414141414146366b55336b34656b47484b57524e6241317256357452356b455644564e4348415163584667415578347046636c4e56676f31575741644e3153594e583874706854414243477343527a424541694238512b41366465702b527a393276687932366c5430416a5a6e3450524c6938426639716f422f434d6b30774967502f526a3250575a3367456a556b546c6844524e415130675877544f3774396e2b563134705a366f6c6a554249514d566d7341616f4e5748564d5330324c6654536530653338384c4e697450613155515a794f6968592b464667414241425941464562324769753663344b4f35595730706677336c4770396a4d55554141413d0a763d32"; fn create_sender_context( - expiration: SystemTime, + expiration: Time, ) -> Result, BoxError> { let endpoint = Url::parse("http://localhost:1234")?; let pj_param = crate::uri::v2::PjParam::new( @@ -553,7 +554,9 @@ mod test { #[test] fn test_serialize_v2() -> Result<(), BoxError> { - let sender = create_sender_context(SystemTime::now() + Duration::from_secs(60))?; + let expiration = Time::from_consensus(crate::uri::v2::now_as_unix_seconds() + 3600) + .expect("Valid timestamp"); + let sender = create_sender_context(expiration)?; let body = serialize_v2_body( &sender.psbt_ctx.original_psbt, sender.psbt_ctx.output_substitution, @@ -566,7 +569,9 @@ mod test { #[test] fn test_extract_v2_success() -> Result<(), BoxError> { - let sender = create_sender_context(SystemTime::now() + Duration::from_secs(60))?; + let expiration = Time::from_consensus(crate::uri::v2::now_as_unix_seconds() + 3600) + .expect("Valid timestamp"); + let sender = create_sender_context(expiration)?; let ohttp_relay = EXAMPLE_URL.clone(); let result = sender.create_v2_post_request(ohttp_relay); let (request, context) = result.expect("Result should be ok"); @@ -581,8 +586,11 @@ mod test { #[test] fn test_extract_v2_fails_when_expired() -> Result<(), BoxError> { - let expected_error = "session expired at SystemTime"; - let sender = create_sender_context(SystemTime::now() - Duration::from_secs(60))?; + let expected_error = "session expired at Time"; + // Create a sender with an already expired timestamp + let expiration = Time::from_consensus(crate::uri::v2::now_as_unix_seconds() - 3600) + .expect("Valid timestamp"); + let sender = create_sender_context(expiration)?; let ohttp_relay = EXAMPLE_URL.clone(); let result = sender.create_v2_post_request(ohttp_relay); assert!(result.is_err(), "Extract v2 expected expiry error, but it succeeded"); diff --git a/payjoin/src/core/send/v2/session.rs b/payjoin/src/core/send/v2/session.rs index 930227d16..03f22fcb4 100644 --- a/payjoin/src/core/send/v2/session.rs +++ b/payjoin/src/core/send/v2/session.rs @@ -99,12 +99,14 @@ pub enum SessionEvent { #[cfg(test)] mod tests { + use bitcoin::absolute::Time; use bitcoin::{FeeRate, ScriptBuf}; use payjoin_test_utils::{KEM, KEY_ID, PARSED_ORIGINAL_PSBT, SYMMETRIC}; use super::*; use crate::output_substitution::OutputSubstitution; use crate::persist::test_utils::InMemoryTestPersister; + #[cfg(feature = "v1")] use crate::send::v1::SenderBuilder; use crate::send::v2::Sender; use crate::send::PsbtContext; @@ -118,10 +120,12 @@ mod tests { let keypair = HpkeKeyPair::gen_keypair(); let id = crate::uri::ShortId::try_from(&b"12345670"[..]).expect("valid short id"); let endpoint = url::Url::parse("http://localhost:1234").expect("valid url"); + let now_seconds = crate::uri::v2::now_as_unix_seconds(); + let expiry = Time::from_consensus(now_seconds + 60).expect("Valid timestamp"); let pj_param = crate::uri::v2::PjParam::new( endpoint, id, - std::time::SystemTime::now() + std::time::Duration::from_secs(60), + expiry, crate::OhttpKeys( ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), ), @@ -191,6 +195,7 @@ mod tests { } #[test] + #[cfg(feature = "v1")] fn test_sender_session_history_with_reply_key_event() { let psbt = PARSED_ORIGINAL_PSBT.clone(); let sender = SenderBuilder::new( @@ -207,10 +212,12 @@ mod tests { let endpoint = sender.endpoint().clone(); let fallback_tx = sender.psbt_ctx.original_psbt.clone().extract_tx_unchecked_fee_rate(); let id = crate::uri::ShortId::try_from(&b"12345670"[..]).expect("valid short id"); + let now_seconds = crate::uri::v2::now_as_unix_seconds(); + let expiry = Time::from_consensus(now_seconds + 60).expect("Valid timestamp"); let pj_param = crate::uri::v2::PjParam::new( endpoint, id, - std::time::SystemTime::now() + std::time::Duration::from_secs(60), + expiry, crate::OhttpKeys( ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"), ), diff --git a/payjoin/src/core/uri/mod.rs b/payjoin/src/core/uri/mod.rs index b327929f1..955a2b065 100644 --- a/payjoin/src/core/uri/mod.rs +++ b/payjoin/src/core/uri/mod.rs @@ -441,6 +441,7 @@ mod tests { /// Test that rejects HTTP URLs that are not onion addresses #[test] + #[cfg(feature = "v1")] fn test_http_non_onion_rejected() { // HTTP to regular domain should be rejected let url = "http://example.com"; diff --git a/payjoin/src/core/uri/v2.rs b/payjoin/src/core/uri/v2.rs index cc1b4f43e..0be2826c4 100644 --- a/payjoin/src/core/uri/v2.rs +++ b/payjoin/src/core/uri/v2.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; use std::str::FromStr; +use bitcoin::absolute::Time; use bitcoin::bech32::Hrp; use bitcoin::consensus::encode::Decodable; use bitcoin::consensus::Encodable; @@ -10,6 +11,18 @@ use crate::hpke::HpkePublicKey; use crate::ohttp::OhttpKeys; use crate::uri::ShortId; +/// Get the current time as Unix seconds (u32). +pub(crate) fn now_as_unix_seconds() -> u32 { + std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap_or_default().as_secs() + as u32 +} + +/// Get the current time as a bitcoin::absolute::Time with second precision. +pub(crate) fn now() -> Time { + Time::from_consensus(now_as_unix_seconds()) + .expect("Current time should always be a valid timestamp") +} + /// Retrieve the receiver's public key from the URL fragment fn receiver_pubkey(url: &Url) -> Result { let value = get_param(url, "RK1") @@ -50,7 +63,7 @@ fn ohttp(url: &Url) -> Result { fn set_ohttp(url: &mut Url, ohttp: &OhttpKeys) { set_param(url, &ohttp.to_string()) } /// Retrieve the exp parameter from the URL fragment -fn exp(url: &Url) -> Result { +fn exp(url: &Url) -> Result { let value = get_param(url, "EX1") .map_err(ParseExpParamError::InvalidFragment)? .ok_or(ParseExpParamError::MissingExp)?; @@ -63,17 +76,18 @@ fn exp(url: &Url) -> Result { return Err(ParseExpParamError::InvalidHrp(hrp)); } - u32::consensus_decode(&mut &bytes[..]) - .map(|timestamp| std::time::UNIX_EPOCH + std::time::Duration::from_secs(timestamp as u64)) - .map_err(ParseExpParamError::InvalidExp) + let seconds = u32::consensus_decode(&mut &bytes[..]).map_err(ParseExpParamError::InvalidExp)?; + Time::from_consensus(seconds).map_err(|_| { + ParseExpParamError::InvalidExp(bitcoin::consensus::encode::Error::Io( + std::io::Error::new(std::io::ErrorKind::InvalidData, "Invalid timestamp: out of range") + .into(), + )) + }) } /// Set the exp parameter in the URL fragment -fn set_exp(url: &mut Url, exp: &std::time::SystemTime) { - let t = match exp.duration_since(std::time::UNIX_EPOCH) { - Ok(duration) => duration.as_secs().try_into().unwrap(), // TODO Result type instead of Option & unwrap - Err(_) => 0u32, - }; +fn set_exp(url: &mut Url, exp: &Time) { + let t = exp.to_consensus_u32(); let mut buf = [0u8; 4]; t.consensus_encode(&mut &mut buf[..]).unwrap(); // TODO no unwrap @@ -90,7 +104,7 @@ fn set_exp(url: &mut Url, exp: &std::time::SystemTime) { pub struct PjParam { directory: Url, id: ShortId, - expiration: std::time::SystemTime, + expiration: Time, ohttp_keys: OhttpKeys, receiver_pubkey: HpkePublicKey, } @@ -99,7 +113,7 @@ impl PjParam { pub fn new( directory: Url, id: ShortId, - expiration: std::time::SystemTime, + expiration: Time, ohttp_keys: OhttpKeys, receiver_pubkey: HpkePublicKey, ) -> Self { @@ -135,7 +149,7 @@ impl PjParam { pub fn ohttp_keys(&self) -> &OhttpKeys { &self.ohttp_keys } - pub fn expiration(&self) -> std::time::SystemTime { self.expiration } + pub fn expiration(&self) -> Time { self.expiration } pub fn endpoint(&self) -> Url { let mut endpoint = self.directory.clone().join(&self.id.to_string()).unwrap(); @@ -432,8 +446,7 @@ mod tests { fn test_exp_get_set() { let mut url = EXAMPLE_URL.clone(); - let exp_time = - std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1720547781); + let exp_time = Time::from_consensus(1720547781).expect("Valid timestamp"); set_exp(&mut url, &exp_time); assert_eq!(url.fragment(), Some("EX1C4UC6ES")); @@ -535,12 +548,17 @@ mod tests { &pjos=0&pj=HTTPS://EXAMPLE.COM/missing_short_id\ %23oh1qypm5jxyns754y4r45qwe336qfx6zr8dqgvqculvztv20tfveydmfqc"; let extras = Uri::try_from(uri).unwrap().extras; + #[cfg(feature = "v1")] match extras { crate::uri::MaybePayjoinExtras::Supported(extras) => { assert!(matches!(extras.pj_param, crate::uri::PjParam::V1(_))); } _ => panic!("Expected v1 pjparam"), } + #[cfg(not(feature = "v1"))] + match extras { + _ => {} + } let uri = "bitcoin:12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX?amount=0.01\ &pjos=0&pj=HTTPS://EXAMPLE.COM/TXJCGKTKXLUUZ\