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 132 forks source link

`Error: IlegalInputValue` when run cairo program. #1794

Closed FilipLaurentiu closed 2 days ago

FilipLaurentiu commented 1 week ago

Describe the bug I have a cairo program that performs a few pedersen hashes and check if a leaf is part of a merkle tree The program work successfully with scarb cairo-run, but fail to run with the cairo1-vm, with the same parameters

To Reproduce

scarb cairo-run '[10, 11, "1129815197211541481934112806673325772687763881719835256646064516195041515616", "2786116088662035069066189777680990419908396521409751409107279532930231316343", "1715556295878999972957474070461491436465516895623517391664966219403971354436", 1, 8, ["1953494062994346031473676762198846975365628378496072945247633132004575093152", "126113334767614658176188594640568076708777092902948464648204141774749582367"], ["2786116088662035069066189777680990419908396521409751409107279532930231316343", "3144957507973559441671210571674558258320337923190994230670584137810138721781"]]'

Run completed successfully, returning []

./tools/cairo1-run ./target/dev/starkswirl.sierra.json \                                                                          git:(main|…1 
    --layout recursive \
    --air_public_input public_input.json \
    --air_private_input private_input.json \
    --trace_file trace \
    --memory_file memory \
    --proof_mode \
    --args '10 11 1129815197211541481934112806673325772687763881719835256646064516195041515616 2786116088662035069066189777680990419908396521409751409107279532930231316343 1715556295878999972957474070461491436465516895623517391664966219403971354436 1 8 [1953494062994346031473676762198846975365628378496072945247633132004575093152 126113334767614658176188594640568076708777092902948464648204141774749582367] [2786116088662035069066189777680990419908396521409751409107279532930231316343 3144957507973559441671210571674558258320337923190994230670584137810138721781]'

Error: IlegalInputValue

The repo is public

Expected behavior generate the artifacts

What version/commit are you on? latest from main branch, 8b5d1c6

Additional context I try it with the v1.0.0-rc3 and the error is different

Error: VirtualMachine(Memory(AddressNotRelocatable))

fmoletta commented 1 week ago

Running in proof_mode only allows Array<felt252> as input and output value, as stated in the crate's documentation. This is currently necessary to make cairo 1 programs provable

FilipLaurentiu commented 1 week ago

Running in proof_mode only allows Array<felt252> as input and output value, as stated in the crate's documentation. This is currently necessary to make cairo 1 programs provable

Sure. The program requires 2 span as input, so should be fine

fmoletta commented 1 week ago

Running in proof_mode only allows Array<felt252> as input and output value, as stated in the crate's documentation. This is currently necessary to make cairo 1 programs provable

Sure. The program requires 2 span as input, so should be fine

From how the cli command looks like it seems like the program receives 7 integer values followed by an array/span. The program should only receive a single array of felt values in order to be run in proof_mode

FilipLaurentiu commented 1 week ago

Running in proof_mode only allows Array<felt252> as input and output value, as stated in the crate's documentation. This is currently necessary to make cairo 1 programs provable

Sure. The program requires 2 span as input, so should be fine

From how the cli command looks like it seems like the program receives 7 integer values followed by an array/span. The program should only receive a single array of felt values in order to be run in proof_mode

ok, so I got it wrong. Now I understand.

I create a struct that represent my input and my main input is now just an Array<felt252>

#[derive(Serde)]
pub struct Input {
    secret: felt252,
    nullifier: felt252,
    nullifier_hash: felt252,
    commitment: felt252,
    root: felt252,
    index: usize,
    last_pos: usize,
    peaks: Span<felt252>,
    proof: Span<felt252>
}

but how can I specify the two arrays from CLI ? In order to get deserialized I should mention the length, I think. If I just send the same input as before without square brackets the deserialisation will fail, obviously

--args '10 11 1129815197211541481934112806673325772687763881719835256646064516195041515616 2786116088662035069066189777680990419908396521409751409107279532930231316343 1715556295878999972957474070461491436465516895623517391664966219403971354436 1 8 1953494062994346031473676762198846975365628378496072945247633132004575093152 126113334767614658176188594640568076708777092902948464648204141774749582367 2786116088662035069066189777680990419908396521409751409107279532930231316343 3144957507973559441671210571674558258320337923190994230670584137810138721781'

FilipLaurentiu commented 1 week ago

I figure it out, I have to prepend the arrays with their length. The deserialisation work but now I have another error

./tools/cairo1-run ./target/dev/starkswirl.sierra.json \                                                                        git:(main|✚2…5 
    --layout recursive \
    --air_public_input public_input.json \
    --air_private_input private_input.json \
    --trace_file trace \
    --memory_file memory \
    --proof_mode \
    --args '[10 11 1129815197211541481934112806673325772687763881719835256646064516195041515616 2786116088662035069066189777680990419908396521409751409107279532930231316343 1715556295878999972957474070461491436465516895623517391664966219403971354436 1 8 2 1953494062994346031473676762198846975365628378496072945247633132004575093152 126113334767614658176188594640568076708777092902948464648204141774749582367 2 2786116088662035069066189777680990419908396521409751409107279532930231316343 3144957507973559441671210571674558258320337923190994230670584137810138721781]'

Error: VirtualMachine(Memory(AddressNotRelocatable))

pefontana commented 1 week ago

Hi @FilipLaurentiu ! We will take a look at the error

FrancoGiachetta commented 2 days ago

Hi @FilipLaurentiu ! I was trying to recreate to error you were having, so I ran the same command.

./tools/cairo1-run ./target/dev/starkswirl.sierra.json \
    --layout recursive \
    --air_public_input public_input.json \
    --air_private_input private_input.json \
    --trace_file trace \
    --memory_file memory \
    --proof_mode \
    --args '[10 11 1129815197211541481934112806673325772687763881719835256646064516195041515616 2786116088662035069066189777680990419908396521409751409107279532930231316343 1715556295878999972957474070461491436465516895623517391664966219403971354436 1 8 2 1953494062994346031473676762198846975365628378496072945247633132004575093152 126113334767614658176188594640568076708777092902948464648204141774749582367 2 2786116088662035069066189777680990419908396521409751409107279532930231316343 3144957507973559441671210571674558258320337923190994230670584137810138721781]'

Though this time I got a different error:

Run panicked with: [1 (''), 1569537929006189670479869870450564906403592805 ('Fail to deserialize')]

Which seems to be an error thrown by lib.cairo in your repo, am I correct?

FrancoGiachetta commented 2 days ago

Done a little research about the error that got. Seems like the args used in the commands that README suggests in the main branch of your repo might be outdated. Your Input struct has 10 attributes be the number of args passed are 9, so there's a mismatch and fail's to deserialize.

FrancoGiachetta commented 2 days ago

Talking about the actual issue, running the same command you've used but using --layout starknet_with_keccak, with main's version 0145e17, returns an empty array as expected. The thing is that there are some builtins not supported by the recursive layout which starknet_with_keccak do actually support.

FilipLaurentiu commented 2 days ago

Hey @FrancoGiachetta Thank you for taking a look. Yes, I added one more parameters in the last commit, probably this is why you get that error.

To make things clear, I can reproduce the error with the default layout in one of your example from the repo

cd cairo1-run &&
 cargo run -r ../cairo_programs/cairo-1-programs/serialized_output/bytes31_ret.cairo \
 --air_public_input public_input.json \
 --air_private_input private_input.json \
 --trace_file trace \
 --memory_file memory \
 --proof_mode

Error:

VirtualMachine(Memory(AddressNotRelocatable))

FilipLaurentiu commented 2 days ago

Talking about the actual issue, running the same command you've used but using --layout starknet_with_keccak, with main's version 0145e17, returns an empty array as expected. The thing is that there are some builtins not supported by the recursive layout which starknet_with_keccak do actually support.

This seems to be the case, changing the layout to starknet_with_keccak will run without an error. But changing the layout of the program require changes to the verifier ? In my use case the proof will be verified on-chain by the herodotous integrity verifier

FrancoGiachetta commented 2 days ago

Exactly, I recommend you to get the sierra out of your cairo program and then check which builtins it is using, then you may want to have a look at this to see which of the layouts might fit to your use case.

pefontana commented 2 days ago

Hi @FilipLaurentiu!

Regarding the verifier, I am not entirely sure of the current state of the Herodotus Integrity Verifier. The prover and verifier need to implement certain constraints to verify each built-in. I really don't know if the Herodotus verifier implements those built-in constraints, but it should. Maybe you can ask them directly.

FrancoGiachetta commented 2 days ago

To finish, here are the layouts that you could use:

I would recommend you to use recursive_with_poseidon since it supports just the builtns you need.

pefontana commented 2 days ago

@FilipLaurentiu If you don't mind I will pass this error as solved Any problem just ping us

FilipLaurentiu commented 2 days ago

Thank you @pefontana @FrancoGiachetta for taking your time and help me. I think I am good now and I got a good understanding of what I should change for this to work. Thanks !