Skip to content
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

refactor(config): consolidate scroll flags #262

Merged
merged 10 commits into from Apr 17, 2023
Merged

refactor(config): consolidate scroll flags #262

merged 10 commits into from Apr 17, 2023

Conversation

HAOYUatHZ
Copy link
Member

@HAOYUatHZ HAOYUatHZ commented Apr 6, 2023

baseFee & EIP-2718 & EIP-1559 related PRs:


we can also move FeeVaultAddress into rollup/rcfg/config.go, but that's a bigger refactor so can be left for future

@HAOYUatHZ HAOYUatHZ force-pushed the consolidate branch 5 times, most recently from 6ad4822 to 57c7eee Compare April 6, 2023 07:05
@HAOYUatHZ HAOYUatHZ changed the title refactor: consolidate scroll flags refactor(config): consolidate scroll flags Apr 6, 2023
@HAOYUatHZ HAOYUatHZ force-pushed the consolidate branch 6 times, most recently from 7708d06 to 0bc0b32 Compare April 6, 2023 10:39
@HAOYUatHZ HAOYUatHZ marked this pull request as ready for review April 6, 2023 11:16
@HAOYUatHZ
Copy link
Member Author

not sure which version number should I use? 3.2.0? @Thegaram

miner/worker.go Outdated Show resolved Hide resolved
params/config.go Outdated Show resolved Hide resolved
params/config.go Outdated Show resolved Hide resolved
params/config.go Outdated Show resolved Hide resolved
Thegaram
Thegaram previously approved these changes Apr 15, 2023
@Thegaram
Copy link

not sure which version number should I use? 3.2.0? @Thegaram

I'd consider this a backward compatible change (using l2geth on non-scroll networks was not supported anyway). So bump path or no bump is enough.

@HAOYUatHZ HAOYUatHZ merged commit df1d37e into develop Apr 17, 2023
3 checks passed
@HAOYUatHZ HAOYUatHZ deleted the consolidate branch April 17, 2023 10:56
// Scroll genesis extension: enable scroll rollup-related traces & state transition
// TODO: merge with these config: Zktrie, FeeVaultAddress, EnableEIP2718, EnableEIP1559 & MaxTxPerBlock
UsingScroll bool `json:"usingScroll,omitempty"`
func (s ScrollConfig) L1FeeEnabled() bool {
Copy link
Member

Choose a reason for hiding this comment

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

The function name is a bit inaccurate. Using fee vault & enable L1 fee is different. Fee vault collects the tx fee that includes both L2 exec fee and L1 rollup fee, right?

if api.backend.ChainConfig().UsingScroll {
blockTrace.WithdrawTrieRoot = withdrawtrie.ReadWTRSlot(rcfg.L2MessageQueueAddress, env.state)
}
blockTrace.WithdrawTrieRoot = withdrawtrie.ReadWTRSlot(rcfg.L2MessageQueueAddress, env.state)
Copy link
Member

Choose a reason for hiding this comment

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

This is not controlled by any config now. Is it compatible with other geth configs?

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.

None yet

5 participants