Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add: CPC extension for SelfCall #1585

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

supragya
Copy link
Contributor

@supragya supragya commented Apr 18, 2024

Adds support for programs calling themselves under the context of Cross-Program-Calls.

Relevant design discussions: https://github.com/orgs/0xmozak/discussions/1324

@supragya supragya changed the title add: CPC extension for SelfCall add: CPC extension for SelfCall {WIP} Apr 18, 2024
@supragya supragya changed the title add: CPC extension for SelfCall {WIP} add: CPC extension for SelfCall Apr 18, 2024

## Native

To run on your system, use the following command (kindly change [target triple](https://doc.rust-lang.org/cargo/appendix/glossary.html#target) as per your machine's architecture):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like copy and paste?

examples/target/riscv32im-mozak-mozakvm-elf/release/selfcallerbin \
--system-tape examples/selfcaller.tape.json \
--self-prog-id \
MZK-5b7b6135be198533f7c7ec46651216b762e6d47e69b408d1bc79d641f9ae06de;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
MZK-5b7b6135be198533f7c7ec46651216b762e6d47e69b408d1bc79d641f9ae06de;
MZK-5b7b6135be198533f7c7ec46651216b762e6d47e69b408d1bc79d641f9ae06de

self_prog_id: get_self_prog_id(),
extended_self_prog_id: SelfCallExtendedProgramIdentifier(
get_self_prog_id(),
SelfCallExtensionFlag::default(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to extend this?

not(target_os = "mozakvm"),
derive(Debug, serde::Serialize, serde::Deserialize)
)]
/// `SelfCallExtension` is a boolean value that differentiates the
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sounds very weird.

/// tuple as: `((P, 0), (P, 1))` or `((P, 1), (P, 0))`. The added digits
/// help separate the two instances of call and hold no numeric significance
/// apart from mere differentiators.
pub struct SelfCallExtensionFlag(pub u8);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you only need something that flips back and forth, use a bool, and 'not' to flip.

But I'm not convinced yet this whole design is a good idea.

Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Please don't merge as is. I need to think through the design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants