Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrong type for bytecode_segment_lengths #651

Merged
merged 1 commit into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Loading