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(ethexe): remove to_vec and mem::transmute from ethexe #4419

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

refactor(ethexe): remove to_vec and mem::transmute from ethexe #4419

wants to merge 18 commits into from

Conversation

playX18
Copy link
Member

@playX18 playX18 commented Dec 23, 2024

@playX18 playX18 requested a review from breathx December 23, 2024 06:59
@playX18 playX18 added the A0-pleasereview PR is ready to be reviewed by the team label Dec 24, 2024
breathx
breathx reviewed Jan 11, 2025
View reviewed changes
Copy link
Member

@breathx breathx left a comment

Choose a reason for hiding this comment

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

impl the same packers/unpackers for i64 across (u32, u32) or even (usize, usize) with according checks. they could be placed in exe/runtime/common, since both exe/runtime and exe/processor(/host) use it

breathx
breathx approved these changes Jan 13, 2025
View reviewed changes
ethexe/runtime/src/wasm/interface/database.rs Outdated Show resolved Hide resolved
ethexe/runtime/src/wasm/interface/mod.rs Show resolved Hide resolved
sandbox/host/src/sandbox/wasmer_backend.rs Outdated Show resolved Hide resolved
sandbox/sandbox/src/host_executor.rs Outdated Show resolved Hide resolved
@breathx breathx added the A5-dontmerge PR should not be merged yet label Jan 13, 2025
breathx
breathx requested changes Jan 14, 2025
View reviewed changes
Copy link
Member

@breathx breathx left a comment

Choose a reason for hiding this comment

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

Discussed

@playX18 playX18 requested a review from breathx January 15, 2025 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grishasobol grishasobol Awaiting requested review from grishasobol

@ByteNacked ByteNacked Awaiting requested review from ByteNacked

@breathx breathx Awaiting requested review from breathx

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
A0-pleasereview PR is ready to be reviewed by the team A5-dontmerge PR should not be merged yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@playX18 @ByteNacked @grishasobol @breathx