updated to sierra version 2.16 - #1557
Conversation
0fb96f5 to
fe05bc2
Compare
|
✅ Code is now correctly formatted. |
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 5 files and all commit messages, and made 3 comments.
Reviewable status: 5 of 28 files reviewed, 3 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
Makefile line 4 at r1 (raw file):
UNAME := $(shell uname) SCARB_VERSION = 2.14.0
Suggestion:
SCARB_VERSION = 2.16.0binaries/starknet-native-compile/src/main.rs line 113 at r1 (raw file):
.extract_sierra_program(false) .context("Error extracting Sierra program from contract class.")?.program, sierra_version,
Suggestion:
let extracted = contract_class
.extract_sierra_program(false)
.context("Error extracting Sierra program from contract class.")?;
Ok((
contract_class.clone(),
extracted.program,
extracted.sierra_version,debug_utils/sierra-emu/examples/contract.rs line 44 at r1 (raw file):
&contract.entry_points_by_type, version_id, );
Suggestion:
)?;
let extracted = contract.extract_sierra_program(false)?;
// Find entrypoint to execute
let entrypoint = contract
.entry_points_by_type
.external
.first()
.ok_or("contract should contain at least one external entrypoint")?
.clone();
// Build virtual machine
let mut vm = VirtualMachine::new_starknet(
Arc::new(extracted.program),
&contract.entry_points_by_type,
extracted.sierra_version,
);
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and made 2 comments.
Reviewable status: 6 of 28 files reviewed, 5 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
debug_utils/sierra-emu/src/main.rs line 93 at r1 (raw file):
&contract.entry_points_by_type, sierra_version, );
Suggestion:
let extracted = contract.extract_sierra_program(false).unwrap();
let entry_point = contract.entry_points_by_type.external.first().unwrap();
let mut vm = VirtualMachine::new_starknet(
extracted.program.into(),
&contract.entry_points_by_type,
extracted.sierra_version,
);debug_utils/sierra-emu/src/main.rs line 141 at r1 (raw file):
&contract.entry_points_by_type, sierra_version, );
Suggestion:
let extracted = contract.extract_sierra_program(false).unwrap();
let entry_point = contract.entry_points_by_type.constructor.first().unwrap();
let mut vm = VirtualMachine::new_starknet(
extracted.program.into(),
&contract.entry_points_by_type,
extracted.sierra_version,
);
orizi
left a comment
There was a problem hiding this comment.
@orizi made 7 comments.
Reviewable status: 6 of 28 files reviewed, 12 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
debug_utils/sierra-emu/src/vm.rs line 377 at r1 (raw file):
) .unwrap(); frame.state = new_state;
Suggestion:
frame.pc = frame.pc.next(invocation.branches[branch_idx].target);
frame.state
.put_vars(
invocation.branches[branch_idx].results.iter().zip(results),
)
.unwrap();debug_utils/sierra-emu/src/vm.rs line 420 at r1 (raw file):
.put_vars(target_branch.results.iter().zip(values)) .unwrap(); prev_frame.state = new_state;
Suggestion:
prev_frame.state
.put_vars(target_branch.results.iter().zip(values))
.unwrap();debug_utils/sierra-emu/src/vm/enum.rs line 27 at r1 (raw file):
EnumConcreteLibfunc::Match(info) => eval_match(registry, info, args), EnumConcreteLibfunc::SnapshotMatch(info) => eval_snapshot_match(registry, info, args), EnumConcreteLibfunc::BoxedMatch(_) => todo!(),
Suggestion:
todo!("string with meaning"),debug_utils/src/bin/contract-to-sierra.rs line 26 at r1 (raw file):
.expect("failed to extract sierra program").program; print!("{}", sierra);
Suggestion:
print!("{}", contract
.extract_sierra_program(false)
.expect("failed to extract sierra program").program);src/compiler.rs line 437 at r1 (raw file):
state.put_vars(values.into_iter())?; state };
Suggestion:
let initial_state = {
let mut values = OrderedHashMap::default();
let mut count = 0;
for param in &function.params {
let type_info = registry.get_type(¶m.ty)?;
let location = Location::new(
context,
"program.sierra",
sierra_stmt_start_offset + function.entry_point.0,
0,
);
values.insert(
param.id.clone(),
if type_info.is_builtin() && type_info.is_zst(registry)? {
pre_entry_block
.append_operation(llvm::undef(
type_info.build(context, module, registry, metadata, ¶m.ty)?,
location,
))
.result(0)?
.into()
} else {
let value = entry_block.argument(count)?.into();
count += 1;
value
},
);
#[cfg(feature = "with-trace-dump")]
var_types.insert(param.id.clone(), param.ty.clone());
}
values
};src/compiler.rs line 488 at r1 (raw file):
.collect::<Result<Vec<_>, Error>>()? .into_iter(), )?;
Suggestion:
let mut new_state = state
.keys()
.sorted_by_key(|x| x.id)
.enumerate()
.map(|(idx, var_id)| Ok((var_id, landing_block.argument(idx)?.into())))
.collect::<Result<_, Error>>()?;src/compiler.rs line 506 at r1 (raw file):
), )); }
Suggestion:
landing_block.append_operation(cf::br(
block,
&state.clone().take_vars(match &statements[statement_idx.0] {
Statement::Invocation(x) => &x.args,
Statement::Return(x) => x,
}.iter())?,
Location::name(
context,
&format!("landing_block(stmt_idx={})", statement_idx),
fn_location,
),
));
}
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 6 files and made 9 comments.
Reviewable status: 12 of 28 files reviewed, 21 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
src/compiler.rs line 556 at r1 (raw file):
); let mut state = state;
src/compiler.rs line 1072 at r1 (raw file):
)?; state };
Suggestion:
let initial_state: OrderedHashMap::<_, Type> =
function
.params
.iter()
.zip(&function.signature.param_types)
.map(|(param, ty)| {
let type_info = registry.get_type(ty)?;
Ok((
¶m.id,
type_info.build(context, module, registry, metadata_storage, ty)?,
))
})
.collect::<Result<_, Error>>()?;src/executor/contract.rs line 857 at r1 (raw file):
.extract_sierra_program(false) .unwrap() .program,
Suggestion:
let extracted = starknet_program
.extract_sierra_program(false)
.unwrap();
let executor = Arc::new(
AotContractExecutor::new(
&extracted.program,src/executor/contract.rs line 859 at r1 (raw file):
.program, &starknet_program.entry_points_by_type, sierra_version,
Suggestion:
extracted.sierra_version,src/executor/contract.rs line 902 at r1 (raw file):
.program, &starknet_program.entry_points_by_type, sierra_version,
same
src/executor/contract.rs line 945 at r1 (raw file):
.program, &starknet_program_factorial.entry_points_by_type, sierra_version,
same
src/executor/contract.rs line 989 at r1 (raw file):
.program, &starknet_program_empty.entry_points_by_type, sierra_version,
same
src/utils/sierra_gen.rs line 468 at r1 (raw file):
let cache = self.type_info_cache.borrow(); let ptr = cache.get(id).unwrap() as *const TypeInfo; return Some(unsafe { &*ptr });
i don't see any actual reason for this entire file to exist. @gabrielbosio @igaray
tests/common/mod.rs line 473 at r1 (raw file):
&contract.extract_sierra_program(false).unwrap().program, &contract.entry_points_by_type, sierra_version,
same
fe05bc2 to
5a12e37
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi partially reviewed 21 files and resolved 18 discussions.
Reviewable status: 30 of 34 files reviewed, 3 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.515 ± 0.080 | 11.455 | 11.728 | 5.50 ± 0.07 |
cairo-native (embedded AOT) |
2.092 ± 0.022 | 2.065 | 2.128 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.125 ± 0.030 | 2.081 | 2.164 | 1.02 ± 0.02 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
564.8 ± 4.3 | 558.2 | 573.2 | 1.00 |
cairo-native (embedded AOT) |
1727.2 ± 27.3 | 1682.1 | 1768.1 | 3.06 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1842.4 ± 12.1 | 1812.7 | 1853.5 | 3.26 ± 0.03 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.936 ± 0.038 | 4.903 | 5.038 | 2.29 ± 0.03 |
cairo-native (embedded AOT) |
2.158 ± 0.020 | 2.133 | 2.199 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.220 ± 0.017 | 2.205 | 2.252 | 1.03 ± 0.01 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.832 ± 0.025 | 4.797 | 4.871 | 2.81 ± 0.03 |
cairo-native (embedded AOT) |
1.717 ± 0.015 | 1.696 | 1.744 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.802 ± 0.008 | 1.789 | 1.814 | 1.05 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
613.8 ± 5.1 | 607.0 | 623.5 | 1.00 |
cairo-native (embedded AOT) |
1753.0 ± 30.7 | 1724.0 | 1811.7 | 2.86 ± 0.06 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1876.5 ± 20.2 | 1849.3 | 1918.4 | 3.06 ± 0.04 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
513.3 ± 6.3 | 507.5 | 524.9 | 1.00 |
cairo-native (embedded AOT) |
1898.7 ± 21.0 | 1874.0 | 1948.6 | 3.70 ± 0.06 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2077.1 ± 17.4 | 2044.2 | 2102.9 | 4.05 ± 0.06 |
7121fd3 to
caf228a
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 16 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: 41 of 42 files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
tests/common.rs line 0 at r3 (raw file):
what is this file?
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 5 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
caf228a to
5e82362
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 5 files and made 1 comment.
Reviewable status: 43 of 46 files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
tests/common/mod.rs line 350 at r5 (raw file):
0.into(), 0.into(), ];
Suggestion:
let builtin_costs: Vec<MaybeRelocatable> = vec![0.into(); 8];
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and all commit messages.
Reviewable status: 44 of 46 files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 2 files.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, pefontana, and TomerStarkware).
|
Two pending tasks here:
There are some CI jobs that fail in the Also, the sequencer PR should point to the |
5e82362 to
17ef1a1
Compare
23468de to
a84d1df
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1557 +/- ##
==========================================
+ Coverage 72.87% 73.03% +0.15%
==========================================
Files 110 110
Lines 27152 27188 +36
==========================================
+ Hits 19788 19856 +68
+ Misses 7364 7332 -32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
494ac90 to
8e72a11
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 17 comments.
Reviewable status: 38 of 84 files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, orizi, and pefontana).
src/executor/contract.rs line 902 at r1 (raw file):
Previously, orizi wrote…
same
Done.
src/executor/contract.rs line 945 at r1 (raw file):
Previously, orizi wrote…
same
Done.
src/executor/contract.rs line 989 at r1 (raw file):
Previously, orizi wrote…
same
Done.
tests/common/mod.rs line 473 at r1 (raw file):
Previously, orizi wrote…
same
Done.
tests/common.rs line at r3 (raw file):
Previously, orizi wrote…
what is this file?
was accedintaly resored due to merge conflics
debug_utils/sierra-emu/src/vm.rs line 377 at r1 (raw file):
) .unwrap(); frame.state = new_state;
Done.
debug_utils/sierra-emu/src/vm.rs line 420 at r1 (raw file):
.put_vars(target_branch.results.iter().zip(values)) .unwrap(); prev_frame.state = new_state;
Done.
binaries/starknet-native-compile/src/main.rs line 113 at r1 (raw file):
.extract_sierra_program(false) .context("Error extracting Sierra program from contract class.")?.program, sierra_version,
Done.
debug_utils/src/bin/contract-to-sierra.rs line 26 at r1 (raw file):
.expect("failed to extract sierra program").program; print!("{}", sierra);
Done.
src/compiler.rs line 437 at r1 (raw file):
state.put_vars(values.into_iter())?; state };
Done.
src/compiler.rs line 488 at r1 (raw file):
.collect::<Result<Vec<_>, Error>>()? .into_iter(), )?;
Done.
src/compiler.rs line 556 at r1 (raw file):
); let mut state = state;
Done.
src/compiler.rs line 1072 at r1 (raw file):
)?; state };
Done.
Makefile line 4 at r1 (raw file):
UNAME := $(shell uname) SCARB_VERSION = 2.14.0
Done.
src/executor/contract.rs line 857 at r1 (raw file):
.extract_sierra_program(false) .unwrap() .program,
Done.
src/executor/contract.rs line 859 at r1 (raw file):
.program, &starknet_program.entry_points_by_type, sierra_version,
Done.
debug_utils/sierra-emu/src/vm/enum.rs line 27 at r1 (raw file):
EnumConcreteLibfunc::Match(info) => eval_match(registry, info, args), EnumConcreteLibfunc::SnapshotMatch(info) => eval_snapshot_match(registry, info, args), EnumConcreteLibfunc::BoxedMatch(_) => todo!(),
Done.
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 1 comment.
Reviewable status: 38 of 84 files reviewed, 2 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, orizi, and pefontana).
tests/common/mod.rs line 350 at r5 (raw file):
0.into(), 0.into(), ];
done
8e72a11 to
50085a2
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 8 comments.
Reviewable status: 38 of 84 files reviewed, 11 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, orizi, and pefontana).
50085a2 to
4e3680d
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 1 comment.
Reviewable status: 36 of 85 files reviewed, 11 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, orizi, and pefontana).
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 6 comments.
Reviewable status: 36 of 85 files reviewed, 12 unresolved discussions (waiting on azteca1998, edg-l, entropidelic, gabrielbosio, igaray, jrchatruc, Oppen, orizi, and pefontana).
4e3680d to
284ca64
Compare
Title
Closes #NA
Introduces Breaking Changes?
No.
starknet-blocks.ymlworkflow to use these PRs.These PRs should be merged after this one right away, in that order.
Checklist
This change is