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

Implement circuit for CALLER and CALLVALUE opcode#287

Merged
ed255 merged 1 commit into
mainfrom
feature/opcode-caller
Feb 18, 2022
Merged

Implement circuit for CALLER and CALLVALUE opcode#287
ed255 merged 1 commit into
mainfrom
feature/opcode-caller

Conversation

@ed255
Copy link
Copy Markdown
Contributor

@ed255 ed255 commented Jan 18, 2022

This PR depends on

Based on privacy-ethereum/zkevm-specs#90 and privacy-ethereum/zkevm-specs#96

Take a special look at the approach I've followed to verify the equivalence between caller address in the StackOp (rlc encoded) and the CallContext (raw value) .

@ed255 ed255 added crate-bus-mapping Issues related to the bus-mapping workspace member T-opcode Type: opcode-related and focused PR/Issue labels Jan 18, 2022
@ed255 ed255 requested review from CPerezz and han0110 January 18, 2022 12:20
@ed255 ed255 marked this pull request as draft January 18, 2022 12:20
@github-actions github-actions Bot added crate-keccak Issues related to the keccak workspace member crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels Jan 18, 2022
@ed255 ed255 removed request for CPerezz and han0110 January 18, 2022 12:21
@CPerezz CPerezz mentioned this pull request Jan 18, 2022
@CPerezz
Copy link
Copy Markdown
Contributor

CPerezz commented Jan 18, 2022

This will be a lot easier to review once #279 and #278 get merged. As the diff is quite big.

Should we wait for now on the review? And also, I've already reviewed it while debugging. So probably @han0110 or others might have a more unaltered view on that! :)

@ed255
Copy link
Copy Markdown
Contributor Author

ed255 commented Jan 18, 2022

This will be a lot easier to review once #279 and #278 get merged. As the diff is quite big.

Completely agree!

Should we wait for now on the review? And also, I've already reviewed it while debugging. So probably @han0110 or others might have a more unaltered view on that! :)

Yes I'd suggest waiting until #279 and #278 is merged before reviewing this PR. I'll remove the draft mode once it's ready for review.

@ed255 ed255 force-pushed the feature/opcode-caller branch from 7c79ae2 to 4cd9863 Compare January 18, 2022 15:43
@ed255 ed255 force-pushed the feature/opcode-caller branch from 4cd9863 to 0e7c179 Compare January 27, 2022 10:29
@ed255 ed255 changed the title [WIP] Implement circuit for CALLER opcode [WIP] Implement circuit for CALLER and CALLVALUE opcode Jan 27, 2022
@ed255 ed255 force-pushed the feature/opcode-caller branch from 0e7c179 to 08ab69e Compare February 17, 2022 16:39
@github-actions github-actions Bot removed crate-zkevm-circuits Issues related to the zkevm-circuits workspace member crate-keccak Issues related to the keccak workspace member labels Feb 17, 2022
@ed255 ed255 force-pushed the feature/opcode-caller branch from 08ab69e to 7adc5cc Compare February 17, 2022 16:59
@ed255 ed255 marked this pull request as ready for review February 17, 2022 17:00
@ed255 ed255 requested a review from CPerezz February 17, 2022 17:32
@ed255
Copy link
Copy Markdown
Contributor Author

ed255 commented Feb 17, 2022

@CPerezz the PR is now ready for review :)

@ed255 ed255 changed the title [WIP] Implement circuit for CALLER and CALLVALUE opcode Implement circuit for CALLER and CALLVALUE opcode Feb 18, 2022
Copy link
Copy Markdown
Contributor

@CPerezz CPerezz left a comment

Choose a reason for hiding this comment

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

LGTM

Just the const nit for the ADDR_BYTE_LENGTH

Comment thread zkevm-circuits/src/evm_circuit/execution/caller.rs Outdated
@ed255 ed255 force-pushed the feature/opcode-caller branch from 7adc5cc to a51473f Compare February 18, 2022 12:48
@ed255 ed255 merged commit 6aa6705 into main Feb 18, 2022
@CPerezz CPerezz deleted the feature/opcode-caller branch February 23, 2022 08:54
@CPerezz CPerezz mentioned this pull request Mar 4, 2022
19 tasks
zemse pushed a commit to zemse/zkevm-circuits that referenced this pull request Mar 22, 2023
* many fixes after sync upstream

* fixing

* limit mainnet test k = 22

* fix CircuitTestBuilder k

* revert to upstream test_util

* fix k; disable ecdsa
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 T-opcode Type: opcode-related and focused PR/Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants