Skip to content

Commit

Permalink
[Verifier] verify-bytecode-meter prints module/function breakdown (#1…
Browse files Browse the repository at this point in the history
…6963)

## Description

Use a custom meter to track every function and module that is verified,
and display all of them.

## Test Plan

```
deepbook$ cargo run --bin sui -p sui \
  -- client verify-bytecode-meter    \
  --module build/DeepBook/bytecode_modules/math.mv

╭──────────────────────────────────────╮
│ Package will pass metering check!    │
├──────────────────────────────────────┤
│ Limits                               │
├─────────────────────────┬────────────┤
│ packages                │ 16,000,000 │
│   modules               │ 16,000,000 │
│     functions           │ 16,000,000 │
├─────────────────────────┴────────────┤
│ Ticks Used                           │
├─────────────────────────┬────────────┤
│ <unknown>               │     41,915 │
│   math                  │     41,915 │
│     unsafe_mul          │      1,225 │
│     unsafe_mul_round    │      5,675 │
│     mul                 │      2,410 │
│     mul_round           │      2,905 │
│     unsafe_div          │      1,225 │
│     unsafe_div_round    │      6,085 │
│     div_round           │      2,905 │
│     count_leading_zeros │     19,485 │
├─────────────────────────┴────────────┤
│ Package will pass metering check!    │
╰──────────────────────────────────────╯
```

## Stack

- #16903
- #16941 
- #16945 

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

`sui client verify-bytecode-meter` prints a breakdown of verifier cost
by module and function in a package.
  • Loading branch information
amnn authored Apr 8, 2024
1 parent ef431c7 commit d296555
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 43 deletions.
155 changes: 112 additions & 43 deletions crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use crate::client_ptb::ptb::PTB;
use crate::{
client_ptb::ptb::PTB,
verifier_meter::{AccumulatingMeter, Accumulator},
};
use std::{
collections::{btree_map::Entry, BTreeMap},
fmt::{Debug, Display, Formatter, Write},
Expand All @@ -21,10 +24,9 @@ use fastcrypto::{
};

use move_binary_format::CompiledModule;
use move_bytecode_verifier_meter::{bound::BoundMeter, Scope};
use move_bytecode_verifier_meter::Scope;
use move_core_types::language_storage::TypeTag;
use move_package::BuildConfig as MoveBuildConfig;
use move_vm_config::verifier::MeterConfig;
use prometheus::Registry;
use serde::Serialize;
use serde_json::{json, Value};
Expand Down Expand Up @@ -72,8 +74,11 @@ use json_to_table::json_to_table;
use tabled::{
builder::Builder as TableBuilder,
settings::{
object::Cell as TableCell, style::HorizontalLine, Border as TableBorder,
Modify as TableModify, Panel as TablePanel, Style as TableStyle,
object::{Cell as TableCell, Columns as TableCols, Rows as TableRows},
span::Span as TableSpan,
style::HorizontalLine,
Alignment as TableAlignment, Border as TableBorder, Modify as TableModify,
Panel as TablePanel, Style as TableStyle,
},
};

Expand Down Expand Up @@ -1123,7 +1128,7 @@ impl SuiClientCommands {
let registry = &Registry::new();
let bytecode_verifier_metrics = Arc::new(BytecodeVerifierMetrics::new(registry));

let modules = match (module_path, package_path) {
let (pkg_name, modules) = match (module_path, package_path) {
(Some(_), Some(_)) => {
bail!("Cannot specify both a module path and a package path")
}
Expand All @@ -1133,13 +1138,18 @@ impl SuiClientCommands {
fs::read(module_path).context("Failed to read module file")?;
let module = CompiledModule::deserialize_with_defaults(&module_bytes)
.context("Failed to deserialize module")?;
vec![module]
("<unknown>".to_string(), vec![module])
}

(None, package_path) => {
let package_path = package_path.unwrap_or_else(|| PathBuf::from("."));
let package = compile_package_simple(build_config, package_path)?;
package.get_modules().cloned().collect()
let name = package
.package
.compiled_package_info
.package_name
.to_string();
(name, package.get_modules().cloned().collect())
}
};

Expand All @@ -1156,22 +1166,31 @@ impl SuiClientCommands {
if modules.len() != 1 { "s" } else { "" },
);

// Need to pass some bounds because otherwise this meter doesn't accumulate
// anything.
let mut meter = BoundMeter::new(MeterConfig {
max_per_fun_meter_units: Some(u128::MAX),
max_per_mod_meter_units: Some(u128::MAX),
max_per_pkg_meter_units: Some(u128::MAX),
});

let mut meter = AccumulatingMeter::new();
verifier.meter_compiled_modules(&protocol_config, &modules, &mut meter)?;

let mut used_ticks = meter.accumulator(Scope::Package).clone();
used_ticks.name = pkg_name;

let meter_config = protocol_config.meter_config();

let exceeded = matches!(
meter_config.max_per_pkg_meter_units,
Some(allowed_ticks) if allowed_ticks < used_ticks.max_ticks(Scope::Package)
) || matches!(
meter_config.max_per_mod_meter_units,
Some(allowed_ticks) if allowed_ticks < used_ticks.max_ticks(Scope::Module)
) || matches!(
meter_config.max_per_fun_meter_units,
Some(allowed_ticks) if allowed_ticks < used_ticks.max_ticks(Scope::Function)
);

SuiClientCommandResult::VerifyBytecodeMeter {
success: !exceeded,
max_package_ticks: meter_config.max_per_pkg_meter_units,
max_module_ticks: meter_config.max_per_mod_meter_units,
max_function_ticks: meter_config.max_per_fun_meter_units,
used_function_ticks: meter.get_usage(Scope::Function),
used_module_ticks: meter.get_usage(Scope::Module),
used_ticks,
}
}

Expand Down Expand Up @@ -2131,42 +2150,91 @@ impl Display for SuiClientCommandResult {
writeln!(writer, "Source verification succeeded!")?;
}
SuiClientCommandResult::VerifyBytecodeMeter {
success,
max_package_ticks,
max_module_ticks,
max_function_ticks,
used_function_ticks,
used_module_ticks,
used_ticks,
} => {
let mut builder = TableBuilder::default();
builder.set_header(vec!["", "Module", "Function"]);

/// Convert ticks to string, using commas as thousands separators
fn format_ticks(ticks: u128) -> String {
let ticks = ticks.to_string();
let mut formatted = String::with_capacity(ticks.len() + ticks.len() / 3);
for (i, c) in ticks.chars().rev().enumerate() {
if i != 0 && (i % 3 == 0) {
formatted.push(',');
}
formatted.push(c);
}
formatted.chars().rev().collect()
}

// Build up the limits table
builder.push_record(vec!["Limits"]);
builder.push_record(vec![
"Max".to_string(),
max_module_ticks.map_or_else(|| "None".to_string(), |x| x.to_string()),
max_function_ticks.map_or_else(|| "None".to_string(), |x| x.to_string()),
"packages".to_string(),
max_package_ticks.map_or_else(|| "None".to_string(), format_ticks),
]);
builder.push_record(vec![
"Used".to_string(),
used_module_ticks.to_string(),
used_function_ticks.to_string(),
" modules".to_string(),
max_module_ticks.map_or_else(|| "None".to_string(), format_ticks),
]);
builder.push_record(vec![
" functions".to_string(),
max_function_ticks.map_or_else(|| "None".to_string(), format_ticks),
]);
let mut table = builder.build();
table.with(TableStyle::rounded());

let module_exceeded_ticks = matches!(
max_module_ticks,
Some(ticks) if ticks < used_module_ticks,
);
// Build up usage table
builder.push_record(vec!["Ticks Used"]);
let mut stack = vec![used_ticks];
while let Some(usage) = stack.pop() {
let indent = match usage.scope {
Scope::Transaction => 0,
Scope::Package => 0,
Scope::Module => 2,
Scope::Function => 4,
};

let function_exceeded_ticks = matches!(
max_function_ticks,
Some(ticks) if ticks < used_function_ticks,
);
builder.push_record(vec![
format!("{:indent$}{}", "", usage.name),
format_ticks(usage.ticks),
]);

if module_exceeded_ticks || function_exceeded_ticks {
table.with(TablePanel::header("Module will NOT pass metering check!"));
} else {
table.with(TablePanel::header("Module will pass metering check!"));
stack.extend(usage.children.iter().rev())
}

let mut table = builder.build();

let message = if *success {
"Package will pass metering check!"
} else {
"Package will NOT pass metering check!"
};

// Add overall header and footer message;
table.with(TablePanel::header(message));
table.with(TablePanel::footer(message));

// Set-up spans for headers
table.with(TableModify::new(TableRows::new(0..2)).with(TableSpan::column(2)));
table.with(TableModify::new(TableRows::single(5)).with(TableSpan::column(2)));

// Styling
table.with(TableStyle::rounded());
table.with(TableModify::new(TableCols::new(1..)).with(TableAlignment::right()));

// Separators before and after headers/footers
let hl = TableStyle::modern().get_horizontal();
let last = table.count_rows() - 1;
table.with(HorizontalLine::new(2, hl));
table.with(HorizontalLine::new(5, hl));
table.with(HorizontalLine::new(6, hl));
table.with(HorizontalLine::new(last, hl));

table.with(tabled::settings::style::BorderSpanCorrection);

writeln!(f, "{}", table)?;
}
SuiClientCommandResult::NoOutput => {}
Expand Down Expand Up @@ -2443,10 +2511,11 @@ pub enum SuiClientCommandResult {
TransferSui(SuiTransactionBlockResponse),
Upgrade(SuiTransactionBlockResponse),
VerifyBytecodeMeter {
success: bool,
max_package_ticks: Option<u128>,
max_module_ticks: Option<u128>,
max_function_ticks: Option<u128>,
used_function_ticks: u128,
used_module_ticks: u128,
used_ticks: Accumulator,
},
VerifySource,
}
Expand Down
1 change: 1 addition & 0 deletions crates/sui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ pub mod keytool;
pub mod shell;
pub mod sui_commands;
pub mod validator_commands;
mod verifier_meter;
pub mod zklogin_commands_util;
99 changes: 99 additions & 0 deletions crates/sui/src/verifier_meter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use move_binary_format::errors::PartialVMResult;
use move_bytecode_verifier_meter::{Meter, Scope};
use serde::Serialize;

/// A meter that accumulates all the scopes that it sees, without enforcing a limit.
#[derive(Debug)]
pub(crate) struct AccumulatingMeter {
pkg_acc: Accumulator,
mod_acc: Accumulator,
fun_acc: Accumulator,
}

/// Ticks and child scopes recorded for an individual scope.
#[derive(Clone, Debug, Serialize)]
pub struct Accumulator {
pub name: String,
#[serde(skip)]
pub scope: Scope,
pub ticks: u128,
pub children: Vec<Accumulator>,
}

impl AccumulatingMeter {
pub fn new() -> Self {
Self {
pkg_acc: Accumulator::new("<unknown>", Scope::Package),
mod_acc: Accumulator::new("<unknown>", Scope::Module),
fun_acc: Accumulator::new("<unknown>", Scope::Function),
}
}

pub fn accumulator(&self, scope: Scope) -> &Accumulator {
match scope {
Scope::Transaction => unreachable!("transaction scope is not supported"),
Scope::Package => &self.pkg_acc,
Scope::Module => &self.mod_acc,
Scope::Function => &self.fun_acc,
}
}

pub fn accumulator_mut(&mut self, scope: Scope) -> &mut Accumulator {
match scope {
Scope::Transaction => unreachable!("transaction scope is not supported"),
Scope::Package => &mut self.pkg_acc,
Scope::Module => &mut self.mod_acc,
Scope::Function => &mut self.fun_acc,
}
}
}

impl Accumulator {
fn new(name: &str, scope: Scope) -> Self {
Self {
name: name.to_string(),
scope,
ticks: 0,
children: vec![],
}
}

/// Find the max ticks spent verifying `scope`s within this scope (including itself).
pub fn max_ticks(&self, scope: Scope) -> u128 {
let mut accs = vec![self];

let mut curr = 0u128;
while let Some(acc) = accs.pop() {
if acc.scope == scope {
curr = curr.max(acc.ticks);
}

accs.extend(acc.children.iter());
}

curr
}
}

impl Meter for AccumulatingMeter {
fn enter_scope(&mut self, name: &str, scope: Scope) {
*self.accumulator_mut(scope) = Accumulator::new(name, scope);
}

fn transfer(&mut self, from: Scope, to: Scope, factor: f32) -> PartialVMResult<()> {
let from_acc = self.accumulator(from).clone();
let to_acc = self.accumulator_mut(to);

to_acc.ticks += (from_acc.ticks as f32 * factor) as u128;
to_acc.children.push(from_acc);
Ok(())
}

fn add(&mut self, scope: Scope, units: u128) -> PartialVMResult<()> {
self.accumulator_mut(scope).ticks += units;
Ok(())
}
}

0 comments on commit d296555

Please sign in to comment.