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.

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

refactor(Inspector): Add CreateOutcome in create/create_end return #980

Merged
merged 57 commits into from
Jan 17, 2024
Merged

refactor(Inspector): Add CreateOutcome in create/create_end return #980

merged 57 commits into from
Jan 17, 2024

Conversation

loocapro
Copy link
Contributor

@loocapro loocapro commented Jan 15, 2024 •
edited
Loading

Related to #873

I worked on top of external_field branch as asked by @rakita.

I defined a CreateOutcome struct and defined some helper methods to encapsulate some logic.

It's a first draft, @mattsse may I get some feedback please?

@rakita
rename main Evm structs, introduce wip external type
e35d741
@rakita
tests
fc03f9b
@rakita
Split evm and external context
90a81f0
@rakita
continue previous commit
fda7f8f
@rakita
wip inspector handle register
99ec673
@rakita
add few more handlers for frame and host
ee65aa5
@rakita
Add instruction handle
006a9c2
@rakita
add instruction handler registration and Inspector wrap
73f5ebd
@rakita
Rm Spec generic, more handlers, start factory
42dfcf4
@rakita
move towards the builder, allow EVM modify
6252606
@rakita
wip on EvmBuilder and modify functionality
a8eba54
@rakita
EvmBuilder with stages wip
ed188d7
@rakita
Merge remote-tracking branch 'origin/main' into handlers
85be2c5
@rakita
Add wip stages for builer
64a1bd6
@rakita
make handle register simple function, add raw instruction table, spli…
45d7209 
…t external data from registers
@rakita
wip on simple builder functions and handler registry
5ca9580
@rakita
Examples and cleanup
aef8257
@rakita
fix lifetime and fmt
bebc79d
@rakita
Add more handlers, deduct caller, validate tx agains state
580ddf8
@rakita
All handlers counted, started on docs, some cleanup
6352f90
@rakita
renaming and docs
900ff89
@rakita
Support all Inspector functionality with Handler
55f8924
@rakita
Handler restructured. Documentation added
c930aeb
@rakita
more docs on registers
5dc55bd
@rakita
integrate builder, fmt, move optimism l1block
238ea1b
@rakita
add utility builder stage functions
66b8642
@rakita
add precompiles, fix bugs with journal spec
697a3e3
@rakita
spec to generic, optimism build
9375f35
@rakita
fix optimism test
65997b7
@rakita
fuck macros
bfc5d8b
@loocapro loocapro changed the base branch from main to external_field January 15, 2024 13:54
mattsse
mattsse previously requested changes Jan 15, 2024
View reviewed changes
Copy link
Collaborator

@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.

I like the wrapper type a lot, though I think the additional functions are not that useful

wdyt @rakita

crates/interpreter/src/interpreter.rs Outdated Show resolved Hide resolved
crates/interpreter/src/interpreter.rs Outdated Show resolved Hide resolved
crates/interpreter/src/interpreter.rs Outdated Show resolved Hide resolved
@loocapro
Copy link
Contributor Author

I like the wrapper type a lot, though I think the additional functions are not that useful

wdyt @rakita

Thanks for the feedback! I have followed your comments and have added a possible improvement over encapsulation of the InstructionResult structure.

What you guys think?

@loocapro loocapro requested a review from mattsse January 15, 2024 15:08
rakita
rakita approved these changes Jan 16, 2024
View reviewed changes
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

two nits left.

Do you want to do similar thing for CallOutcome? It can be in this PR or the next one

@loocapro
Copy link
Contributor Author

two nits left.

Do you want to do similar thing for CallOutcome? It can be in this PR or the next one

Sure, i am gonna do in a new pr!

@loocapro loocapro marked this pull request as ready for review January 16, 2024 15:59
@loocapro loocapro requested a review from rakita January 16, 2024 15:59
@rakita
Copy link
Member

rakita commented Jan 17, 2024

Hey @loocapro, can you rebase these changed to main, external_field branch is merged to it?

@loocapro loocapro changed the base branch from external_field to main January 17, 2024 11:11
rakita
rakita approved these changes Jan 17, 2024
View reviewed changes
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm

@rakita rakita merged commit 0629883 into bluealloy:main Jan 17, 2024
25 checks passed
@rakita rakita changed the title Improve/inspector create refactor(Inspector): Add CreateOutcome in create/create_end return Jan 17, 2024
@github-actions github-actions bot mentioned this pull request Jan 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rakita rakita rakita approved these changes

@mattsse mattsse Awaiting requested review from mattsse

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

Successfully merging this pull request may close these issues.
3 participants
@loocapro @rakita @mattsse