From 4151379d442237c9e7240aea964c62c255046aa1 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Fri, 17 Jun 2022 14:33:48 -0700 Subject: [PATCH] [messages] Split out objects into their own enum (#2607) - To make room for vectors of objects, object args should be a nested enum --- crates/sui-adapter/src/adapter.rs | 13 ++++++---- crates/sui-core/src/execution_engine.rs | 3 ++- crates/sui-core/src/gateway_state.rs | 16 +++++++------ crates/sui-core/src/generate_format.rs | 3 ++- .../unit_tests/authority_aggregator_tests.rs | 6 ++--- .../src/unit_tests/authority_tests.rs | 10 ++++---- .../src/unit_tests/consensus_tests.rs | 6 +++-- crates/sui-core/src/unit_tests/gas_tests.rs | 4 +++- crates/sui-core/tests/staged/sui.yaml | 24 ++++++++++++------- crates/sui-json-rpc-api/src/rpc_types.rs | 6 ++--- .../sui-transactional-test-runner/src/args.rs | 6 ++--- crates/sui-types/src/messages.rs | 16 +++++++++---- .../sui/src/benchmark/transaction_creator.rs | 2 +- crates/sui/tests/checkpoints_tests.rs | 4 ++-- crates/sui/tests/shared_objects_tests.rs | 23 ++++++++++-------- crates/test-utils/src/messages.rs | 5 ++-- 16 files changed, 90 insertions(+), 57 deletions(-) diff --git a/crates/sui-adapter/src/adapter.rs b/crates/sui-adapter/src/adapter.rs index 69f99477f8b8e..760e1e937d195 100644 --- a/crates/sui-adapter/src/adapter.rs +++ b/crates/sui-adapter/src/adapter.rs @@ -18,7 +18,7 @@ use sui_types::{ event::{Event, TransferType}, gas::SuiGasStatus, id::VersionedID, - messages::{CallArg, InputObjectKind}, + messages::{CallArg, InputObjectKind, ObjectArg}, object::{self, Data, MoveObject, Object, Owner}, storage::{DeleteKind, Storage}, }; @@ -72,7 +72,8 @@ pub fn execute + ModuleResolver None, - CallArg::ImmOrOwnedObject((id, _, _)) | CallArg::SharedObject(id) => { + CallArg::Object(ObjectArg::ImmOrOwnedObject((id, _, _))) + | CallArg::Object(ObjectArg::SharedObject(id)) => { Some((*id, state_view.read_object(id)?)) } }) @@ -746,8 +747,12 @@ pub fn resolve_and_type_check( } return Ok(arg); } - CallArg::ImmOrOwnedObject(ref_) => InputObjectKind::ImmOrOwnedMoveObject(ref_), - CallArg::SharedObject(id) => InputObjectKind::SharedMoveObject(id), + CallArg::Object(ObjectArg::ImmOrOwnedObject(ref_)) => { + InputObjectKind::ImmOrOwnedMoveObject(ref_) + } + CallArg::Object(ObjectArg::SharedObject(id)) => { + InputObjectKind::SharedMoveObject(id) + } }; let id = object_kind.object_id(); diff --git a/crates/sui-core/src/execution_engine.rs b/crates/sui-core/src/execution_engine.rs index 53be73a00098b..bb3e6b99e2ad6 100644 --- a/crates/sui-core/src/execution_engine.rs +++ b/crates/sui-core/src/execution_engine.rs @@ -11,6 +11,7 @@ use sui_types::committee::EpochId; use sui_types::error::ExecutionError; use sui_types::gas::GasCostSummary; use sui_types::gas_coin::GasCoin; +use sui_types::messages::ObjectArg; use sui_types::object::{MoveObject, Owner, OBJECT_START_VERSION}; use sui_types::{ base_types::{ObjectID, ObjectRef, SuiAddress, TransactionDigest, TxContext}, @@ -175,7 +176,7 @@ fn execute_transaction( &function, vec![], vec![ - CallArg::SharedObject(SUI_SYSTEM_STATE_OBJECT_ID), + CallArg::Object(ObjectArg::SharedObject(SUI_SYSTEM_STATE_OBJECT_ID)), CallArg::Pure(bcs::to_bytes(&epoch).unwrap()), CallArg::Pure(bcs::to_bytes(&storage_charge).unwrap()), CallArg::Pure(bcs::to_bytes(&computation_charge).unwrap()), diff --git a/crates/sui-core/src/gateway_state.rs b/crates/sui-core/src/gateway_state.rs index d7b7912bbb3d5..d33b0e09882db 100644 --- a/crates/sui-core/src/gateway_state.rs +++ b/crates/sui-core/src/gateway_state.rs @@ -694,7 +694,9 @@ where let signer = certificate.data.signer(); let (gas_payment, _, _) = certificate.data.gas(); let (coin_object_id, split_arg) = match call.arguments.as_slice() { - [CallArg::ImmOrOwnedObject((id, _, _)), CallArg::Pure(arg)] => (id, arg), + [CallArg::Object(ObjectArg::ImmOrOwnedObject((id, _, _))), CallArg::Pure(arg)] => { + (id, arg) + } _ => { return Err(SuiError::InconsistentGatewayResult { error: "Malformed transaction data".to_string(), @@ -747,7 +749,7 @@ where ) -> Result { let call = Self::try_get_move_call(&certificate)?; let primary_coin = match call.arguments.first() { - Some(CallArg::ImmOrOwnedObject((id, _, _))) => id, + Some(CallArg::Object(ObjectArg::ImmOrOwnedObject((id, _, _)))) => id, _ => { return Err(SuiError::InconsistentGatewayResult { error: "Malformed transaction data".to_string(), @@ -883,9 +885,9 @@ where SuiJsonCallArg::Object(id) => { let obj = self.get_object_internal(&id).await?; let arg = if obj.is_shared() { - CallArg::SharedObject(id) + CallArg::Object(ObjectArg::SharedObject(id)) } else { - CallArg::ImmOrOwnedObject(obj.compute_object_reference()) + CallArg::Object(ObjectArg::ImmOrOwnedObject(obj.compute_object_reference())) }; objects.insert(id, obj); arg @@ -1205,7 +1207,7 @@ where vec![coin_type], gas, vec![ - CallArg::ImmOrOwnedObject(coin_object_ref), + CallArg::Object(ObjectArg::ImmOrOwnedObject(coin_object_ref)), CallArg::Pure(bcs::to_bytes(&split_amounts)?), ], gas_budget, @@ -1243,8 +1245,8 @@ where vec![coin_type], gas, vec![ - CallArg::ImmOrOwnedObject(primary_coin_ref), - CallArg::ImmOrOwnedObject(coin_to_merge_ref), + CallArg::Object(ObjectArg::ImmOrOwnedObject(primary_coin_ref)), + CallArg::Object(ObjectArg::ImmOrOwnedObject(coin_to_merge_ref)), ], gas_budget, ); diff --git a/crates/sui-core/src/generate_format.rs b/crates/sui-core/src/generate_format.rs index 467fb63dc35db..ca8e3d4f54324 100644 --- a/crates/sui-core/src/generate_format.rs +++ b/crates/sui-core/src/generate_format.rs @@ -16,7 +16,7 @@ use sui_types::{ batch::UpdateItem, crypto::{get_key_pair, AuthoritySignature, Signature}, messages::{ - CallArg, ExecutionFailureStatus, ExecutionStatus, ObjectInfoRequestKind, + CallArg, ExecutionFailureStatus, ExecutionStatus, ObjectArg, ObjectInfoRequestKind, SingleTransactionKind, TransactionKind, }, object::{Data, Owner}, @@ -64,6 +64,7 @@ fn get_registry() -> Result { tracer.trace_type::(&samples)?; tracer.trace_type::(&samples)?; tracer.trace_type::(&samples)?; + tracer.trace_type::(&samples)?; tracer.trace_type::(&samples)?; tracer.trace_type::(&samples)?; tracer.trace_type::(&samples)?; diff --git a/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs b/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs index dc29738523717..552e9a7703129 100644 --- a/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_aggregator_tests.rs @@ -124,7 +124,7 @@ fn transfer_object_move_transaction( gas_object_ref: ObjectRef, ) -> Transaction { let args = vec![ - CallArg::ImmOrOwnedObject(object_ref), + CallArg::Object(ObjectArg::ImmOrOwnedObject(object_ref)), CallArg::Pure(bcs::to_bytes(&AccountAddress::from(dest)).unwrap()), ]; @@ -187,7 +187,7 @@ pub fn delete_object_move_transaction( ident_str!("delete").to_owned(), Vec::new(), gas_object_ref, - vec![CallArg::ImmOrOwnedObject(object_ref)], + vec![CallArg::Object(ObjectArg::ImmOrOwnedObject(object_ref))], GAS_VALUE_FOR_TESTING / 2, ), secret, @@ -203,7 +203,7 @@ pub fn set_object_move_transaction( gas_object_ref: ObjectRef, ) -> Transaction { let args = vec![ - CallArg::ImmOrOwnedObject(object_ref), + CallArg::Object(ObjectArg::ImmOrOwnedObject(object_ref)), CallArg::Pure(bcs::to_bytes(&value).unwrap()), ]; diff --git a/crates/sui-core/src/unit_tests/authority_tests.rs b/crates/sui-core/src/unit_tests/authority_tests.rs index 30a5a194c1d69..bca4b6e1e08b9 100644 --- a/crates/sui-core/src/unit_tests/authority_tests.rs +++ b/crates/sui-core/src/unit_tests/authority_tests.rs @@ -39,9 +39,11 @@ impl TestCallArg { Self::Object(object_id) => { let object = state.get_object(&object_id).await.unwrap().unwrap(); if object.is_shared() { - CallArg::SharedObject(object_id) + CallArg::Object(ObjectArg::SharedObject(object_id)) } else { - CallArg::ImmOrOwnedObject(object.compute_object_reference()) + CallArg::Object(ObjectArg::ImmOrOwnedObject( + object.compute_object_reference(), + )) } } Self::U64(value) => CallArg::Pure(bcs::to_bytes(&value).unwrap()), @@ -220,7 +222,7 @@ async fn test_handle_shared_object_with_max_sequence_number() { gas_object_ref, /* args */ vec![ - CallArg::SharedObject(shared_object_id), + CallArg::Object(ObjectArg::SharedObject(shared_object_id)), CallArg::Pure(16u64.to_le_bytes().to_vec()), CallArg::Pure(bcs::to_bytes(&AccountAddress::from(sender)).unwrap()), ], @@ -1739,7 +1741,7 @@ async fn shared_object() { gas_object_ref, /* args */ vec![ - CallArg::SharedObject(shared_object_id), + CallArg::Object(ObjectArg::SharedObject(shared_object_id)), CallArg::Pure(16u64.to_le_bytes().to_vec()), CallArg::Pure(bcs::to_bytes(&AccountAddress::from(sender)).unwrap()), ], diff --git a/crates/sui-core/src/unit_tests/consensus_tests.rs b/crates/sui-core/src/unit_tests/consensus_tests.rs index b1cf23432d6c4..9ff7d78f192ad 100644 --- a/crates/sui-core/src/unit_tests/consensus_tests.rs +++ b/crates/sui-core/src/unit_tests/consensus_tests.rs @@ -12,7 +12,9 @@ use sui_types::{ base_types::{ObjectID, TransactionDigest}, crypto::Signature, gas_coin::GasCoin, - messages::{CallArg, CertifiedTransaction, SignatureAggregator, Transaction, TransactionData}, + messages::{ + CallArg, CertifiedTransaction, ObjectArg, SignatureAggregator, Transaction, TransactionData, + }, object::{MoveObject, Object, Owner, OBJECT_START_VERSION}, }; use test_utils::test_keys; @@ -60,7 +62,7 @@ pub async fn test_certificates(authority: &AuthorityState) -> Vec SuiResult { ident_str!("delete").to_owned(), vec![], gas_object.compute_object_reference(), - vec![CallArg::ImmOrOwnedObject(created_object_ref)], + vec![CallArg::Object(ObjectArg::ImmOrOwnedObject( + created_object_ref, + ))], expected_gas_balance, ); let signature = Signature::new(&data, &sender_key); diff --git a/crates/sui-core/tests/staged/sui.yaml b/crates/sui-core/tests/staged/sui.yaml index f82204bd7daba..9d6131a9a34b8 100644 --- a/crates/sui-core/tests/staged/sui.yaml +++ b/crates/sui-core/tests/staged/sui.yaml @@ -38,16 +38,9 @@ CallArg: NEWTYPE: SEQ: U8 1: - ImmOrOwnedObject: + Object: NEWTYPE: - TUPLE: - - TYPENAME: ObjectID - - TYPENAME: SequenceNumber - - TYPENAME: ObjectDigest - 2: - SharedObject: - NEWTYPE: - TYPENAME: ObjectID + TYPENAME: ObjectArg ChangeEpoch: STRUCT: - epoch: U64 @@ -179,6 +172,19 @@ MoveTypeLayout: TYPENAME: MoveStructLayout 7: signer: UNIT +ObjectArg: + ENUM: + 0: + ImmOrOwnedObject: + NEWTYPE: + TUPLE: + - TYPENAME: ObjectID + - TYPENAME: SequenceNumber + - TYPENAME: ObjectDigest + 1: + SharedObject: + NEWTYPE: + TYPENAME: ObjectID ObjectDigest: NEWTYPESTRUCT: BYTES ObjectFormatOptions: diff --git a/crates/sui-json-rpc-api/src/rpc_types.rs b/crates/sui-json-rpc-api/src/rpc_types.rs index 71ae662901ad8..5c25a90259005 100644 --- a/crates/sui-json-rpc-api/src/rpc_types.rs +++ b/crates/sui-json-rpc-api/src/rpc_types.rs @@ -35,7 +35,7 @@ use sui_types::event::{Event, TransferType}; use sui_types::gas::GasCostSummary; use sui_types::gas_coin::GasCoin; use sui_types::messages::{ - CallArg, CertifiedTransaction, ExecutionStatus, InputObjectKind, MoveModulePublish, + CallArg, CertifiedTransaction, ExecutionStatus, InputObjectKind, MoveModulePublish, ObjectArg, SingleTransactionKind, TransactionData, TransactionEffects, TransactionKind, }; use sui_types::move_package::disassemble_modules; @@ -914,10 +914,10 @@ impl TryFrom for SuiTransactionKind { .into_iter() .map(|arg| match arg { CallArg::Pure(p) => SuiJsonValue::from_bcs_bytes(&p), - CallArg::ImmOrOwnedObject((id, _, _)) => { + CallArg::Object(ObjectArg::ImmOrOwnedObject((id, _, _))) => { SuiJsonValue::new(Value::String(id.to_hex_literal())) } - CallArg::SharedObject(id) => { + CallArg::Object(ObjectArg::SharedObject(id)) => { SuiJsonValue::new(Value::String(id.to_hex_literal())) } }) diff --git a/crates/sui-transactional-test-runner/src/args.rs b/crates/sui-transactional-test-runner/src/args.rs index 6ad646b3daedf..7f470f882a209 100644 --- a/crates/sui-transactional-test-runner/src/args.rs +++ b/crates/sui-transactional-test-runner/src/args.rs @@ -8,7 +8,7 @@ use move_command_line_common::{parser::Parser as MoveCLParser, values::ValueToke use move_compiler::shared::parse_u128; use move_core_types::identifier::Identifier; use move_core_types::value::{MoveStruct, MoveValue}; -use sui_types::messages::CallArg; +use sui_types::messages::{CallArg, ObjectArg}; use crate::test_adapter::SuiTestAdapter; @@ -92,10 +92,10 @@ impl SuiValue { None => bail!("INVALID TEST. Could not load object argument {}", id), }; if obj.is_shared() { - CallArg::SharedObject(id) + CallArg::Object(ObjectArg::SharedObject(id)) } else { let obj_ref = obj.compute_object_reference(); - CallArg::ImmOrOwnedObject(obj_ref) + CallArg::Object(ObjectArg::ImmOrOwnedObject(obj_ref)) } } SuiValue::MoveValue(v) => CallArg::Pure(v.simple_serialize().unwrap()), diff --git a/crates/sui-types/src/messages.rs b/crates/sui-types/src/messages.rs index f8af11f446a54..8a32bfef5db31 100644 --- a/crates/sui-types/src/messages.rs +++ b/crates/sui-types/src/messages.rs @@ -42,7 +42,13 @@ mod messages_tests; pub enum CallArg { // contains no structs or objects Pure(Vec), + // an object + Object(ObjectArg), // TODO support more than one object (object vector of some sort) +} + +#[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)] +pub enum ObjectArg { // A Move object, either immutable, or owned mutable. ImmOrOwnedObject(ObjectRef), // A Move object that's shared and mutable. @@ -123,8 +129,8 @@ impl SingleTransactionKind { match &self { Self::Call(MoveCall { arguments, .. }) => { Either::Left(arguments.iter().filter_map(|arg| match arg { - CallArg::Pure(_) | CallArg::ImmOrOwnedObject(_) => None, - CallArg::SharedObject(id) => Some(id), + CallArg::Pure(_) | CallArg::Object(ObjectArg::ImmOrOwnedObject(_)) => None, + CallArg::Object(ObjectArg::SharedObject(id)) => Some(id), })) } _ => Either::Right(std::iter::empty()), @@ -146,10 +152,12 @@ impl SingleTransactionKind { .iter() .filter_map(|arg| match arg { CallArg::Pure(_) => None, - CallArg::ImmOrOwnedObject(object_ref) => { + CallArg::Object(ObjectArg::ImmOrOwnedObject(object_ref)) => { Some(InputObjectKind::ImmOrOwnedMoveObject(*object_ref)) } - CallArg::SharedObject(id) => Some(InputObjectKind::SharedMoveObject(*id)), + CallArg::Object(ObjectArg::SharedObject(id)) => { + Some(InputObjectKind::SharedMoveObject(*id)) + } }) .chain([InputObjectKind::MovePackage(package.0)]) .collect(), diff --git a/crates/sui/src/benchmark/transaction_creator.rs b/crates/sui/src/benchmark/transaction_creator.rs index f1a2833d18e39..f2293fbe084d1 100644 --- a/crates/sui/src/benchmark/transaction_creator.rs +++ b/crates/sui/src/benchmark/transaction_creator.rs @@ -36,7 +36,7 @@ fn make_transfer_transaction( function: ident_str!("transfer").to_owned(), type_arguments: Vec::new(), arguments: vec![ - CallArg::ImmOrOwnedObject(object_ref), + CallArg::Object(ObjectArg::ImmOrOwnedObject(object_ref)), CallArg::Pure(bcs::to_bytes(&AccountAddress::from(recipient)).unwrap()), ], }) diff --git a/crates/sui/tests/checkpoints_tests.rs b/crates/sui/tests/checkpoints_tests.rs index ff2ce6be4015e..cf07f619c3a85 100644 --- a/crates/sui/tests/checkpoints_tests.rs +++ b/crates/sui/tests/checkpoints_tests.rs @@ -9,7 +9,7 @@ use sui_core::{ use sui_types::{ base_types::{ExecutionDigests, TransactionDigest}, crypto::get_key_pair_from_rng, - messages::{CallArg, ExecutionStatus}, + messages::{CallArg, ExecutionStatus, ObjectArg}, }; use test_utils::transaction::publish_counter_package; use test_utils::{ @@ -269,7 +269,7 @@ async fn checkpoint_with_shared_objects() { "counter", "increment", package_ref, - vec![CallArg::SharedObject(counter_id)], + vec![CallArg::Object(ObjectArg::SharedObject(counter_id))], ); let replies = submit_shared_object_transaction( increment_counter_transaction.clone(), diff --git a/crates/sui/tests/shared_objects_tests.rs b/crates/sui/tests/shared_objects_tests.rs index bf6e91dd2fddb..99b98b845d3b5 100644 --- a/crates/sui/tests/shared_objects_tests.rs +++ b/crates/sui/tests/shared_objects_tests.rs @@ -4,7 +4,9 @@ use std::sync::Arc; use sui_core::authority_client::AuthorityAPI; use sui_core::gateway_state::{GatewayAPI, GatewayState}; -use sui_types::messages::{CallArg, ExecutionStatus, ObjectInfoRequest, ObjectInfoRequestKind}; +use sui_types::messages::{ + CallArg, ExecutionStatus, ObjectArg, ObjectInfoRequest, ObjectInfoRequestKind, +}; use sui_types::object::OBJECT_START_VERSION; use test_utils::authority::{get_client, test_authority_aggregator}; use test_utils::transaction::{ @@ -98,7 +100,7 @@ async fn call_shared_object_contract() { "assert_value", package_ref, vec![ - CallArg::SharedObject(counter_id), + CallArg::Object(ObjectArg::SharedObject(counter_id)), CallArg::Pure(0u64.to_le_bytes().to_vec()), ], ); @@ -114,7 +116,7 @@ async fn call_shared_object_contract() { "counter", "increment", package_ref, - vec![CallArg::SharedObject(counter_id)], + vec![CallArg::Object(ObjectArg::SharedObject(counter_id))], ); let effects = submit_shared_object_transaction(transaction, &configs.validator_set()[0..1]) .await @@ -129,7 +131,7 @@ async fn call_shared_object_contract() { "assert_value", package_ref, vec![ - CallArg::SharedObject(counter_id), + CallArg::Object(ObjectArg::SharedObject(counter_id)), CallArg::Pure(1u64.to_le_bytes().to_vec()), ], ); @@ -177,7 +179,7 @@ async fn shared_object_flood() { "assert_value", package_ref, vec![ - CallArg::SharedObject(counter_id), + CallArg::Object(ObjectArg::SharedObject(counter_id)), CallArg::Pure(0u64.to_le_bytes().to_vec()), ], ); @@ -193,7 +195,7 @@ async fn shared_object_flood() { "counter", "increment", package_ref, - vec![CallArg::SharedObject(counter_id)], + vec![CallArg::Object(ObjectArg::SharedObject(counter_id))], ); let effects = submit_shared_object_transaction(transaction, configs.validator_set()) .await @@ -208,7 +210,7 @@ async fn shared_object_flood() { "assert_value", package_ref, vec![ - CallArg::SharedObject(counter_id), + CallArg::Object(ObjectArg::SharedObject(counter_id)), CallArg::Pure(1u64.to_le_bytes().to_vec()), ], ); @@ -278,7 +280,7 @@ async fn shared_object_sync() { "counter", "increment", package_ref, - vec![CallArg::SharedObject(counter_id)], + vec![CallArg::Object(ObjectArg::SharedObject(counter_id))], ); // Let's submit the transaction to the first authority (the only one up-to-date). @@ -409,7 +411,8 @@ async fn shared_object_on_gateway() { "counter", "increment", package_ref, - /* arguments */ vec![CallArg::SharedObject(shared_object_id)], + /* arguments */ + vec![CallArg::Object(ObjectArg::SharedObject(shared_object_id))], ); async move { g.execute_transaction(increment_counter_transaction).await } }) @@ -431,7 +434,7 @@ async fn shared_object_on_gateway() { "assert_value", package_ref, vec![ - CallArg::SharedObject(shared_object_id), + CallArg::Object(ObjectArg::SharedObject(shared_object_id)), CallArg::Pure((increment_amount as u64).to_le_bytes().to_vec()), ], ); diff --git a/crates/test-utils/src/messages.rs b/crates/test-utils/src/messages.rs index f8550fe0fdeb1..a1baf6a43a7b3 100644 --- a/crates/test-utils/src/messages.rs +++ b/crates/test-utils/src/messages.rs @@ -10,7 +10,8 @@ use std::path::PathBuf; use sui_adapter::genesis; use sui_types::base_types::ObjectRef; use sui_types::messages::{ - CertifiedTransaction, SignatureAggregator, SignedTransaction, Transaction, TransactionData, + CertifiedTransaction, ObjectArg, SignatureAggregator, SignedTransaction, Transaction, + TransactionData, }; use sui_types::object::Object; use sui_types::{base_types::SuiAddress, crypto::Signature}; @@ -100,7 +101,7 @@ pub fn test_shared_object_transactions() -> Vec { gas_object.compute_object_reference(), /* args */ vec![ - CallArg::SharedObject(shared_object_id), + CallArg::Object(ObjectArg::SharedObject(shared_object_id)), CallArg::Pure(16u64.to_le_bytes().to_vec()), CallArg::Pure(bcs::to_bytes(&AccountAddress::from(sender)).unwrap()), ],