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
508 stars 142 forks source link

Make it easier to test hints implemented out of the cairo-vm repository #1595

Open odesenfans opened 7 months ago

odesenfans commented 7 months ago

Hello,

We're currently developing some hints for a project. These hints live in a different Git repository. We developed hints for the bootloader in a fork of cairo-vm so I hadn't noticed yet, but it is surprisingly way more difficult to write unit tests out of the repository. Example:

If we take a hint

%{
    a = ids.a
%}

We can easily write a unit test for this in the cairo-vm repository like this:

let mut vm = vm!();
vm.segments = segments![((1, 0), 42)];
vm.run_context.fp = 1;
let mut exec_scopes = ExecutionScopes::new();
let ids_data = ids_data!["a"];

hint(&mut vm, &mut exec_scopes, &ids_data).unwrap();

let a: u64 = exec_scopes.get("a").unwrap();
assert_eq!(a, 42);

But out of tree, things become more complex / impossible.

// let mut vm = vm!();   // vm! is private
let mut vm = VirtualMachine::new(false);   // but easily replaced by a one-liner, so this is fine

// vm.segments = segments![((1, 0), 42)];   // segments! is also private
vm.add_memory_segment();
vm.add_memory_segment();
vm.insert_value(Relocatable::from((1, 0), 42);  // A bit more verbose, but OK

// vm.run_context.fp = 1;
// Incrementing FP is impossible because `run_context` is `pub(crate)`.

// let ids_data = ids_data!["a"];   // ids_data is private as well
let ids_data = HashMap::from([("a".to_string(), HintReference::new_simple(-1))];

So we're in a situation where AP/FP manipulations are impossible because of VM members being pub(crate) and the utils macros are not public so they need to be reimplemented manually.

The first point is the one that actually is blocking, so I would like your opinion on the matter. What's a good solution there? Making VM fields pub instead of pub(crate)? Adding accessors? Another solution I have not thought of?

Describe the solution you'd like Ideally, it should be possible to write unit tests the same way independently of where the implementation lives.

Additional context This is related to the implementation of hints for the bootloader and Starknet OS.

fmoletta commented 7 months ago

For the first point, the VirtualMachine methods set_ap, set_fp and set_pc are public, they are hidden from the docs but they should be safe to use in tests. And I agree that some testing macros could be made public under the test_utils feature (particularly the segments and memory macros as their usage is not tied to knowing how the vm works internally)

odesenfans commented 7 months ago

The problem with segments! is that it requires vm.segments to be public as well. I can make a PR and we can discuss it there if this suits you.