keep-starknet-strange / snos

Rust Library for running the Starknet OS via the Cairo VM
MIT License
59 stars 28 forks source link

bug: Memory addresses must be relocatable #437

Open whichqua opened 1 week ago

whichqua commented 1 week ago

Description

This issue seems to be caused by DECLARE transactions from contracts that are performing an ERC20 transfer. The following blocks are affected:

- 202083
- 202173
- 202216
- 202240
- 203743
- 204082
- 204120
- 204311
- 205604
- 205638*
- 205658

Next steps

The steps to fix the issue:

whichqua commented 3 days ago

This exception happens on cairo-vm meaning we have to debug from there:

pub fn compute_op1_addr(
        &self,
        instruction: &Instruction,
        op0: Option<&MaybeRelocatable>,
    ) -> Result<Relocatable, VirtualMachineError> {
        let base_addr = match instruction.op1_addr {
            Op1Addr::FP => self.get_fp(),
            Op1Addr::AP => self.get_ap(),
            Op1Addr::Imm => match instruction.off2 == 1 {
                true => self.pc,
                false => return Err(VirtualMachineError::ImmShouldBe1),
            },
            Op1Addr::Op0 => match op0 {
                Some(MaybeRelocatable::RelocatableValue(addr)) => *addr,
                Some(_) => return Err(VirtualMachineError::Memory(AddressNotRelocatable)),
                None => return Err(VirtualMachineError::UnknownOp0),
            },
        };
        if instruction.off2 < 0 {
            Ok((base_addr - abs(instruction.off2) as usize)?)
        } else {
            Ok((base_addr + (instruction.off2 as usize))?)
        }
    }

The issue is an invalid instruction:

[cairo-vm/vm/src/vm/context/run_context.rs:68:9] instruction = Instruction {
    off0: 0,
    off1: -3,
    off2: 0,
    dst_register: AP,
    op0_register: FP,
    op1_addr: Op0,
    res: Op1,
    pc_update: Regular,
    ap_update: Add1,
    fp_update: Regular,
    opcode: AssertEq,
}

In this case it receives a op1_addr: Op0, which is an integer and it expects a relocatable.

Next steps include looking for the culprit. Debugging continues.

whichqua commented 3 days ago

Here is the trace for better understanding of the call:

Encountered an invalid relocatable Int(0)
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4ac7bcbaad8d6fd7a51bdf1b696cbc3ba4c796cf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/4ac7bcbaad8d6fd7a51bdf1b696cbc3ba4c796cf/library/core/src/panicking.rs:74:14
   2: cairo_vm::vm::context::run_context::RunContext::compute_op1_addr
             at cairo-vm/vm/src/vm/context/run_context.rs:77:28
   3: cairo_vm::vm::vm_core::VirtualMachine::compute_operands
             at cairo-vm/vm/src/vm/vm_core.rs:634:24
   4: cairo_vm::vm::vm_core::VirtualMachine::run_instruction
             at cairo-vm/vm/src/vm/vm_core.rs:406:13
   5: cairo_vm::vm::vm_core::VirtualMachine::step_instruction
             at cairo-vm/vm/src/vm/vm_core.rs:538:17
   6: cairo_vm::vm::vm_core::VirtualMachine::step
             at cairo-vm/vm/src/vm/vm_core.rs:567:9
   7: cairo_vm::vm::runners::cairo_runner::CairoRunner::run_until_pc
             at cairo-vm/vm/src/vm/runners/cairo_runner.rs:660:13
   8: starknet_os::run_os
             at snos/crates/starknet-os/src/lib.rs:87:5
whichqua commented 3 days ago

Messing around with the Relocatable to provide a custom one yields:

inner_exc: DiffAssertValues((Int(0), Int(1)))

which is an error that we have seen somewhere else 😌

whichqua commented 2 days ago

What This Instruction Does

whichqua commented 2 days ago

After giving a try to the changes in this PR , the issue still persists.