Skip to content

Conversation

@zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented May 15, 2025

Currently L2 block time is available in op-node here, but sadly not available in op-geth.

In the codebase there're many places that actually want the next block time, but it's computed as parentHead.Time+1, which is not accurate.

This PR introduces a BlockTime field to chain config and replaces all those places that want the next block time with config.NextBlockTime(parentHead.Time).

@zhiqiangxu zhiqiangxu requested a review from a team as a code owner May 15, 2025 12:59
@zhiqiangxu zhiqiangxu requested a review from protolambda May 15, 2025 12:59
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

There is some code that hydrates the chain config from superchain-registry data. That will need to be updated too. And the op-deployer code that generates a genesis config too. cc @bitwiseguy maybe you can help bridge with platforms team here on chain config changes?

Comment on lines 454 to 456

// Seconds per L2 block
BlockTime uint64 `json:"blockTime,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to call this config var out as an op-stack diff with a comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in fed1b3c.

@zhiqiangxu
Copy link
Contributor Author

zhiqiangxu commented May 15, 2025

There is some code that hydrates the chain config from superchain-registry data. That will need to be updated too.

Done in 41cbd5d.

And the op-deployer code that generates a genesis config too.

Yeah I was planning to modify op-deployer after this PR is merged.

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.

2 participants