Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

fix(callop): fix LastCalleeId, and other fixes #1594

Merged
merged 3 commits into from
Sep 12, 2023
Merged

Conversation

lispc
Copy link
Collaborator

@lispc lispc commented Sep 10, 2023

Description

  1. for callop, when there is no bytecode (empty account or non-existed account) or pre check fails, the return data should be flushed to empty, and the LastCalleeId (used in RETURNDATACOPY opcode) should be set to correct callee id instead of 0.
  2. precompile can fail. (eg: out of gas)
  3. "precompile account exists" is only the current status of eth mainnet. It is not part of evm spec. Following spec, precompile account can be non-existed.
  4. when ErrDepth, there is no transfer
  5. (not 100% sure) I also changed the 'is_static' check for only call opcode. It was a fix i made long ago during fixing each fail case of testool. I guess maybe we can construct this test case to check this behavior: delegatecall inside callcode(with non 0 value) inside staticcall. It is a valid tx, but the 'value' is not zero?

Issue Link

[link issue here]

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@github-actions github-actions bot added crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels Sep 10, 2023
@lispc
Copy link
Collaborator Author

lispc commented Sep 10, 2023

oh i cannot see the err logs from github actions... everything ok with CI?

@ed255
Copy link
Member

ed255 commented Sep 12, 2023

oh i cannot see the err logs from github actions... everything ok with CI?

I can't see them either in https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/6135267810/job/16648876291 if this happens again we should investigate it.
On the last run I can see the full logs (all the checks passed).

Copy link
Member

@ed255 ed255 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the various fixes!

Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

LGTM.
Thank you @lispc for upstreaming this fix.
Thank @ed255 for investigating the CALLCODE with non-zero value.

For the CI issue https://github.com/privacy-scaling-explorations/zkevm-circuits/actions/runs/6135267810/job/16648876291, it happened from time to time roughly after #1537. It usually works after a rerun, so we didn't investigate it.

@ChihChengLiang ChihChengLiang added this pull request to the merge queue Sep 12, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 12, 2023
@ChihChengLiang ChihChengLiang added this pull request to the merge queue Sep 12, 2023
Merged via the queue into main with commit 7449c4f Sep 12, 2023
11 checks passed
@ChihChengLiang ChihChengLiang deleted the fix/call-op branch September 12, 2023 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crate-bus-mapping Issues related to the bus-mapping workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants