feat: add pbtime util package

[robert-zaremba]

Description

Adding pbtime package bring more higher level functionality to protobuf.Timestamp (notably comparison).
With new proto directions we are going away from go stdlib time.Time.

Note, I put it into root, but maybe there is a better place?

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• included the correct type prefix in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• updated the relevant documentation or specification
• reviewed "Files changed" and left comments if necessary
• confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[aaronc]

Thanks @robert-zaremba. I wonder if this maybe does fit in the scope of cosmos-proto as it is pretty generic. @fdymylja would you feel comfortable including it there?

[aaronc]

this isn't generically true. it's sufficient if we assume that the time is in the future, but technically 1970-01-01 is a valid timestamp. generically IsZero is equivalent to t == nil

[robert-zaremba]

This function doesn't do a validation. The goal is to make a canonical zero value which work with a non nil version of tspb.Timestamp.

[aaronc]

But it's not zero. We can't use this to infer that

[robert-zaremba]

It's zero value of tspb.Timestamp and I thought that yesterday we said to add a support for a non nil zero value.

That being said, my if we can change non nil to nil in proto and handle that in a normal way, then that would be my preference.

[aaronc]

I think we should use durationpb here

[robert-zaremba]

OK,. we can have both: time.Duration and durationpb

[fdymylja]

Thanks @robert-zaremba. I wonder if this maybe does fit in the scope of cosmos-proto as it is pretty generic. @fdymylja would you feel comfortable including it there?

TBH, I think ideally you'd want all the time.Time methods on timestampb.Timestamp which accept timestamppb.Timestamp and durationpb.Duration as arguments (and maybe even variants which accept time.Time and time.Duration for easier modifications).

I don't feel the sdk is the correct place for this package, I don't even think cosmos-proto is, Ideally you'd want these upstreamed to go-protobuf (as methods on those types and not as utility function).

But if I had to choose between sdk or cosmos-proto, then better have those in a types/wellknown/time pkg in cosmos-proto.

[robert-zaremba]

I think ideally you'd want all the time.Time methods

We can add them later, when needed. For the moment I mostly need IsZero and Compare for #11095

[robert-zaremba]

I've updated the PR to:

• Add now accepts PB Duration
• new AddStd - accepts time.Duration
• Added overflow checks
• Added case for Compare when either of t1 and t2 is nil -> it will return false. nil is treated here as undefined, although if we want to follow the IsZero then:
  • if both are nil, then either we return true or update the IsZero function. what do you think?

[robert-zaremba]

I think moving it to cosmos-proto is a good idea. Anyone against?

[aaronc]

Moving to cosmos-proto makes sense

[robert-zaremba]

OK, will keep this PR open until I will move it (plan: tomorrow) to let other comment on other things.
BTW: the motivation is to have a canonical functions and representation rather than copy-pasting.

[robert-zaremba]

Moved to cosmos/cosmos-proto#60
Please review there.