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

Increase FinalityDepth & HistoryDepth defaults to match Polygon mainnet #12348

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

reductionista
Copy link
Contributor

We have verified that finality tags on zkevm polygon are just as far behind latest as they are on other polygon chains (often 300-500 blocks). HistoryDepth needs to be larger for the same reason, and allow room for uncles.

Copy link
Contributor

github-actions bot commented Mar 7, 2024

I see that you haven't updated any README files. Would it make sense to do so?

@jmank88
Copy link
Contributor

jmank88 commented Mar 7, 2024

You'll need to run make config-docs to pick up these default changes.

simsonraj
simsonraj previously approved these changes Mar 8, 2024
docs/CONFIG.md Outdated
@@ -3993,13 +3993,13 @@ TipCapMin = '1 wei'

[GasEstimator.BlockHistory]
BatchSize = 25
BlockHistorySize = 12
BlockHistorySize = 1000
Copy link
Contributor

Choose a reason for hiding this comment

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

Why increase the number of blocks used to calculate gas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was an accidental change I made at some point yesterday and rolled back soon after. It's not changed in the actual defaults, must be that I forgot to regenerate the config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patrickhuie19 Thanks for catch, I've re-run "make config-docs" which removed those changes. Approve?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry just saw this, but LGTM! - my github notifications are not surfacing replies to comments :/

We have verified that finality tags on zkevm polygon are just as far
behind latest as they are on other polygon chains (often 300-500
blocks). HistoryDepth needs to be larger for the same reason, and allow
room for uncles.
and increase FinalityDepth & BlockHistoryDepth for both chains
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@reductionista reductionista added this pull request to the merge queue Mar 8, 2024
Merged via the queue into develop with commit efead72 Mar 8, 2024
175 checks passed
ogtownsend pushed a commit that referenced this pull request Mar 14, 2024
…et (#12348)

* Increase FinalityDepth & HistoryDepth defaults to match Polygon mainnet

We have verified that finality tags on zkevm polygon are just as far
behind latest as they are on other polygon chains (often 300-500
blocks). HistoryDepth needs to be larger for the same reason, and allow
room for uncles.

* Add Cardona defaults based on Goerli

and increase FinalityDepth & BlockHistoryDepth for both chains

* make config-docs

* pnpm changeset

* Re-run "make config-docs"
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.

5 participants