Skip to content
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

fix: enforce UTC where time.Time is used #1194

Merged
merged 2 commits into from
Jun 21, 2022
Merged

fix: enforce UTC where time.Time is used #1194

merged 2 commits into from
Jun 21, 2022

Conversation

technicallyty
Copy link
Contributor

Description

Enforces UTC values when a raw time.Time value is used. timestamppb (both gogo, and google) uses UTC in conversions to and from time.Time. All of our state is stored using timestamppb, not time.Time, so we already have UTC across state. The only lingering problem would be batch denom formatting which is fixed with this PR (alongside expiration checks).

I pulled all batches from mainnet and double checked that formatting their batch denom using the stored start/end date results in the same batch denom stored in state.

Closes: n/a


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)

@technicallyty technicallyty changed the title fix(all): enforce UTC where time.Time is used fix: enforce UTC where time.Time is used Jun 16, 2022
@technicallyty technicallyty mentioned this pull request Jun 16, 2022
19 tasks
@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #1194 (714ae65) into master (22622fb) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1194      +/-   ##
==========================================
+ Coverage   64.87%   64.95%   +0.07%     
==========================================
  Files         231      231              
  Lines       21709    21711       +2     
==========================================
+ Hits        14084    14102      +18     
+ Misses       6276     6254      -22     
- Partials     1349     1355       +6     
Flag Coverage Δ
stable-codecov 64.95% <100.00%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ryanchristo ryanchristo merged commit 22d6564 into master Jun 21, 2022
@ryanchristo ryanchristo deleted the ty/UTC branch June 21, 2022 15:27
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