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

feat: crate docs #39

Merged
merged 12 commits into from
Apr 8, 2024
Merged

feat: crate docs #39

merged 12 commits into from
Apr 8, 2024

Conversation

fgimenez
Copy link
Member

@fgimenez fgimenez commented Apr 6, 2024

While writting examples of how to insert precompiles and instructions in a custom EVM I've realized that we didn't have code to clear the instructions context, added in this PR too.

@fgimenez fgimenez marked this pull request as ready for review April 7, 2024 16:03
@fgimenez fgimenez requested review from Rjected and mattsse April 7, 2024 16:04
Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

some typos/nits on the docs

bin/alphanet/src/main.rs Outdated Show resolved Hide resolved
crates/instructions/src/context.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
crates/instructions/src/eip3074.rs Outdated Show resolved Hide resolved
fgimenez and others added 4 commits April 8, 2024 09:38
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
@fgimenez
Copy link
Member Author

fgimenez commented Apr 8, 2024

some typos/nits on the docs

thanks!

Copy link
Contributor

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm,

left one note about a change we need to do when bumping reth deps

Comment on lines 120 to +121
fn evm_with_inspector<'a, DB: Database + 'a, I>(&self, db: DB, inspector: I) -> Evm<'a, I, DB> {
let instructions_context = InstructionsContext::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

heads up, this function signature was changed on main because this is currently incorrect, because we're not appending the inspector register

https://github.com/paradigmxyz/reth/blob/c6857efa8a10e2e86396ac6385d1008b8128e694/crates/evm/src/lib.rs#L66-L70

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

nice, lgtm!

@fgimenez fgimenez merged commit bbcdd43 into main Apr 8, 2024
13 checks passed
@fgimenez fgimenez deleted the fgimenez/docs branch April 8, 2024 16:16
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

4 participants