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

Add secp256k1_new request and response. #678

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

ilyalesokhin-starkware
Copy link
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware commented Jun 28, 2023

This change is Reviewable

@ilyalesokhin-starkware
Copy link
Contributor Author

crates/blockifier/src/execution/syscalls/mod.rs line 491 at r1 (raw file):

    // The id of the ec point.
    pub ec_point: usize,

do you want me to put
pub ec_point: Option

here and move the comment to the impl?

Code quote:

    // The syscall returns `Option<Secp256k1Point>` which is represented as two felts in Cairo0.

    // 1 if the point is not on the curve, 0 otherwise.
    pub not_on_curve: bool,

    // The id of the ec point.
    pub ec_point: usize,

elintul
elintul previously approved these changes Jun 28, 2023
Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware and @noaov1)


crates/blockifier/src/execution/execution_utils.rs line 109 at r1 (raw file):

) -> Result<BigUint, VirtualMachineError> {
    let low = vm.get_integer(*ptr)?;
    *ptr += 1;

x2

Suggestion:

*ptr = (*ptr + 1)?;

crates/blockifier/src/execution/syscalls/mod.rs line 22 at r1 (raw file):

    SyscallHintProcessor,
};
use super::execution_utils::u256_from_ptr;

Suggestion:

crate::execution

crates/blockifier/src/execution/syscalls/mod.rs line 484 at r1 (raw file):

#[derive(Debug, Eq, PartialEq)]

pub struct Secp256k1NewResponse {

Remove blank line above.

Code quote:

pub struct Secp256k1NewResponse {

elintul
elintul previously approved these changes Jun 28, 2023
Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware and @noaov1)


crates/blockifier/src/execution/syscalls/secp.rs line 28 at r2 (raw file):

#[derive(Debug, Eq, PartialEq)]

pub struct Secp256k1NewResponse {

Remove blank line above.

Code quote:

pub struct Secp256k1NewResponse {

Copy link
Contributor Author

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @elintul and @noaov1)


crates/blockifier/src/execution/syscalls/secp.rs line 28 at r2 (raw file):

Previously, elintul (Elin) wrote…

Remove blank line above.

done.

Copy link
Collaborator

@elintul elintul left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @noaov1)

@ilyalesokhin-starkware ilyalesokhin-starkware merged commit 511b551 into main Jun 28, 2023
@ilyalesokhin-starkware ilyalesokhin-starkware deleted the ilya/ec_point_req branch June 28, 2023 12:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants