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

Propagate value of caller when DELEGATECALL#975

Closed
han0110 wants to merge 1 commit into
privacy-ethereum:mainfrom
han0110:fix/correct-value-of-delegatecall
Closed

Propagate value of caller when DELEGATECALL#975
han0110 wants to merge 1 commit into
privacy-ethereum:mainfrom
han0110:fix/correct-value-of-delegatecall

Conversation

@han0110
Copy link
Copy Markdown
Contributor

@han0110 han0110 commented Dec 9, 2022

This PR aims to fix parse_call to propagate caller's value to next call, and calculate the gas cost correctly (when it's delegate call with propagated value, no need to add extra gas).

Resolves #969.

@github-actions github-actions Bot added the crate-bus-mapping Issues related to the bus-mapping workspace member label Dec 9, 2022
@zhuoatscroll
Copy link
Copy Markdown

no i think this fix is not correct. Eg: in bus-mapping callop, there is a transfer call.value. With this PR, there will be a transfer from the delegate caller to delegate callee, which is not the correct behavior.

@lispc
Copy link
Copy Markdown
Collaborator

lispc commented Dec 9, 2022

@silathdiir is working a fix for this problem. #973

@han0110
Copy link
Copy Markdown
Contributor Author

han0110 commented Dec 9, 2022

I see I see, I thought it's a separate issue, but it's actually related, thanks for noticing!

Close this one in favor of #973.

@han0110 han0110 closed this Dec 9, 2022
@han0110 han0110 deleted the fix/correct-value-of-delegatecall branch December 9, 2022 08:19
silathdiir referenced this pull request in scroll-tech/zkevm-circuits Dec 23, 2022
han0110 added a commit that referenced this pull request Dec 23, 2022
…ALL`) (#973)

* Add function `call_context_as_word` and fix `current_value` to Word type.

* Fix to not invoke `transfer` for call related opcodes (CALLCODE, DELEGATECALL and STATICCALL) expect CALL.

* Fix to use `current_call.value` to setup next-call context for DELEGATECALL.

* Update to set first transaction with value `1000`. It sets `current_call.value` to 1000.

* Fix to setup next call `is_static` as current `is_static` or `is_staticcall`.

* Update according to spec code and integrate with non-existing proof.

* Merge PR #975 into this PR, and revert the `current_call.value` fix in bus-mapping.

* Update zkevm-circuits/src/evm_circuit/execution/callop.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* Update bus-mapping/src/evm/opcodes/callop.rs

Co-authored-by: Han <tinghan0110@gmail.com>

* Revert with non-existing callee in test cases, and update `has_value` in circuit.

* Revert `current_value` to a Cell.

* Enable non-existing accounts in test cases, and set empty hash if account doesn't exist.

* Move getting callee existent before `transfer` in bus-mapping.

* Fix to reuse `transfer.caller_balance_pair` for `caller_balance` in CALLCODE.

* Fix lint and wrong assignment.

Co-authored-by: Han <tinghan0110@gmail.com>
noel2004 pushed a commit to noel2004/zkevm-circuits that referenced this pull request Nov 29, 2023
* Return `ChunkProof` from `chunk_prove` test.

* Add `batch_prove` test.
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prover error StateCircuit

3 participants