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

Initialize LastCallee* of CallContext#298

Merged
han0110 merged 1 commit into
privacy-ethereum:mainfrom
han0110:feature/untrack-last-callee-info
Feb 26, 2022
Merged

Initialize LastCallee* of CallContext#298
han0110 merged 1 commit into
privacy-ethereum:mainfrom
han0110:feature/untrack-last-callee-info

Conversation

@han0110
Copy link
Copy Markdown
Contributor

@han0110 han0110 commented Jan 21, 2022

This PR is once #278, and aims to initialize last callee's information of CallContext in BeginTx.

For spec, see privacy-ethereum/zkevm-specs#99.

It depends on #292.

@github-actions github-actions Bot added crate-bus-mapping Issues related to the bus-mapping workspace member T-opcode Type: opcode-related and focused PR/Issue crate-zkevm-circuits Issues related to the zkevm-circuits workspace member labels Jan 21, 2022
@han0110 han0110 mentioned this pull request Jan 21, 2022
6 tasks
@han0110 han0110 force-pushed the feature/untrack-last-callee-info branch 2 times, most recently from c84f32f to e9d94e0 Compare January 21, 2022 14:15
),
(CallContextFieldTag::Value, tx_value.expr()),
(CallContextFieldTag::IsStatic, 0.expr()),
(CallContextFieldTag::LastCalleeId, 0.expr()),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

want to know why call last callee in begin_tx ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To avoid a malicious prover set these fields to read arbitrary call's memory, we need to make sure it's initilized to 0 properly

@han0110 han0110 force-pushed the feature/untrack-last-callee-info branch 2 times, most recently from 1ec98b0 to 7d398d7 Compare January 30, 2022 09:50
@han0110 han0110 force-pushed the feature/untrack-last-callee-info branch 2 times, most recently from df8d1ae to 189b340 Compare February 18, 2022 03:37
@github-actions github-actions Bot removed the crate-zkevm-circuits Issues related to the zkevm-circuits workspace member label Feb 18, 2022
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!

I have a question though:
LastCallee* rows are always READ right?

But in the same call, we can have different values of LastCallee.
Imagine the following situation

Begin CALL 1
  Begin CALL 2
  End CALL 2
  Begin CALL 3
  End CALL 3
End Call 1

I imagine that the LastCalleeId rows for callId = 1 (in rw_table) will be:

0 rwc 1 isWrite 2 Key0 (Tag) 3 callId 5 Key3 7 Value0
1 false CallContext 1 LastCalleeId 0
2 false CallContext 1 LastCalleeId 2
3 false CallContext 1 LastCalleeId 3

Is this right? If that's the case, then these rows don't follow the READ constraint, because for the same set of keys, the value is changing. Am I missing something?

@han0110
Copy link
Copy Markdown
Contributor Author

han0110 commented Feb 18, 2022

Fields LastCallee* will be written when callee switch context back to caller (e.g. STOP, RETURN, REVERT, all the other ExecutionState that halt). So in your case it would be like:

0 rwc 1 isWrite 2 Key0 (Tag) 3 callId 5 Key3 7 Value0
1 false CallContext 1 LastCalleeId 0
2 true CallContext 1 LastCalleeId 2
3 false CallContext 1 LastCalleeId 2
4 true CallContext 1 LastCalleeId 3
5 false CallContext 1 LastCalleeId 3

There is also implemented in privacy-ethereum/zkevm-specs#100 and the writes are here

@ed255
Copy link
Copy Markdown
Contributor

ed255 commented Feb 18, 2022

Fields LastCallee* will be written when callee switch context back to caller (e.g. STOP, RETURN, REVERT, all the other ExecutionState that halt). So in your case it would be like:
...

Thanks! This resolves my doubt. I had the misunderstanding that LastCalleeId* rows where never WRITE.

Copy link
Copy Markdown
Collaborator

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

/// IsStatic
IsStatic,
/// LastCalleeId
LastCalleeId,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

just thinking how about if we only record LastCalleeId without LastCalleeReturnDataOffset LastCalleeReturnDataOffset ? then we can look up those two items by LastCalleeId as already have ReturnDataOffset ReturnDataOffset , something missing ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fields LastCalleeReturnData* are decided by the callee's step which halts.

Fields ReturnData* are decided by the caller's step which triggers the call, to ask callee to copy the ReturnData to the specified range.

So they are for different purposes.

@CPerezz
Copy link
Copy Markdown
Contributor

CPerezz commented Feb 23, 2022

Should we update and merge this @han0110 ?

@han0110 han0110 force-pushed the feature/untrack-last-callee-info branch 2 times, most recently from a99b432 to ace926d Compare February 25, 2022 05:03
@han0110 han0110 force-pushed the feature/untrack-last-callee-info branch from ace926d to e0cbfd9 Compare February 26, 2022 07:43
@han0110 han0110 merged commit a07200f into privacy-ethereum:main Feb 26, 2022
@han0110 han0110 deleted the feature/untrack-last-callee-info branch February 26, 2022 08:34
zemse pushed a commit to zemse/zkevm-circuits that referenced this pull request Mar 22, 2023
Co-authored-by: Zhang Zhuo <mycinbrin@gmail.com>
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.

4 participants