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

Read-only System Burners #179

Merged
merged 27 commits into from
Jan 24, 2022
Merged

Read-only System Burners #179

merged 27 commits into from
Jan 24, 2022

Conversation

rachel-bousfield
Copy link
Contributor

@rachel-bousfield rachel-bousfield commented Jan 23, 2022

System Burners may now be configured to be read-only, producing errors when one attempts to use them to do work that mutates state. This is useful in scenarios such as block production, where state must be read but cannot be written to without breaking tracing.

Additionally, this PR refactors internal txes to include a Type field

arbos/arbosState/arbosstate.go Outdated Show resolved Hide resolved
@rachel-bousfield rachel-bousfield added the geth Involves changes to Go-Ethereum label Jan 23, 2022
Copy link
Contributor

@edfelten edfelten left a comment

Choose a reason for hiding this comment

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

Minor comments.

arbos/block_processor.go Outdated Show resolved Hide resolved
arbos/block_processor.go Outdated Show resolved Hide resolved
arbos/block_processor.go Outdated Show resolved Hide resolved
arbos/burn/burn.go Outdated Show resolved Hide resolved
edfelten
edfelten previously approved these changes Jan 24, 2022
Copy link
Contributor

@edfelten edfelten left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -85,7 +85,7 @@ func CreateTestL1BlockChain(t *testing.T, l1info info) (info, *eth.Ethereum, *no

nodeConf := ethconfig.Defaults
nodeConf.NetworkId = chainConfig.ChainID.Uint64()
l1Genesys = core.DeveloperGenesisBlock(0, l2pricing.PerBlockGasLimit, l1info.GetAddress("Faucet"))
l1Genesys = core.DeveloperGenesisBlock(0, l2pricing.InitialPerBlockGasLimit, l1info.GetAddress("Faucet"))
Copy link
Contributor

Choose a reason for hiding this comment

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

#47 shifted this to be a constant since it should be unrelated to the L2 gas limit. This change is fine, the the change in #47 should take precedence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted, I only changed this because PerBlockGasLimit is now a function, which means needing an arbosState. We'll move to #47 when merging

precompiles/ArbAddressTable_test.go Outdated Show resolved Hide resolved
solgen/src/precompiles/ArbGasInfo.sol Outdated Show resolved Hide resolved
arbos/tx_processor.go Show resolved Hide resolved
hkalodner
hkalodner previously approved these changes Jan 24, 2022
Copy link
Contributor

@hkalodner hkalodner left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@hkalodner hkalodner left a comment

Choose a reason for hiding this comment

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

LGTM

@hkalodner hkalodner merged commit 4b72600 into master Jan 24, 2022
@rachel-bousfield rachel-bousfield deleted the read-write-burners branch January 24, 2022 16:37
rachel-bousfield added a commit that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ArbOS geth Involves changes to Go-Ethereum
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants