Repository navigation
std::time::SystemTime has nanosecond precision but Payjoin only sees u32 UnixTime second precision #893
Description
In roundtrip testing a new concrete v2::PjParams struct I realized SystemTime includes nanosecond precision. After serializing SystemTime into the EX1 u32 expiration parameter, and deserializing back into the type that precision is lost and an assert_eq failed, raising this issue, since we are storing time as SystemTime in many parts of the payjoin crate.
It seems to me that encoding this information internally as precise as the URL-serialized representation would be more accurate. For this purpose we may even use bitcoin::absolute::Time. Other than accuracy, changing to a u32 UnixTime abstraction gives us the advantage that u32 would work in wasm where SystemTime is not available.
I think this may be a good first issue to start, but it might be tricky to get the ergonomics right to completely close this issue. We'd need to figure out what type (u64, u32, or bitcoin::absolute::Time?) to expose in the API arguments and across the FFI boundary even if the internal representation becomes absolute::Time. We may take a look at how conversion ergonomics from SystemTime::now() to bitcoin::absolute::Time (or similar functionality) are designed as inspiration.
Metadata
Metadata
Assignees
Type
Projects
- Status
Activity
Mshehu5 commentedon Jul 22, 2025
Can I be assigned this issue
DanGould commentedon Jul 28, 2025
From @Mshehu5, somewhere else:
Specifically the issue is replacing the use of SystemTime in the functional state machine of the
payjoincrate. The structured data (structs, enums, errors, etc.) relying onstd::time::SystemTimeneed to rely onu64,u32, orbitcoin::absolute::Timeinstead, since only the latter encodes the second (rather than nanosecond) precision with which the protocol is concerned. I suggest studying the difference between the types, making a suggestion, and writing up rationale for whatever decision you find most appropriate or otherwise asking specific questions about the differences between them.bitcoin::absolute::Timecame up because it's already a dependency and makes it explicit we're talking about a concrete unix time as a seconds-precision unsigned integer under the hood, but you could argue for u64 or u32 as well.Working around
SystemTime::now()calls is outside of the scope of this issue. That call does have side-effects that should be eventually worked around by making the state machine methods that currently callSystemTime::now()takenowin seconds as input (in the type you choose) instead. This can be done as a follow up or separate concern.Mshehu5 commentedon Aug 8, 2025
Thank you for clarifying, and sorry for the late reply.
This is just based on my research, feel free to correct me
After analyzing the
SystemTimeprecision issue and comparing different time representations,bitcoin::absolute::Timeseems like the optimal choice for Payjoin.Comparison Analysis
bitcoin::absolute::Time(Recommended)u32Unix timestamp, matching Payjoin’s precision exactlyrust-bitcointeam with extensive testingu32Unix Timestampu64Unix Timestampu32u32, so conversion would still be necessaryThe core issue lies in
SystemTime’s nanosecond precision, which leads to roundtrip test failures when serialized tou32.bitcoin::absolute::Timeresolves this by maintaining second-level precision internally while offering robust time handling.This abstraction layer is an advantage, not a cost it prevents common time-related bugs and is maintained by domain experts.
Since
rust-bitcoinis already a dependency, this introduces no additional overhead while significantly improving type safety and developer experience.Let me know if you're okay with this direction, and I’ll go ahead and push the PR.
DanGould commentedon Aug 17, 2025
I'm ok with this direction. thank you for thinking it through @Mshehu5. These notes and reasoning are easy to follow and make sense to me.
arminsabouri commentedon Sep 18, 2025
Closed by #1047