feat: forgive timestamps provided as ms/μs

[Bilb]

No description provided.

[jagerman]

I think that this change more properly belongs in the binding code for nodejs, so that what libsession-nodejs would expose is a function to nodejs that takes millisecond input, and produces millisecond output, as those are what native nodejs tools expect, and does the ÷1000 or ×1000 in the function wrapper.

Milliseconds unix timestamps are a non-common concept in general; pretty much every other language defines timestamps in terms of seconds: plain integer seconds (older C apis, and languages that adopted that convention), a seconds/nanosecond pair (newer C APIs, Java), or a floating point seconds value (Python, Swift, and loads of other languages). Older Java used milliseconds, but when they revamped the date time API to fix various limitations in it a decade ago they moved away from it. That's not to say all language should be the same, by any means, but it's the job of the specific language binding code to pave over conceptual differences in what the specific language wants, rather than make the library worry about possible different language conventions.

[mpretty-cyro]

I think that this change more properly belongs in the binding code for nodejs, so that what libsession-nodejs would expose is a function to nodejs that takes millisecond input, and produces millisecond output, as those are what native nodejs tools expect, and does the ÷1000 or ×1000 in the function wrapper.

This is unfortunately less of a nodejs-specific thing and more an attempt to fix some libSession misuse that happened at some point - there was some invalid conversions on one or more of the platforms which resulted in incorrect timestamps being added into libSession which, unsurprisingly, caused a bunch of bugs

We could look at implementing this timestamp conversion fix across all 3 platforms or wrappers but I guess the arguments for doing it in libSession are:

1. Isn't one of the points of libSession to help reduce duplicating work across platforms
2. There are a bunch of other inputs that libSession validates so it makes sense in my mind for it to validate that the durations provided are correct (ie. in seconds), and if not valid then automatically convert them when possible (eg. similar to how we auto-truncate strings to the max lengths in some cases)

Since some users will already have configs which have invalid timestamps just throwing when given an invalid value wouldn't help those users and may result in unexpected crashes (we introduced a crash on Desktop when originally adding exceptions for the name length requirement in UserProfile) hence the conversion approach

If we are able to get some more time to dedicate to libSession next year I'm hoping we can start to move more of the business logic to the libSession side so that these types of bugs become less likely (eg. if there were createGroup(name:) & joinGroup(ed25519PubKey:authData:) functions then the timestamps wouldn't need to be set directly by the frontends)

[jagerman]

This is unfortunately less of a nodejs-specific thing and more an attempt to fix some libSession misuse that happened at some point - there was some invalid conversions on one or more of the platforms which resulted in incorrect timestamps being added into libSession which, unsurprisingly, caused a bunch of bugs

I see. Yeah, if that broken data is out there in the wild then we need to deal with it, and here is the place that makes sense (sort of like the protobuf double-wrapping issue).

Are current (or in-progress) clients now passing the correct value, so that we can treat this as a workaround for past issues?

If we are able to get some more time to dedicate to libSession next year I'm hoping we can start to move more of the business logic to the libSession side so that these types of bugs become less likely (eg. if there were createGroup(name:) & joinGroup(ed25519PubKey:authData:) functions then the timestamps wouldn't need to be set directly by the frontends)

100% yes, and that is indeed on the agenda for starting right away in 2025.

[jagerman]

One minor nitpick: to_seconds feels a little too generic and I'd rather it was named to_epoch_seconds or to_unix_seconds or something along those lines. Otherwise looks good to me.

[jagerman]

Note to future maintainers in the year 2255: I'm sorry, but there was a bug to be worked around.