Enforce consistent datetime serialization #3389
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a587e9b
to
1ef2adf
Compare
tzemanovic
previously approved these changes
Jun 7, 2024
1ef2adf
to
7f38267
Compare
7f38267
to
3352219
Compare
3352219
to
834a468
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3389 +/- ##
=======================================
Coverage 53.92% 53.93%
=======================================
Files 317 317
Lines 107575 107610 +35
=======================================
+ Hits 58011 58037 +26
- Misses 49564 49573 +9 ☔ View full report in Codecov by Sentry. |
Closed
834a468
to
c18864e
Compare
murisi
previously approved these changes
Jun 8, 2024
murisi
requested changes
Jun 8, 2024
murisi
reviewed
Jun 8, 2024
brentstone
added a commit
that referenced
this pull request
Jun 12, 2024
* tiago/consistent-datetime-serialization: Changelog for #3389 Improve tx salting Fix from tm time impl for `DateTimeUtc` Misc fixes Increase gas limit in `FinalizeBlock` tests Rebuild tx fixtures Resign localnet genesis txs Rebuild wasms for tests gen_localnet.py: Fix genesis time string Add datetime encoding tests Enforce fixed RFC3339 format
Co-authored-by: Yuji Ito <[email protected]>
0ac361d
to
7f90393
Compare
This PR works with the hardware wallet. We just need to make a (hopefully) minor change on the hardware wallet to increase the precision it displays. See for instance the following where |
murisi
previously approved these changes
Jun 18, 2024
Test vectors contain data not found in serializations.
murisi
previously requested changes
Jun 18, 2024
There was a problem hiding this comment.
Could we please change the line
namada/crates/sdk/src/signing.rs
Line 1917 in 7f90393
format!("Timestamp : {}", tx.header.timestamp), ? This will change the expected output on the Ledger app from 2 | Timestamp : 4900-04-22 06:26:45.041754692 UTC to something like 3 | Timestamp : 4440-02-19T23:31:34.674481+00:00. This is required because the Borsh serialization does not contain the last 3 decimal places.
Either we do the above, or we increase the number of decimal places in the Borsh serialization of a DateTimeUtc so that the hardware wallet is able to meet the precise expectations.
murisi
approved these changes
Jun 18, 2024
brentstone
added a commit
that referenced
this pull request
Jun 25, 2024
* tiago/consistent-datetime-serialization: Changelog for #3389 Regen tx fixtures Resign localnet genesis txs Rebuild wasms for tests gen_localnet.py: Fix genesis time string Increase precision of timestamps to 9 nanos Fix genesis time in tests Keep nanoseconds during CometBFT time conversions Switch to fixed offset format in UTC Increase robustness of datetime test Improve tx salting Fix from tm time impl for `DateTimeUtc` Misc fixes Increase gas limit in `FinalizeBlock` tests Add datetime encoding tests Enforce fixed RFC3339 format
brentstone
added a commit
that referenced
this pull request
Jun 25, 2024
* tiago/consistent-datetime-serialization: Changelog for #3389 Regen tx fixtures Resign localnet genesis txs Rebuild wasms for tests gen_localnet.py: Fix genesis time string Increase precision of timestamps to 9 nanos Fix genesis time in tests Keep nanoseconds during CometBFT time conversions Switch to fixed offset format in UTC Increase robustness of datetime test Improve tx salting Fix from tm time impl for `DateTimeUtc` Misc fixes Increase gas limit in `FinalizeBlock` tests Add datetime encoding tests Enforce fixed RFC3339 format
brentstone
added a commit
that referenced
this pull request
Jun 25, 2024
brentstone
added a commit
that referenced
this pull request
Jul 4, 2024
* origin/tiago/consistent-datetime-serialization: Changelog for #3389 Regen tx fixtures Resign localnet genesis txs Rebuild wasms for tests gen_localnet.py: Fix genesis time string Increase precision of timestamps to 9 nanos Fix genesis time in tests Keep nanoseconds during CometBFT time conversions Switch to fixed offset format in UTC Increase robustness of datetime test Improve tx salting Fix from tm time impl for `DateTimeUtc` Misc fixes Increase gas limit in `FinalizeBlock` tests Add datetime encoding tests Enforce fixed RFC3339 format
brentstone
added a commit
that referenced
this pull request
Jul 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Our
DateTimeUtctype allowed a relaxed representation of RFC3339 strings. We now enforce a string subset of this format, to guarantee deterministic serialization.Indicate on which release or other PRs this topic is based on
v0.39.0Checklist before merging to
draft