Skip to content

Commit

Permalink
fix: wrong type for bytecode_segment_lengths
Browse files Browse the repository at this point in the history
  • Loading branch information
xJonathanLEI committed Aug 26, 2024
1 parent 95fb1b6 commit 4214c00
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 37 deletions.
85 changes: 48 additions & 37 deletions starknet-core/src/types/contract/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ pub struct CompiledClass {
/// Represents the structure of the bytecode segments, using a nested list of segment lengths.
/// For example, [2, [3, 4]] represents a bytecode with 2 segments, the first is a leaf of
/// length 2 and the second is a node with 2 children of lengths 3 and 4.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub bytecode_segment_lengths: Vec<IntOrList>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub bytecode_segment_lengths: Option<IntOrList>,
/// Hints for non-determinism.
pub hints: Vec<Hint>,
/// Same as `hints` but represented in Python code, which can be generated by the compiler but
Expand Down Expand Up @@ -658,42 +658,45 @@ impl CompiledClass {
);

// Hashes bytecode
hasher.update(if self.bytecode_segment_lengths.is_empty() {
// Pre-Sierra-1.5.0 compiled classes
poseidon_hash_many(&self.bytecode)
} else {
// `bytecode_segment_lengths` was added since Sierra 1.5.0 and changed hash calculation.
// This implementation here is basically a direct translation of the Python code from
// `cairo-lang` v0.13.1. The goal was simply to have a working implementation as quickly
// as possible. There should be some optimizations to be made here.
// TODO: review how this can be optimized

// NOTE: this looks extremely inefficient. Maybe just use a number for tracking instead?
let mut rev_visited_pcs: Vec<u64> = (0..(self.bytecode.len() as u64)).rev().collect();

let (res, total_len) = Self::create_bytecode_segment_structure_inner(
&self.bytecode,
&IntOrList::List(self.bytecode_segment_lengths.clone()),
&mut rev_visited_pcs,
&mut 0,
)?;

if total_len != self.bytecode.len() as u64 {
return Err(ComputeClassHashError::BytecodeSegmentLengthMismatch(
BytecodeSegmentLengthMismatchError {
segment_length: total_len as usize,
bytecode_length: self.bytecode.len(),
},
));
}
if !rev_visited_pcs.is_empty() {
return Err(ComputeClassHashError::PcOutOfRange(PcOutOfRangeError {
pc: rev_visited_pcs[rev_visited_pcs.len() - 1],
}));
}
hasher.update(
if let Some(bytecode_segment_lengths) = self.bytecode_segment_lengths.clone() {
// `bytecode_segment_lengths` was added since Sierra 1.5.0 and changed hash calculation.
// This implementation here is basically a direct translation of the Python code from
// `cairo-lang` v0.13.1. The goal was simply to have a working implementation as quickly
// as possible. There should be some optimizations to be made here.
// TODO: review how this can be optimized

// NOTE: this looks extremely inefficient. Maybe just use a number for tracking instead?
let mut rev_visited_pcs: Vec<u64> =
(0..(self.bytecode.len() as u64)).rev().collect();

let (res, total_len) = Self::create_bytecode_segment_structure_inner(
&self.bytecode,
&bytecode_segment_lengths,
&mut rev_visited_pcs,
&mut 0,
)?;

if total_len != self.bytecode.len() as u64 {
return Err(ComputeClassHashError::BytecodeSegmentLengthMismatch(
BytecodeSegmentLengthMismatchError {
segment_length: total_len as usize,
bytecode_length: self.bytecode.len(),
},
));
}
if !rev_visited_pcs.is_empty() {
return Err(ComputeClassHashError::PcOutOfRange(PcOutOfRangeError {
pc: rev_visited_pcs[rev_visited_pcs.len() - 1],
}));
}

res.hash()
});
res.hash()
} else {
// Pre-Sierra-1.5.0 compiled classes
poseidon_hash_many(&self.bytecode)
},
);

Ok(hasher.finalize())
}
Expand Down Expand Up @@ -1022,6 +1025,7 @@ mod tests {
include_str!("../../../test-data/contracts/cairo2/artifacts/abi_types_sierra.txt"),
include_str!("../../../test-data/contracts/cairo2/artifacts/erc20_sierra.txt"),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/erc20_sierra.txt"),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/trivial_sierra.txt"),
] {
let direct_deser = serde_json::from_str::<SierraClass>(raw_artifact).unwrap();
let via_contract_artifact = match serde_json::from_str::<ContractArtifact>(raw_artifact)
Expand All @@ -1047,6 +1051,7 @@ mod tests {
include_str!("../../../test-data/contracts/cairo2/artifacts/abi_types_compiled.txt"),
include_str!("../../../test-data/contracts/cairo2/artifacts/erc20_compiled.txt"),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/erc20_compiled.txt"),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/trivial_compiled.txt"),
] {
let direct_deser = serde_json::from_str::<CompiledClass>(raw_artifact).unwrap();
let via_contract_artifact = match serde_json::from_str::<ContractArtifact>(raw_artifact)
Expand Down Expand Up @@ -1133,6 +1138,12 @@ mod tests {
include_str!("../../../test-data/contracts/cairo2.6/artifacts/erc20_compiled.txt"),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/erc20.hashes.json"),
),
(
include_str!(
"../../../test-data/contracts/cairo2.6/artifacts/trivial_compiled.txt"
),
include_str!("../../../test-data/contracts/cairo2.6/artifacts/trivial.hashes.json"),
),
] {
let compiled_class = serde_json::from_str::<CompiledClass>(raw_artifact).unwrap();
let computed_hash = compiled_class.class_hash().unwrap();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"sierra_class_hash": "0x7585639b4e793743860f2761d81e070157ae8d0fc8e518a8cd9069eb2a40010",
"compiled_class_hash": "0x317d3ac2cf840e487b6d0014a75f0cf507dff0bc143c710388e323487089bfa"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"prime": "0x800000000000011000000000000000000000000000000000000000000000001",
"compiler_version": "2.6.2",
"bytecode": [],
"bytecode_segment_lengths": 0,
"hints": [],
"entry_points_by_type": {
"EXTERNAL": [],
"L1_HANDLER": [],
"CONSTRUCTOR": []
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"sierra_program": [
"0x1",
"0x5",
"0x0",
"0x2",
"0x6",
"0x2",
"0x1",
"0xff",
"0x0",
"0x4",
"0x0"
],
"sierra_program_debug_info": {
"type_names": [],
"libfunc_names": [],
"user_func_names": []
},
"contract_class_version": "0.1.0",
"entry_points_by_type": {
"EXTERNAL": [],
"L1_HANDLER": [],
"CONSTRUCTOR": []
},
"abi": [
{
"type": "event",
"name": "trivial::trivial::Trivial::Event",
"kind": "enum",
"variants": []
}
]
}
10 changes: 10 additions & 0 deletions starknet-core/test-data/contracts/cairo2.6/contracts/trivial.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#[starknet::contract]
mod Trivial {
#[storage]
struct Storage {}

#[abi(embed_v0)]
fn something(ref self: ContractState) -> felt252 {
1
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ compile () {
}

compile "/contracts/erc20.cairo" "/artifacts/erc20"
compile "/contracts/trivial.cairo" "/artifacts/trivial"
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ hash () {
}

hash "/artifacts/erc20"
hash "/artifacts/trivial"

0 comments on commit 4214c00

Please sign in to comment.