Skip to content

Commit

Permalink
[future proof/hardening] Hide internals of MoveObjectType (MystenLabs…
Browse files Browse the repository at this point in the history
…#9964)

## Description 

Made the enum for MoveObjectType private to avoid manually constructing
an Other. This is potentially an issue as the type does not support
equality currently over similar types, e.g. `GasCoin !=
Other(GasCoin::type_())`.
In other words, this is more about trying to make sure buggy code isn't
introduced in the future.

## Test Plan 

- Internal change 

---
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 35bbdc5 commit 197d436
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 101 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1132,8 +1132,9 @@ fn build_move_args<S: StorageView, Mode: ExecutionMode>(
let TypeTag::Struct(struct_tag) = type_tag else {
invariant_violation!("Struct type make a non struct type tag")
};
let type_ = (*struct_tag).into();
ValueKind::Object {
type_: MoveObjectType::Other(*struct_tag),
type_,
has_public_transfer: *has_public_transfer,
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-core/src/generate_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use move_core_types::{
use pretty_assertions::assert_str_eq;
use serde_reflection::{Registry, Result, Samples, Tracer, TracerConfig};
use std::{fs::File, io::Write};
use sui_types::crypto::Signer;
use sui_types::{base_types::MoveObjectType_, crypto::Signer};
use sui_types::{
base_types::{
self, MoveObjectType, ObjectDigest, ObjectID, TransactionDigest, TransactionEffectsDigest,
Expand Down Expand Up @@ -86,6 +86,7 @@ fn get_registry() -> Result<Registry> {
tracer.trace_type::<MoveStructLayout>(&samples)?;
tracer.trace_type::<MoveTypeLayout>(&samples)?;
tracer.trace_type::<MoveObjectType>(&samples)?;
tracer.trace_type::<MoveObjectType_>(&samples)?;
tracer.trace_type::<base_types::SuiAddress>(&samples)?;
tracer.trace_type::<DeleteKind>(&samples)?;
tracer.trace_type::<Argument>(&samples)?;
Expand Down
3 changes: 3 additions & 0 deletions crates/sui-core/tests/staged/sui.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,9 @@ MoveObject:
TYPENAME: SequenceNumber
- contents: BYTES
MoveObjectType:
NEWTYPESTRUCT:
TYPENAME: MoveObjectType_
MoveObjectType_:
ENUM:
0:
Other:
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-json-rpc-types/src/sui_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use sui_types::sui_serde::SuiStructTag;

use sui_protocol_config::ProtocolConfig;
use sui_types::base_types::{
MoveObjectType, ObjectDigest, ObjectID, ObjectInfo, ObjectRef, ObjectType, SequenceNumber,
SuiAddress, TransactionDigest,
ObjectDigest, ObjectID, ObjectInfo, ObjectRef, ObjectType, SequenceNumber, SuiAddress,
TransactionDigest,
};
use sui_types::error::{SuiObjectResponseError, UserInputError, UserInputResult};
use sui_types::gas_coin::GasCoin;
Expand Down Expand Up @@ -209,7 +209,7 @@ impl SuiObjectData {

pub fn is_gas_coin(&self) -> bool {
match self.type_.as_ref() {
Some(ObjectType::Struct(MoveObjectType::GasCoin)) => true,
Some(ObjectType::Struct(ty)) if ty.is_gas_coin() => true,
Some(_) => false,
None => false,
}
Expand Down
19 changes: 7 additions & 12 deletions crates/sui-json-rpc/src/balance_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use tokio::sync::RwLock;

use sui_core::authority::AuthorityState;
use sui_json_rpc_types::BalanceChange;
use sui_types::base_types::{MoveObjectType, ObjectID, ObjectRef, SequenceNumber};
use sui_types::base_types::{ObjectID, ObjectRef, SequenceNumber};
use sui_types::coin::Coin;
use sui_types::error::SuiError;
use sui_types::gas_coin::GAS;
Expand Down Expand Up @@ -98,20 +98,15 @@ async fn fetch_coins<P: ObjectProvider<Error = E>, E>(
// TODO: use multi get object
if let Ok(o) = object_provider.get_object(id, version).await {
if let Some(type_) = o.type_() {
match type_ {
MoveObjectType::GasCoin => all_mutated_coins.push((
if type_.is_coin() {
let [coin_type]: [TypeTag; 1] =
type_.clone().into_type_params().try_into().unwrap();
all_mutated_coins.push((
o.owner,
GAS::type_tag(),
coin_type,
// we know this is a coin, safe to unwrap
Coin::extract_balance_if_coin(&o).unwrap().unwrap(),
)),
MoveObjectType::Coin(coin_type) => all_mutated_coins.push((
o.owner,
coin_type.clone(),
// we know this is a coin, safe to unwrap
Coin::extract_balance_if_coin(&o).unwrap().unwrap(),
)),
_ => {}
))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-json-rpc/src/governance_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl GovernanceReadApi {
async fn get_staked_sui(&self, owner: SuiAddress) -> Result<Vec<StakedSui>, Error> {
Ok(self
.state
.get_move_objects(owner, MoveObjectType::StakedSui)
.get_move_objects(owner, MoveObjectType::staked_sui())
.await?)
}

Expand Down
20 changes: 4 additions & 16 deletions crates/sui-json-rpc/src/object_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
use std::collections::BTreeMap;

use sui_json_rpc_types::ObjectChange;
use sui_types::base_types::{MoveObjectType, ObjectID, ObjectRef, SequenceNumber, SuiAddress};
use sui_types::coin::Coin;
use sui_types::gas_coin::GasCoin;
use sui_types::governance::StakedSui;
use sui_types::base_types::{ObjectID, ObjectRef, SequenceNumber, SuiAddress};
use sui_types::object::Owner;
use sui_types::storage::{DeleteKind, WriteKind};

Expand All @@ -30,12 +27,7 @@ pub async fn get_object_changes<P: ObjectProvider<Error = E>, E>(
for ((id, version, digest), owner, kind) in all_changed_objects {
let o = object_provider.get_object(id, version).await?;
if let Some(type_) = o.type_() {
let object_type = match type_ {
MoveObjectType::Other(type_) => type_.clone(),
MoveObjectType::StakedSui => StakedSui::type_(),
MoveObjectType::GasCoin => GasCoin::type_(),
MoveObjectType::Coin(t) => Coin::type_(t.clone()),
};
let object_type = type_.clone().into();

match kind {
WriteKind::Mutate => object_changes.push(ObjectChange::Mutated {
Expand Down Expand Up @@ -76,12 +68,8 @@ pub async fn get_object_changes<P: ObjectProvider<Error = E>, E>(
.await?;
if let Some(o) = o {
if let Some(type_) = o.type_() {
let type_ = match type_ {
MoveObjectType::Other(type_) => Some(type_.clone()),
MoveObjectType::StakedSui => Some(StakedSui::type_()),
_ => None,
};
if let Some(object_type) = type_ {
if !type_.is_coin() {
let object_type = type_.clone().into();
match kind {
DeleteKind::Normal => object_changes.push(ObjectChange::Deleted {
sender,
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-open-rpc/src/examples.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ impl RpcExampleProvider {
object_id,
version: SequenceNumber::from_u64(1),
digest: ObjectDigest::new(self.rng.gen()),
type_: Some(ObjectType::Struct(MoveObjectType::GasCoin)),
type_: Some(ObjectType::Struct(MoveObjectType::gas_coin())),
bcs: None,
display: None,
});
Expand Down Expand Up @@ -263,7 +263,7 @@ impl RpcExampleProvider {
object_id,
version: SequenceNumber::from_u64(4),
digest: ObjectDigest::new(self.rng.gen()),
type_: Some(ObjectType::Struct(MoveObjectType::GasCoin)),
type_: Some(ObjectType::Struct(MoveObjectType::gas_coin())),
bcs: None,
display: None,
});
Expand Down Expand Up @@ -313,7 +313,7 @@ impl RpcExampleProvider {
object_id: ObjectID::new(self.rng.gen()),
version: Default::default(),
digest: ObjectDigest::new(self.rng.gen()),
type_: Some(ObjectType::Struct(MoveObjectType::GasCoin)),
type_: Some(ObjectType::Struct(MoveObjectType::gas_coin())),
owner: Some(Owner::AddressOwner(owner)),
previous_transaction: Some(TransactionDigest::new(self.rng.gen())),
storage_rebate: None,
Expand Down
Loading

0 comments on commit 197d436

Please sign in to comment.