Question about current timestamp for LSPS5

[martinsaposnic]

Hey all, I have a question about handling time for some LSPS5 features. The spec needs the current time for things like:

• webhook's last used property
• cooldowns for notifications
• signing and verifying signatures
• pruning old signatures
• cleaning up peer state.

I tried using a clock feature from chrono on this PR, but @tnull commented that it depends on std so I should not use it.

I saw some messages from discord explaining that we usually use the block time (±2 hours) to get the time in a no-std/wasm context. My worry is that using block time might not be accurate enough or could lead to unexpected behavior.

I have browsed Discord and GitHub issues but couldn’t find a definitive solution. I'm considering two options:

• Option 1:

For each public method defined on the spec that uses time, have both std and no-std versions (e.g., handle_set_webhook() and handle_set_webhook_no_std()). In the no-std version, add an extra parameter current_time: Duration.

• Option 2:

Have a method like this:

pub fn timestamp_since_epoch(channelmanager: ChannelManager) -> Duration {
    #[cfg(feature = "std")]
    let duration_since_epoch = {
        use std::time::SystemTime;
        SystemTime::now()
            .duration_since(SystemTime::UNIX_EPOCH)
            .expect("SystemTime::now() should be after UNIX_EPOCH")
    };

    #[cfg(not(feature = "std"))]
    let duration_since_epoch =
        Duration::from_secs(channelmanager.highest_seen_timestamp.load(Ordering::Acquire) as u64);

    duration_since_epoch
}

Note that this option would require changing ChannelManager to expose highest_seen_timestamp.

Also, take into account that for verifying signatures, the spec says that the LSPS5 client expects a time difference of up to 10 minutes, so in the no-std context this can be problematic, since the timestamp would have a +- 2 hour error.

Is there a recommended way to get a more reliable current time in this no-std context, or should I just use the block time approximation and maybe review the spec to take this into consideration?

Thanks for any advice!

[tnull]

Excuse the delay answering on the PR.

So, as mentioned there, std::time::SystemTime is not available in no-std environments. Generally we'd prefer it to make the whole crate no-std compatible by finding ways to have the user supply the current time when needed. However, I guess in this case (and especially as this is the initial LSPS5 PR) we should be fine feature gating the entirety of lsps5/service.rs behind the std feature, as realistically chances are rather slim a user wants to run the service side in a no-std or WASM environment (and we can revisit it when one shows up^^). However, it's much more likely for the client side, which we should def. make no-std compatible.

From the options above I think I'd prefer Option 1.

FWIW, another option would be to create a trait TimeProvider or similar via which users can hand in their time source implementations via an alternative constructor, while we provide the default implementation/constructor on std based on SystemTime::now.