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

demote log level for TIMESTAMP_TOO_FAR_IN_FUTURE errors #16101

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Conversation

arvidn
Copy link
Contributor

@arvidn arvidn commented Aug 17, 2023

Purpose:

This patch aims to match the log level with the attention needed from the user.

Current Behavior:

When a node receives an unfinished block with a timestamp that's more than 2 minutes ahead of its own clock, it's rejected with a ConsensusError exception, printing a very serious looking error to the log. We expect to receive more of these invalid blocks while transitioning to more 1.8 full nodes.

Prior to version 1.8.0 the upper timestamp limit was 5 minutes, now it's 2. This means that a sizeable part of the network still forwards blocks more than 2 minutes ahead of wall-clock.

New Behavior:

Instead of printing the stack trace at ERROR log level, print a message at INFO log level describing the validation failure.

Testing Notes:

I'm running this locally and have two instances printed to my log so far:

2023-08-17T22:09:41.847 full_node full_node_server        : INFO     Received block with timestamp too far into the future
2023-08-17T23:11:12.479 full_node full_node_server        : INFO     Received block with timestamp too far into the future

@arvidn arvidn requested a review from a team as a code owner August 17, 2023 21:47
@arvidn arvidn added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Aug 17, 2023
Copy link
Contributor

@danieljperry danieljperry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with changing the severity of this message to INFO.

@github-actions
Copy link
Contributor

File Coverage Missing Lines
chia/full_node/full_node.py 50.0% lines 1648-1649, 1831
chia/server/ws_connection.py 60.0% lines 415, 442
chia/util/errors.py 60.0% lines 200-201
Total Missing Coverage
16 lines 7 lines 56%

@emlowe
Copy link
Contributor

emlowe commented Aug 29, 2023

Approved with coverage diff exception

@wallentx wallentx merged commit ec9d009 into main Aug 29, 2023
204 of 205 checks passed
@wallentx wallentx deleted the timestamp-error branch August 29, 2023 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants