Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

feat: add OOG error handling for CALLCODE, DELEGATECALL and STATICCALL to ErrorOOGCall#262

Closed
silathdiir wants to merge 6 commits into
scroll-stablefrom
feat/add-callcode-delegatecall-staticcall-oog-errors
Closed

feat: add OOG error handling for CALLCODE, DELEGATECALL and STATICCALL to ErrorOOGCall#262
silathdiir wants to merge 6 commits into
scroll-stablefrom
feat/add-callcode-delegatecall-staticcall-oog-errors

Conversation

@silathdiir
Copy link
Copy Markdown

@silathdiir silathdiir commented Jan 18, 2023

Upstream spec issue privacy-ethereum/zkevm-specs#360

Spec PR scroll-tech/zkevm-specs#56

Summary

  1. Merge enum value OogError::CallCode, DelegateCall and StaticCall into OogError::Call.

  2. Merge enum value ExecutionState::ErrorOutOfGasCALLCODE, ErrorOutOfGasDELEGATECALL and ErrorOutOfGasSTATICCALL into ExecutionState::ErrorOutOfGasCALL.

  3. Update bus-mapping and zkevm-circuit error_oog_call to handle CALLCODE, DELEGATECALL and STATICCALL opcodes.

  4. Update both root and internal test cases in circuit.

@silathdiir silathdiir marked this pull request as draft January 18, 2023 12:17
@silathdiir silathdiir force-pushed the feat/add-callcode-delegatecall-staticcall-oog-errors branch 2 times, most recently from 64a5573 to 595bf8e Compare January 19, 2023 11:17
@silathdiir silathdiir changed the title [WIP] feat: add OOG error handling for CALLCODE, DELEGATECALL and STATICCALL to ErrorOOGCall feat: add OOG error handling for CALLCODE, DELEGATECALL and STATICCALL to ErrorOOGCall Jan 19, 2023
@silathdiir silathdiir marked this pull request as ready for review January 19, 2023 11:25
@github-actions github-actions Bot added the CI label Jan 19, 2023
@silathdiir silathdiir marked this pull request as draft January 19, 2023 12:34
@silathdiir silathdiir marked this pull request as ready for review January 19, 2023 12:34
@silathdiir silathdiir force-pushed the feat/add-callcode-delegatecall-staticcall-oog-errors branch from d42e243 to a5467e6 Compare January 19, 2023 13:50
@github-actions github-actions Bot removed the CI label Jan 19, 2023
@silathdiir silathdiir force-pushed the feat/add-callcode-delegatecall-staticcall-oog-errors branch from 21e4422 to 9b50859 Compare January 30, 2023 08:37
ed255 and others added 4 commits January 30, 2023 11:29
* chore: unify circuits in integration tests

- unify circuit proving test in integration tests with a single generic function that takes a SubCircuit
  - implement SubCircuit for SuperCircuit
- fix: Invalid `SubCircuit.instance()` implementation for TxCircuit
- allow setting the mock_randomness for the SuperCircuit externally

* feat(super): remove unnecessary const generics

- Remove MAX_TXS, MAX_CALLDATA and MAX_RWS from SuperCircuitConfig
- Remove MAX_RWS and MAX_CALLDATA from SuperCircuit
* fix(testool) keep errors format

* Add index and reorder html contents

* Make clippy happy

---------

Co-authored-by: adria0 <Nowhere>
Co-authored-by: adria0 <nowhere@>
PUSH1(0)
.write_op(terminator)
};
.write_op(if is_success { OpcodeId::RETURN } else { OpcodeId::REVERT })
Copy link
Copy Markdown

@DreamWuGit DreamWuGit Feb 2, 2023

Choose a reason for hiding this comment

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

I think don't need is_success check since it is an error case. it is always failed. can remove that parameter.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deleted is_success parameter and always use OpcodeId::REVERT in test cases of OOG call.
Fixed in a597c2c

@DreamWuGit
Copy link
Copy Markdown

DreamWuGit commented Feb 2, 2023

overall good to me .

@silathdiir
Copy link
Copy Markdown
Author

Thanks for @DreamWuGit 's help for code review. Moved to upstream privacy-ethereum#1127

@silathdiir silathdiir closed this Feb 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants