Public inputs refactor

[juan518munoz]

Refactor Public Inputs

Description

Modify the MemorySegmentMap field of PublicInputs to be a HashMap with values of type Segment instead of Range.

This pr also updates the cairo-vm crate to it's latest main revision.

Type of change

• Optimization

[codecov-commenter]

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (cd957b1) 96.23% compared to head (58a5d4e) 96.16%.

Files	Patch %	Lines
provers/cairo/src/air.rs	72.00%	14 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #676      +/-   ##
==========================================
- Coverage   96.23%   96.16%   -0.07%     
==========================================
  Files         133      133              
  Lines       29801    29678     -123     
==========================================
- Hits        28678    28541     -137     
- Misses       1123     1137      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[juan518munoz]

Initial implementation, the struct Segment replaces Range<u64> as the value of the MemorySegmentMap : HashMap.

Range<u64> is still used (and instanced like n..m) on some functions, for this reason From and Into has been implemented beetwen this types.

[juan518munoz]

Should we implement Iterator for Segment in a similar way Range does?

[entropidelic]

Should we implement Iterator for Segment in a similar way Range does?

@juan518munoz I think it is not necessary right now, is there any benefit on doing so?

[juan518munoz]

Should we implement Iterator for Segment in a similar way Range does?

@juan518munoz I think it is not necessary right now, is there any benefit on doing so?

Not really, it would only save us from doing two clones() that are needed to cast into Range.

[entropidelic]

Now that we have the public inputs directly from the cairo vm, there is no need to ask for the codelen, and the public memory should be obtained from the public inputs, this was just a hacky solution until we had access to them.
The approach should be similar to the one you did with the memory segments

[entropidelic]

As a result of this, the PublicInputs field codelen could be removed, since it can be deduced with the length of the Program memory segment

[juan518munoz]

The only function that makes use of the codelen field is get_memory_holes which itself is only called inside build_main_trace. How should we handle this?

[juan518munoz]

The approach should be similar to the one you did with the memory segments

I'm having trouble doing an iteration over the vm public memory, doing something like this:

    let mut public_memory: HashMap<Felt252, Felt252> = HashMap::new();
    vm_public_inputs.public_memory.iter().for_each(|e| {
        let v = e.value.clone().unwrap().to_str_radix(16);
        public_memory.insert(
            Felt252::from((e.address + e.page) as u64),
            Felt252::from_hex_unchecked(&v),
        );
    });

Breaks some of our tests

Maybe I'm missing something?

[Oppen]

Why not just public_input.public_memory.iter() and then get the value and key together? It saves an unwrap.
Related question, is this logic dependent on the order of elements? Because public_memory seems to be a HashMap. Maybe a BTreeMap is more appropriate here?