Skip to content

Commit

Permalink
[GraphQL/MoveType] Represent using TypeTag (MystenLabs#15044)
Browse files Browse the repository at this point in the history
##  Description

Previously `MoveType` was represented using its string representation.
This meant that everywhere we needed to create a `MoveType`, we had to
be careful to ensure we canonicalised the string representation first.
This PR flips things around so that the native representation relies on
`TypeTag` and we derive the canonical form of it when requested (by the
accessing the `repr` field).

This makes it harder (or impossible) to accidentally return a
non-canonical type in a query response.

## Test Plan

```
sui-graphql-rpc$ cargo nextest run
sui-graphql-e2e-tests$ cargo nextest run -j 1 --features pg_integration
```

##  Stack

- MystenLabs#14929 
- MystenLabs#14930 
- MystenLabs#14934
- MystenLabs#14935 
- MystenLabs#14961 
- MystenLabs#14974
- MystenLabs#15013
- MystenLabs#15014
- MystenLabs#15015
- MystenLabs#15016
- MystenLabs#15018
- MystenLabs#15020
- MystenLabs#15021
- MystenLabs#15036 
- MystenLabs#15037 
- MystenLabs#15038 
- MystenLabs#15039 
- MystenLabs#15040
- MystenLabs#15041
- MystenLabs#15042 
- MystenLabs#15043
  • Loading branch information
amnn authored Dec 1, 2023
1 parent 7b749b3 commit 9c4e8fb
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 91 deletions.
89 changes: 46 additions & 43 deletions crates/sui-graphql-rpc/src/context_data/db_data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -899,24 +899,27 @@ impl PgManager {
coin_type: Option<String>,
) -> Result<Option<Balance>, Error> {
let address = address.into_vec();
let coin_type = parse_to_type_tag(coin_type)
.map_err(|e| Error::InvalidCoinType(e.to_string()))?
.to_canonical_string(/* with_prefix */ true);
let result = self.get_balance(address, coin_type).await?;
let coin_type = parse_to_type_tag(coin_type.clone())
.map_err(|e| Error::InvalidCoinType(e.to_string()))?;
let result = self
.get_balance(
address,
coin_type.to_canonical_string(/* with_prefix */ true),
)
.await?;

match result {
Some(result) => match result {
(Some(balance), Some(count), Some(coin_type)) => Ok(Some(Balance {
coin_object_count: Some(count as u64),
total_balance: Some(BigInt::from(balance)),
coin_type: Some(MoveType::new(coin_type)),
})),
(None, None, None) => Ok(None),
_ => Err(Error::Internal(
"Expected fields are missing on balance calculation".to_string(),
)),
},
None => Ok(None),
None | Some((None, None, None)) => Ok(None),

Some((Some(balance), Some(count), Some(_coin_type))) => Ok(Some(Balance {
coin_object_count: Some(count as u64),
total_balance: Some(BigInt::from(balance)),
coin_type: Some(MoveType::new(coin_type)),
})),

_ => Err(Error::Internal(
"Expected fields are missing on balance calculation".to_string(),
)),
}
}

Expand All @@ -929,33 +932,35 @@ impl PgManager {
before: Option<String>,
) -> Result<Option<Connection<String, Balance>>, Error> {
let address = address.into_vec();

let balances = self
let Some(balances) = self
.multi_get_balances(address, first, after, last, before)
.await?;
.await?
else {
return Ok(None);
};

if let Some(balances) = balances {
let mut connection = Connection::new(false, false);
for (balance, count, coin_type) in balances {
if let (Some(balance), Some(count), Some(coin_type)) = (balance, count, coin_type) {
connection.edges.push(Edge::new(
coin_type.clone(),
Balance {
coin_object_count: Some(count as u64),
total_balance: Some(BigInt::from(balance)),
coin_type: Some(MoveType::new(coin_type)),
},
));
} else {
return Err(Error::Internal(
"Expected fields are missing on balance calculation".to_string(),
));
}
}
Ok(Some(connection))
} else {
Ok(None)
let mut connection = Connection::new(false, false);
for (balance, count, coin_type) in balances {
let (Some(balance), Some(count), Some(coin_type)) = (balance, count, coin_type) else {
return Err(Error::Internal(
"Expected fields are missing on balance calculation".to_string(),
));
};

let coin_tag = TypeTag::from_str(&coin_type)
.map_err(|e| Error::Internal(format!("Error parsing type '{coin_type}': {e}")))?;

connection.edges.push(Edge::new(
coin_type.clone(),
Balance {
coin_object_count: Some(count as u64),
total_balance: Some(BigInt::from(balance)),
coin_type: Some(MoveType::new(coin_tag)),
},
));
}

Ok(Some(connection))
}

/// Fetches all coins owned by the given address that match the given coin type.
Expand Down Expand Up @@ -1268,9 +1273,7 @@ impl PgManager {
let event = Event {
sending_package: SuiAddress::from(e.package_id),
sending_module: e.transaction_module.to_string(),
event_type: Some(MoveType::new(
e.type_.to_canonical_string(/* with_prefix */ true),
)),
event_type: Some(MoveType::new(TypeTag::from(e.type_.clone()))),
senders: Some(vec![Address {
address: SuiAddress::from_array(e.sender.to_inner()),
}]),
Expand Down
6 changes: 1 addition & 5 deletions crates/sui-graphql-rpc/src/types/balance_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ impl BalanceChange {

/// The inner type of the coin whose balance has changed (e.g. `0x2::sui::SUI`).
async fn coin_type(&self) -> Option<MoveType> {
Some(MoveType::new(
self.stored
.coin_type
.to_canonical_string(/* with_prefix */ true),
))
Some(MoveType::new(self.stored.coin_type.clone()))
}

/// The signed balance change.
Expand Down
7 changes: 2 additions & 5 deletions crates/sui-graphql-rpc/src/types/dynamic_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,7 @@ impl DynamicField {
.map_err(|e| Error::Internal(format!("Failed to serialize object: {e}")))
.extend()?;

Ok(Some(MoveValue::new(
type_tag.to_canonical_string(true),
Base64::from(bcs),
)))
Ok(Some(MoveValue::new(type_tag, Base64::from(bcs))))
}

/// The actual data stored in the dynamic field.
Expand Down Expand Up @@ -118,7 +115,7 @@ impl DynamicField {
.extend()?;

Ok(Some(DynamicFieldValue::MoveValue(MoveValue::new(
type_tag.to_canonical_string(true),
type_tag,
Base64::from(bcs),
))))
}
Expand Down
9 changes: 3 additions & 6 deletions crates/sui-graphql-rpc/src/types/move_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use super::{coin::Coin, object::Object};
use crate::error::Error;
use crate::types::stake::StakedSui;
use async_graphql::*;
use move_core_types::language_storage::StructTag;
use sui_types::object::{Data, MoveObject as NativeMoveObject};
use sui_types::TypeTag;

#[derive(Clone)]
pub(crate) struct MoveObject {
Expand All @@ -30,11 +30,8 @@ impl MoveObject {
/// provides the flat representation of the type signature, and the bcs of the corresponding
/// data
async fn contents(&self) -> Option<MoveValue> {
let type_ = StructTag::from(self.native.type_().clone());
Some(MoveValue::new(
type_.to_canonical_string(/* with_prefix */ true),
self.native.contents().into(),
))
let type_ = TypeTag::from(self.native.type_().clone());
Some(MoveValue::new(type_, self.native.contents().into()))
}

/// Determines whether a tx can transfer this object
Expand Down
49 changes: 21 additions & 28 deletions crates/sui-graphql-rpc/src/types/move_type.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use std::str::FromStr;

use crate::context_data::package_cache::PackageCache;
use async_graphql::*;
use move_core_types::{annotated_value as A, language_storage::TypeTag};
Expand All @@ -11,12 +9,9 @@ use sui_package_resolver::Resolver;

use crate::error::Error;

/// Represents concrete types (no type parameters, no references)
#[derive(SimpleObject, Clone, Debug, PartialEq, Eq)]
#[graphql(complex)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct MoveType {
/// Flat representation of the type signature, as a displayable string.
repr: String,
native: TypeTag,
}

scalar!(
Expand Down Expand Up @@ -96,8 +91,14 @@ pub(crate) struct MoveFieldLayout {
layout: MoveTypeLayout,
}

#[ComplexObject]
/// Represents concrete types (no type parameters, no references)
#[Object]
impl MoveType {
/// Flat representation of the type signature, as a displayable string.
async fn repr(&self) -> String {
self.native.to_canonical_string(/* with_prefix */ true)
}

/// Structured representation of the type signature.
async fn signature(&self) -> Result<MoveTypeSignature> {
// Factor out into its own non-GraphQL, non-async function for better testability
Expand All @@ -116,30 +117,28 @@ impl MoveType {
}

impl MoveType {
pub(crate) fn new(repr: String) -> MoveType {
Self { repr }
pub(crate) fn new(native: TypeTag) -> MoveType {
Self { native }
}

fn signature_impl(&self) -> Result<MoveTypeSignature, Error> {
MoveTypeSignature::try_from(self.native_type_tag()?)
MoveTypeSignature::try_from(self.native.clone())
}

pub(crate) async fn layout_impl(
&self,
resolver: &Resolver<PackageCache>,
) -> Result<A::MoveTypeLayout, Error> {
resolver
.type_layout(self.native_type_tag()?)
.type_layout(self.native.clone())
.await
.map_err(|e| {
Error::Internal(format!("Error calculating layout for {}: {e}", self.repr))
Error::Internal(format!(
"Error calculating layout for {}: {e}",
self.native.to_canonical_display(/* with_prefix */ true),
))
})
}

fn native_type_tag(&self) -> Result<TypeTag, Error> {
TypeTag::from_str(&self.repr)
.map_err(|e| Error::Internal(format!("Error parsing type '{}': {e}", self.repr)))
}
}

impl TryFrom<TypeTag> for MoveTypeSignature {
Expand Down Expand Up @@ -227,12 +226,15 @@ pub(crate) fn unexpected_signer_error() -> Error {

#[cfg(test)]
mod tests {
use std::str::FromStr;

use super::*;

use expect_test::expect;

fn signature(repr: impl Into<String>) -> Result<MoveTypeSignature, Error> {
MoveType::new(repr.into()).signature_impl()
let tag = TypeTag::from_str(repr.into().as_str()).unwrap();
MoveType::new(tag).signature_impl()
}

#[test]
Expand All @@ -255,15 +257,6 @@ mod tests {
expect.assert_eq(&format!("{sig:#?}"));
}

#[test]
fn tag_parse_error() {
let err = signature("not_a_type").unwrap_err();
let expect = expect![[
r#"Internal("Error parsing type 'not_a_type': unexpected token Name(\"not_a_type\"), expected type tag")"#
]];
expect.assert_eq(&format!("{err:?}"));
}

#[test]
fn signer_type() {
let err = signature("signer").unwrap_err();
Expand Down
9 changes: 5 additions & 4 deletions crates/sui-graphql-rpc/src/types/move_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl MoveValue {
}

impl MoveValue {
pub fn new(repr: String, bcs: Base64) -> Self {
let type_ = MoveType::new(repr);
pub fn new(tag: TypeTag, bcs: Base64) -> Self {
let type_ = MoveType::new(tag);
Self { type_, bcs }
}

Expand Down Expand Up @@ -437,14 +437,15 @@ mod tests {
layout: A::MoveTypeLayout,
data: T,
) -> Result<MoveData, Error> {
let type_ = MoveType::new(tag.into());
let tag = TypeTag::from_str(tag.into().as_str()).unwrap();
let type_ = MoveType::new(tag);
let bcs = Base64(bcs::to_bytes(&data).unwrap());
MoveValue { type_, bcs }.data_impl(layout)
}

fn json<T: Serialize>(layout: A::MoveTypeLayout, data: T) -> Result<Json, Error> {
let tag: TypeTag = (&layout).try_into().expect("Error fetching type tag");
let type_ = MoveType::new(tag.to_canonical_string(/* with_prefix */ true));
let type_ = MoveType::new(tag);
let bcs = Base64(bcs::to_bytes(&data).unwrap());
MoveValue { type_, bcs }.json_impl(layout)
}
Expand Down

0 comments on commit 9c4e8fb

Please sign in to comment.