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: (levm) Flow operations #553

Merged
merged 25 commits into from
Sep 27, 2024
Merged

feat: (levm) Flow operations #553

merged 25 commits into from
Sep 27, 2024

Conversation

belfortep
Copy link
Contributor

Motivation

Description

Closes #490
Closes #491
Closes #489
Closes #494

@belfortep belfortep marked this pull request as ready for review September 25, 2024 18:49
@belfortep belfortep requested a review from a team as a code owner September 25, 2024 18:49
@belfortep belfortep requested a review from ilitteri September 26, 2024 18:56
Comment on lines 45 to 56
fn valid_jump(&self, offset: U256) -> bool {
// In the future this should be the Opcode::Invalid and halt
self.opcode_at(offset).eq(&Opcode::JUMPDEST)
}

fn opcode_at(&self, offset: U256) -> Opcode {
self.bytecode
.get(offset.as_usize())
.copied()
.map(Opcode::from)
.unwrap_or(Opcode::STOP)
}
Copy link
Contributor

@ilitteri ilitteri Sep 27, 2024

Choose a reason for hiding this comment

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

Firstly, I misunderstood the purpose of the unwrap_or, I thought that we could abstract that logic into opcode_at to reuse it in other functions if needed. After discussing this, I came up with this new suggestion:

Suggested change
fn valid_jump(&self, offset: U256) -> bool {
// In the future this should be the Opcode::Invalid and halt
self.opcode_at(offset).eq(&Opcode::JUMPDEST)
}
fn opcode_at(&self, offset: U256) -> Opcode {
self.bytecode
.get(offset.as_usize())
.copied()
.map(Opcode::from)
.unwrap_or(Opcode::STOP)
}
fn valid_jump(&self, offset: U256) -> bool {
// In the future this should be the Opcode::Invalid and halt
self.bytecode
.get(offset.as_usize())
.copied()
.map(Opcode::from)
.map(|opcode| opcode.eq(&Opcode::JUMPDEST))
.is_some_and(|is_jumpdest| is_jumpdest)
}

Copy link
Contributor

@ilitteri ilitteri Sep 27, 2024

Choose a reason for hiding this comment

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

We can later define an opcode_at function used by next_opcode and valid_jump that returns an Option<Opcode>:

fn opcode_at(&self, offset: U256) -> Option<Opcode> {
    self.bytecode
        .get(offset.as_usize())
        .copied()
        .map(Opcode::from)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just updated it and added the opcode_at function, changing next_opcode and valid_jump to use it

belfortep and others added 3 commits September 27, 2024 12:24
Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
Copy link
Contributor

@ilitteri ilitteri left a comment

Choose a reason for hiding this comment

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

LGTM

@ilitteri ilitteri enabled auto-merge September 27, 2024 15:46
@ilitteri ilitteri added this pull request to the merge queue Sep 27, 2024
Merged via the queue into main with commit 21bff69 Sep 27, 2024
7 checks passed
@ilitteri ilitteri deleted the levm/feat/flow_operations branch September 27, 2024 15:55
mpaulucci pushed a commit to mpaulucci/lambda_ethereum_rust that referenced this pull request Oct 16, 2024
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 -->

Closes lambdaclass#490
Closes lambdaclass#491 
Closes lambdaclass#489 
Closes lambdaclass#494

---------

Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
emirongrr pushed a commit to emirongrr/ethrex that referenced this pull request Nov 10, 2024
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 -->

Closes lambdaclass#490
Closes lambdaclass#491 
Closes lambdaclass#489 
Closes lambdaclass#494

---------

Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
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.

Opcode: JUMPDEST Opcode: PC Opcode: JUMPI Opcode: JUMP
4 participants