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

insufficient balance error circuit #919

Conversation

DreamWuGit
Copy link
Collaborator

@DreamWuGit DreamWuGit commented Nov 21, 2022

this error execution is different from others(OOG, stack error etc.) we have done, when this error happens, the result was pushed into stack and execution will consume instead of restore to caller's context.
spec PR privacy-scaling-explorations/zkevm-specs#313

@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 Nov 21, 2022
@DreamWuGit DreamWuGit marked this pull request as ready for review November 24, 2022 04:03
@DreamWuGit DreamWuGit changed the title draft insufficient balance error state insufficient balance error circuit Nov 24, 2022
@CPerezz CPerezz self-requested a review November 30, 2022 14:45
Copy link
Member

@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.

Just noticed this and after, I saw that the specs PR that this relies on is not merged yet. So I'd please require you to put this as Draft and first work on getting the spec PR merged.

Then, happy to review this PR !

let fn_gen_error_associated_ops = fn_gen_error_state_associated_ops(&exec_error);
return fn_gen_error_associated_ops(state, geth_steps);
}
let fn_gen_error_associated_ops = fn_gen_error_state_associated_ops(&exec_error);
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what is done whith this new variable. Which seems to be dropped immediately and does nothing and is not passed anywhere.

Copy link
Collaborator Author

@DreamWuGit DreamWuGit Dec 2, 2022

Choose a reason for hiding this comment

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

@CPerezz trying to merge this error state into CallOp code as found many of code shares with it especially for gas cost related stuff. if it works, will update spec accordingly.

@aguzmant103 aguzmant103 requested a review from icemelon December 1, 2022 15:27
@DreamWuGit DreamWuGit marked this pull request as draft December 2, 2022 01:27
@DreamWuGit
Copy link
Collaborator Author

closing this, will open new one for combining into Callop version.

@DreamWuGit DreamWuGit closed this Dec 21, 2022
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.

2 participants