-
Notifications
You must be signed in to change notification settings - Fork 585
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: use UTC time for packet timeout error #4476
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4476 +/- ##
=======================================
Coverage 79.47% 79.47%
=======================================
Files 188 188
Lines 13045 13045
=======================================
Hits 10367 10367
Misses 2245 2245
Partials 433 433
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @GAtom22!
Can you confirm whether or not these errors msgs are propagated to the block's last results hash? As I understand some validators flagged this issue as reason for halt.
From investigation, I found that the error msg itself should be dropped from ExecTxResult
before being put into consensus.
The error msg which is contained in the ResponseDeliverTx.Log
is flagged as non-deterministic and should be stripped from results.
Hey @damiannolan, For cosmos txs seems to be OK. In Evmos this caused a |
Thank you @GAtom22! This had me scratching my head for a while. cc. @tac0turtle edit: do I understand correctly that changing ANY error string would then essentially be a state machine breaking change for evmos nodes? |
Not any. This is introduced with the EVM extensions that enable to use cosmos/ibc modules with EVM transactions. We're working on handling this on our side to avoid this issue. |
Makes a lot of sense! Thank you 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @GAtom22.
(cherry picked from commit 674d832)
(cherry picked from commit 674d832)
(cherry picked from commit 674d832) Co-authored-by: Tom <[email protected]>
* fix: use UTC time for packet timeout error (#4476) (cherry picked from commit 674d832) * add changelog --------- Co-authored-by: Tom <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>
Description
Problem
When using a timeout timestamp, the packet timeout error message will return a different time depending on the node's timezone configuration.
For example, for the same packet, we'll have:
In a node with CET timezone:
In a node with UTC timezone:
Solution
Unify this using UTC timezone independently of server timezone configuration
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
).godoc
comments.Files changed
in the Github PR explorer.Codecov Report
in the comment section below once CI passes.