Skip to content

Commit

Permalink
[CLI-PTB] Add integer inferrence (#17850)
Browse files Browse the repository at this point in the history
## Description 

Fixes an issue where if a type was not annotated on an integer value we
would set its type to `u64`. This could lead to errors with things like
```
--make-move-vec <u16> [1,2,3,4]
```

This now adds an `InferredNum` argument, and also adds type arguments to
pure value serialization at which point we will select the type for the
inferred num, if no type is provided we default to a `u64`.

This also has the added benefit of performing (lightweight!) type
checking during the building of the PTB, so we will now get build errors
for things like
```
--make-move-vec <std::option::Option<u64>> [some(some(1))]
```

## Test plan 

Added additional tests and updated existing tests.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [X] CLI: Added better type inference for integer types, and added
better build-time errors for invalid value types.
- [ ] Rust SDK:
  • Loading branch information
tzakian committed May 21, 2024
1 parent c98650c commit b958760
Show file tree
Hide file tree
Showing 21 changed files with 576 additions and 167 deletions.
103 changes: 101 additions & 2 deletions crates/sui/src/client_ptb/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ use move_command_line_common::{
types::{ParsedFqName, ParsedModuleId, ParsedStructType, ParsedType},
};
use move_core_types::runtime_value::MoveValue;
use sui_types::{base_types::ObjectID, Identifier};
use sui_types::{
base_types::{ObjectID, RESOLVED_ASCII_STR, RESOLVED_STD_OPTION, RESOLVED_UTF8_STR},
Identifier, TypeTag,
};

use crate::{error, sp};
use crate::{err, error, sp};

use super::error::{PTBResult, Span, Spanned};

Expand Down Expand Up @@ -148,6 +151,7 @@ pub enum Argument {
U64(u64),
U128(u128),
U256(move_core_types::u256::U256),
InferredNum(move_core_types::u256::U256),
Gas,
Identifier(String),
VariableAccess(Spanned<String>, Vec<Spanned<String>>),
Expand All @@ -158,6 +162,72 @@ pub enum Argument {
}

impl Argument {
/// Resolve an `Argument` into a `MoveValue` if possible. Errors if the `Argument` is not
/// convertible to a `MoveValue` of the provided `tag`.
pub fn checked_to_pure_move_value(&self, loc: Span, tag: &TypeTag) -> PTBResult<MoveValue> {
Ok(match (self, tag) {
(Argument::Bool(b), TypeTag::Bool) => MoveValue::Bool(*b),
(Argument::U8(u), TypeTag::U8) => MoveValue::U8(*u),
(Argument::U16(u), TypeTag::U16) => MoveValue::U16(*u),
(Argument::U32(u), TypeTag::U32) => MoveValue::U32(*u),
(Argument::U64(u), TypeTag::U64) => MoveValue::U64(*u),
(Argument::U128(u), TypeTag::U128) => MoveValue::U128(*u),
(Argument::U256(u), TypeTag::U256) => MoveValue::U256(*u),
// Inferred numbers, unless they need to be used as a specific type default to u64.
(Argument::InferredNum(u), tag) => Self::cast_inferrred_num(*u, tag, loc)?,
(Argument::Address(a), TypeTag::Address) => MoveValue::Address(a.into_inner()),
(Argument::Vector(vs), TypeTag::Vector(ty)) => MoveValue::Vector(
vs.iter()
.map(|sp!(loc, v)| v.checked_to_pure_move_value(*loc, ty))
.collect::<PTBResult<Vec<_>>>()
.map_err(|e| {
e.with_help("Literal vectors cannot contain object values.".to_string())
})?,
),
(Argument::String(s), TypeTag::Vector(ty)) if **ty == TypeTag::U8 => {
MoveValue::Vector(s.bytes().map(MoveValue::U8).collect::<Vec<_>>())
}
(Argument::String(s), TypeTag::Struct(stag))
if {
let resolved = (
&stag.address,
stag.module.as_ident_str(),
stag.name.as_ident_str(),
);
resolved == RESOLVED_ASCII_STR || resolved == RESOLVED_UTF8_STR
} =>
{
MoveValue::Vector(s.bytes().map(MoveValue::U8).collect::<Vec<_>>())
}
(Argument::Option(sp!(loc, o)), TypeTag::Struct(stag))
if (
&stag.address,
stag.module.as_ident_str(),
stag.name.as_ident_str(),
) == RESOLVED_STD_OPTION
&& stag.type_params.len() == 1 =>
{
if let Some(v) = o {
let v = v
.as_ref()
.checked_to_pure_move_value(*loc, &stag.type_params[0])
.map_err(|e| {
e.with_help(
"Literal option values cannot contain object values.".to_string(),
)
})?;
MoveValue::Vector(vec![v])
} else {
MoveValue::Vector(vec![])
}
}
(Argument::Identifier(_) | Argument::VariableAccess(_, _) | Argument::Gas, _) => {
error!(loc, "Unable to convert '{self}' to non-object value.")
}
(arg, tag) => error!(loc, "Unable to serialize '{arg}' as a {tag} value"),
})
}

/// Resolve an `Argument` into a `MoveValue` if possible. Errors if the `Argument` is not
/// convertible to a `MoveValue`.
pub fn to_pure_move_value(&self, loc: Span) -> PTBResult<MoveValue> {
Expand All @@ -169,6 +239,8 @@ impl Argument {
Argument::U64(u) => MoveValue::U64(*u),
Argument::U128(u) => MoveValue::U128(*u),
Argument::U256(u) => MoveValue::U256(*u),
// Inferred numbers, unless they need to be used as a specific type default to u64.
Argument::InferredNum(u) => Self::cast_inferrred_num(*u, &TypeTag::U64, loc)?,
Argument::Address(a) => MoveValue::Address(a.into_inner()),
Argument::Vector(vs) => MoveValue::Vector(
vs.iter()
Expand Down Expand Up @@ -198,6 +270,32 @@ impl Argument {
}
})
}

fn cast_inferrred_num(
val: move_core_types::u256::U256,
tag: &TypeTag,
loc: Span,
) -> PTBResult<MoveValue> {
match tag {
TypeTag::U8 => u8::try_from(val)
.map(MoveValue::U8)
.map_err(|_| err!(loc, "Value {val} is too large to be a u8 value")),
TypeTag::U16 => u16::try_from(val)
.map(MoveValue::U16)
.map_err(|_| err!(loc, "Value {val} is too large to be a u16 value")),
TypeTag::U32 => u32::try_from(val)
.map(MoveValue::U32)
.map_err(|_| err!(loc, "Value {val} is too large to be a u32 value")),
TypeTag::U64 => u64::try_from(val)
.map(MoveValue::U64)
.map_err(|_| err!(loc, "Value {val} is too large to be a u64 value")),
TypeTag::U128 => u128::try_from(val)
.map(MoveValue::U128)
.map_err(|_| err!(loc, "Value {val} is too large to be a u128 value")),
TypeTag::U256 => Ok(MoveValue::U256(val)),
_ => error!(loc, "Expected an integer type but got {tag} for '{val}'"),
}
}
}

impl fmt::Display for Argument {
Expand All @@ -210,6 +308,7 @@ impl fmt::Display for Argument {
Argument::U64(u) => write!(f, "{u}u64"),
Argument::U128(u) => write!(f, "{u}u128"),
Argument::U256(u) => write!(f, "{u}u256"),
Argument::InferredNum(u) => write!(f, "{u}"),
Argument::Gas => write!(f, "gas"),
Argument::Identifier(i) => write!(f, "{i}"),
Argument::VariableAccess(sp!(_, head), accesses) => {
Expand Down
55 changes: 42 additions & 13 deletions crates/sui/src/client_ptb/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use move_command_line_common::{
address::{NumericalAddress, ParsedAddress},
parser::NumberFormat,
};
use move_core_types::{account_address::AccountAddress, ident_str, runtime_value::MoveValue};
use move_core_types::{
account_address::AccountAddress, annotated_value::MoveTypeLayout, ident_str,
};
use move_package::BuildConfig;
use std::{collections::BTreeMap, path::PathBuf};
use sui_json::{is_receiving_argument, primitive_type};
Expand Down Expand Up @@ -59,9 +61,10 @@ trait Resolver<'a>: Send {
&mut self,
builder: &mut PTBBuilder<'a>,
loc: Span,
val: MoveValue,
argument: PTBArg,
) -> PTBResult<Tx::Argument> {
builder.ptb.pure(val).map_err(|e| err!(loc, "{e}"))
let value = argument.to_pure_move_value(loc)?;
builder.ptb.pure(value).map_err(|e| err!(loc, "{e}"))
}

async fn resolve_object_id(
Expand Down Expand Up @@ -147,10 +150,34 @@ impl<'a> Resolver<'a> for ToObject {
}

/// A resolver that resolves object IDs that it encounters to pure PTB values.
struct ToPure;
struct ToPure {
type_: TypeTag,
}

impl ToPure {
pub fn new(type_: TypeTag) -> Self {
Self { type_ }
}

pub fn new_from_layout(layout: MoveTypeLayout) -> Self {
Self {
type_: TypeTag::from(&layout),
}
}
}

#[async_trait]
impl<'a> Resolver<'a> for ToPure {
async fn pure(
&mut self,
builder: &mut PTBBuilder<'a>,
loc: Span,
argument: PTBArg,
) -> PTBResult<Tx::Argument> {
let value = argument.checked_to_pure_move_value(loc, &self.type_)?;
builder.ptb.pure(value).map_err(|e| err!(loc, "{e}"))
}

async fn resolve_object_id(
&mut self,
builder: &mut PTBBuilder<'a>,
Expand Down Expand Up @@ -415,12 +442,14 @@ impl<'a> PTBBuilder<'a> {
sp!(loc, arg): Spanned<PTBArg>,
param: &SignatureToken,
) -> PTBResult<Tx::Argument> {
let (is_primitive, _) = primitive_type(view, ty_args, param);
let (is_primitive, layout) = primitive_type(view, ty_args, param);

// If it's a primitive value, see if we've already resolved this argument. Otherwise, we
// need to resolve it.
if is_primitive {
return self.resolve(loc.wrap(arg), ToPure).await;
return self
.resolve(loc.wrap(arg), ToPure::new_from_layout(layout.unwrap()))
.await;
}

// Otherwise it's ambiguous what the value should be, and we need to turn to the signature
Expand Down Expand Up @@ -602,12 +631,10 @@ impl<'a> PTBBuilder<'a> {
| PTBArg::U64(_)
| PTBArg::U128(_)
| PTBArg::U256(_)
| PTBArg::InferredNum(_)
| PTBArg::String(_)
| PTBArg::Option(_)
| PTBArg::Vector(_)) => {
ctx.pure(self, arg_loc, a.to_pure_move_value(arg_loc)?)
.await
}
| PTBArg::Vector(_)) => ctx.pure(self, arg_loc, a).await,
PTBArg::Gas => Ok(Tx::Argument::GasCoin),
// NB: the ordering of these lines is important so that shadowing is properly
// supported.
Expand Down Expand Up @@ -766,7 +793,9 @@ impl<'a> PTBBuilder<'a> {
// let sp!(cmd_span, tok) = &command.name;
match command {
ParsedPTBCommand::TransferObjects(obj_args, to_address) => {
let to_arg = self.resolve(to_address, ToPure).await?;
let to_arg = self
.resolve(to_address, ToPure::new(TypeTag::Address))
.await?;
let mut transfer_args = vec![];
for o in obj_args.value.into_iter() {
let arg = self.resolve(o, ToObject::default()).await?;
Expand Down Expand Up @@ -804,7 +833,7 @@ impl<'a> PTBBuilder<'a> {
let mut vec_args: Vec<Tx::Argument> = vec![];
if is_primitive_type_tag(&ty_arg) {
for arg in args.into_iter() {
let arg = self.resolve(arg, ToPure).await?;
let arg = self.resolve(arg, ToPure::new(ty_arg.clone())).await?;
vec_args.push(arg);
}
} else {
Expand All @@ -822,7 +851,7 @@ impl<'a> PTBBuilder<'a> {
let coin = self.resolve(pre_coin, ToObject::default()).await?;
let mut args = vec![];
for arg in amounts.into_iter() {
let arg = self.resolve(arg, ToPure).await?;
let arg = self.resolve(arg, ToPure::new(TypeTag::U64)).await?;
args.push(arg);
}
let res = self.ptb.command(Tx::Command::SplitCoins(coin, args));
Expand Down
11 changes: 7 additions & 4 deletions crates/sui/src/client_ptb/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ impl<'a, I: Iterator<Item = &'a str>> ProgramParser<'a, I> {
fn parse_gas_budget(&mut self) -> PTBResult<Spanned<u64>> {
Ok(match self.parse_argument()? {
sp!(sp, Argument::U64(u)) => sp.wrap(u),
sp!(sp, Argument::InferredNum(n)) => {
sp.wrap(u64::try_from(n).map_err(|_| err!(sp, "Value does not fit within a u64"))?)
}
sp!(sp, _) => error!(sp, "Expected a u64 value"),
})
}
Expand Down Expand Up @@ -628,10 +631,10 @@ impl<'a, I: Iterator<Item = &'a str>> ProgramParser<'a, I> {
L(T::Ident, A::U128) => parse_num!(parse_u128, V::U128),
L(T::Ident, A::U256) => parse_num!(parse_u256, V::U256),

// If there's no suffix, assume u64, and don't consume the peeked character.
_ => match parse_u64(contents.value) {
Ok((value, _)) => contents.span.wrap(V::U64(value)),
Err(e) => error!(contents.span, "{e}"),
// If there's no suffix, parse as `InferredNum`, and don't consume the peeked character.
_ => match parse_u256(contents.value) {
Ok((value, _)) => contents.span.wrap(V::InferredNum(value)),
Err(_) => error!(contents.span, "Invalid integer literal"),
},
})
}
Expand Down
Loading

0 comments on commit b958760

Please sign in to comment.