Skip to content

Commit

Permalink
Gas limit to limit execution of unbounded methods (MystenLabs#18)
Browse files Browse the repository at this point in the history
After Jan's fix on adding terminators to basic blocks with
infinite loops, all the remaining tests can be fixed by having
a reasonable compute budget. This patch creates a new compute
budget for Solana tests.
    
A possible extension will be to allow each test to specify
a compute budget (added as a TODO in
test_runner.rs:exec_module_tests_solana)
  • Loading branch information
ksolana authored Apr 17, 2024
2 parents eb55588 + 6912324 commit 5d9eaf6
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::{env, path::Path};
use move_command_line_common::{env::read_bool_env_var, testing::EXP_EXT};
use move_prover_test_utils::baseline_test::verify_or_update_baseline;
use move_stdlib::move_stdlib_files;
use move_unit_test::UnitTestingConfig;
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND_SOLANA};

fn test_runner(path: &Path) -> datatest_stable::Result<()> {
env::set_var("NO_COLOR", "1");
Expand All @@ -29,7 +29,7 @@ fn test_runner(path: &Path) -> datatest_stable::Result<()> {

let test_plan = config.build_test_plan().unwrap();
let mut buffer = vec![];
config.run_and_report_unit_tests(test_plan, None, None, &mut buffer)?;
config.run_and_report_unit_tests(test_plan, None, None, &mut buffer, DEFAULT_EXECUTION_BOUND_SOLANA)?;
let output = String::from_utf8(buffer)?;

let baseline_path = path.with_extension(EXP_EXT);
Expand Down
4 changes: 2 additions & 2 deletions external-crates/move/crates/move-cli/src/base/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use move_compiler::{
};
use move_coverage::coverage_map::{output_map_to_file, CoverageMap};
use move_package::{compilation::build_plan::BuildPlan, BuildConfig};
use move_unit_test::UnitTestingConfig;
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND_SOLANA_STDLIB_TEST};
use move_vm_test_utils::gas_schedule::CostTable;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -241,7 +241,7 @@ pub fn run_move_unit_tests<W: Write + Send>(
// Run the tests. If any of the tests fail, then we don't produce a coverage report, so cleanup
// the trace files.
if !unit_test_config
.run_and_report_unit_tests(test_plan, Some(natives), cost_table, writer)
.run_and_report_unit_tests(test_plan, Some(natives), cost_table, writer, DEFAULT_EXECUTION_BOUND_SOLANA_STDLIB_TEST)
.unwrap()
.1
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use move_command_line_common::files::find_filenames;
use move_vm_runtime::native_functions::NativeFunctionTable;
use move_vm_test_utils::gas_schedule::CostTable;

use crate::UnitTestingConfig;
use crate::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND};

pub fn run_tests_with_config_and_filter(
mut config: UnitTestingConfig,
Expand Down Expand Up @@ -40,6 +40,7 @@ pub fn run_tests_with_config_and_filter(
native_function_table,
cost_table,
std::io::stdout(),
DEFAULT_EXECUTION_BOUND
)
.expect("Failed to execute tests");

Expand Down
15 changes: 10 additions & 5 deletions external-crates/move/crates/move-unit-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ use std::{
};

/// The default value bounding the amount of gas consumed in a test.
const DEFAULT_EXECUTION_BOUND: u64 = 1_000_000;
pub const DEFAULT_EXECUTION_BOUND: u64 = 1_000_000;
pub const DEFAULT_EXECUTION_BOUND_SOLANA: u64 = 10000;
// TODO: Remove this after tests have the ability to provide gas_limit. anza-xyz/sui/issues/20
pub const DEFAULT_EXECUTION_BOUND_SOLANA_STDLIB_TEST: u64 = u64::MAX;

#[derive(Debug, Parser, Clone)]
#[clap(author, version, about)]
Expand Down Expand Up @@ -195,6 +198,7 @@ impl UnitTestingConfig {
native_function_table: Option<NativeFunctionTable>,
cost_table: Option<CostTable>,
writer: W,
gas_limit: u64,
) -> Result<(W, bool)> {
let shared_writer = Mutex::new(writer);

Expand All @@ -213,13 +217,14 @@ impl UnitTestingConfig {
}

writeln!(shared_writer.lock().unwrap(), "Running Move unit tests")?;
let num_threads = if cfg!(feature = "solana-backend") {
1 // enforce single threaded execution for Solana, as llvm-sys is not re-entrant.
let (num_threads, cgas_limit) = if cfg!(feature = "solana-backend") {
(1, gas_limit) // enforce single threaded execution for Solana, as llvm-sys is not re-entrant.
} else {
self.num_threads
(self.num_threads, self.gas_limit.unwrap())
};

let mut test_runner = TestRunner::new(
self.gas_limit.unwrap_or(DEFAULT_EXECUTION_BOUND),
cgas_limit,
num_threads,
self.check_stackless_vm,
self.verbose,
Expand Down
4 changes: 2 additions & 2 deletions external-crates/move/crates/move-unit-test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
// SPDX-License-Identifier: Apache-2.0

use clap::*;
use move_unit_test::UnitTestingConfig;
use move_unit_test::{UnitTestingConfig, DEFAULT_EXECUTION_BOUND};

pub fn main() {
let args = UnitTestingConfig::parse();

let test_plan = args.build_test_plan();
if let Some(test_plan) = test_plan {
args.run_and_report_unit_tests(test_plan, None, None, std::io::stdout())
args.run_and_report_unit_tests(test_plan, None, None, std::io::stdout(), DEFAULT_EXECUTION_BOUND)
.unwrap();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,8 @@ impl SharedTestingConfig {
}

let gen_options = move_to_solana::options::Options::default();
// TODO: Get the compute budget from the test file. anza-xyz/sui/issues/20
let compute_budget = move_to_solana::runner::compute_budget(self.execution_bound);

for (function_name, test_info) in &test_plan.tests {
let shared_object = match move_to_solana::run_for_unit_test(
Expand All @@ -555,7 +557,7 @@ impl SharedTestingConfig {
}
};

let (result, duration) = move_to_solana::runner::run_solana_vm(shared_object);
let (result, duration) = move_to_solana::runner::run_solana_vm(shared_object, compute_budget);
let test_run_info = || -> TestRunInfo {
TestRunInfo::new(function_name.to_string(), duration, result.compute_units)
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use move_command_line_common::testing::{
add_update_baseline_fix, format_diff, read_env_update_baseline, EXP_EXT,
};
use move_unit_test::{self, UnitTestingConfig};
use move_unit_test::{self, UnitTestingConfig, DEFAULT_EXECUTION_BOUND, DEFAULT_EXECUTION_BOUND_SOLANA};
use regex::RegexBuilder;
use std::{
fs,
Expand Down Expand Up @@ -34,12 +34,12 @@ fn run_test_with_modifiers(

if !cfg!(feature = "solana-backend") {
results.push((
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer)?,
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer, DEFAULT_EXECUTION_BOUND)?,
path.with_extension(EXP_EXT),
));
} else {
results.push((
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer)?,
unit_test_config.run_and_report_unit_tests(test_plan.unwrap(), None, None, buffer, DEFAULT_EXECUTION_BOUND_SOLANA)?,
path.with_extension(format!("{}.{}", "solana", EXP_EXT)),
));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ module 0x42::m {
#[test]
#[expected_failure(out_of_gas, location=Self)]
fun t4() {
// fixme solana - bb without terminator
//loop {}
loop {}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ Failures in 0x42::m:


┌── t4 ──────
│ Test did not error as expected
│ Failed to run a program on Solana VM.
│ Err(ExceededMaxInstructions(3248))
└──────────────────

Test result: FAILED. Total tests: 5; passed: 0; failed: 5
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ fun t0() {}
#[test]
#[expected_failure(arithmetic_error, location=Self)]
fun t1() {
// fixme solana - bb without terminator
//loop {}
loop {}
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ Failures in 0x42::m:


┌── t1 ──────
│ Test did not error as expected
│ Failed to run a program on Solana VM.
│ Err(ExceededMaxInstructions(3182))
└──────────────────


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ address 0x1 {
module M {
#[test]
fun timeout_fail() {
// fixme solana - bb without terminator
//while (true) {}
while (true) {}
}

#[test]
#[expected_failure]
fun timeout_fail_with_expected_failure() {
// fixme solana - bb without terminator
//while (true) {}
while (true) {}
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Running Move unit tests
[ PASS ] 0x1::M::no_timeout
[ FAIL ] 0x1::M::no_timeout_fail
[ PASS ] 0x1::M::no_timeout_while_loop
[ PASS ] 0x1::M::timeout_fail
[ FAIL ] 0x1::M::timeout_fail
[ FAIL ] 0x1::M::timeout_fail_with_expected_failure

Test failures:
Expand All @@ -14,8 +14,19 @@ Failures in 0x1::M:
└──────────────────


┌── timeout_fail ──────
│ Failed to run a program on Solana VM.
│ Err(ExceededMaxInstructions(110))
└──────────────────


┌── timeout_fail_with_expected_failure ──────
│ Test did not error as expected
│ Failed to run a program on Solana VM.
│ Err(ExceededMaxInstructions(112))
└──────────────────

Test result: FAILED. Total tests: 5; passed: 3; failed: 2
Test result: FAILED. Total tests: 5; passed: 2; failed: 3
17 changes: 10 additions & 7 deletions external-crates/move/solana/move-to-solana/src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ pub struct ExecuteResult {
pub log: String,
}

pub fn compute_budget(execution_bound: u64) -> ComputeBudget {
ComputeBudget {
compute_unit_limit: execution_bound,
heap_size: Some(10000000),
max_call_depth: 8192,
..ComputeBudget::default()
}
}

fn load_input(path: PathBuf) -> serde_json::Result<Input> {
debug!("Reading input file {path:?}");
let file = fs::File::open(path).unwrap();
Expand Down Expand Up @@ -314,17 +323,11 @@ fn execution_result(
}
}

pub fn run_solana_vm(exe: String) -> (ExecuteResult, Duration) {
pub fn run_solana_vm(exe: String, compute_budget: ComputeBudget) -> (ExecuteResult, Duration) {
let exe = Path::new(&exe);
let (instruction_accounts, transaction_accounts, instruction_data, program_id) =
parse_input("input.json");

let compute_budget = ComputeBudget {
compute_unit_limit: i64::MAX as u64,
heap_size: Some(10000000),
max_call_depth: 8192,
..ComputeBudget::default()
};
let mut transaction_context = TransactionContext::new(
transaction_accounts,
Some(Rent::default()),
Expand Down

0 comments on commit 5d9eaf6

Please sign in to comment.