Skip to content

Adding a trait to provide current time through the client config #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

emarteca
Copy link
Contributor

There are various places in the source code currently where MlsTime::now() is called. This calls the SystemTime::now (unless we are running in WASM).

This PR adds the ability to specialize the clock. This is useful both for testing (setting a deterministic clock), and also for allowing an application to provide an external clock.

@emarteca emarteca requested a review from a team as a code owner April 25, 2025 08:28
@mulmarta
Copy link
Contributor

mulmarta commented May 6, 2025

Hey @emarteca ! I'm a bit hesitant to add yet more trait - something we explicitly want to avoid in the 1.x branch... Going through the code, now() is not called in too many places. Maybe using the builder pattern plus explicit inputs we can make the API clean?

  • tests -- calling this in tests seems wrong, it should be some constant?
  • commit -- we can have optional custom time in CommitBuilder
  • validate key package -- we can have input
  • key package lifetime -- custom time can be input for now, I think 1.x already has KeyPackageBuilder

@emarteca
Copy link
Contributor Author

Thanks for the review -- and please excuse the delay in response! That makes sense. I will refactor and ping back with changes soon.

@emarteca
Copy link
Contributor Author

Thanks again for your review -- I've refactored the code as you suggested, in #282

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants