Skip to content

feat: add custom duration type, fix metrics server and flaky catalyst tests #94

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 19, 2025
Merged

feat: add custom duration type, fix metrics server and flaky catalyst tests #94

merged 6 commits into from
May 19, 2025

Conversation

@hominlee-wemade
Copy link
Contributor

@hominlee-wemade hominlee-wemade commented Apr 30, 2025 •
edited
Loading

Summary

This PR addresses two main issues in Config.toml configuration: incorrect handling of time.Duration fields, and misconfiguration that prevented the metrics server from starting correctly.

1. feat: parse time.Duration fields using custom Duration type in config.toml

  • Fields such as Recommit and NewPayloadTimeout now support string values like "1s" or "500ms" in the TOML file.
  • A custom Duration type wrapping time.Duration was introduced, implementing the encoding.TextUnmarshaler and TextMarshaler interfaces to enable automatic parsing during TOML decoding.
  • This ensures that time.Duration fields, which were previously ignored due to parsing issues, are now correctly applied from the configuration.

2. fix: metrics server not starting correctly when configured via config.toml

  • Moved the metrics server initialization to occur after Config.toml is loaded, so that it can start correctly based on the configuration values.
  • Adopted upstream PR #30814 from go-ethereum

3. fix: eth/catalyst, trie/pathdb: fix flaky tests

  • This change fixes three flaky tests TestEth2AssembleBlock,TestEth2NewBlock, TestEth2PrepareAndGetPayload and TestDisable.
  • Adopted upstream PR #29571 from go-ethereum

@hominlee-wemade hominlee-wemade self-assigned this Apr 30, 2025
@hominlee-wemade hominlee-wemade added the bug Something isn't working label Apr 30, 2025
@jed-wemade
Copy link
Contributor

jed-wemade commented May 7, 2025 •
edited
Loading

  1. feat: parse time.Duration fields from string values in config.toml

I recommend BurntSushi/toml package which works as we want.
Or, implement custom interface like an example in naoina/toml.

@hominlee-wemade
refactor: replace string-based duration parsing with custom Duration …
7b1542f 
…type

- Introduced a Duration type implementing encoding.TextUnmarshaler and TextMarshaler
- Replaced fields like ForceSyncCycleStr, TdSyncIntervalStr, etc. with unified Duration type
- Removed redundant ParseDurations() method
- Ensures durations are automatically parsed during TOML decoding
@hominlee-wemade
fix: eth/catalyst, trie/pathdb: fix flaky tests(PR #29571 from go-eth…
c837377 
…ereum)

- This change fixes three flaky tests `TestEth2AssembleBlock`,`TestEth2NewBlock`, `TestEth2PrepareAndGetPayload` and `TestDisable`.
egonspace
egonspace approved these changes May 19, 2025
View reviewed changes
Copy link
Contributor

@egonspace egonspace left a comment

Choose a reason for hiding this comment

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

LGTM

@hominlee-wemade hominlee-wemade changed the title fix: config_toml_apply feat: add custom duration type, fix metrics server and flaky catalyst tests May 19, 2025
@hominlee-wemade hominlee-wemade merged commit 39d55fb into dev May 19, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@egonspace egonspace egonspace approved these changes

@jed-wemade jed-wemade Awaiting requested review from jed-wemade

Assignees

@hominlee-wemade hominlee-wemade

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hominlee-wemade @jed-wemade @egonspace