lambdaclass / cairo-vm

cairo-vm is a Rust implementation of the Cairo VM. Cairo (CPU Algebraic Intermediate Representation) is a programming language for writing provable programs, where one party can prove to another that a certain computation was executed correctly without the need for this party to re-execute the same program.
https://lambdaclass.github.io/cairo-vm
Apache License 2.0
485 stars 133 forks source link

Unexpected Behavior in Representation of PanicResult(Struct) Return Type #1735

Closed Okm165 closed 1 month ago

Okm165 commented 2 months ago
#[derive(Drop, Serde)]
struct Input {
    a: u32, // =3
    b: u32, // =4
    c: u32, // =5
}

struct Output {
    a_2: u32, // =9
    b_2: u32, // =16
    c_2: u32, // =25
}

fn main(input: Array<felt252>) -> Output {
    let mut input_span = input.span();
    let input = Serde::<Input>::deserialize(ref input_span).unwrap();

    let a_2 = input.a * input.a;
    let b_2 = input.b * input.b;
    let c_2 = input.c * input.c;
    assert (a_2 + b_2 == c_2, 'invalid value');

    Output {
        a_2, // =9
        b_2, // =16
        c_2, // =25
    }
}

This program when finished returns object PanicResult(Output). When i print return_values i see [Int(0), Int(9), Int(16), Int(25)] so in order to successfully serialize output i needed to change line to the code below:

return_values = return_values[1..].to_vec()

because otherwise it is removing a_2=Int(9) element of Output struct and fails on serialization step.

Program behaves differently when return value is of type for ex. u32

fn main(input: Array<felt252>) -> u32 {
    let mut input_span = input.span();
    let input = Serde::<Input>::deserialize(ref input_span).unwrap();

    let a_2 = input.a * input.a;
    let b_2 = input.b * input.b;
    let c_2 = input.c * input.c;
    assert (a_2 + b_2 == c_2, 'invalid value');

    a_2
}

This program when finished returns object PanicResult(u32) When i print return_values i see [Int(0), Int(0), Int(9)] so line does remove [Int(0), Int(0)] part and it works well.

Do u know the reason why struct was not represented by [Int(0), Int(0), Int(9), Int(16), Int(25)] as code expects it to be? Do u know what causes this behavior? Tell me wdyt i may misunderstand smth?

branch main commit: 3f1f9ecfffcc52e6d30ce3254b55a7b7cb94eb7f

fmoletta commented 2 months ago

The PanicResult is not implicit it the code itself, so we decided to handle the panic wrapper when fetching return values, and either return the decodified error in the case of a Panic::Error or return the wrapped result in the case of Panic::Ok, consuming the wrapper in the process. This makes it more clear for users of the CLI to see when a run panicked, and avoids possible confusion with the panic wrapper being returned, as otherwise you would have to look at all the corelib functions used by the program to know if the first return value will be a panic wrapper or not.

fmoletta commented 2 months ago

This is also compatible with how cairo's cairo-run handles its outputs. If you run the program (removing the input arg and declaring it inside the program instead), it will yield: Run completed successfully, returning [9, 16, 25]

Okm165 commented 2 months ago

In order to make it work with existing impl on branch main commit: https://github.com/lambdaclass/cairo-vm/commit/3f1f9ecfffcc52e6d30ce3254b55a7b7cb94eb7f i need to define program like this:

#[derive(Drop, Serde)]
struct Input {
    a: felt252,
    b: felt252,
    c: felt252,
}

struct Output {
    a_2: felt252,
    b_2: felt252,
    c_2: felt252,
}

fn main() -> Output {
    let input = Input {
        a: 3,
        b: 4,
        c: 5,
    };

    let a_2 = input.a * input.a;
    let b_2 = input.b * input.b;
    let c_2 = input.c * input.c;
    // assert (a_2 + b_2 == c_2, 'invalid value');

    Output {
        a_2,
        b_2,
        c_2,
    }
}

But this is kinda obvious because this prog is not returning PanicResult(Output) but just Output struct.


What i wanna understand: Given PanicResult(u32) = [Int(0), Int(0), Int(9)] why PanicResult(Output) = [Int(0), Int(9), Int(16), Int(25)] (with just single Int(0) element), this line suggests me that code expects it to be a list of [Int(0), Int(0), Int(9), Int(16), Int(25)].

fmoletta commented 2 months ago

In order to make it work with existing impl on branch main commit: 3f1f9ec i need to define program like this:

#[derive(Drop, Serde)]
struct Input {
    a: felt252,
    b: felt252,
    c: felt252,
}

struct Output {
    a_2: felt252,
    b_2: felt252,
    c_2: felt252,
}

fn main() -> Output {
    let input = Input {
        a: 3,
        b: 4,
        c: 5,
    };

    let a_2 = input.a * input.a;
    let b_2 = input.b * input.b;
    let c_2 = input.c * input.c;
    // assert (a_2 + b_2 == c_2, 'invalid value');

    Output {
        a_2,
        b_2,
        c_2,
    }
}

But this is kinda obvious because this prog is not returning PanicResult(Output) but just Output struct.

What i wanna understand: Given PanicResult(u32) = [Int(0), Int(0), Int(9)] why PanicResult(Output) = [Int(0), Int(9), Int(16), Int(25)] (with just single Int(0) element), this line suggests me that code expects it to be a list of [Int(0), Int(0), Int(9), Int(16), Int(25)].

This program doesn't call function that can cause a panic, and therefore doesn't return a PanicResult<Output>, it only returns Output