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

validate EOF bytecode on Genesis. #10172

Closed
rakita opened this issue Aug 7, 2024 · 17 comments · Fixed by #10584
Closed

validate EOF bytecode on Genesis. #10172

rakita opened this issue Aug 7, 2024 · 17 comments · Fixed by #10584
Assignees
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started E-prague Related to the prague network upgrade

Comments

@rakita
Copy link
Collaborator

rakita commented Aug 7, 2024

Describe the feature

If there ar EOF bytecodes in the Genesis file we should validate those bytes so that only valid EOF bytecodes are found in the state.

The function is found in use revm::interpreter::analysis::validate_raw_eof_inner and it needs to be CodeType::ReturnOrStop type (Will renaming it to ContainerKind::Runtime)

Additional context

It does not affect mainnet, it is a guard for new chains that have EOF enabled from the start.

@rakita rakita added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Aug 7, 2024
@rakita rakita added D-good-first-issue Nice and easy! A great choice to get started E-prague Related to the prague network upgrade and removed S-needs-triage This issue needs to be labelled labels Aug 7, 2024
@danielcdz
Copy link
Contributor

Hello @rakita Can I help with this one?

@rakita
Copy link
Collaborator Author

rakita commented Aug 9, 2024

Hello @rakita Can I help with this one?

Go for it

@danielcdz
Copy link
Contributor

Hello @rakita can you give me more context about this? Is a validation that needs to be moved from to a different function?

@nkysg
Copy link
Contributor

nkysg commented Aug 22, 2024

Comment for interests

@emnul
Copy link
Contributor

emnul commented Aug 23, 2024

Hey @danielcdz any updates on your progress here? Happy to take over if you haven't been able to get around to it

@danielcdz
Copy link
Contributor

Hello! @emnul I definitely need some help here, can you provide me more context about the issue?

@emnul
Copy link
Contributor

emnul commented Aug 23, 2024

I think this will help https://github.com/ipsilon/eof/blob/main/spec/eof.md. Although, I'm not sure what "Genesis" refers to in description here. I would reach out to the telegram or open a discussion for more information / clarification.

This is the function that's referred to in the description https://github.com/bluealloy/revm/blob/1ad860469755e3cf71383f45d71c3faaf61d3029/crates/interpreter/src/interpreter/analysis.rs#L75

@danielcdz
Copy link
Contributor

@emnul thank you! do yo have the link to the telegram?

@emnul
Copy link
Contributor

emnul commented Aug 24, 2024

@emnul thank you! do yo have the link to the telegram?

Check the README.md file

@emnul
Copy link
Contributor

emnul commented Aug 26, 2024

The genesis file in the description refers to the genesis.json file which is a JSON-formatted file used to define the initial state of the blockchain at the time of its creation. Here's a good usage example. The struct definition of a Genesis block.

It's possible to define smart contracts in the genesis block (src). If there are EVM Object Format bytecodes in the Genesis file they should be validated so that only valid EOF bytecodes are found in the state.

@emnul
Copy link
Contributor

emnul commented Aug 26, 2024

Hey @rakita is this not already validated on this line? Bytecode::new_raw has EOF validation built-in. Or was the intention here to perform validation in the ChainSpecBuilder or somewhere else? Please clarify

@rakita
Copy link
Collaborator Author

rakita commented Aug 26, 2024

Hey @rakita is this not already validated on this line? Bytecode::new_raw has EOF validation built-in. Or was the intention here to perform validation in the ChainSpecBuilder or somewhere else? Please clarify

Hey, was on vacation last week. This looks like a good place but the function that needs to be called is Bytecode::new_raw_checked and the error needs to be propagated as invalid json.

@emnul
Copy link
Contributor

emnul commented Aug 26, 2024

This looks like a good place but the function that needs to be called is Bytecode::new_raw_checked and the error needs to be propagated as invalid json.

Gotcha, so to be clear, @danielcdz should modify the insert_state function in reth/crates/storage/db-common/src/init.rs to use Bytecode::new_raw_checked and with added error handling such that the error is propagated as invalid json?

@emnul
Copy link
Contributor

emnul commented Aug 26, 2024

@danielcdz feel free to reach out if you run into any other blockers.

@danielcdz
Copy link
Contributor

Hey @emnul and @rakita I really appreciate your help and clarifications here, I'll be working on this and if a get stuck I'll let you guys know!

@nkysg
Copy link
Contributor

nkysg commented Aug 26, 2024

Thanks all,I also understand.

@danielcdz
Copy link
Contributor

Hey @emnul I opened a PR for this, I don't know if it is good at all, but I would appreciate it if you take a look and provide feedback.

@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement New feature or request D-good-first-issue Nice and easy! A great choice to get started E-prague Related to the prague network upgrade
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants