remove new_timestamp(id) standalone function, keep it only as a method of the Session

[milyin]

Describe the release item

This will guarantee that artifitial timestamp with fake ZenohId is never created. But this requires rework of zenoh/plugins/zenoh-plugin-storage-manager/src/replica/snapshotter.rs and several plugins.
Make final decision after discussion with @J-Loudet and @Mallets

[milyin]

Currenly we could safely make the standalone new_timestamp() function internal

[wyfo]

Actually, Session is not the only type to expose an id, there are also Config (which should be the same as the session) and Hello (and maybe others). And Session also exposed peers/routers ids.

The question is then, should we allow timestamp generation only for the local id (from session/config), or do we allow to generate timestamp also for "foreign" ids (peers/routers). I don't think enabling it for "foreign" ids makes sense, but I don't know anything about timestamp in zenoh.
By the way, if the problem is "fake" ZenohId, I think the signature should be changed to accept ZenohId directly instead of impl Into<TimestampId>.

Also, maybe we should hide these uhlc implementation details, provide our own Timestamp type, only compatible with ZenohId, not arbitrary uhlc::ID.

[Charles-Schleich]

My understanding is that we wanted to have a Timestamp with an Idea of the Session, and the current implementation with a ZenohId being set to 1 for new timestamps was a bug.

I agree with the point of hiding uhlc implementation details, a user should not have to create a NonZeroX in order to make a timestamp.

[Mallets]

To guarantee the correct behaviour of Zenoh and avoid API misusage/abuse we should not expose the possibility of generate timestamp for foreign ids. The role of ZenohId in the timestamp is to guarantee unicity in the system, if unicity is not guaranteed then the correct behaviour can't be guaranteed either.

Therefore, I suggest to have something like:

impl Session {
    fn new_timestamp(&self) -> Timestamp { .. }
}

[Charles-Schleich]

Removed Timestamp entirely, PR:remove+rework_timestamp
If we arent exposing a session to the Storage Trait, then each backend that needs to generate a timestamp will have to do so via a Clone of Runtime that can be kept in the Struct that implements Storage.

RocksDB PR: eclipse-zenoh/zenoh-backend-rocksdb#126

[Mallets]

Completed