Skip to content

Commit

Permalink
[programmable-transactions] No more generic Error parameter (MystenLa…
Browse files Browse the repository at this point in the history
…bs#9930)

## Description

`StorageView` (and by extension `ExecutionContext`) had an error
parameter, but it was always supplied with `SuiError`, and introduces a
bunch of line noise passing the generic parameter around.

Avoid that, by just requiring that implementations of `StorageView`
communicate errors through `SuiError`. Main motivation was to avoid
propagating this parameter further when introducing the wrapping type
that implements `LinkageResolver`.

## Test Plan

```
$ cargo simtest
$ env SUI_SKIP_SIMTESTS=1 cargo nextest run
```
  • Loading branch information
amnn authored Mar 28, 2023
1 parent d72c42e commit 6632fe4
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 116 deletions.
8 changes: 4 additions & 4 deletions crates/sui-adapter/src/execution_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ fn execution_loop<
Ok(Mode::empty_results())
}
TransactionKind::ProgrammableTransaction(pt) => {
programmable_transactions::execution::execute::<_, _, Mode>(
programmable_transactions::execution::execute::<_, Mode>(
protocol_config,
move_vm,
temporary_store,
Expand Down Expand Up @@ -471,7 +471,7 @@ fn advance_epoch<S: BackingPackageStore + ParentSync + ChildObjectResolver>(
epoch_start_timestamp_ms: change_epoch.epoch_start_timestamp_ms,
};
let advance_epoch_pt = construct_advance_epoch_pt(&params)?;
let result = programmable_transactions::execution::execute::<_, _, execution_mode::System>(
let result = programmable_transactions::execution::execute::<_, execution_mode::System>(
protocol_config,
move_vm,
temporary_store,
Expand All @@ -490,7 +490,7 @@ fn advance_epoch<S: BackingPackageStore + ParentSync + ChildObjectResolver>(
);
temporary_store.drop_writes();
let advance_epoch_safe_mode_pt = construct_advance_epoch_safe_mode_pt(&params)?;
programmable_transactions::execution::execute::<_, _, execution_mode::System>(
programmable_transactions::execution::execute::<_, execution_mode::System>(
protocol_config,
move_vm,
temporary_store,
Expand Down Expand Up @@ -569,7 +569,7 @@ fn setup_consensus_commit<S: BackingPackageStore + ParentSync + ChildObjectResol
);
builder.finish()
};
programmable_transactions::execution::execute::<_, _, execution_mode::System>(
programmable_transactions::execution::execute::<_, execution_mode::System>(
protocol_config,
move_vm,
temporary_store,
Expand Down
45 changes: 22 additions & 23 deletions crates/sui-adapter/src/execution_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

use move_core_types::language_storage::TypeTag;
use std::fmt;
use sui_types::{error::ExecutionError, messages::Argument};

use crate::programmable_transactions::{
Expand Down Expand Up @@ -33,15 +32,15 @@ pub trait ExecutionMode {

fn empty_results() -> Self::ExecutionResults;

fn add_argument_update<E: fmt::Debug, S: StorageView<E>>(
context: &mut ExecutionContext<E, S>,
fn add_argument_update<S: StorageView>(
context: &mut ExecutionContext<S>,
acc: &mut Self::ArgumentUpdates,
arg: Argument,
_new_value: &Value,
) -> Result<(), ExecutionError>;

fn finish_command<E: fmt::Debug, S: StorageView<E>>(
context: &mut ExecutionContext<E, S>,
fn finish_command<S: StorageView>(
context: &mut ExecutionContext<S>,
acc: &mut Self::ExecutionResults,
argument_updates: Self::ArgumentUpdates,
command_result: &[Value],
Expand Down Expand Up @@ -71,17 +70,17 @@ impl ExecutionMode for Normal {

fn empty_results() -> Self::ExecutionResults {}

fn add_argument_update<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn add_argument_update<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ArgumentUpdates,
_arg: Argument,
_new_value: &Value,
) -> Result<(), ExecutionError> {
Ok(())
}

fn finish_command<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn finish_command<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ExecutionResults,
_argument_updates: Self::ArgumentUpdates,
_command_result: &[Value],
Expand Down Expand Up @@ -113,17 +112,17 @@ impl ExecutionMode for Genesis {

fn empty_results() -> Self::ExecutionResults {}

fn add_argument_update<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn add_argument_update<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ArgumentUpdates,
_arg: Argument,
_new_value: &Value,
) -> Result<(), ExecutionError> {
Ok(())
}

fn finish_command<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn finish_command<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ExecutionResults,
_argument_updates: Self::ArgumentUpdates,
_command_result: &[Value],
Expand Down Expand Up @@ -158,17 +157,17 @@ impl ExecutionMode for System {

fn empty_results() -> Self::ExecutionResults {}

fn add_argument_update<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn add_argument_update<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ArgumentUpdates,
_arg: Argument,
_new_value: &Value,
) -> Result<(), ExecutionError> {
Ok(())
}

fn finish_command<E: fmt::Debug, S: StorageView<E>>(
_context: &mut ExecutionContext<E, S>,
fn finish_command<S: StorageView>(
_context: &mut ExecutionContext<S>,
_acc: &mut Self::ExecutionResults,
_argument_updates: Self::ArgumentUpdates,
_command_result: &[Value],
Expand Down Expand Up @@ -211,8 +210,8 @@ impl ExecutionMode for DevInspect {
vec![]
}

fn add_argument_update<E: fmt::Debug, S: StorageView<E>>(
context: &mut ExecutionContext<E, S>,
fn add_argument_update<S: StorageView>(
context: &mut ExecutionContext<S>,
acc: &mut Self::ArgumentUpdates,
arg: Argument,
new_value: &Value,
Expand All @@ -222,8 +221,8 @@ impl ExecutionMode for DevInspect {
Ok(())
}

fn finish_command<E: fmt::Debug, S: StorageView<E>>(
context: &mut ExecutionContext<E, S>,
fn finish_command<S: StorageView>(
context: &mut ExecutionContext<S>,
acc: &mut Self::ExecutionResults,
argument_updates: Self::ArgumentUpdates,
command_result: &[Value],
Expand All @@ -237,8 +236,8 @@ impl ExecutionMode for DevInspect {
}
}

fn value_to_bytes_and_tag<E: fmt::Debug, S: StorageView<E>>(
context: &mut ExecutionContext<E, S>,
fn value_to_bytes_and_tag<S: StorageView>(
context: &mut ExecutionContext<S>,
value: &Value,
) -> Result<(Vec<u8>, TypeTag), ExecutionError> {
let (type_tag, bytes) = match value {
Expand Down
24 changes: 7 additions & 17 deletions crates/sui-adapter/src/programmable_transactions/context.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use std::{
collections::{BTreeMap, BTreeSet, HashMap},
fmt,
marker::PhantomData,
};
use std::collections::{BTreeMap, BTreeSet, HashMap};

use move_binary_format::{
errors::{Location, VMError},
Expand Down Expand Up @@ -36,7 +32,7 @@ use crate::{
use super::types::*;

/// Maintains all runtime state specific to programmable transactions
pub struct ExecutionContext<'vm, 'state, 'a, 'b, E: fmt::Debug, S: StorageView<E>> {
pub struct ExecutionContext<'vm, 'state, 'a, 'b, S: StorageView> {
/// The protocol config
pub protocol_config: &'a ProtocolConfig,
/// The MoveVM
Expand Down Expand Up @@ -68,7 +64,6 @@ pub struct ExecutionContext<'vm, 'state, 'a, 'b, E: fmt::Debug, S: StorageView<E
/// Map of arguments that are currently borrowed in this command, true if the borrow is mutable
/// This gets cleared out when new results are pushed, i.e. the end of a command
borrowed: HashMap<Argument, /* mut */ bool>,
_e: PhantomData<E>,
}

/// A write for an object that was generated outside of the Move ObjectRuntime
Expand All @@ -83,11 +78,7 @@ struct AdditionalWrite {
bytes: Vec<u8>,
}

impl<'vm, 'state, 'a, 'b, E, S> ExecutionContext<'vm, 'state, 'a, 'b, E, S>
where
E: fmt::Debug,
S: StorageView<E>,
{
impl<'vm, 'state, 'a, 'b, S: StorageView> ExecutionContext<'vm, 'state, 'a, 'b, S> {
pub fn new(
protocol_config: &'a ProtocolConfig,
vm: &'vm MoveVM,
Expand Down Expand Up @@ -183,7 +174,6 @@ where
new_packages: vec![],
user_events: vec![],
borrowed: HashMap::new(),
_e: PhantomData,
})
}

Expand Down Expand Up @@ -787,7 +777,7 @@ where
}

/// Load an input object from the state_view
fn load_object<E: fmt::Debug, S: StorageView<E>>(
fn load_object<S: StorageView>(
vm: &MoveVM,
state_view: &S,
session: &Session<S>,
Expand Down Expand Up @@ -827,7 +817,7 @@ fn load_object<E: fmt::Debug, S: StorageView<E>>(
}

/// Load an a CallArg, either an object or a raw set of BCS bytes
fn load_call_arg<E: fmt::Debug, S: StorageView<E>>(
fn load_call_arg<S: StorageView>(
vm: &MoveVM,
state_view: &S,
session: &Session<S>,
Expand All @@ -843,7 +833,7 @@ fn load_call_arg<E: fmt::Debug, S: StorageView<E>>(
}

/// Load an ObjectArg from state view, marking if it can be treated as mutable or not
fn load_object_arg<E: fmt::Debug, S: StorageView<E>>(
fn load_object_arg<S: StorageView>(
vm: &MoveVM,
state_view: &S,
session: &Session<S>,
Expand Down Expand Up @@ -928,7 +918,7 @@ fn refund_max_gas_budget(
///
/// This function assumes proper generation of has_public_transfer, either from the abilities of
/// the StructTag, or from the runtime correctly propagating from the inputs
unsafe fn create_written_object<E: fmt::Debug, S: StorageView<E>>(
unsafe fn create_written_object<S: StorageView>(
vm: &MoveVM,
state_view: &S,
session: &Session<S>,
Expand Down
Loading

0 comments on commit 6632fe4

Please sign in to comment.