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

fix delegate call with insufficient balance#1069

Merged
ed255 merged 5 commits into
privacy-ethereum:mainfrom
scroll-tech:delegatecall_insufficient
Jan 20, 2023
Merged

fix delegate call with insufficient balance#1069
ed255 merged 5 commits into
privacy-ethereum:mainfrom
scroll-tech:delegatecall_insufficient

Conversation

@DreamWuGit
Copy link
Copy Markdown
Collaborator

fix two issues:

  1. only check insufficient balance when call/callcode, met error with delegatecall
  2. use caller.address for balance reading.

@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 Jan 16, 2023
Comment thread bus-mapping/src/evm/opcodes/callop.rs
Copy link
Copy Markdown
Contributor

@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 fix!


let is_insufficient = value > caller_balance;

let is_insufficient = (value > caller_balance) && (is_call || is_callcode);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The corresponding circuit expression only does is_insufficient = value > caller_balance. This is because when not(is_call || is_callcode) then value = 0 so value > caller_balance == 0 > caller_balance == false.

So I believe adding the && (is_call || is_callcode here is redundant. Nevertheless maybe it makes things more clear? This is just an observation, not a request to change anything.

@ed255
Copy link
Copy Markdown
Contributor

ed255 commented Jan 19, 2023

@lispc do you consider this PR approved? Or would you like to see some changes?

@ed255 ed255 merged commit adf6eba into privacy-ethereum:main Jan 20, 2023
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

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants