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

feat: add validation time to block proposal response #5474

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Nov 18, 2024

Fixes #5464

Copy link
Contributor

@hstove hstove left a comment

Choose a reason for hiding this comment

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

This LGTM, but my question is - what exactly do we want to compute as the "computation time"? In this case we have some checks around the block's payload, checks around valid tenure/parent, etc. Is there anything in there that's not relevant to what we care about?

My guess is that it's all stuff we care about, but just asking the question.

@obycode
Copy link
Contributor Author

obycode commented Nov 18, 2024

This LGTM, but my question is - what exactly do we want to compute as the "computation time"? In this case we have some checks around the block's payload, checks around valid tenure/parent, etc. Is there anything in there that's not relevant to what we care about?

My guess is that it's all stuff we care about, but just asking the question.

That's a good question. We basically are trying to get some estimate of the time it will take a node to process this block if it were broadcast as a real block. These same kinds of checks happen there, e.g. in process_next_nakamoto_block, so I think it is correct that we capture all of that. I don't think there is an exact 1:1 mapping, but it should be a reasonable estimate.

@obycode obycode merged commit 9308df9 into feat/time-based-tenure-extend Nov 18, 2024
141 of 147 checks passed
@obycode obycode deleted the feat/update-block-validation branch November 18, 2024 16:48
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