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

Enable tracing in the execution layer #21165

Merged
merged 14 commits into from
Feb 13, 2025
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exclude = [
"external-crates/move/crates/move-stdlib-natives",
"external-crates/move/crates/move-symbol-pool",
"external-crates/move/crates/move-transactional-test-runner",
"external-crates/move/crates/move-trace-format",
"external-crates/move/crates/move-unit-test",
"external-crates/move/crates/move-vm-config",
"external-crates/move/crates/move-vm-integration-tests",
Expand Down Expand Up @@ -584,6 +585,9 @@ move-symbol-pool = { path = "external-crates/move/crates/move-symbol-pool" }
move-abstract-interpreter = { path = "external-crates/move/crates/move-abstract-interpreter" }
move-abstract-stack = { path = "external-crates/move/crates/move-abstract-stack" }
move-analyzer = { path = "external-crates/move/crates/move-analyzer" }
move-trace-format = { path = "external-crates/move/crates/move-trace-format" }
move-proc-macros = { path = "external-crates/move/crates/move-proc-macros" }
enum-compat-util = { path = "external-crates/move/crates/enum-compat-util" }
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of a "my bad" situation here -- I don't think move-trace-format needs all these different crates (or at least doesn't use them). So I think removing them from the move-trace-format crate should allow you to remove them here and just import the move-trace-format crate.


fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "69d496c71fb37e3d22fe85e5bbfd4256d61422b9" }
fastcrypto-tbls = { git = "https://github.com/MystenLabs/fastcrypto", rev = "69d496c71fb37e3d22fe85e5bbfd4256d61422b9" }
Expand Down
1 change: 1 addition & 0 deletions crates/simulacrum/src/epoch_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ impl EpochState {
kind,
signer,
tx_digest,
&mut None,
);
Ok((inner_temp_store, gas_status, effects, result))
}
Expand Down
3 changes: 3 additions & 0 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1688,6 +1688,7 @@ impl AuthorityState {
kind,
signer,
tx_digest,
&mut None,
);

fail_point_if!("cp_execution_nondeterminism", || {
Expand Down Expand Up @@ -1880,6 +1881,7 @@ impl AuthorityState {
kind,
signer,
transaction_digest,
&mut None,
);
let tx_digest = *effects.transaction_digest();

Expand Down Expand Up @@ -2070,6 +2072,7 @@ impl AuthorityState {
kind,
signer,
transaction.digest(),
&mut None,
);

Ok(SimulateTransactionResult {
Expand Down
1 change: 1 addition & 0 deletions crates/sui-genesis-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ fn create_genesis_transaction(
kind,
signer,
genesis_digest,
&mut None,
);
assert!(inner_temp_store.input_objects.is_empty());
assert!(inner_temp_store.mutable_inputs.is_empty());
Expand Down
2 changes: 2 additions & 0 deletions crates/sui-replay/src/replay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ impl LocalExec {
transaction_kind.clone(),
tx_info.sender,
*tx_digest,
&mut None,
);

if let Err(err) = self.pretty_print_for_tracing(
Expand Down Expand Up @@ -939,6 +940,7 @@ impl LocalExec {
kind,
signer,
*executable.digest(),
&mut None,
);

let effects =
Expand Down
1 change: 1 addition & 0 deletions crates/sui-single-node-benchmark/src/single_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ impl SingleValidator {
kind,
signer,
*executable.digest(),
&mut None,
);
assert!(effects.status().is_ok());
store.commit_objects(inner_temp_store);
Expand Down
1 change: 1 addition & 0 deletions crates/sui-swarm-config/src/network_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ mod test {
kind,
signer,
genesis_digest,
&mut None,
);

assert_eq!(&effects, genesis.effects());
Expand Down
1 change: 1 addition & 0 deletions external-crates/move/crates/move-trace-format/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.0.1"
authors = ["Move Core Contributors"]
description = "Move Trace Format"
license = "Apache-2.0"
publish = false
edition = "2021"

[dependencies]
Expand Down
12 changes: 12 additions & 0 deletions external-crates/move/crates/move-trace-format/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ impl MoveTrace {
}
}

impl Default for MoveTrace {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh clippy...

Copy link
Contributor

Choose a reason for hiding this comment

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

We have this clippy note disabled in external-crates fwiw. So we can remove this if we want

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have this clippy note disabled in external-crates fwiw. So we can remove this if we want

Fwiw, I got clippy complaining about it in CI...

fn default() -> Self {
Self::new()
}
}

impl MoveTraceBuilder {
/// Create a new `MoveTraceBuilder` with no additional tracing.
pub fn new() -> Self {
Expand Down Expand Up @@ -314,6 +320,12 @@ impl MoveTraceBuilder {
}
}

impl Default for MoveTraceBuilder {
fn default() -> Self {
Self::new()
}
}

impl Display for TraceValue {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ impl TraceState {
}
}

impl Default for TraceState {
Copy link
Contributor

Choose a reason for hiding this comment

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

fn default() -> Self {
Self::new()
}
}

impl Tracer for TraceState {
fn notify(&mut self, event: &TraceEvent, mut write: Writer<'_>) {
self.apply_event(event);
Expand Down
1 change: 1 addition & 0 deletions sui-execution/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ sui-types.workspace = true

move-binary-format.workspace = true
move-bytecode-verifier-meter.workspace = true
move-trace-format.workspace = true
move-vm-config.workspace = true

sui-adapter-latest = { path = "latest/sui-adapter" }
Expand Down
1 change: 1 addition & 0 deletions sui-execution/latest/sui-adapter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ move-binary-format.workspace = true
move-bytecode-utils.workspace = true
move-bytecode-verifier-meter.workspace = true
move-core-types.workspace = true
move-trace-format.workspace = true
move-vm-config.workspace = true
move-vm-types.workspace = true
mysten-metrics.workspace = true
Expand Down
28 changes: 28 additions & 0 deletions sui-execution/latest/sui-adapter/src/execution_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod checked {

use crate::execution_mode::{self, ExecutionMode};
use move_binary_format::CompiledModule;
use move_trace_format::format::MoveTraceBuilder;
use move_vm_runtime::move_vm::MoveVM;
use std::{collections::HashSet, sync::Arc};
use sui_types::balance::{
Expand Down Expand Up @@ -92,6 +93,7 @@ mod checked {
metrics: Arc<LimitsMetrics>,
enable_expensive_checks: bool,
certificate_deny_set: &HashSet<TransactionDigest>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> (
InnerTemporaryStore,
SuiGasStatus,
Expand Down Expand Up @@ -145,6 +147,7 @@ mod checked {
deny_cert,
contains_deleted_input,
cancelled_objects,
trace_builder_opt,
);

let status = if let Err(error) = &execution_result {
Expand Down Expand Up @@ -262,6 +265,7 @@ mod checked {
tx_context,
&mut gas_charger,
pt,
&mut None,
)
.map_err(|(e, _)| e)?;
temporary_store.update_object_version_and_prev_tx();
Expand All @@ -281,6 +285,7 @@ mod checked {
deny_cert: bool,
contains_deleted_input: bool,
cancelled_objects: Option<(Vec<ObjectID>, SequenceNumber)>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> (
GasCostSummary,
Result<Mode::ExecutionResults, ExecutionError>,
Expand Down Expand Up @@ -340,6 +345,7 @@ mod checked {
gas_charger,
protocol_config,
metrics.clone(),
trace_builder_opt,
)
};

Expand Down Expand Up @@ -566,6 +572,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> ResultWithTimings<Mode::ExecutionResults, ExecutionError> {
let result = match transaction_kind {
TransactionKind::ChangeEpoch(change_epoch) => {
Expand All @@ -579,6 +586,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.map_err(|e| (e, vec![]))?;
Ok((Mode::empty_results(), vec![]))
Expand Down Expand Up @@ -612,6 +620,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.expect("ConsensusCommitPrologue cannot fail");
Ok((Mode::empty_results(), vec![]))
Expand All @@ -625,6 +634,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.expect("ConsensusCommitPrologueV2 cannot fail");
Ok((Mode::empty_results(), vec![]))
Expand All @@ -638,6 +648,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.expect("ConsensusCommitPrologueV3 cannot fail");
Ok((Mode::empty_results(), vec![]))
Expand All @@ -651,6 +662,7 @@ mod checked {
tx_ctx,
gas_charger,
pt,
trace_builder_opt,
)
}
TransactionKind::EndOfEpochTransaction(txns) => {
Expand All @@ -669,6 +681,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.map_err(|e| (e, vec![]))?;
return Ok((Mode::empty_results(), vec![]));
Expand Down Expand Up @@ -714,6 +727,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.map_err(|e| (e, vec![]))?;
Ok((Mode::empty_results(), vec![]))
Expand All @@ -727,6 +741,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
)
.map_err(|e| (e, vec![]))?;
Ok((Mode::empty_results(), vec![]))
Expand Down Expand Up @@ -882,6 +897,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> Result<(), ExecutionError> {
let params = AdvanceEpochParams {
epoch: change_epoch.epoch,
Expand All @@ -903,6 +919,7 @@ mod checked {
tx_ctx,
gas_charger,
advance_epoch_pt,
trace_builder_opt,
);

#[cfg(msim)]
Expand Down Expand Up @@ -932,6 +949,7 @@ mod checked {
tx_ctx,
gas_charger,
advance_epoch_safe_mode_pt,
trace_builder_opt,
)
.map_err(|(e, _)| e)
.expect("Advance epoch with safe mode must succeed");
Expand All @@ -953,6 +971,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
);
} else {
process_system_packages(
Expand All @@ -963,6 +982,7 @@ mod checked {
gas_charger,
protocol_config,
metrics,
trace_builder_opt,
);
}
Ok(())
Expand All @@ -976,6 +996,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) {
let binary_config = to_binary_config(protocol_config);
for (version, modules, dependencies) in change_epoch.system_packages.into_iter() {
Expand All @@ -1002,6 +1023,7 @@ mod checked {
tx_ctx,
gas_charger,
publish_pt,
trace_builder_opt,
)
.map_err(|(e, _)| e)
.expect("System Package Publish must succeed");
Expand Down Expand Up @@ -1044,6 +1066,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> Result<(), ExecutionError> {
let pt = {
let mut builder = ProgrammableTransactionBuilder::new();
Expand Down Expand Up @@ -1071,6 +1094,7 @@ mod checked {
tx_ctx,
gas_charger,
pt,
trace_builder_opt,
)
.map_err(|(e, _)| e)?;
Ok(())
Expand Down Expand Up @@ -1183,6 +1207,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> Result<(), ExecutionError> {
let pt = {
let mut builder = ProgrammableTransactionBuilder::new();
Expand Down Expand Up @@ -1214,6 +1239,7 @@ mod checked {
tx_ctx,
gas_charger,
pt,
trace_builder_opt,
)
.map_err(|(e, _)| e)?;
Ok(())
Expand Down Expand Up @@ -1250,6 +1276,7 @@ mod checked {
gas_charger: &mut GasCharger,
protocol_config: &ProtocolConfig,
metrics: Arc<LimitsMetrics>,
trace_builder_opt: &mut Option<MoveTraceBuilder>,
) -> Result<(), ExecutionError> {
let pt = {
let mut builder = ProgrammableTransactionBuilder::new();
Expand Down Expand Up @@ -1282,6 +1309,7 @@ mod checked {
tx_ctx,
gas_charger,
pt,
trace_builder_opt,
)
.map_err(|(e, _)| e)?;
Ok(())
Expand Down
Loading
Loading