Skip to content

feat(feynman): ensure that transition block timestamp is exact #1214

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

Closed
wants to merge 3 commits into from
Closed

feat(feynman): ensure that transition block timestamp is exact #1214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc44107
feat(feynman): ensure that transition block timestamp is exact
Thegaram Jun 25, 2025
8ae5924
re-add witness
Thegaram Jun 25, 2025
2941470
typo
Thegaram Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions eth/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,10 @@ func generateWitness(blockchain *core.BlockChain, block *types.Block) (*stateles
// Collect storage locations that prover needs but sequencer might not touch necessarily
statedb.GetState(rcfg.L2MessageQueueAddress, rcfg.WithdrawTrieRootSlot)

// Note: scroll-revm detects the Feynman transition block using this storage slot,
// since it does not have access to the parent block timestamp. We need to make
// sure that this is always present in the execution witness.
// Note: scroll-reth detects the Feynman transition block using the block
// timestamp, but since there might be multiple blocks with the same timestamp,
// it also needs this value to avoid applying the state update multiple times.
// So we need to make sure that this is always present in the execution witness.
statedb.GetState(rcfg.L1GasPriceOracleAddress, rcfg.IsFeynmanSlot)
Comment on lines -365 to 369
Copy link

Choose a reason for hiding this comment

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

You still include the witness for every block?

Copy link
Author

Choose a reason for hiding this comment

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

See the explanation -- there might be multiple blocks with the same timestamp. Either we do this, or we make sure that the block after Feynman has timestamp at least Feynman + 1

Copy link

Choose a reason for hiding this comment

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

I'm not very familiar on the overhead this state read means for the prover, if it's minimal maybe we just leave has is and avoid over complicating things?

Copy link
Member

@georgehao georgehao Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

it also needs this value to avoid applying the state update multiple times.

why set this can avoid the multiple applying, and what's the result after applying many times

Copy link
Member

@colinlyguo colinlyguo Jun 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

why set this can avoid the multiple applying, and what's the result after applying many times

I think it's because there's only one time that a block is in feynman time and IsFeynmanSlot is false, the later blocks with feynman time will have IsFeynmanSlot true.


receipts, _, usedGas, err := blockchain.Processor().Process(block, statedb, *blockchain.GetVMConfig())
Expand Down
10 changes: 9 additions & 1 deletion miner/scroll_worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,17 @@ func (w *worker) newWork(now time.Time, parent *types.Block, reorging bool, reor
header.Coinbase = common.Address{}
header.Nonce = types.BlockNonce{}
} else {
var timeOverride *uint64
if w.chainConfig.IsFeynmanTransitionBlock(header.Time, parent.Time()) {
// current candidate timestamp suggests that this block is the Feynman transition block,
// we forcibly set timestamp to the exact upgrade timestamp.
timeOverride = new(uint64)
*timeOverride = *w.chainConfig.FeynmanTime // IsFeynmanTransitionBlock guarantees that this is not nil
}

prepareStart := time.Now()
// Note: this call will set header.Time, among other fields.
if err := w.engine.Prepare(w.chain, header, nil); err != nil {
if err := w.engine.Prepare(w.chain, header, timeOverride); err != nil {
return fmt.Errorf("failed to prepare header for mining: %w", err)
}
prepareTimer.UpdateSince(prepareStart)
Expand Down
2 changes: 1 addition & 1 deletion params/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
const (
VersionMajor = 5 // Major version component of the current release
VersionMinor = 8 // Minor version component of the current release
VersionPatch = 58 // Patch version component of the current release
VersionPatch = 59 // Patch version component of the current release
VersionMeta = "mainnet" // Version metadata to append to the version string
)

Expand Down
Loading