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: Constrain the Max to Target block size ratio #43

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

davidterpay
Copy link
Contributor

Much of the overflow issues that can arise in the fee market occur from the ratio of max / target block sizes. At a high level, this value is utilized to determine how much we should increase/decrease the base fee when there is a discrepancy between the two. When this value is too large, it can cause unintended side consequences including overflow issues.

Additionally, I added panic handlers that should default the fee market to the minimum's in the case when the fee market does come across a overflow error. We may want to visit that logic at a later time.

@davidterpay davidterpay changed the title feat: Constrain the Max to Target block size ratio's feat: Constrain the Max to Target block size ratio Dec 4, 2023
x/feemarket/types/params.go Outdated Show resolved Hide resolved
x/feemarket/types/state.go Show resolved Hide resolved
for {
state.Update(params.MaxBlockUtilization, params)
state.UpdateLearningRate(params)
baseFee := state.UpdateBaseFee(params)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can use something like require.Recover() to check that a recover happens

Copy link
Collaborator

Choose a reason for hiding this comment

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

or require.Panics()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lemme see if the API supports that, it won't directly panic since it recovers in the same function but we might be able to see if a recover happened

@davidterpay davidterpay merged commit a76e829 into main Dec 5, 2023
5 checks passed
@davidterpay davidterpay deleted the terpay/max-to-target-ratio branch December 5, 2023 22:53
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.

2 participants