fixup! [1/x][dev-inspect] Adds execution mode for a more expressive d…

…ry-run

crates/sui-adapter/src/adapter.rs

@@ -131,17 +131,11 @@ pub fn execute<
             }
             CallArg::ObjVec(obj_args) => {
                 for obj_arg in obj_args {
-                    match obj_arg {
-                        ObjectArg::ImmOrOwnedObject((id, _, _))
-                        | ObjectArg::SharedObject { id, .. } => {
-                            let obj = state_view.read_object(id);
-                            assert_invariant!(
-                                obj.is_some(),
-                                format!("Object {} does not exist yet", id)
-                            );
-                            objects.insert(*id, obj.unwrap());
-                        }
-                    }
+                    let (ObjectArg::ImmOrOwnedObject((id, _, _))
+                    | ObjectArg::SharedObject { id, .. }) = obj_arg;
+                    let obj = state_view.read_object(id);
+                    assert_invariant!(obj.is_some(), format!("Object {} does not exist yet", id));
+                    objects.insert(*id, obj.unwrap());
                 }
             }
         }
@@ -156,15 +150,7 @@ pub fn execute<
         by_value_objects,
         mutable_ref_objects,
         has_ctx_arg,
-    } = resolve_and_type_check(
-        &objects,
-        &module,
-        function,
-        &type_args,
-        args,
-        is_genesis,
-        Mode::allow_arbitrary_function_calls(),
-    )?;
+    } = resolve_and_type_check::<Mode>(&objects, &module, function, &type_args, args, is_genesis)?;
 
     if has_ctx_arg {
         args.push(ctx.to_vec());
@@ -687,14 +673,13 @@ pub struct TypeCheckSuccess {
 /// - Check that the the signature of `function` is well-typed w.r.t `type_args`, `object_args`, and `pure_args`
 /// - Return the ID of the resolved module, a vector of BCS encoded arguments to pass to the VM, and a partitioning
 /// of the input objects into objects passed by value vs by mutable reference
-pub fn resolve_and_type_check(
+pub fn resolve_and_type_check<Mode: ExecutionMode>(
     objects: &BTreeMap<ObjectID, impl Borrow<Object>>,
     module: &CompiledModule,
     function: &Identifier,
     type_args: &[TypeTag],
     args: Vec<CallArg>,
     is_genesis: bool,
-    allow_arbitrary_function_calls: bool,
 ) -> Result<TypeCheckSuccess, ExecutionError> {
     // Resolve the function we are calling
     let view = &BinaryIndexedView::Module(module);
@@ -723,7 +708,7 @@ pub fn resolve_and_type_check(
     // Similarly, we will bypass this check for dev-inspect, as the mode does not make state changes
     // and is just for developers to check the result of Move functions. This mode is flagged by
     // allow_arbitrary_function_calls
-    if !fdef.is_entry && !is_genesis && !allow_arbitrary_function_calls {
+    if !fdef.is_entry && !is_genesis && !Mode::allow_arbitrary_function_calls() {
         return Err(ExecutionError::new_with_source(
             ExecutionErrorKind::NonEntryFunctionInvoked,
             "Can only call `entry` functions",
@@ -784,7 +769,7 @@ pub fn resolve_and_type_check(
             let object_arg = match arg {
                 // dev-inspect does not make state changes and just a developer aid, so let through
                 // any BCS bytes (they will be checked later by the VM)
-                CallArg::Pure(arg) if allow_arbitrary_function_calls => return Ok(arg),
+                CallArg::Pure(arg) if Mode::allow_arbitrary_function_calls() => return Ok(arg),
                 CallArg::Pure(arg) => {
                     let (is_primitive, type_layout_opt) =
                         primitive_type(view, type_args, param_type);

crates/sui-adapter/src/execution_mode.rs

@@ -7,9 +7,16 @@ use sui_types::error::ExecutionError;
 pub type TransactionIndex = usize;
 
 pub trait ExecutionMode {
+    /// the type of a single Move call execution
     type ExecutionResult;
+
+    /// the gathered results from batched executions
     type ExecutionResults;
 
+    /// Controls two things:
+    /// - the calling of arbitrary Move functions
+    /// - the ability to instantiate any Move function parameter with a Pure call arg.
+    ///   In other words, you can instantiate any struct or object or other value with its BCS bytes.
     fn allow_arbitrary_function_calls() -> bool;
 
     fn make_result(
@@ -49,6 +56,9 @@ impl ExecutionMode for Normal {
     }
 }
 
+/// WARNING! Using this mode will bypass all normal checks around Move entry functions! This
+/// includes the various rules for function arguments, meaning any object can be created just from
+/// BCS bytes!
 pub struct DevInspect;
 
 impl ExecutionMode for DevInspect {

crates/sui-core/src/authority.rs

@@ -28,7 +28,6 @@ use prometheus::{
     exponential_buckets, register_histogram_with_registry, register_int_counter_with_registry,
     register_int_gauge_with_registry, Histogram, IntCounter, IntGauge, Registry,
 };
-use sui_adapter::execution_mode::DevInspect;
 use tap::TapFallible;
 use tokio::sync::broadcast::error::RecvError;
 use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver};
@@ -1089,34 +1088,6 @@ impl AuthorityState {
         SuiTransactionEffects::try_from(effects, self.module_cache.as_ref())
     }
 
-    pub async fn dev_inspect_transaction(
-        &self,
-        transaction: TransactionData,
-        transaction_digest: TransactionDigest,
-    ) -> Result<DevInspectResults, anyhow::Error> {
-        let (gas_status, input_objects) =
-            transaction_input_checker::check_transaction_input(&self.database, &transaction)
-                .await?;
-        let shared_object_refs = input_objects.filter_shared_objects();
-
-        let transaction_dependencies = input_objects.transaction_dependencies();
-        let temporary_store =
-            TemporaryStore::new(self.database.clone(), input_objects, transaction_digest);
-        let (_inner_temp_store, effects, execution_error) =
-            execution_engine::execute_transaction_to_effects::<execution_mode::DevInspect, _>(
-                shared_object_refs,
-                temporary_store,
-                transaction,
-                transaction_digest,
-                transaction_dependencies,
-                &self.move_vm,
-                &self._native_functions,
-                gas_status,
-                self.epoch(),
-            );
-        DevInspectResults::try_from(effects, self.module_cache.as_ref())
-    }
-
     pub fn is_tx_already_executed(&self, digest: &TransactionDigest) -> SuiResult<bool> {
         self.database.effects_exists(digest)
     }

crates/sui-core/src/execution_engine.rs

@@ -150,7 +150,7 @@ fn execute_transaction<
             tx_ctx,
             move_vm,
             native_functions,
-            gas_status,
+            &mut gas_status,
         );
         if execution_result.is_err() {
             // Roll back the temporary store if execution failed.
@@ -182,7 +182,7 @@ fn execution_loop<
     tx_ctx: &mut TxContext,
     move_vm: &Arc<MoveVM>,
     native_functions: &NativeFunctionTable,
-    mut gas_status: SuiGasStatus,
+    gas_status: &mut SuiGasStatus,
 ) -> Result<Mode::ExecutionResults, ExecutionError> {
     let mut results = Mode::empty_results();
     // TODO: Since we require all mutable objects to not show up more than

crates/sui-core/src/gateway_state.rs

@@ -9,6 +9,7 @@ use std::path::Path;
 use std::sync::atomic::AtomicU64;
 use std::sync::Arc;
 use std::time::Duration;
+use sui_adapter::execution_mode;
 
 use anyhow::anyhow;
 use async_trait::async_trait;
@@ -1278,7 +1279,6 @@ where
 
         // Pass in the objects for a deeper check
         let is_genesis = false;
-        let allow_arbitrary_function_calls = false;
         let type_arguments = type_arguments
             .into_iter()
             .map(|arg| arg.try_into())
@@ -1288,14 +1288,13 @@ where
             .try_as_package()
             .ok_or_else(|| anyhow!("Cannot get package from object"))?
             .deserialize_module(&module)?;
-        resolve_and_type_check(
+        resolve_and_type_check::<execution_mode::Normal>(
             &objects,
             &compiled_module,
             &function,
             &type_arguments,
             args.clone(),
             is_genesis,
-            allow_arbitrary_function_calls,
         )?;
         used_object_ids.extend(objects.keys());
 

crates/sui-json-rpc-types/src/lib.rs

@@ -38,7 +38,7 @@ use sui_types::base_types::{
 };
 use sui_types::committee::EpochId;
 use sui_types::crypto::{AuthorityStrongQuorumSignInfo, SignableBytes, Signature};
-use sui_types::error::{ExecutionError, SuiError};
+use sui_types::error::SuiError;
 use sui_types::event::{BalanceChangeType, Event, EventID};
 use sui_types::event::{EventEnvelope, EventType};
 use sui_types::filter::{EventFilter, TransactionFilter};
@@ -1870,30 +1870,6 @@ pub struct SuiTransactionEffects {
     pub dependencies: Vec<TransactionDigest>,
 }
 
-/// The response from processing a dev inspect transaction
-#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, JsonSchema)]
-#[serde(rename = "DevInspectResults", rename_all = "camelCase")]
-pub struct DevInspectResults {
-    /// Summary of effects that likely would be generated if the transaction is actually run.
-    /// Note however, that not all dev-inspect transactions are actually usable as transactions so
-    /// it might not be possible actually generate these effects from a normal transaction.
-    effects: SuiTransactionEffects,
-    /// Execution results (including return values) from executing the transactions
-    /// Currently contains only return values from Move calls
-    results: Result<Vec<(usize, ExecutionResult)>, String>,
-}
-
-#[derive(Eq, PartialEq, Clone, Debug, Serialize, Deserialize, JsonSchema)]
-pub struct ExecutionResult {
-    /// The value of any arguments that were mutably borrowed.
-    /// Non-mut borrowed values are not included
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub mutable_reference_outputs: Vec<(u8, Vec<u8>, MoveTypeLayout)>,
-    /// The return values from the function
-    #[serde(default, skip_serializing_if = "Vec::is_empty")]
-    pub return_values: Vec<(Vec<u8>, MoveTypeLayout)>,
-}
-
 impl SuiTransactionEffects {
     /// Return an iterator of mutated objects, but excluding the gas object.
     pub fn mutated_excluding_gas(&self) -> impl Iterator<Item = &OwnedObjectRef> {

crates/sui-transaction-builder/src/lib.rs

@@ -13,6 +13,7 @@ use move_core_types::identifier::Identifier;
 use move_core_types::language_storage::TypeTag;
 
 use sui_adapter::adapter::resolve_and_type_check;
+use sui_adapter::execution_mode;
 use sui_json::{resolve_move_function_args, SuiJsonCallArg, SuiJsonValue};
 use sui_json_rpc_types::GetRawObjectDataResponse;
 use sui_json_rpc_types::SuiObjectInfo;
@@ -349,15 +350,14 @@ impl TransactionBuilder {
         }
         let compiled_module = package.deserialize_module(module)?;
 
-        resolve_and_type_check(
+        // TODO set the Mode from outside?
+        resolve_and_type_check::<execution_mode::Normal>(
             &objects,
             &compiled_module,
             function,
             type_args,
             args.clone(),
             false,
-            // TODO this needs to be set from outside?
-            false,
         )?;
 
         Ok(args)

crates/sui-transactional-test-runner/src/test_adapter.rs

@@ -36,7 +36,7 @@ use std::{
     path::Path,
     sync::Arc,
 };
-use sui_adapter::{adapter::new_move_vm, genesis};
+use sui_adapter::{adapter::new_move_vm, execution_mode, genesis};
 use sui_core::{execution_engine, test_utils::to_sender_signed_transaction};
 use sui_framework::DEFAULT_FRAMEWORK_PATH;
 use sui_types::temporary_store::TemporaryStore;
@@ -488,7 +488,7 @@ impl<'a> SuiTestAdapter<'a> {
                 ..
             },
             execution_error,
-        ) = execution_engine::execute_transaction_to_effects(
+        ) = execution_engine::execute_transaction_to_effects::<execution_mode::Normal, _>(
             shared_object_refs,
             temporary_store,
             transaction.into_inner().into_data().data,
@@ -548,7 +548,7 @@ impl<'a> SuiTestAdapter<'a> {
                 Err(anyhow::anyhow!(self.stabilize_str(format!(
                     "Transaction Effects Status: {}\nExecution Error: {}",
                     error,
-                    execution_error.expect(
+                    execution_error.err().expect(
                         "to have an execution error if a transaction's status is a failure"
                     )
                 ))))