dojo icon indicating copy to clipboard operation
dojo copied to clipboard

Chain id issue

Open greged93 opened this issue 1 year ago • 5 comments

Describe the bug The loaded chain id using --chain-id flag is not the same value as returned when using the get_tx_info syscall.

To Reproduce Steps to reproduce the behavior (the issue is due to the use of the parse_cairo_short_string from Starknet-rs):

use std::str::FromStr as _;
use starknet::core::{types::FieldElement, utils::parse_cairo_short_string};

fn main() {
    let f = FieldElement::from_str("0xc72dd9d5e883e").unwrap();
    println!("{:?}", f);

    // Used in Katana to convert the chain id to a String, as required by the chain id field in the BlockContext type.
    let s = parse_cairo_short_string(&f).unwrap();

    // How Blockifier converts the string reprensentation of the chain id back to a FieldElement.
    let f = FieldElement::from_byte_slice_be(s.as_bytes()).unwrap();
    println!("{:?}", f);
}

Expected behavior Both chain id's should be the same.

greged93 avatar Mar 01 '24 15:03 greged93

I think a way to produce the correct result is by using the unsafe method from_utf8_unchecked, which stores values even if they aren't valid utf8 values. The issue from the above example is that the byte representation of the field element "0xc72dd9d5e883e" contains the byte 136 (hex 88) which isn't a valid utf8 value. This causes the parse_cairo_short_string to add a value in the String to make it a valid utf8 char. I don't think adding from_utf8_unchecked is a solution here because of course, I'm from the start violating the invariants lol. Below is a working example

use std::str::FromStr as _;

use starknet::core::types::FieldElement;

fn main() {
    let f_old = FieldElement::from_str("0xc72dd9d5e883e").unwrap();

    let binding = f_old.to_bytes_be();
    let s = match String::from_utf8(binding.to_vec()) {
        Ok(str) => str,
        Err(_) => unsafe { String::from_utf8_unchecked(binding.to_vec()) },
    };

    let f_new = FieldElement::from_byte_slice_be(s.as_bytes()).unwrap();
    assert_eq!(f_old, f_new);
}

greged93 avatar Mar 01 '24 16:03 greged93

hey @kariy, is it possible at all to fix this or does it require too much changes on Blockifier,...?

greged93 avatar Jun 13 '24 11:06 greged93

@greged93 I've just run some tests on the update of blockifier pushed by @kariy in #2180. The issue seems to be resolved:

# Katana run with --chain-id ABCD

# Output from contract get_tx_info().unbox().chain_id in Katana prints
1094861636 # 0x41424344

# Starkli
0x41424344 (ABCD)

glihm avatar Jul 17 '24 22:07 glihm

@greged93 i believe this issue has been fixed in the latest blockifier (v0.8.0-dev.2). i've added a test for this in https://github.com/dojoengine/dojo/pull/2204

kariy avatar Jul 23 '24 15:07 kariy

hey @kariy @glihm just provided a failing example for the chain id in the latest linked PR, I don't think this can be fixed without changes to blockifier, but let me know :)

greged93 avatar Aug 08 '24 11:08 greged93