Skip to content

Commit

Permalink
[move object type] Fix bad address function (MystenLabs#10096)
Browse files Browse the repository at this point in the history
## Description 

- `fn address` was incorrect for StakedSui
- Fixes issue with the transaction builder

## Test Plan 

- Added tests 

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
tnowacki authored Mar 29, 2023
1 parent e3e6741 commit 2804512
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 18 deletions.
26 changes: 13 additions & 13 deletions crates/sui-transaction-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ use anyhow::{anyhow, bail, ensure, Ok};
use async_trait::async_trait;
use futures::future::join_all;
use move_binary_format::file_format::SignatureToken;
use move_binary_format::file_format_common::VERSION_MAX;
use move_core_types::identifier::Identifier;
use move_core_types::language_storage::{StructTag, TypeTag};

use sui_adapter::adapter::CheckCallArg;
use sui_adapter::adapter::{resolve_and_type_check, CheckCallArg};
use sui_adapter::execution_mode::ExecutionMode;
use sui_json::{resolve_move_function_args, ResolvedCallArg, SuiJsonValue};
use sui_json_rpc_types::{
Expand Down Expand Up @@ -412,18 +413,17 @@ impl<Mode: ExecutionMode> TransactionBuilder<Mode> {
}
})
}
// FIXME: uncomment once we figure out what is going on.
// let compiled_module = package.deserialize_module(module, VERSION_MAX)?;

// // TODO set the Mode from outside?
// resolve_and_type_check::<Mode>(
// &objects,
// &compiled_module,
// function,
// type_args,
// check_args.clone(),
// false,
// )?;
let compiled_module = package.deserialize_module(module, VERSION_MAX)?;

// TODO set the Mode from outside?
resolve_and_type_check::<Mode>(
&objects,
&compiled_module,
function,
type_args,
check_args.clone(),
false,
)?;
let args = check_args
.into_iter()
.map(|check_arg| match check_arg {
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-types/src/base_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::signature::GenericSignature;
use crate::sui_serde::HexAccountAddress;
use crate::sui_serde::Readable;
use crate::SUI_FRAMEWORK_ADDRESS;
use crate::SUI_SYSTEM_ADDRESS;
use anyhow::anyhow;
use fastcrypto::encoding::decode_bytes_hex;
use fastcrypto::encoding::{Encoding, Hex};
Expand Down Expand Up @@ -138,9 +139,8 @@ pub enum MoveObjectType {
impl MoveObjectType {
pub fn address(&self) -> AccountAddress {
match self {
MoveObjectType::GasCoin | MoveObjectType::StakedSui | MoveObjectType::Coin(_) => {
SUI_FRAMEWORK_ADDRESS
}
MoveObjectType::GasCoin | MoveObjectType::Coin(_) => SUI_FRAMEWORK_ADDRESS,
MoveObjectType::StakedSui => SUI_SYSTEM_ADDRESS,
MoveObjectType::Other(s) => s.address,
}
}
Expand Down
18 changes: 16 additions & 2 deletions crates/sui-types/src/dynamic_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use crate::sui_serde::SuiTypeTag;
use crate::{MoveTypeTagTrait, ObjectID, SequenceNumber, SUI_FRAMEWORK_ADDRESS};
use fastcrypto::encoding::Base58;
use fastcrypto::hash::HashFunction;
use move_core_types::ident_str;
use move_core_types::identifier::IdentStr;
use move_core_types::language_storage::{StructTag, TypeTag};
use move_core_types::value::{MoveStruct, MoveValue};
use schemars::JsonSchema;
Expand All @@ -24,6 +26,9 @@ use shared_crypto::intent::HashingIntentScope;
use std::fmt;
use std::fmt::{Display, Formatter};

const DYNAMIC_FIELD_MODULE_NAME: &IdentStr = ident_str!("dynamic_field");
const DYNAMIC_FIELD_FIELD_STRUCT_NAME: &IdentStr = ident_str!("Field");

/// Rust version of the Move sui::dynamic_field::Field type
#[derive(Clone, Serialize, Deserialize, Debug)]
pub struct Field<N, V> {
Expand Down Expand Up @@ -77,8 +82,17 @@ pub enum DynamicFieldType {
impl DynamicFieldInfo {
pub fn is_dynamic_field(tag: &StructTag) -> bool {
tag.address == SUI_FRAMEWORK_ADDRESS
&& tag.module.as_str() == "dynamic_field"
&& tag.name.as_str() == "Field"
&& tag.module.as_ident_str() == DYNAMIC_FIELD_MODULE_NAME
&& tag.name.as_ident_str() == DYNAMIC_FIELD_FIELD_STRUCT_NAME
}

pub fn dynamic_field_type(key: TypeTag, value: TypeTag) -> StructTag {
StructTag {
address: SUI_FRAMEWORK_ADDRESS,
name: DYNAMIC_FIELD_FIELD_STRUCT_NAME.to_owned(),
module: DYNAMIC_FIELD_MODULE_NAME.to_owned(),
type_params: vec![key, value],
}
}

pub fn try_extract_field_name(tag: &StructTag, type_: &DynamicFieldType) -> SuiResult<TypeTag> {
Expand Down
47 changes: 47 additions & 0 deletions crates/sui-types/src/unit_tests/base_types_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::crypto::{
get_key_pair, get_key_pair_from_bytes, AccountKeyPair, AuthorityKeyPair, AuthoritySignature,
Signature, SuiAuthoritySignature, SuiSignature,
};
use crate::id::{ID, UID};
use crate::OBJECT_START_VERSION;
use crate::{gas_coin::GasCoin, object::Object, SUI_FRAMEWORK_ADDRESS};
use shared_crypto::intent::{Intent, IntentMessage, IntentScope};
Expand Down Expand Up @@ -406,3 +407,49 @@ fn test_address_backwards_compatibility() {
also require updates if they use fixed values generated by the old algorithm."
);
}

// tests translating into and out of a MoveObjectType from a StructTag
#[test]
fn move_object_type_consistency() {
// Tests consistency properties for the relationship between a StructTag and a MoveObjectType
fn assert_consistent(tag: &StructTag) -> MoveObjectType {
let ty: MoveObjectType = tag.clone().into();
// check into/out of the tag works
assert!(ty.is(tag));
let ty_as_tag: StructTag = ty.clone().into();
assert_eq!(&ty_as_tag, tag);
// test same type information
assert_eq!(ty.address(), tag.address);
assert_eq!(ty.module(), tag.module.as_ident_str());
assert_eq!(ty.name(), tag.name.as_ident_str());
assert_eq!(&ty.type_params(), &tag.type_params);
assert_eq!(ty.module_id(), tag.module_id());
// sanity check special cases
assert!(!ty.is_gas_coin() || ty.is_coin());
let cases = [
ty.is_coin(),
ty.is_staked_sui(),
ty.is_coin_metadata(),
ty.is_dynamic_field(),
];
assert!(cases.into_iter().map(|is_ty| is_ty as u8).sum::<u8>() <= 1);
ty
}

let ty = assert_consistent(&GasCoin::type_());
assert!(ty.is_coin());
assert!(ty.is_gas_coin());
let ty = assert_consistent(&StakedSui::type_());
assert!(ty.is_staked_sui());
let ty = assert_consistent(&Coin::type_(TypeTag::U64));
assert!(ty.is_coin());
let ty = assert_consistent(&CoinMetadata::type_(GasCoin::type_()));
assert!(ty.is_coin_metadata());
let ty = assert_consistent(&DynamicFieldInfo::dynamic_field_type(
TypeTag::Struct(Box::new(ID::type_())),
TypeTag::U64,
));
assert!(ty.is_dynamic_field());
assert_consistent(&UID::type_());
assert_consistent(&ID::type_());
}

0 comments on commit 2804512

Please sign in to comment.