From eb2c808089c7610ec1b997f30ed41f135e22711c Mon Sep 17 00:00:00 2001 From: Harsh Bajpai <harshbajpai@Harshs-MacBook-Pro.local> Date: Thu, 28 Sep 2023 17:39:13 +0700 Subject: [PATCH 1/6] serialize program and shared data --- .../hint_processor_definition.rs | 3 +- vm/src/serde/deserialize_program.rs | 70 +++++++++++++------ vm/src/types/program.rs | 59 +++++++++++----- 3 files changed, 92 insertions(+), 40 deletions(-) diff --git a/vm/src/hint_processor/hint_processor_definition.rs b/vm/src/hint_processor/hint_processor_definition.rs index 6e2912af3d..7d7ce0abcf 100644 --- a/vm/src/hint_processor/hint_processor_definition.rs +++ b/vm/src/hint_processor/hint_processor_definition.rs @@ -13,6 +13,7 @@ use crate::vm::vm_core::VirtualMachine; use super::builtin_hint_processor::builtin_hint_processor_definition::HintProcessorData; use felt::Felt252; +use serde::{Deserialize, Serialize}; #[cfg(feature = "arbitrary")] use arbitrary::Arbitrary; @@ -79,7 +80,7 @@ fn get_ids_data( } #[cfg_attr(feature = "arbitrary", derive(Arbitrary))] -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct HintReference { pub offset1: OffsetValue, pub offset2: OffsetValue, diff --git a/vm/src/serde/deserialize_program.rs b/vm/src/serde/deserialize_program.rs index 52c5e70a42..9e8316794f 100644 --- a/vm/src/serde/deserialize_program.rs +++ b/vm/src/serde/deserialize_program.rs @@ -126,7 +126,7 @@ impl Default for ApTracking { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct Identifier { pub pc: Option<usize>, - #[serde(rename(deserialize = "type"))] + #[serde(rename = "type")] pub type_: Option<String>, #[serde(default)] #[serde(deserialize_with = "felt_from_number")] @@ -225,23 +225,49 @@ fn felt_from_number<'de, D>(deserializer: D) -> Result<Option<Felt252>, D::Error where D: Deserializer<'de>, { - let n = Number::deserialize(deserializer)?; - match Felt252::parse_bytes(n.to_string().as_bytes(), 10) { - Some(x) => Ok(Some(x)), - None => { - // Handle de Number with scientific notation cases - // e.g.: n = Number(1e27) - let felt = deserialize_scientific_notation(n); - if felt.is_some() { - return Ok(felt); - } - Err(de::Error::custom(String::from( - "felt_from_number parse error", - ))) + + // This value can be of 3 possible types + // Felt252, Number, None + #[derive(Serialize, Deserialize)] + #[serde(untagged)] + enum Tmp{ + Felt252(Option<Number>), + SerializedFelt252(Felt252) + } + + let n: Tmp = Tmp::deserialize(deserializer)?; + + match n { + Tmp::Felt252(n) => { + match n { + Some(n) => { + match Felt252::parse_bytes(n.to_string().as_bytes(), 10) { + Some(x) => Ok(Some(x)), + None => { + // Handle de Number with scientific notation cases + // e.g.: n = Number(1e27) + let felt = deserialize_scientific_notation(n); + if felt.is_some() { + return Ok(felt); + } + + Err(de::Error::custom(String::from( + "felt_from_number parse error", + ))) + } + } + }, + None => { + Ok(None) + } } + }, + Tmp::SerializedFelt252 (value ) => { + Ok(Some(value)) } } +} fn deserialize_scientific_notation(n: Number) -> Option<Felt252> { match n.as_f64() { @@ -580,7 +606,7 @@ mod tests { "attributes": [], "debug_info": { "instruction_locations": {} - }, + }, "builtins": [], "data": [ "0x480680017fff8000", @@ -1107,7 +1133,7 @@ mod tests { "attributes": [], "debug_info": { "instruction_locations": {} - }, + }, "builtins": [], "data": [ ], @@ -1188,10 +1214,10 @@ mod tests { "start_pc": 402, "value": "SafeUint256: subtraction overflow" } - ], + ], "debug_info": { "instruction_locations": {} - }, + }, "builtins": [], "data": [ ], @@ -1245,7 +1271,7 @@ mod tests { let valid_json = r#" { "prime": "0x800000000000011000000000000000000000000000000000000000000000001", - "attributes": [], + "attributes": [], "debug_info": { "file_contents": {}, "instruction_locations": { @@ -1296,7 +1322,7 @@ mod tests { } } } - }, + }, "builtins": [], "data": [ ], @@ -1354,7 +1380,7 @@ mod tests { let valid_json = r#" { "prime": "0x800000000000011000000000000000000000000000000000000000000000001", - "attributes": [], + "attributes": [], "debug_info": { "file_contents": {}, "instruction_locations": { @@ -1401,7 +1427,7 @@ mod tests { } } } - }, + }, "builtins": [], "data": [ ], diff --git a/vm/src/types/program.rs b/vm/src/types/program.rs index 2ba347e6a2..c6251c1eb8 100644 --- a/vm/src/types/program.rs +++ b/vm/src/types/program.rs @@ -27,6 +27,7 @@ use crate::{ use cairo_lang_starknet::casm_contract_class::CasmContractClass; use core::num::NonZeroUsize; use felt::{Felt252, PRIME_STR}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; #[cfg(feature = "std")] use std::path::Path; @@ -55,18 +56,18 @@ use arbitrary::{Arbitrary, Unstructured}; // exceptional circumstances, such as when reconstructing a backtrace on execution // failures. // Fields in `Program` (other than `SharedProgramData` itself) are used by the main logic. -#[derive(Clone, Default, Debug, PartialEq, Eq)] -pub(crate) struct SharedProgramData { - pub(crate) data: Vec<MaybeRelocatable>, - pub(crate) hints_collection: HintsCollection, - pub(crate) main: Option<usize>, +#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct SharedProgramData { + pub data: Vec<MaybeRelocatable>, + pub hints_collection: HintsCollection, + pub main: Option<usize>, //start and end labels will only be used in proof-mode - pub(crate) start: Option<usize>, - pub(crate) end: Option<usize>, - pub(crate) error_message_attributes: Vec<Attribute>, - pub(crate) instruction_locations: Option<HashMap<usize, InstructionLocation>>, - pub(crate) identifiers: HashMap<String, Identifier>, - pub(crate) reference_manager: Vec<HintReference>, + pub start: Option<usize>, + pub end: Option<usize>, + pub error_message_attributes: Vec<Attribute>, + pub instruction_locations: Option<HashMap<usize, InstructionLocation>>, + pub identifiers: HashMap<String, Identifier>, + pub reference_manager: Vec<HintReference>, } #[cfg(all(feature = "arbitrary", feature = "std"))] @@ -102,8 +103,8 @@ impl<'a> Arbitrary<'a> for SharedProgramData { } } -#[derive(Clone, Default, Debug, PartialEq, Eq)] -pub(crate) struct HintsCollection { +#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct HintsCollection { hints: Vec<HintParams>, /// This maps a PC to the range of hints in `hints` that correspond to it. hints_ranges: Vec<HintRange>, @@ -176,11 +177,35 @@ impl From<&HintsCollection> for BTreeMap<usize, Vec<HintParams>> { type HintRange = Option<(usize, NonZeroUsize)>; #[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))] -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Program { - pub(crate) shared_program_data: Arc<SharedProgramData>, - pub(crate) constants: HashMap<String, Felt252>, - pub(crate) builtins: Vec<BuiltinName>, + #[serde( + serialize_with = "serialize_shared_program_data", + deserialize_with = "deserialize_shared_program_data" + )] + pub shared_program_data: Arc<SharedProgramData>, + pub constants: HashMap<String, Felt252>, + pub builtins: Vec<BuiltinName>, +} + +pub fn serialize_shared_program_data<S>( + shared_program_data: &Arc<SharedProgramData>, + serializer: S, +) -> Result<S::Ok, S::Error> +where + S: Serializer, +{ + shared_program_data.serialize(serializer) +} + +pub fn deserialize_shared_program_data<'de, D>( + deserializer: D, +) -> Result<Arc<SharedProgramData>, D::Error> +where + D: Deserializer<'de>, +{ + let shared_program_data = SharedProgramData::deserialize(deserializer)?; + Ok(Arc::new(shared_program_data)) } impl Program { From 9550841970a8b602673b4842d2aa8d2017859e14 Mon Sep 17 00:00:00 2001 From: Gregory Edison <gregory.edison1993@gmail.com> Date: Mon, 27 Nov 2023 11:43:36 +0100 Subject: [PATCH 2/6] fix lint and change log --- CHANGELOG.md | 2 ++ vm/src/serde/deserialize_program.rs | 18 ++++++------------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2a21be759..e5f28ff69e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## Cairo-VM Changelog +* feat: add Serialize and Deserialize derivation for Program. + #### Upcoming Changes * BREAKING: Partially Revert `Optimize trace relocation #906` [#1492](https://github.com/lambdaclass/cairo-vm/pull/1492) diff --git a/vm/src/serde/deserialize_program.rs b/vm/src/serde/deserialize_program.rs index 9e8316794f..537d9ed8dc 100644 --- a/vm/src/serde/deserialize_program.rs +++ b/vm/src/serde/deserialize_program.rs @@ -225,15 +225,13 @@ fn felt_from_number<'de, D>(deserializer: D) -> Result<Option<Felt252>, D::Error where D: Deserializer<'de>, { - - // This value can be of 3 possible types // Felt252, Number, None #[derive(Serialize, Deserialize)] #[serde(untagged)] - enum Tmp{ + enum Tmp { Felt252(Option<Number>), - SerializedFelt252(Felt252) + SerializedFelt252(Felt252), } let n: Tmp = Tmp::deserialize(deserializer)?; @@ -256,18 +254,14 @@ where "felt_from_number parse error", ))) } + } } - }, - None => { - Ok(None) - } + None => Ok(None), + } } - }, - Tmp::SerializedFelt252 (value ) => { - Ok(Some(value)) + Tmp::SerializedFelt252(value) => Ok(Some(value)), } } -} fn deserialize_scientific_notation(n: Number) -> Option<Felt252> { match n.as_f64() { From 34f97ce83b5d1f84231dee30e29f358142e8308f Mon Sep 17 00:00:00 2001 From: Gregory Edison <gregory.edison1993@gmail.com> Date: Mon, 27 Nov 2023 14:53:30 +0100 Subject: [PATCH 3/6] fix cancel duplicate --- .github/workflows/cancel_duplicate.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cancel_duplicate.yml b/.github/workflows/cancel_duplicate.yml index 5b05d23f0a..0a8f1fb92f 100644 --- a/.github/workflows/cancel_duplicate.yml +++ b/.github/workflows/cancel_duplicate.yml @@ -22,4 +22,4 @@ jobs: cancelMode: allDuplicates cancelFutureDuplicates: true token: ${{ secrets.GITHUB_TOKEN }} - workflowFileName: bench_pull_request.yml + workflowFileName: bench.yml From 4485b415cb62d13b304c32efc83e32a53fb2d3a9 Mon Sep 17 00:00:00 2001 From: Gregory Edison <gregory.edison1993@gmail.com> Date: Mon, 27 Nov 2023 15:45:25 +0100 Subject: [PATCH 4/6] add issues permission --- .github/workflows/hyperfine.yml | 210 ++++++++++++++++---------------- 1 file changed, 105 insertions(+), 105 deletions(-) diff --git a/.github/workflows/hyperfine.yml b/.github/workflows/hyperfine.yml index 87bf26f3e3..61a4435974 100644 --- a/.github/workflows/hyperfine.yml +++ b/.github/workflows/hyperfine.yml @@ -2,7 +2,7 @@ name: Hyperfine Benchmark on: pull_request: - branches: [ '**' ] + branches: ["**"] concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -11,11 +11,14 @@ concurrency: env: CARGO_TERM_COLOR: always +permissions: + issues: write + jobs: build-programs: strategy: matrix: - branch: [ base, head ] + branch: [base, head] name: Build Cairo programs for ${{ matrix.branch }} runs-on: ubuntu-22.04 outputs: @@ -39,7 +42,7 @@ jobs: if: ${{ steps.cache.outputs.cache-hit != 'true' }} uses: actions/setup-python@v4 with: - python-version: '3.9' + python-version: "3.9" - name: Install Cairo compiler if: ${{ steps.cache.outputs.cache-hit != 'true' }} @@ -54,20 +57,18 @@ jobs: - name: Export benchmark hashes id: export-hashes - run: - echo "benchmark-hashes-${{ matrix.branch }}=${{ hashFiles( 'cairo_programs/benchmarks/*.cairo' ) }}" >> "$GITHUB_OUTPUT" - + run: echo "benchmark-hashes-${{ matrix.branch }}=${{ hashFiles( 'cairo_programs/benchmarks/*.cairo' ) }}" >> "$GITHUB_OUTPUT" build-binaries: strategy: matrix: - branch: [ base, head ] + branch: [base, head] name: Build cairo-vm-cli for ${{ matrix.branch }} runs-on: ubuntu-22.04 steps: - name: Populate cache uses: actions/cache@v3 - id: cache + id: cache with: path: bin/cairo-vm-cli-${{ matrix.branch }} key: binary-${{ github.event.pull_request[matrix.branch].sha }} @@ -93,106 +94,105 @@ jobs: mkdir bin cp target/release/cairo-vm-cli bin/cairo-vm-cli-${{ matrix.branch }} - run-hyperfine: strategy: matrix: - program_state: [ modified, unmodified ] + program_state: [modified, unmodified] name: Run benchmarks for ${{ matrix.program_state }} programs - needs: [ build-programs, build-binaries ] + needs: [build-programs, build-binaries] runs-on: ubuntu-22.04 steps: - - name: Install Hyperfine - uses: taiki-e/install-action@v2 - with: - tool: hyperfine@1.16 - - - name: Fetch base binary - uses: actions/cache/restore@v3 - with: - path: bin/cairo-vm-cli-base - key: binary-${{ github.event.pull_request.base.sha }} - - - name: Fetch HEAD binary - uses: actions/cache/restore@v3 - with: - path: bin/cairo-vm-cli-head - key: binary-${{ github.event.pull_request.head.sha }} - - - name: Fetch base programs - uses: actions/cache/restore@v3 - with: - path: base_programs/*.json - key: benchmarks-base-${{ needs.build-programs.outputs.benchmark-hashes-base }} - - - name: Fetch head programs - uses: actions/cache/restore@v3 - with: - path: head_programs/*.json - key: benchmarks-head-${{ needs.build-programs.outputs.benchmark-hashes-head }} - - - name: Benchmark ${{ matrix.program_state }} programs - id: run-benchmarks - run: | - sudo swapoff -a - mkdir target_programs - if [ 'modified' = ${{ matrix.program_state }} ]; then - BINS=head - for f in head_programs/*.json; do - # Only run new or modified benchmarks - if ! cmp -s ${f/head/base} $f; then - cp $f target_programs/ - fi - done - else - BINS="base,head" - for f in base_programs/*.json; do - # Only run unmodified benchmarks - if cmp -s ${f/base/head} $f; then - cp $f target_programs/ - fi - done - fi - find target_programs -name '*.json' -exec basename -s .json '{}' '+' | \ - sort | xargs -I '{program}' \ - hyperfine -N -r 10 --export-markdown "target_programs/{program}.md" \ - -L bin "$BINS" -n "{bin} {program}" \ - -s "cat ./bin/cairo-vm-cli-{bin} target_programs/{program}.json" \ - "./bin/cairo-vm-cli-{bin} --proof_mode --layout starknet_with_keccak \ - --memory_file /dev/null --trace_file /dev/null target_programs/{program}.json" - echo "benchmark_count=$(ls target_programs/*.md | wc -l)" >> $GITHUB_OUTPUT - - - name: Print tables - if: steps.run-benchmarks.outputs.benchmark_count != 0 - run: | - { - echo "Benchmark Results for ${{ matrix.program_state }} programs :rocket:" - for f in target_programs/*.md; do - echo - cat $f - done - } | tee -a comment_body.md - - - name: Find comment - if: ${{ steps.run-benchmarks.outputs.benchmark_count != 0 }} - uses: peter-evans/find-comment@v2 - id: fc - with: - issue-number: ${{ github.event.pull_request.number }} - comment-author: 'github-actions[bot]' - body-includes: Benchmark Results for ${{ matrix.program_state }} programs - - - name: Create comment - if: steps.fc.outputs.comment-id == '' && steps.run-benchmarks.outputs.benchmark_count != 0 - uses: peter-evans/create-or-update-comment@v3 - with: - issue-number: ${{ github.event.pull_request.number }} - body-path: comment_body.md - - - name: Update comment - if: steps.fc.outputs.comment-id != '' && steps.run-benchmarks.outputs.benchmark_count != 0 - uses: peter-evans/create-or-update-comment@v3 - with: - comment-id: ${{ steps.fc.outputs.comment-id }} - body-path: comment_body.md - edit-mode: replace + - name: Install Hyperfine + uses: taiki-e/install-action@v2 + with: + tool: hyperfine@1.16 + + - name: Fetch base binary + uses: actions/cache/restore@v3 + with: + path: bin/cairo-vm-cli-base + key: binary-${{ github.event.pull_request.base.sha }} + + - name: Fetch HEAD binary + uses: actions/cache/restore@v3 + with: + path: bin/cairo-vm-cli-head + key: binary-${{ github.event.pull_request.head.sha }} + + - name: Fetch base programs + uses: actions/cache/restore@v3 + with: + path: base_programs/*.json + key: benchmarks-base-${{ needs.build-programs.outputs.benchmark-hashes-base }} + + - name: Fetch head programs + uses: actions/cache/restore@v3 + with: + path: head_programs/*.json + key: benchmarks-head-${{ needs.build-programs.outputs.benchmark-hashes-head }} + + - name: Benchmark ${{ matrix.program_state }} programs + id: run-benchmarks + run: | + sudo swapoff -a + mkdir target_programs + if [ 'modified' = ${{ matrix.program_state }} ]; then + BINS=head + for f in head_programs/*.json; do + # Only run new or modified benchmarks + if ! cmp -s ${f/head/base} $f; then + cp $f target_programs/ + fi + done + else + BINS="base,head" + for f in base_programs/*.json; do + # Only run unmodified benchmarks + if cmp -s ${f/base/head} $f; then + cp $f target_programs/ + fi + done + fi + find target_programs -name '*.json' -exec basename -s .json '{}' '+' | \ + sort | xargs -I '{program}' \ + hyperfine -N -r 10 --export-markdown "target_programs/{program}.md" \ + -L bin "$BINS" -n "{bin} {program}" \ + -s "cat ./bin/cairo-vm-cli-{bin} target_programs/{program}.json" \ + "./bin/cairo-vm-cli-{bin} --proof_mode --layout starknet_with_keccak \ + --memory_file /dev/null --trace_file /dev/null target_programs/{program}.json" + echo "benchmark_count=$(ls target_programs/*.md | wc -l)" >> $GITHUB_OUTPUT + + - name: Print tables + if: steps.run-benchmarks.outputs.benchmark_count != 0 + run: | + { + echo "Benchmark Results for ${{ matrix.program_state }} programs :rocket:" + for f in target_programs/*.md; do + echo + cat $f + done + } | tee -a comment_body.md + + - name: Find comment + if: ${{ steps.run-benchmarks.outputs.benchmark_count != 0 }} + uses: peter-evans/find-comment@v2 + id: fc + with: + issue-number: ${{ github.event.pull_request.number }} + comment-author: "github-actions[bot]" + body-includes: Benchmark Results for ${{ matrix.program_state }} programs + + - name: Create comment + if: steps.fc.outputs.comment-id == '' && steps.run-benchmarks.outputs.benchmark_count != 0 + uses: peter-evans/create-or-update-comment@v3 + with: + issue-number: ${{ github.event.pull_request.number }} + body-path: comment_body.md + + - name: Update comment + if: steps.fc.outputs.comment-id != '' && steps.run-benchmarks.outputs.benchmark_count != 0 + uses: peter-evans/create-or-update-comment@v3 + with: + comment-id: ${{ steps.fc.outputs.comment-id }} + body-path: comment_body.md + edit-mode: replace From f6e02dd71dfa5951b5d9084422c4e724ec7666f3 Mon Sep 17 00:00:00 2001 From: Gregory Edison <gregory.edison1993@gmail.com> Date: Mon, 27 Nov 2023 16:06:50 +0100 Subject: [PATCH 5/6] all write --- .github/workflows/hyperfine.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/hyperfine.yml b/.github/workflows/hyperfine.yml index 61a4435974..a5110a9496 100644 --- a/.github/workflows/hyperfine.yml +++ b/.github/workflows/hyperfine.yml @@ -12,7 +12,19 @@ env: CARGO_TERM_COLOR: always permissions: + actions: write + checks: write + contents: write + deployments: write + id-token: write issues: write + discussions: write + packages: write + pages: write + pull-requests: write + repository-projects: write + security-events: write + statuses: write jobs: build-programs: From 16777fedaf5f677d04d7cad7a3e401f7be15a06e Mon Sep 17 00:00:00 2001 From: Gregory Edison <gregory.edison1993@gmail.com> Date: Mon, 27 Nov 2023 16:22:14 +0100 Subject: [PATCH 6/6] clean permissions --- .github/workflows/hyperfine.yml | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/.github/workflows/hyperfine.yml b/.github/workflows/hyperfine.yml index a5110a9496..bd2cd3a9bc 100644 --- a/.github/workflows/hyperfine.yml +++ b/.github/workflows/hyperfine.yml @@ -11,21 +11,6 @@ concurrency: env: CARGO_TERM_COLOR: always -permissions: - actions: write - checks: write - contents: write - deployments: write - id-token: write - issues: write - discussions: write - packages: write - pages: write - pull-requests: write - repository-projects: write - security-events: write - statuses: write - jobs: build-programs: strategy: @@ -200,6 +185,7 @@ jobs: with: issue-number: ${{ github.event.pull_request.number }} body-path: comment_body.md + permissions: write - name: Update comment if: steps.fc.outputs.comment-id != '' && steps.run-benchmarks.outputs.benchmark_count != 0