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

Add Monad Testnet Config and Error Mapping #15993

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

flodesi
Copy link
Contributor

@flodesi flodesi commented Jan 20, 2025

No description provided.

@flodesi flodesi requested review from a team as code owners January 20, 2025 16:12
@flodesi flodesi requested a review from krehermann January 20, 2025 16:12
@@ -0,0 +1,23 @@
ChainID = '10143'
# finality_depth was: 0
FinalityDepth = 10
Copy link
Contributor

@simsonraj simsonraj Jan 20, 2025

Choose a reason for hiding this comment

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

did the finality committee approve the number?

Copy link
Contributor Author

@flodesi flodesi Jan 20, 2025

Choose a reason for hiding this comment

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

I haven't reached out, was going off a previous conversation I had with @fernandezlautaro regarding finality where a chain with instant finality we default to using a finality depth of 10 (example being sei). Please let me know if I should

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please atleast make them aware, Max would be a good person to start with

simsonraj
simsonraj previously approved these changes Jan 20, 2025
Copy link
Contributor

@simsonraj simsonraj Jan 20, 2025

Choose a reason for hiding this comment

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

I think if we dont have the DF. we dont need to override all the configs inside core, do we?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like we always need to have this override. CCIP's is the option one.

Copy link
Contributor

Choose a reason for hiding this comment

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

yea but shouldn't the chainID only be enough & the rest will be picked up from the ccip defaults?

LogBroadcasterEnabled = false
FinalityTagEnabled = false
# finality_depth * block_time / 60 secs = < 1 min (finality time)
NoNewFinalizedHeadsThreshold = '1m'
Copy link
Collaborator

@dhaidashenko dhaidashenko Jan 20, 2025

Choose a reason for hiding this comment

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

nit: Should we also set NoNewHeadsThreshold to a lower value (current 3m)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. added!

dhaidashenko
dhaidashenko previously approved these changes Jan 20, 2025
@flodesi flodesi dismissed stale reviews from dhaidashenko and simsonraj via 05617d8 January 20, 2025 19:06
@cl-sonarqube-production
Copy link

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