-
Notifications
You must be signed in to change notification settings - Fork 1k
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: calc block base reward use block number only #8876
Conversation
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.
i don't think the idea was removing the ttd check entirely, just avoiding fetching ttd where possible by checking with the block number first, e.g. if paris is not activated at a specific block number, we'd need to fetch the ttd and check again because it might be a false negative (in the case where we don't know the number of the merge block)
cc @mattsse for clarification
@onbjerg I have the same thoughts as you, but after some consideration, if the chain has passed merge, it must be set |
The block is only set if its present in the chain spec, which is not the case for all chains, e.g. if the block is not known, but the TTD is. We don't update the chain spec while the node is running, so we don't set the block when we detect TTD has been passed. |
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.
we can't safely do that, but what we can do this, add a new function that checks the base block reward pre-merge based on the block number
…ase block reward Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
This reverts commit 7a896ee. Signed-off-by: jsvisa <[email protected]>
…o calc base block reward" This reverts commit e5379e2. Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
@mattsse ptal, seems the |
-.- adding back in #8896 |
nvm, I'll include it here, ty |
c57eab8
to
58f52b9
Compare
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.
lgtm
ref #8868 (comment)