Skip to content

fix: use MiningChain #398

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: use MiningChain #398

wants to merge 1 commit into from

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Apr 25, 2025 •
edited
Loading

as of py-evm 0.12.x, MainnetChain behaves differently (doesn't have all the header fields filled in). use MiningChain as going forward it will apparently be more future-proof

What I did

fix for #397

How I did it

How to verify it

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@charles-cooper
fix: use MiningChain
f1e2393 
as of py-evm 0.12.x, MainnetChain behaves differently (doesn't have all
the header fields filled in). use MiningChain as going forward it will
apparently be more future-proof
@fselmo
Copy link

fselmo commented Apr 25, 2025

I am getting a fix in for that AttributeError but I do think the MiningChain is a better way to instantiate, if all you want is the latest VM starting at genesis. The MainnetChain has the added burden of not having the Prague mainnet block yet because it has not yet hit mainnet. So it shouldn't really be used to configure a chain that doesn't care about non-mainnet properties.

I can't speak to the integration test failures here but this is the recommended way to instantiate the chain. I suggested another approach that can likely make things more modular for future iterations and allow you to control what VM you are at rather than relying on the changing latest_mainnet_at().

fselmo
fselmo reviewed Apr 25, 2025
View reviewed changes
boa/vm/py_evm.py
@@ -565,13 +565,13 @@ def get_storage_slot(self, address: Address, slot: int) -> bytes:
return data.to_bytes(32, "big")


GENESIS_PARAMS = {"difficulty": constants.GENESIS_DIFFICULTY, "gas_limit": int(1e8)}
GENESIS_PARAMS = {"gas_limit": int(1e8)}


# TODO make fork configurable - ex. "latest", "frontier", "berlin"
# TODO make genesis params+state configurable
def _make_chain():
# TODO should we use MiningChain? is there a perf difference?
Copy link

Choose a reason for hiding this comment

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

These can probably go now.

boa/vm/py_evm.py


# TODO make fork configurable - ex. "latest", "frontier", "berlin"
# TODO make genesis params+state configurable
def _make_chain():
# TODO should we use MiningChain? is there a perf difference?
# TODO debug why `fork_at()` cannot accept 0 as block num
_Chain = chain.build(MainnetChain, chain.latest_mainnet_at(1))
_Chain = chain.build(MiningChain, chain.latest_mainnet_at(0))
Copy link

@fselmo fselmo Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I would definitely recommend using MiningChain over MainnetChain for stability and your own control. Alternatively, you can configure the chain directly:

_Chain = MiningChain.configure(chain_id=1, vm_configuration=[(0, PragueVM)])

Seeing as one of the comments above has a goal to make this configurable, you could eventually configure it as, for example, Cancun to Prague at 5:

vm_configuration=[(0, CancunVM), (5, PragueVM)])

boa/vm/py_evm.py
@@ -565,13 +565,13 @@ def get_storage_slot(self, address: Address, slot: int) -> bytes:
return data.to_bytes(32, "big")


GENESIS_PARAMS = {"difficulty": constants.GENESIS_DIFFICULTY, "gas_limit": int(1e8)}
GENESIS_PARAMS = {"gas_limit": int(1e8)}


# TODO make fork configurable - ex. "latest", "frontier", "berlin"
Copy link

Choose a reason for hiding this comment

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

See suggestion below.

@charles-cooper
Copy link
Member Author

I am getting a fix in for that AttributeError but I do think the MiningChain is a better way to instantiate, if all you want is the latest VM starting at genesis. The MainnetChain has the added burden of not having the Prague mainnet block yet because it has not yet hit mainnet. So it shouldn't really be used to configure a chain that doesn't care about non-mainnet properties.

I can't speak to the integration test failures here but this is the recommended way to instantiate the chain. I suggested another approach that can likely make things more modular for future iterations and allow you to control what VM you are at rather than relying on the changing latest_mainnet_at().

i suspect the test failures have to do with changing the start block from 1 to 0, but when i try that, i get

File ~/.venvs/boa/lib/python3.11/site-packages/eth/chains/base.py:128, in BaseChain.get_vm_class_for_block_number(cls, block_number)
    126         return vm_class
    127 else:
--> 128     raise VMNotFound(f"No vm available for block #{block_number}")

VMNotFound: No vm available for block #0

@charles-cooper charles-cooper mentioned this pull request Apr 25, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fselmo fselmo fselmo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@charles-cooper @fselmo