Added a test of formatting an error of entry point execution failure.

[yuvalsw]

This change is 

[dorimedini-starkware]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @orizi and @yuvalsw)

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 100 at r1 (raw file):

pub const BYTE_ARRAY_MAGIC: &str =
    "0x46a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3";

This constant should be defined in the compiler repo, correct?
And imported into the blockifier?

Code quote:

pub const BYTE_ARRAY_MAGIC: &str =
    "0x46a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3";

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 629 at r1 (raw file):

        "Execution failed. Failure reason: \
         \"0x046a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3, , Execution \
         failure, \\u{11}\"."

use format! and the BYTE_ARRAY_MAGIC const, don't hard code the string twice

Code quote:

        "Execution failed. Failure reason: \
         \"0x046a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3, , Execution \
         failure, \\u{11}\"."

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 629 at r1 (raw file):

        "Execution failed. Failure reason: \
         \"0x046a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3, , Execution \
         failure, \\u{11}\"."

I don't understand what would fail if we upgrade to 2.4.0?
the to_string of the error variant is defined in the blockifier, right? so even after upgrading the compiler version this test will still pass...?

Code quote:

        get_dummy_string_execution_error().to_string(),
        "Execution failed. Failure reason: \
         \"0x046a6158a16a947e5916b2a2ca68501a45e93d7110e81aa2d6438b1c57c879a3, , Execution \
         failure, \\u{11}\"."

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 633 at r1 (raw file):

}

// "Execution failure"

I don't understand from this docstring what the function does

Code quote:

// "Execution failure"

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 634 at r1 (raw file):

// "Execution failure"
const DUMMY_EXECUTION_FAILURE_ERROR: &str = "0x457865637574696f6e206661696c757265";

1. this const isn't needed outside of get_dummy_string_execution_error, right?
   move the definition inside the function; or just hard-code the string in the function.
2. Please document what this string represents; looks like a highly specific value

Code quote:

const DUMMY_EXECUTION_FAILURE_ERROR: &str = "0x457865637574696f6e206661696c757265";

[codecov-commenter]

Codecov Report

Merging #1071 (bd28894) into main (434bc5e) will increase coverage by 0.95%.
Report is 10 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1071      +/-   ##
==========================================
+ Coverage   67.76%   68.72%   +0.95%     
==========================================
  Files          51       51              
  Lines        6382     6672     +290     
  Branches     6382     6672     +290     
==========================================
+ Hits         4325     4585     +260     
- Misses       1674     1707      +33     
+ Partials      383      380       -3     

see 5 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dorimedini-starkware]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @orizi and @yuvalsw)

[yuvalsw]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware and @orizi)

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 629 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I don't understand what would fail if we upgrade to 2.4.0?
the to_string of the error variant is defined in the blockifier, right? so even after upgrading the compiler version this test will still pass...?

The to_string uses as_cairo_short_string from cairo-lang-runner under the hood.
It changed since 2.3.0-rc0 to 2.4.0 which makes non-printable characters be formatted differently. Here, the 0x11 is non printable and thus is printed differently. I tested it by copying the current as_cairo_short_string and using it instead (in felts_as_str), and the test indeed failed.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 633 at r1 (raw file):

Previously, dorimedini-starkware wrote…

I don't understand from this docstring what the function does

This was the doc for the constant. Anyway, inlined the function.

---

crates/blockifier/src/execution/syscalls/syscalls_test.rs line 634 at r1 (raw file):

Previously, dorimedini-starkware wrote…

1. this const isn't needed outside of get_dummy_string_execution_error, right?
   move the definition inside the function; or just hard-code the string in the function.
2. Please document what this string represents; looks like a highly specific value

Yes, it's "Execution failure", as documented, the same way as constants are defined in hint_processor.rs.
I hard coded it with a comment trying to make it clearer.

---

crates/blockifier/src/execution/syscalls/hint_processor.rs line 100 at r1 (raw file):

Previously, dorimedini-starkware wrote…

This constant should be defined in the compiler repo, correct?
And imported into the blockifier?

Done.

[dorimedini-starkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @orizi)