Key lifecycle #459
Key lifecycle #459
Conversation
There was a problem hiding this comment.
This is really neat! I can't verify all the logic of the key state transitions, but I've commented on the implementation code.
I'm concerned about how users are supposed to react if one of the key transition functions returns an error. I suppose this relates back to the TODO comment for aborting or canceling rollovers.
I think the single most important thing is to make KeyState.keys a hash map. This should greatly simplify a lot of the code accessing those keys, and it'll make the control flow and logic in the PR a lot easier. It should also noticeably reduce the size of the PR.
src/sign/keyset.rs
Outdated
| #[derive(Clone, Debug, Deserialize, Serialize)] | ||
| /// A type that contains Unix time. | ||
| /// | ||
| /// Unix time is the number of seconds since midnight January first | ||
| /// 1970 GMT. | ||
| pub struct UnixTime(Duration); |
There was a problem hiding this comment.
Is there a reason why std::time::SystemTime was not satisfactory? SystemTime::duration_since(UNIX_EPOCH) already provides a measure of Unix time, and users of this crate will expect standard time types instead of custom ones. I strongly recommend using SystemTime or adding a comment here explaining the need for UnixTime.
There was a problem hiding this comment.
The reason is serde. I want to make sure that the time goes out as seconds since unix epoch. I don't think SystemTime guarantees that. After discussion with @partim we concluded that a type that is clearly defined as Unix time could be useful in more places.
There was a problem hiding this comment.
That's understandable. But I had a peek at serde: the impl Serialize for SystemTime shows that it serializes as seconds and nanoseconds since the Unix Epoch. I'm sure they can't change this now, for backward compatibility, so you can rely on it.
There was a problem hiding this comment.
We can change that later if it makes sense. I don't like relying on undocumented features. It is also important that any time related type is easy to mock.
Basic support for a collection of keys that sign a zone and key rolls.