[Request] Remove (or make optional) dependency on time (`Timestamp::now_utc()`)

[frederikrothenberger]

Description

We'd like to contribute to these crates a PR that removes (or makes optional via feature flag) the dependency on a specific way to obtain the current time (currently provided by Timestamp::now_utc()).

Motivation

The reason for this feature request / contribution is that we'd like to make use of this library in ICP smart contracts. These smart contracts have a custom way of accessing the current time. As such, it would be great if the current time could be provided externally by the library user.

The ultimate goal is to make the library wasm32-unknown-unknown compatible without requiring WASI or JS bindings.

Requirements

Write a list of what you want this feature to do.

1. Remove the dependency on now_utc or allow it's implementation to be provided by the library user

Open questions

I have the following open questions:

• Do I need to implement this without breaking changes, or are breaking changes an option?
  • If it must be non-breaking: The only option I see there is adding a default feature time that can be explicitly disabled by clients. Having that will add some complexity toggling all the code paths to now_utc off. Of course, I'll try to minimize the impact.
• Currently, we only use the identity_core, identity_credential and identity_jose crates. Should I / is it desired to remove the dependency on a specific time provider from all other crates too or should I scope it to only exactly what we need?

Request for feedback on a possible approach

The need to access time seems to mostly come from builders and options that allow to not specify the time which results in defaulting to now_utc somewhere deeply nested in the library code.

The simplest approach I see is to move the point at which now_utc is being called to an earlier point in time. One way of doing that is to change the interfaces of the functions that take these options from fn foo(options: &OptionsWithOptionalTime) -> ... to something like fn foo(options: &impl Into<OptionsWithTime> -> ... and then provide an implementation From<OptionsWithOptionalTime> for OptionsWithTime and toggle OptionsWithOptionalTime behind the time feature flag.

Similarly for the Credential::from_builder, we could introduce a CredentialBuilderWithMandatoryIssuance and put the issuance_date into the conversion from the current builder to the new one.

Do you see this as a feasible approach? Happy to hear any type of feedback on this. Especially open to suggestions that make the implementation easier. 😉

One disadvantage of that approach I see is that it leads to potentially many conversions with cloning behind the scenes for existing users of OptionsWithOptionalTime. Or we make the conversion zero-copy (i.e. OptionsWithTimeReferencingOptionsWithOptionalTime) which will introduce a lot more complexity in terms of lifetimes, etc. I'd rather avoid that. Should I be concerned about the costs of conversions or not?

Are you planning to do it yourself in a pull request?

Yes

[eike-hass]

Hi there 👋 After some internal discussion we would prefer if we can introduce the needed extensibility in a non-breaking way. We are in the process of designing the next mayor release, where we would revisit the topic and provide a more complete interface, but for now the quickest way to release the requested extensibility is to add a new cfg for the wasm32-unknown-unknown target.

My colleague @itsyaasir will support the effort 💪

[itsyaasir]

Hi there 👋🏾 , expounding on what @eike-hass has said, currently the Timestamp::now_utc() has a cfg check for wasi and non-wasi compilation, we can have additional for wasm32-unknown-unknown target. This is a very minimal approach with limited moving parts. What do you think @frederikrothenberger ?

[itsyaasir]

Hi @frederikrothenberger , Is it possible to import and call an external function from a WebAssembly module in an Internet Computer (IC) canister?

One of the approaches suggested by the team involves implementing a now_utc function by importing an external function from a Wasm module. While this approach would involve using the unsafe keyword in Rust, it is considered acceptable within this context. Here is a rough outline of the implementation:

#[link(wasm_import_module = "time")]
extern "C" {
    fn now() -> u64;
}

fn utc_now() {
    unsafe { now() }
}

This method allows us to use the external function to get the current time. Would this approach be feasible ?

[frederikrothenberger]

@itsyaasir: We actually do something similar in our SDK:

#[link(wasm_import_module = "ic0")]
extern "C" {
    pub fn time() -> i64;
}

But I can't assume the existence of that module for generic wasm32-unknown-unknown. Or is the idea to add a feature flag for ICP specifically?