Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crates/simulacrum/src/epoch_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ impl EpochState {
tx_digest,
&mut None,
);
Ok((inner_temp_store, gas_status, effects, result))
Ok((
inner_temp_store,
gas_status,
effects,
result.map_err(sui_types::error::ExecutionError::from),
))
}
}
156 changes: 104 additions & 52 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ use sui_types::effects::{
InputConsensusObject, SignedTransactionEffects, TransactionEffects, TransactionEffectsAPI,
TransactionEvents, VerifiedSignedTransactionEffects,
};
use sui_types::error::{ExecutionError, SuiErrorKind, UserInputError};
use sui_types::error::{
ExecutionError, ExecutionErrorContext, ExecutionErrorMetadata, SuiErrorKind, UserInputError,
};
use sui_types::event::EventID;
use sui_types::executable_transaction::VerifiedExecutableTransaction;
use sui_types::execution_status::ExecutionErrorKind;
Expand Down Expand Up @@ -1905,40 +1907,80 @@ impl AuthorityState {
rewritten_inputs: Option<Vec<bool>>,
signer: SuiAddress,
tx_digest: TransactionDigest,
is_fullnode: bool,
) -> (
InnerTemporaryStore,
SuiGasStatus,
TransactionEffects,
Vec<ExecutionTiming>,
Result<(), ExecutionError>,
Option<ExecutionErrorMetadata>,
) {
let (inner_temp_store, gas_status, effects, timings, execution_error) = executor
// TODO only run this function on FullNodes, use `execute_transaction_to_effects` on validators.
Comment thread
jordanjennings-mysten marked this conversation as resolved.
.execute_transaction_to_effects_and_execution_error(
store,
protocol_config,
self.metrics.execution_metrics.clone(),
enable_expensive_checks,
execution_params,
epoch_id,
epoch_timestamp_ms,
input_objects,
gas_data,
// Fullnodes retain source errors and VM metadata as sidecar data for richer user-facing
// error reporting. Validators use the lean execution path and intentionally return no
// sidecar metadata so effects stay independent of non-consensus data.
if is_fullnode {
Comment thread
jordanjennings-mysten marked this conversation as resolved.
let (inner_temp_store, gas_status, effects, timings, execution_error) = executor
.execute_transaction_to_effects_and_execution_error(
store,
protocol_config,
self.metrics.execution_metrics.clone(),
enable_expensive_checks,
execution_params,
epoch_id,
epoch_timestamp_ms,
input_objects,
gas_data,
gas_status,
kind,
rewritten_inputs,
signer,
tx_digest,
&mut None,
);

let execution_error_metadata = execution_error
.as_ref()
.err()
.and_then(ExecutionErrorContext::metadata_with_source);

(
inner_temp_store,
gas_status,
kind,
rewritten_inputs,
signer,
tx_digest,
&mut None,
);
effects,
timings,
execution_error.map_err(ExecutionError::from),
execution_error_metadata,
)
} else {
let (inner_temp_store, gas_status, effects, timings, execution_failure) = executor
.execute_transaction_to_effects(
store,
protocol_config,
self.metrics.execution_metrics.clone(),
enable_expensive_checks,
execution_params,
epoch_id,
epoch_timestamp_ms,
input_objects,
gas_data,
gas_status,
kind,
rewritten_inputs,
signer,
tx_digest,
&mut None,
);

(
inner_temp_store,
gas_status,
effects,
timings,
execution_error,
)
(
inner_temp_store,
gas_status,
effects,
timings,
execution_failure.map_err(ExecutionError::from),
None,
)
}
}

/// execute_certificate validates the transaction input, and executes the certificate,
Expand Down Expand Up @@ -2025,30 +2067,39 @@ impl AuthorityState {
let tracking_store = TrackingBackingStore::new(self.get_backing_store().as_ref());

#[allow(unused_mut)]
let (inner_temp_store, _, mut effects, timings, execution_error_opt) = self
.execute_transaction_to_effects(
&**epoch_store.executor(),
&tracking_store,
protocol_config,
// TODO: would be nice to pass the whole NodeConfig here, but it creates a
// cyclic dependency w/ sui-adapter
self.config
.expensive_safety_check_config
.enable_deep_per_tx_sui_conservation_check(),
execution_params,
&epoch_store.epoch_start_config().epoch_data().epoch_id(),
epoch_store
.epoch_start_config()
.epoch_data()
.epoch_start_timestamp(),
input_objects,
gas_data,
gas_status,
kind,
rewritten_inputs,
signer,
tx_digest,
);
let (
inner_temp_store,
_,
mut effects,
timings,
execution_error_opt,
execution_error_metadata,
) = self.execute_transaction_to_effects(
&**epoch_store.executor(),
&tracking_store,
protocol_config,
// TODO: would be nice to pass the whole NodeConfig here, but it creates a
// cyclic dependency w/ sui-adapter
self.config
.expensive_safety_check_config
.enable_deep_per_tx_sui_conservation_check(),
execution_params,
&epoch_store.epoch_start_config().epoch_data().epoch_id(),
epoch_store
.epoch_start_config()
.epoch_data()
.epoch_start_timestamp(),
input_objects,
gas_data,
gas_status,
kind,
rewritten_inputs,
signer,
tx_digest,
self.is_fullnode(epoch_store),
);

let execution_error_opt = execution_error_opt.err();

let object_funds_checker = self.object_funds_checker.load();
if let Some(object_funds_checker) = object_funds_checker.as_ref()
Expand Down Expand Up @@ -2174,6 +2225,7 @@ impl AuthorityState {
effects,
inner_temp_store,
unchanged_loaded_runtime_objects,
execution_error_metadata,
);

let elapsed = prepare_certificate_start_time.elapsed().as_micros() as f64;
Expand All @@ -2187,7 +2239,7 @@ impl AuthorityState {
);
}

ExecutionOutput::Success((transaction_outputs, timings, execution_error_opt.err()))
ExecutionOutput::Success((transaction_outputs, timings, execution_error_opt))
}

pub fn prepare_certificate_for_benchmark(
Expand Down Expand Up @@ -2339,7 +2391,7 @@ impl AuthorityState {
let execution_error_source = execution_result
.as_ref()
.err()
.and_then(|e| e.source().as_ref().map(|e| e.to_string()));
.and_then(|e| std::error::Error::source(e).map(|e| e.to_string()));
Comment thread
jordanjennings-mysten marked this conversation as resolved.
Outdated

let response = DryRunTransactionBlockResponse {
suggested_gas_price,
Expand Down
19 changes: 18 additions & 1 deletion crates/sui-core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use move_core_types::resolver::{ModuleResolver, SerializedPackage};
use serde::{Deserialize, Serialize};
use sui_config::node::AuthorityStorePruningConfig;
use sui_macros::fail_point_arg;
use sui_types::error::{SuiErrorKind, UserInputError};
use sui_types::error::{ExecutionErrorMetadata, SuiErrorKind, UserInputError};
use sui_types::execution::TypeLayoutStore;
use sui_types::global_state_hash::GlobalStateHash;
use sui_types::message_envelope::Message;
Expand Down Expand Up @@ -344,6 +344,13 @@ impl AuthorityStore {
.get(digest)
}

pub fn get_execution_error_metadata(
Comment thread
jordanjennings-mysten marked this conversation as resolved.
&self,
digest: &TransactionDigest,
) -> Result<Option<ExecutionErrorMetadata>, TypedStoreError> {
self.perpetual_tables.execution_error_metadata.get(digest)
}

pub fn multi_get_effects<'a>(
&self,
effects_digests: impl Iterator<Item = &'a TransactionEffectsDigest>,
Expand Down Expand Up @@ -791,6 +798,7 @@ impl AuthorityStore {
written,
events,
unchanged_loaded_runtime_objects,
execution_error_metadata,
locks_to_delete,
new_locks_to_init,
..
Expand Down Expand Up @@ -863,6 +871,15 @@ impl AuthorityStore {
)?;
}

if let Some(metadata) = execution_error_metadata
&& !metadata.attributes.is_empty()
{
write_batch.insert_batch(
&self.perpetual_tables.execution_error_metadata,
[(transaction_digest, metadata)],
)?;
}

self.initialize_live_object_markers_impl(write_batch, new_locks_to_init, false)?;

// Note: deletes locks for received objects as well (but not for objects that were in
Expand Down
2 changes: 2 additions & 0 deletions crates/sui-core/src/authority/authority_store_pruner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,8 @@ impl AuthorityStorePruner {
&perpetual_db.unchanged_loaded_runtime_objects,
transactions.iter(),
)?;
perpetual_batch
.delete_batch(&perpetual_db.execution_error_metadata, transactions.iter())?;
perpetual_batch.delete_batch(&perpetual_db.effects, effect_digests)?;

let mut checkpoints_batch = checkpoint_db.tables.certified_checkpoints.batch();
Expand Down
14 changes: 14 additions & 0 deletions crates/sui-core/src/authority/authority_store_tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::path::Path;
use std::sync::atomic::AtomicU64;
use sui_types::base_types::SequenceNumber;
use sui_types::effects::{TransactionEffects, TransactionEvents};
use sui_types::error::ExecutionErrorMetadata;
use sui_types::global_state_hash::GlobalStateHash;
use sui_types::storage::MarkerValue;
use typed_store::metrics::SamplingInterval;
Expand Down Expand Up @@ -100,6 +101,9 @@ pub struct AuthorityPerpetualTables {
// Loaded (and unchanged) runtime object references.
pub(crate) unchanged_loaded_runtime_objects: DBMap<TransactionDigest, Vec<ObjectKey>>,

// Local execution error metadata, keyed by the digest of the transaction that produced it.
Comment thread
jordanjennings-mysten marked this conversation as resolved.
pub(crate) execution_error_metadata: DBMap<TransactionDigest, ExecutionErrorMetadata>,

/// DEPRECATED in favor of the table of the same name in authority_per_epoch_store.
/// Please do not add new accessors/callsites.
/// When transaction is executed via checkpoint executor, we store association here
Expand Down Expand Up @@ -331,6 +335,16 @@ impl AuthorityPerpetualTables {
digest_prefix.clone(),
),
),
(
"execution_error_metadata".to_string(),
ThConfig::new_with_rm_prefix(
32,
mutexes,
uniform_key,
KeySpaceConfig::default().with_relocation_filter(|_, _| Decision::Remove),
digest_prefix.clone(),
),
),
(
"executed_transactions_to_checkpoint".to_string(),
ThConfig::new_with_rm_prefix(
Expand Down
7 changes: 7 additions & 0 deletions crates/sui-core/src/checkpoints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4190,6 +4190,13 @@ mod tests {
unimplemented!()
}

fn get_execution_error_metadata(
&self,
_digest: &TransactionDigest,
) -> Option<sui_types::error::ExecutionErrorMetadata> {
unimplemented!()
}

fn transaction_executed_in_last_epoch(&self, _: &TransactionDigest, _: EpochId) -> bool {
unimplemented!()
}
Expand Down
7 changes: 6 additions & 1 deletion crates/sui-core/src/execution_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use sui_protocol_config::ProtocolVersion;
use sui_types::base_types::{FullObjectID, VerifiedExecutionData};
use sui_types::digests::{TransactionDigest, TransactionEffectsDigest};
use sui_types::effects::{TransactionEffects, TransactionEvents};
use sui_types::error::{SuiError, SuiErrorKind, SuiResult, UserInputError};
use sui_types::error::{ExecutionErrorMetadata, SuiError, SuiErrorKind, SuiResult, UserInputError};
use sui_types::executable_transaction::VerifiedExecutableTransaction;
use sui_types::messages_checkpoint::CheckpointSequenceNumber;
use sui_types::object::Object;
Expand Down Expand Up @@ -534,6 +534,11 @@ pub trait TransactionCacheRead: Send + Sync {
digest: &TransactionDigest,
) -> Option<Vec<ObjectKey>>;

fn get_execution_error_metadata(
Comment thread
jordanjennings-mysten marked this conversation as resolved.
&self,
digest: &TransactionDigest,
) -> Option<ExecutionErrorMetadata>;

fn take_accumulator_events(&self, digest: &TransactionDigest) -> Option<Vec<AccumulatorEvent>>;

fn notify_read_executed_effects_digests<'a>(
Expand Down
Loading
Loading