Skip to content

Conversation

@fjl
Copy link
Contributor

@fjl fjl commented Jul 11, 2025

This is a resubmit of #31820 against the master branch.

@fjl fjl requested review from Copilot and removed request for karalabe July 11, 2025 14:28
Copy link
Contributor

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

Adds support for five new "BPO" hardfork timestamps (BPO1–BPO5) and integrates them into chain configuration and blob gas scheduling.

  • Introduce BPO1TimeBPO5Time in ChainConfig, update display banner, fork-order validation, and compatibility checks.
  • Extend BlobScheduleConfig with BPO entries and refactor blob fee and blob scheduling functions to use a new latestBlobConfig helper.
  • Replace legacy switch on LatestFork in eip4844.go with unified logic that accounts for BPO forks.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
params/config.go Add BPO1–BPO5 time fields, IsBPO methods, integrate into Description, fork ordering, and compatibility checks
consensus/misc/eip4844/eip4844.go Refactor blob fee and capacity functions to use latestBlobConfig with BPO support; remove direct fork switch
Comments suppressed due to low confidence (4)

params/config.go:782

  • [nitpick] The name field uses "bpo1" which is inconsistent with other entries like "pragueTime"; consider using "bpo1Time" to match the JSON key and improve clarity.
		{name: "bpo1", timestamp: c.BPO1Time, optional: true},

params/config.go:414

  • New fork fields like BPO1Time through BPO5Time introduce behavior that should be covered by unit tests (e.g., IsBPO methods and fork ordering) to prevent regressions.
	BPO1Time     *uint64 `json:"bpo1Time,omitempty"`     // BPO1 switch time (nil = no fork, 0 = already on bpo1)

params/config.go:684

  • Using c.IsLondon for BPO forks may prematurely activate forks before the intended predecessor (e.g., Cancun) is reached; consider using c.IsCancun or the appropriate previous fork condition.
	return c.IsLondon(num) && isTimestampForked(c.BPO1Time, time)

consensus/misc/eip4844/eip4844.go:123

  • Using cfg.LondonBlock as the block number when determining blob config may ignore the actual block height for later forks; consider passing the real block number instead of a fixed London threshold.
		london = cfg.LondonBlock

Comment on lines +682 to +694
// IsBPO1 returns whether time is either equal to the BPO1 fork time or greater.
func (c *ChainConfig) IsBPO1(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO1Time, time)
}

// IsBPO2 returns whether time is either equal to the BPO2 fork time or greater.
func (c *ChainConfig) IsBPO2(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO2Time, time)
}

// IsBPO3 returns whether time is either equal to the BPO3 fork time or greater.
func (c *ChainConfig) IsBPO3(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO3Time, time)
Copy link

Copilot AI Jul 11, 2025

Choose a reason for hiding this comment

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

[nitpick] The IsBPO1 through IsBPO5 methods are nearly identical; consider refactoring into a helper or loop to reduce code duplication and ease future additions.

Suggested change
// IsBPO1 returns whether time is either equal to the BPO1 fork time or greater.
func (c *ChainConfig) IsBPO1(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO1Time, time)
}
// IsBPO2 returns whether time is either equal to the BPO2 fork time or greater.
func (c *ChainConfig) IsBPO2(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO2Time, time)
}
// IsBPO3 returns whether time is either equal to the BPO3 fork time or greater.
func (c *ChainConfig) IsBPO3(num *big.Int, time uint64) bool {
return c.IsLondon(num) && isTimestampForked(c.BPO3Time, time)
// IsForked returns whether time is either equal to the specified fork time or greater.
func (c *ChainConfig) IsForked(num *big.Int, time uint64, forkTime uint64) bool {
return c.IsLondon(num) && isTimestampForked(forkTime, time)
}
// IsBPO1 returns whether time is either equal to the BPO1 fork time or greater.
func (c *ChainConfig) IsBPO1(num *big.Int, time uint64) bool {
return c.IsForked(num, time, c.BPO1Time)
}
// IsBPO2 returns whether time is either equal to the BPO2 fork time or greater.
func (c *ChainConfig) IsBPO2(num *big.Int, time uint64) bool {
return c.IsForked(num, time, c.BPO2Time)
}
// IsBPO3 returns whether time is either equal to the BPO3 fork time or greater.
func (c *ChainConfig) IsBPO3(num *big.Int, time uint64) bool {
return c.IsForked(num, time, c.BPO3Time)

Copilot uses AI. Check for mistakes.
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.

LGTM

@fjl fjl merged commit a327ffe into ethereum:master Jul 14, 2025
4 of 5 checks passed
@fjl fjl added this to the 1.16.2 milestone Jul 14, 2025
howjmay pushed a commit to iotaledger/go-ethereum that referenced this pull request Aug 27, 2025
This is a resubmit of ethereum#31820
against the `master` branch.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
gballet pushed a commit to gballet/go-ethereum that referenced this pull request Sep 11, 2025
This is a resubmit of ethereum#31820
against the `master` branch.

---------

Co-authored-by: Marius van der Wijden <[email protected]>
Co-authored-by: Gary Rong <[email protected]>
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