Skip to content

eth/catalyst: sanitize simulated beacon period to avoid overflowing time.Duration #31407

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

Merged
merged 9 commits into from
Apr 17, 2025

Conversation

jwasinger
Copy link
Contributor

closes #31401

Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

SGTM

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
@gballet gballet requested a review from Copilot March 23, 2025 13:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a check in the simulated beacon initialization to cap the period value, preventing an overflow when converting to time.Duration.

  • Added an import for math.
  • Introduced a maximum period check based on math.MaxInt64/time.Second with a corresponding log warning.

@gballet gballet modified the milestones: 1.15.6, 1.15.7 Mar 23, 2025
@fjl fjl modified the milestones: 1.15.7, 1.15.8 Mar 31, 2025
@fjl fjl removed this from the 1.15.8 milestone Apr 11, 2025
@fjl fjl added this to the 1.15.9 milestone Apr 11, 2025
fjl
fjl previously approved these changes Apr 17, 2025
@fjl fjl self-assigned this Apr 17, 2025
@fjl fjl changed the title eth/catalyst: sanitize simulated beacon period to avoid overflowing time.Duration it will occupy eth/catalyst: sanitize simulated beacon period to avoid overflowing time.Duration Apr 17, 2025
@fjl fjl merged commit 074da25 into ethereum:master Apr 17, 2025
3 of 4 checks passed
sivaratrisrinivas pushed a commit to sivaratrisrinivas/go-ethereum that referenced this pull request Apr 21, 2025
…ime.Duration (ethereum#31407)

closes ethereum#31401

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
0g-wh pushed a commit to 0glabs/0g-geth that referenced this pull request Apr 22, 2025
…ime.Duration (ethereum#31407)

closes ethereum#31401

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
0g-wh pushed a commit to 0g-wh/0g-geth that referenced this pull request May 8, 2025
…ime.Duration (ethereum#31407)

closes ethereum#31401

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
Rampex1 pushed a commit to streamingfast/go-ethereum that referenced this pull request May 15, 2025
…ime.Duration (ethereum#31407)

closes ethereum#31401

---------

Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Guillaume Ballet <3272758+gballet@users.noreply.github.com>
Co-authored-by: Felix Lange <fjl@twurst.com>
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.

dev.period should be clamped to max int64 / time.Second
4 participants