diff --git a/crates/sui-graphql-rpc/src/types/dot_move/named_type.rs b/crates/sui-graphql-rpc/src/types/dot_move/named_type.rs index 14b41af424639e..3d1e953c670764 100644 --- a/crates/sui-graphql-rpc/src/types/dot_move/named_type.rs +++ b/crates/sui-graphql-rpc/src/types/dot_move/named_type.rs @@ -2,17 +2,18 @@ // SPDX-License-Identifier: Apache-2.0 use std::collections::HashMap; +use std::str::FromStr; use async_graphql::Context; use futures::future; -use move_core_types::parser::parse_type_tag; -use sui_types::base_types::ObjectID; +use regex::{Captures, Regex}; +use sui_types::{base_types::ObjectID, TypeTag}; use crate::error::Error; use super::{ - config::{DotMoveServiceError, VERSIONED_NAME_UNBOUND_REG}, - named_move_package::NamedMovePackage, + error::MoveRegistryError, named_move_package::NamedMovePackage, + on_chain::VERSIONED_NAME_UNBOUND_REG, }; pub(crate) struct NamedType; @@ -22,7 +23,7 @@ impl NamedType { ctx: &Context<'_>, name: &str, checkpoint_viewed_at: u64, - ) -> Result { + ) -> Result { // we do not de-duplicate the names here, as the dataloader will do this for us. let names = Self::parse_names(name)?; @@ -41,18 +42,20 @@ impl NamedType { // doing it in reverse so we can pop instead of shift for name in names.into_iter().rev() { - // safe unwrap: we know that the amount of results has to equal the amount of names. + // SAFETY: we know that the amount of results has to equal the amount of names. let Some(package) = results.pop().unwrap() else { - return Err(Error::DotMove(DotMoveServiceError::NameNotFound(name))); + return Err(Error::MoveNameRegistry(MoveRegistryError::NameNotFound( + name, + ))); }; name_package_id_mapping.insert(name, package.native.id()); } - let correct_type_tag = Self::replace_names(name, &name_package_id_mapping); + let correct_type_tag: String = Self::replace_names(name, &name_package_id_mapping)?; // now we query the names with futures to utilize data loader - Ok(correct_type_tag) + TypeTag::from_str(&correct_type_tag).map_err(|e| Error::Client(format!("bad type: {e}"))) } // TODO: Should we introduce some overall string limit length here? @@ -64,7 +67,7 @@ impl NamedType { fn parse_names(name: &str) -> Result, Error> { let mut names = vec![]; let struct_tag = VERSIONED_NAME_UNBOUND_REG.replace_all(name, |m: ®ex::Captures| { - // safe unwrap: we know that the regex will always have a match on position 0. + // SAFETY: we know that the regex will always have a match on position 0. names.push(m.get(0).unwrap().as_str().to_string()); "0x0".to_string() }); @@ -72,7 +75,7 @@ impl NamedType { // We attempt to parse the type_tag with these replacements, to make sure there are no other // errors in the type tag (apart from the move names). That protects us from unnecessary // queries to resolve .move names, for a type tag that will be invalid anyway. - parse_type_tag(&struct_tag).map_err(|e| Error::Client(e.to_string()))?; + TypeTag::from_str(&struct_tag).map_err(|e| Error::Client(e.to_string()))?; Ok(names) } @@ -80,25 +83,51 @@ impl NamedType { // This function replaces all the names in the type tag with their corresponding MovePackage address. // The names are guaranteed to be the same and exist (as long as this is called in sequence), // since we use the same parser to extract the names. - fn replace_names(type_name: &str, names: &HashMap) -> String { - let struct_tag_str = - VERSIONED_NAME_UNBOUND_REG.replace_all(type_name, |m: ®ex::Captures| { - // safe unwrap: we know that the regex will have a match on position 0. + fn replace_names(type_name: &str, names: &HashMap) -> Result { + let struct_tag_str = replace_all( + &VERSIONED_NAME_UNBOUND_REG, + type_name, + |m: ®ex::Captures| { + // SAFETY: we know that the regex will have a match on position 0. let name = m.get(0).unwrap().as_str(); // if we are miss-using the function, and we cannot find the name in the hashmap, // we return an empty string, which will make the type tag invalid. if let Some(addr) = names.get(name) { - addr.to_string() + Ok(addr.to_string()) } else { - "".to_string() + Err(Error::MoveNameRegistry(MoveRegistryError::NameNotFound( + name.to_string(), + ))) } - }); + }, + )?; - struct_tag_str.to_string() + Ok(struct_tag_str.to_string()) } } +// Helper to replace all occurrences of a regex with a function that returns a string. +// Used as a replacement of `regex`.replace_all(). +// The only difference is that this function returns a Result, so we can handle errors. +fn replace_all( + re: &Regex, + haystack: &str, + replacement: impl Fn(&Captures) -> Result, +) -> Result { + let mut new = String::with_capacity(haystack.len()); + let mut last_match = 0; + for caps in re.captures_iter(haystack) { + // SAFETY: we know that the regex will have a match on position 0. + let m = caps.get(0).unwrap(); + new.push_str(&haystack[last_match..m.start()]); + new.push_str(&replacement(&caps)?); + last_match = m.end(); + } + new.push_str(&haystack[last_match..]); + Ok(new) +} + #[cfg(test)] mod tests { use std::collections::HashMap; @@ -169,7 +198,7 @@ mod tests { } let replaced = NamedType::replace_names(&data.input_type, &mapping); - assert_eq!(replaced, data.expected_output); + assert_eq!(replaced.unwrap(), data.expected_output); } } diff --git a/crates/sui-graphql-rpc/src/types/query.rs b/crates/sui-graphql-rpc/src/types/query.rs index bc3db8fb1d5e16..0714e5a3fab06b 100644 --- a/crates/sui-graphql-rpc/src/types/query.rs +++ b/crates/sui-graphql-rpc/src/types/query.rs @@ -526,11 +526,7 @@ impl Query { let Watermark { checkpoint, .. } = *ctx.data()?; let type_tag = NamedType::query(ctx, &name, checkpoint).await?; - Ok(MoveType::new( - TypeTag::from_str(&type_tag) - .map_err(|e| Error::Client(format!("Bad type: {e}"))) - .extend()?, - )) + Ok(MoveType::new(type_tag)) } /// The coin metadata associated with the given coin type.