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

improved validation for scarb version and scarb.toml config #2900

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
81 changes: 62 additions & 19 deletions crates/forge-runner/src/backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{Context, Result};
use anyhow::{ensure, Context, Result};
use cairo_annotations::annotations::coverage::{
CodeLocation, ColumnNumber, CoverageAnnotationsV1, LineNumber, VersionedCoverageAnnotations,
};
Expand All @@ -11,16 +11,25 @@ use cairo_lang_starknet_classes::casm_contract_class::CasmContractClass;
use cairo_lang_starknet_classes::contract_class::ContractClass;
use cheatnet::runtime_extensions::forge_runtime_extension::contracts_data::ContractsData;
use cheatnet::state::EncounteredError;
use indoc::indoc;
use indoc::formatdoc;
use itertools::Itertools;
use rayon::prelude::*;
use scarb_api::metadata::Metadata;
use semver::Version;
use starknet_api::core::ClassHash;
use std::collections::HashMap;
use std::fmt::Display;
use std::{env, fmt};
use std::{env, fmt, fs};
use toml_edit::{DocumentMut, Table};

const BACKTRACE_ENV: &str = "SNFORGE_BACKTRACE";
const MINIMAL_SCARB_VERSION: Version = Version::new(2, 8, 0);

const BACKTRACE_REQUIRED_ENTRIES: [(&str, bool); 2] = [
("unstable-add-statements-functions-debug-info", true ),
("unstable-add-statements-code-locations-debug-info", true),
];
#[must_use]
pub fn add_backtrace_footer(
message: String,
contracts_data: &ContractsData,
Expand All @@ -30,8 +39,7 @@ pub fn add_backtrace_footer(
return message;
}

let is_backtrace_enabled = env::var(BACKTRACE_ENV).is_ok_and(|value| value == "1");
if !is_backtrace_enabled {
if !is_backtrace_enabled() {
return format!(
"{message}\nnote: run with `{BACKTRACE_ENV}=1` environment variable to display a backtrace",
);
Expand All @@ -52,6 +60,53 @@ pub fn add_backtrace_footer(
)
}

pub fn can_backtrace_be_generated(scarb_metadata: &Metadata) -> Result<()> {
ensure!(
scarb_metadata.app_version_info.version >= MINIMAL_SCARB_VERSION,
"Backtrace generation requires scarb version >= {MINIMAL_SCARB_VERSION}",
);

let manifest: DocumentMut = fs::read_to_string(&scarb_metadata.runtime_manifest)?.parse::<DocumentMut>()?;

let has_needed_entries = manifest
.get("profile")
.and_then(|profile| profile.get(&scarb_metadata.current_profile))
.and_then(|profile| profile.get("cairo"))
.and_then(|cairo| cairo.as_table())
.is_some_and(|profile_cairo| {
BACKTRACE_REQUIRED_ENTRIES
.iter()
.all(|(key, value)| contains_entry_with_value(profile_cairo, key, *value))
});

ensure!(
has_needed_entries,
formatdoc! {
"Scarb.toml must have the following Cairo compiler configuration to run backtrace:

[profile.{profile}.cairo]
unstable-add-statements-functions-debug-info = true
unstable-add-statements-code-locations-debug-info = true
... other entries ...
",
profile = scarb_metadata.current_profile
},
);

Ok(())
}

#[must_use]
pub fn is_backtrace_enabled() -> bool {
env::var(BACKTRACE_ENV).is_ok_and(|value| value == "1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use this function also here:

let is_backtrace_enabled = env::var(BACKTRACE_ENV).is_ok_and(|value| value == "1");

}

fn contains_entry_with_value(table: &Table, key: &str, value: bool) -> bool {
table
.get(key)
.and_then(toml_edit::Item::as_bool)
.is_some_and(|entry| entry == value)
}
struct ContractBacktraceData {
contract_name: String,
casm_debug_info_start_offsets: Vec<usize>,
Expand Down Expand Up @@ -82,22 +137,10 @@ impl ContractBacktraceData {
.context("debug info not found")?;

let VersionedCoverageAnnotations::V1(coverage_annotations) =
VersionedCoverageAnnotations::try_from_debug_info(sierra_debug_info).context(indoc! {
"perhaps the contract was compiled without the following entry in Scarb.toml under [profile.dev.cairo]:
unstable-add-statements-code-locations-debug-info = true

or scarb version is less than 2.8.0
"
})?;
VersionedCoverageAnnotations::try_from_debug_info(sierra_debug_info)?;

let VersionedProfilerAnnotations::V1(profiler_annotations) =
VersionedProfilerAnnotations::try_from_debug_info(sierra_debug_info).context(indoc! {
"perhaps the contract was compiled without the following entry in Scarb.toml under [profile.dev.cairo]:
unstable-add-statements-functions-debug-info = true

or scarb version is less than 2.8.0
"
})?;
VersionedProfilerAnnotations::try_from_debug_info(sierra_debug_info)?;

// Not optimal, but USC doesn't produce debug info for the contract class
let (_, debug_info) = CasmContractClass::from_contract_class_with_debug_info(
Expand Down
2 changes: 1 addition & 1 deletion crates/forge-runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use tokio::sync::mpsc::{channel, Sender};
use tokio::task::JoinHandle;
use universal_sierra_compiler_api::AssembledProgramWithDebugInfo;

pub mod backtrace;
pub mod build_trace_data;
pub mod coverage_api;
pub mod expected_result;
Expand All @@ -31,7 +32,6 @@ pub mod profiler_api;
pub mod test_case_summary;
pub mod test_target_summary;

mod backtrace;
mod fuzzer;
mod gas;
pub mod printing;
Expand Down
5 changes: 5 additions & 0 deletions crates/forge/src/run_tests/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
warn::warn_if_snforge_std_not_compatible, ColorOption, ExitStatus, TestArgs,
};
use anyhow::{Context, Result};
use forge_runner::backtrace::{can_backtrace_be_generated, is_backtrace_enabled};
use forge_runner::{
coverage_api::can_coverage_be_generated,
test_case_summary::{AnyTestCaseSummary, TestCaseSummary},
Expand Down Expand Up @@ -32,6 +33,10 @@ pub async fn run_for_workspace(args: TestArgs) -> Result<ExitStatus> {
can_coverage_be_generated(&scarb_metadata)?;
}

if is_backtrace_enabled() {
can_backtrace_be_generated(&scarb_metadata)?;
}

warn_if_snforge_std_not_compatible(&scarb_metadata)?;

let artifacts_dir_path =
Expand Down
11 changes: 6 additions & 5 deletions crates/forge/tests/e2e/backtrace.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pleas add similar test:

fn test_fail_on_scarb_version_lt_2_8_0() {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PoulavBhowmick03
It seems that this comment is still unaddressed

Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ fn test_wrong_scarb_toml_configuration() {
assert_stdout_contains(
output,
indoc! {
"Failure data:
(0x454e545259504f494e545f4e4f545f464f554e44 ('ENTRYPOINT_NOT_FOUND'), 0x454e545259504f494e545f4641494c4544 ('ENTRYPOINT_FAILED'))
failed to create backtrace: perhaps the contract was compiled without the following entry in Scarb.toml under [profile.dev.cairo]:
unstable-add-statements-code-locations-debug-info = true
"[ERROR] Scarb.toml must have the following Cairo compiler configuration to run backtrace:

or scarb version is less than 2.8.0"
[profile.dev.cairo]
unstable-add-statements-functions-debug-info = true
unstable-add-statements-code-locations-debug-info = true
... other entries ...
"
},
);
}
Expand Down
Loading