From 7488a1fb7d7f45607312fc0410c411001de5572a Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Tue, 18 Nov 2025 15:38:34 -0800 Subject: [PATCH 01/10] mechanisms for index lifting --- .../typing/translate.rs | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index c76e243bf3cf3..11d8985c4ed14 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -12,7 +12,7 @@ use crate::{ }, }; use indexmap::{IndexMap, IndexSet}; -use std::rc::Rc; +use std::{collections::BTreeMap, rc::Rc}; use sui_types::{ base_types::{ObjectRef, TxContextKind}, coin::RESOLVED_COIN_STRUCT, @@ -45,7 +45,10 @@ struct Context { objects: IndexMap, pure: IndexMap<(T::InputIndex, Type), T::PureInput>, receiving: IndexMap<(T::InputIndex, Type), T::ReceivingInput>, - results: Vec, + commands: Vec, + // map from original result to the lifted result + // the map should only be queried via a range, up to the current command index + result_index_lift: BTreeMap, } impl Context { @@ -59,7 +62,8 @@ impl Context { objects: IndexMap::new(), pure: IndexMap::new(), receiving: IndexMap::new(), - results: vec![], + commands: vec![], + result_index_lift: BTreeMap::new(), }; // clone inputs for debug assertions #[cfg(debug_assertions)] @@ -120,12 +124,13 @@ impl Context { Ok(context) } - fn finish(self, commands: T::Commands) -> T::Transaction { + fn finish(self) -> T::Transaction { let Self { bytes, objects, pure, receiving, + commands, .. } = self; let objects = objects.into_iter().map(|(_, o)| o).collect(); @@ -140,6 +145,52 @@ impl Context { } } + fn push_result(&mut self, command: T::Command_) -> Result<(), ExecutionError> { + self.commands.push(sp(self.current_command, command)); + Ok(()) + } + + /// Push a generated command that was not part of the original PTB + /// It then "lifts" all subsequent result indices by 1 + /// returns the new index of the pushed command + fn push_generated_command(&mut self, command: T::Command_) -> Result { + // we are "overwriting" the command that was normally at `cur`, as such, we need to "lift" + // `cur` and all subsequent result indices by 1 + let cur = self.current_command; + let prev_lift = self + .result_index_lift + .last_key_value() + .map(|(k, v)| { + debug_assert!(*k <= cur); + *v + }) + .unwrap_or(0); + let Some(cur_lift) = prev_lift.checked_add(1) else { + invariant_violation!("Cannot increment lift value of {prev_lift}") + }; + self.result_index_lift.insert(cur, cur_lift + 1); + self.commands.push(sp(self.current_command, command)); + Ok(cur) + } + + fn lift_result_index(&mut self, original_idx: u16) -> Result { + let lift = self + .result_index_lift + .range(0..=original_idx) + .last() + .map(|(_k, v)| *v) + .unwrap_or(0); + original_idx.checked_add(lift).ok_or_else(|| { + make_invariant_violation!( + "Result index lift overflow. Cannot lift {original_idx} by {lift}", + ) + }) + } + + fn result_type(&self, i: u16) -> Option<&T::ResultType> { + self.commands.get(i as usize).map(|c| &c.value.result_type) + } + // Get the fixed type of a location. Returns `None` for Pure and Receiving inputs, fn fixed_type( &mut self, @@ -150,7 +201,7 @@ impl Context { SplatLocation::GasCoin => (T::Location::GasCoin, env.gas_coin_type()?), SplatLocation::Result(i, j) => ( T::Location::Result(i, j), - self.results[i as usize][j as usize].clone(), + self.result_type(i).unwrap()[j as usize].clone(), ), SplatLocation::Input(i) => match &self.input_resolution[i.0 as usize] { InputKind::Object => { @@ -235,7 +286,7 @@ pub fn transaction( ) -> Result { let L::Transaction { inputs, commands } = lt; let mut context = Context::new(inputs)?; - let commands = commands + commands .into_iter() .enumerate() .map(|(i, c)| { @@ -243,7 +294,6 @@ pub fn transaction( context.current_command = idx; let (c_, tys) = command::(env, &mut context, c).map_err(|e| e.with_command_index(i))?; - context.results.push(tys.clone()); let c = T::Command_ { command: c_, result_type: tys, @@ -252,10 +302,10 @@ pub fn transaction( // computed later consumed_shared_objects: vec![], }; - Ok(sp(idx, c)) + context.push_result(c) }) - .collect::, ExecutionError>>()?; - let mut ast = context.finish(commands); + .collect::>()?; + let mut ast = context.finish(); // mark the last usage of references as Move instead of Copy scope_references::transaction(&mut ast); // mark unused results to be dropped @@ -489,13 +539,14 @@ where } res.push(SplatLocation::Input(T::InputIndex(i))) } - L::Argument::NestedResult(i, j) => { - let Some(command_result) = context.results.get(i as usize) else { - return Err(CommandArgumentError::IndexOutOfBounds { idx: i }.into()); + L::Argument::NestedResult(original_i, j) => { + let i = context.lift_result_index(original_i)?; + let Some(command_result) = context.result_type(i) else { + return Err(CommandArgumentError::IndexOutOfBounds { idx: original_i }.into()); }; if j as usize >= command_result.len() { return Err(CommandArgumentError::SecondaryIndexOutOfBounds { - result_idx: i, + result_idx: original_i, secondary_idx: j, } .into()); @@ -503,7 +554,8 @@ where res.push(SplatLocation::Result(i, j)) } L::Argument::Result(i) => { - let Some(result) = context.results.get(i as usize) else { + let lifted_i = context.lift_result_index(i)?; + let Some(result) = context.result_type(lifted_i) else { return Err(CommandArgumentError::IndexOutOfBounds { idx: i }.into()); }; let Ok(len): Result = result.len().try_into() else { From af284f4bb5f75eb2c2e79c6a3b01d8780f876e4e Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Tue, 18 Nov 2025 17:53:47 -0800 Subject: [PATCH 02/10] prog --- crates/sui-types/src/funds_accumulator.rs | 6 + .../typing/ast.rs | 17 +- .../typing/translate.rs | 171 ++++++++++++++++-- 3 files changed, 178 insertions(+), 16 deletions(-) diff --git a/crates/sui-types/src/funds_accumulator.rs b/crates/sui-types/src/funds_accumulator.rs index 7eac27b55f3b7..67fd0070fb8eb 100644 --- a/crates/sui-types/src/funds_accumulator.rs +++ b/crates/sui-types/src/funds_accumulator.rs @@ -5,6 +5,7 @@ use crate::SUI_FRAMEWORK_ADDRESS; use crate::base_types::SuiAddress; +use move_core_types::account_address::AccountAddress; use move_core_types::annotated_value::{MoveFieldLayout, MoveStructLayout, MoveTypeLayout}; use move_core_types::ident_str; use move_core_types::identifier::IdentStr; @@ -15,6 +16,11 @@ use serde::Serialize; pub const FUNDS_ACCUMULATOR_MODULE_NAME: &IdentStr = ident_str!("funds_accumulator"); pub const WITHDRAWAL_STRUCT_NAME: &IdentStr = ident_str!("Withdrawal"); +pub const RESOLVED_WITHDRAWAL_STRUCT: (&AccountAddress, &IdentStr, &IdentStr) = ( + &SUI_FRAMEWORK_ADDRESS, + FUNDS_ACCUMULATOR_MODULE_NAME, + WITHDRAWAL_STRUCT_NAME, +); /// Rust bindings for the Move struct `sui::funds_accumulator::Withdrawal`. #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs index 984aca8adec18..6ed441264d87a 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs @@ -4,7 +4,7 @@ use crate::static_programmable_transactions::{ linkage::resolved_linkage::ResolvedLinkage, loading::ast as L, spanned::Spanned, }; -use indexmap::IndexSet; +use indexmap::{IndexMap, IndexSet}; use move_vm_types::values::VectorSpecialization; use std::{cell::OnceCell, vec}; use sui_types::base_types::{ObjectID, ObjectRef}; @@ -23,6 +23,7 @@ pub struct Transaction { pub pure: Vec, /// All receiving inputs pub receiving: Vec, + pub withdrawal_casts: IndexMap, pub commands: Commands, } @@ -60,6 +61,20 @@ pub struct ReceivingInput { pub constraint: BytesConstraint, } +#[derive(Debug)] +pub struct WithdrawalCast { + // Result index to a call to `sui::funds_accumulator::withdrawal_owner` + pub owner_result: u16, + // Result index to cast call + pub cast_result: u16, + pub cast_kind: WithdrawalCastKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WithdrawalCastKind { + ToCoin, +} + pub type Commands = Vec; pub type ObjectArg = L::ObjectArg; diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index 11d8985c4ed14..f4f4ba04626d8 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -5,8 +5,9 @@ use super::{ast as T, env::Env}; use crate::{ execution_mode::ExecutionMode, programmable_transactions::context::EitherError, + sp, static_programmable_transactions::{ - loading::ast::{self as L, Type}, + loading::ast::{self as L, Datatype, Type}, spanned::sp, typing::ast::BytesConstraint, }, @@ -18,9 +19,10 @@ use sui_types::{ coin::RESOLVED_COIN_STRUCT, error::{ExecutionError, ExecutionErrorKind, command_argument_error}, execution_status::CommandArgumentError, + funds_accumulator::RESOLVED_WITHDRAWAL_STRUCT, }; -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] enum SplatLocation { GasCoin, Input(T::InputIndex), @@ -45,10 +47,13 @@ struct Context { objects: IndexMap, pure: IndexMap<(T::InputIndex, Type), T::PureInput>, receiving: IndexMap<(T::InputIndex, Type), T::ReceivingInput>, + withdrawal_casts: IndexMap, commands: Vec, // map from original result to the lifted result // the map should only be queried via a range, up to the current command index result_index_lift: BTreeMap, + // When the input location is used, instead, use the masked one + location_masks: IndexMap, } impl Context { @@ -61,9 +66,11 @@ impl Context { receiving_refs: IndexMap::new(), objects: IndexMap::new(), pure: IndexMap::new(), + withdrawal_casts: IndexMap::new(), receiving: IndexMap::new(), commands: vec![], result_index_lift: BTreeMap::new(), + location_masks: IndexMap::new(), }; // clone inputs for debug assertions #[cfg(debug_assertions)] @@ -187,6 +194,28 @@ impl Context { }) } + fn resolve_location_mask( + &mut self, + location: T::Location, + ) -> Result, ExecutionError> { + if !self.location_masks.contains_key(&location) { + return Ok(None); + } + let mut cur = location; + let mut visited = IndexSet::new(); + loop { + let was_new = visited.insert(cur); + assert_invariant!( + was_new, + "Cycle detected when resolving fixed type for location {location:?}" + ); + let Some(next) = self.location_masks.get(&cur).copied() else { + return Ok(Some(cur)); + }; + cur = next; + } + } + fn result_type(&self, i: u16) -> Option<&T::ResultType> { self.commands.get(i as usize).map(|c| &c.value.result_type) } @@ -197,7 +226,7 @@ impl Context { env: &Env, location: SplatLocation, ) -> Result, ExecutionError> { - Ok(Some(match location { + let (location, ty) = match location { SplatLocation::GasCoin => (T::Location::GasCoin, env.gas_coin_type()?), SplatLocation::Result(i, j) => ( T::Location::Result(i, j), @@ -215,7 +244,14 @@ impl Context { } InputKind::Pure | InputKind::Receiving => return Ok(None), }, - })) + }; + let Some(new_location) = match location { + SplatLocation::GasCoin => T::Location::GasCoin, + SplatLocation::Result(i, j) => T::Location::Result(i, j), + SplatLocation::Input(i) => T::Location::Input(i), + }; + let location = self.resolve_splat_location_mask(location)?; + Ok(Some()) } fn resolve_location( @@ -225,6 +261,7 @@ impl Context { expected_ty: &Type, bytes_constraint: BytesConstraint, ) -> Result<(T::Location, Type), ExecutionError> { + let location = self.resolve_splat_location_mask(location)?; Ok(match location { SplatLocation::GasCoin | SplatLocation::Result(_, _) => self .fixed_type(env, location)? @@ -639,7 +676,7 @@ fn argument_( }; debug_assert!(expected_ty.abilities().has_copy()); // unused since the type is fixed - check_type(&a, b)?; + let location = check_type(context, location, &a, b)?; if needs_freeze { T::Argument__::Freeze(T::Usage::new_copy(location)) } else { @@ -647,7 +684,7 @@ fn argument_( } } (Type::Reference(_, a), b) => { - check_type(&a, b)?; + let location = check_type(context, location, &a, b)?; if !b.abilities().has_copy() { // TODO this should be a different error for missing copy return Err(CommandArgumentError::TypeMismatch.into()); @@ -657,11 +694,11 @@ fn argument_( // Non reference location types (actual_ty, Type::Reference(is_mut, inner)) => { - check_type(&actual_ty, inner)?; + let location = check_type(context, location, &actual_ty, inner)?; T::Argument__::Borrow(/* mut */ *is_mut, location) } (actual_ty, _) => { - check_type(&actual_ty, expected_ty)?; + let location = check_type(context, location, &actual_ty, expected_ty)?; T::Argument__::Use(if expected_ty.abilities().has_copy() { T::Usage::new_copy(location) } else { @@ -671,9 +708,20 @@ fn argument_( }) } -fn check_type(actual_ty: &Type, expected_ty: &Type) -> Result<(), CommandArgumentError> { +fn check_type( + context: &mut Context, + location: T::Location, + actual_ty: &Type, + expected_ty: &Type, +) -> Result { if actual_ty == expected_ty { - Ok(()) + Ok(location) + } else if let Some(expected_inner_t) = coin_inner_type(expected_ty) + && let Some(actual_inner_t) = withdrawal_inner_type(actual_ty) + && actual_inner_t == expected_inner_t + { + let cast_result = cast_withdrawl_to_coin(context, location, actual_inner_t); + Ok(cast_result) } else { Err(CommandArgumentError::TypeMismatch) } @@ -802,18 +850,111 @@ fn coin_mut_ref_argument_( } fn check_coin_type(ty: &Type) -> Result<(), EitherError> { + if coin_inner_type(ty).is_some() { + Ok(()) + } else { + Err(CommandArgumentError::TypeMismatch.into()) + } +} + +/// Returns the inner type `T` if the type is `sui::coin::Coin`, else `None` +fn coin_inner_type(ty: &Type) -> Option<&Type> { let Type::Datatype(dt) = ty else { - return Err(CommandArgumentError::TypeMismatch.into()); + return None; }; + if dt.type_arguments.len() != 1 { + return None; + } let resolved = dt.qualified_ident(); - let is_coin = resolved == RESOLVED_COIN_STRUCT; - if is_coin { - Ok(()) + if resolved == RESOLVED_COIN_STRUCT { + Some(&dt.type_arguments[0]) } else { - Err(CommandArgumentError::TypeMismatch.into()) + None + } +} + +/// Returns the inner type `T` if the type is `sui::funds_accumulator::Withdrawal`, else `None` +fn withdrawal_inner_type(ty: &Type) -> Option<&Type> { + let Type::Datatype(dt) = ty else { + return None; + }; + if dt.type_arguments.len() != 1 { + return None; + } + let resolved = dt.qualified_ident(); + if resolved == RESOLVED_WITHDRAWAL_STRUCT { + Some(&dt.type_arguments[0]) + } else { + None } } +fn cast_withdrawal_to_coin( + env: &Env, + context: &mut Context, + location: T::Location, + withdrawal_type: &Type, + inner_ty: &Type, +) -> Result { + let idx = todo!("actual arg index"); + // first store the owner of the withdrawal + let withdrawal_borrow_ = T::Argument__::Borrow(false, location); + let withdrawl_ref_ty = Type::Reference(false, Rc::new(withdrawal_type.clone())); + let withdrawal_borrow = sp(idx, (withdrawal_borrow_, withdrawl_ref_ty)); + let owner_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { + function: todo!("call sui::funds_accumulator::withdrawal_owner"), + arguments: vec![withdrawal_borrow], + })); + let owner_command_ = T::Command_ { + command: owner_command__, + result_type: vec![Type::Address], + drop_values: vec![], + consumed_shared_objects: vec![], + }; + let owner_idx = context.push_generated_command(owner_command_)?; + // we need to insert a cast command before the current command + let withdrawal_arg_ = T::Argument__::new_move(location); + let withdrawal_arg = sp(idx, (withdrawal_arg_, withdrawal_type.clone())); + let ctx_arg_ = T::Argument__::Borrow(true, T::Location::TxContext); + let ctx_ty = Type::Reference(true, Rc::new(env.tx_context_type()?)); + let ctx_arg = sp(idx, (ctx_arg_, ctx_ty)); + let cast_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { + function: todo!("call sui::coin::redeem_funds"), + arguments: vec![withdrawal_arg, ctx_arg], + })); + let cast_command_ = T::Command_ { + command: cast_command__, + result_type: vec![env.coin_type(inner_ty.clone())?], + drop_values: vec![], + consumed_shared_objects: vec![], + }; + let cast_idx = context.push_generated_command(cast_command_)?; + // add mask + let splat_location = match location { + T::Location::ObjectInput(_) => todo!(), + T::Location::Result(_, _) => todo!(), + T::Location::TxContext => todo!(), + T::Location::GasCoin => todo!(), + T::Location::ReceivingInput(_) => todo!(), + T::Location::PureInput(_) => todo!(), + }; + context + .location_masks + .insert(splat_location, SplatLocation::Result(cast_idx, 0)); + // manage metadata + context.withdrawal_casts.insert( + location, + T::WithdrawalCast { + owner_idx, + cast_idx, + cast_kind: T::WithdrawalCastKind::ToCoin, + }, + ); + + // the result of the cast is at (cast_idx, 0) + T::Location::Result(cast_idx, 0) +} + //************************************************************************************************** // Reference scoping //************************************************************************************************** From 388dfe09da6ac422b13e794e3b023ac68e064d57 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Wed, 19 Nov 2025 11:47:31 -0800 Subject: [PATCH 03/10] prog --- crates/sui-types/src/balance.rs | 7 + .../static_programmable_transactions/env.rs | 18 +- .../typing/translate.rs | 199 ++++++++++++------ 3 files changed, 153 insertions(+), 71 deletions(-) diff --git a/crates/sui-types/src/balance.rs b/crates/sui-types/src/balance.rs index b8ac270e8cdfd..512f70f65af40 100644 --- a/crates/sui-types/src/balance.rs +++ b/crates/sui-types/src/balance.rs @@ -5,6 +5,7 @@ use crate::SUI_FRAMEWORK_ADDRESS; use crate::error::{ExecutionError, ExecutionErrorKind}; use crate::sui_serde::BigInt; use crate::sui_serde::Readable; +use move_core_types::account_address::AccountAddress; use move_core_types::annotated_value::{MoveFieldLayout, MoveStructLayout, MoveTypeLayout}; use move_core_types::ident_str; use move_core_types::identifier::IdentStr; @@ -13,9 +14,15 @@ use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; use serde_with::serde_as; + pub const SUI_MODULE_NAME: &IdentStr = ident_str!("sui"); pub const BALANCE_MODULE_NAME: &IdentStr = ident_str!("balance"); pub const BALANCE_STRUCT_NAME: &IdentStr = ident_str!("Balance"); +pub const RESOLVED_BALANCE_STRUCT: (&AccountAddress, &IdentStr, &IdentStr) = ( + &SUI_FRAMEWORK_ADDRESS, + BALANCE_MODULE_NAME, + BALANCE_STRUCT_NAME, +); pub const BALANCE_CREATE_REWARDS_FUNCTION_NAME: &IdentStr = ident_str!("create_staking_rewards"); pub const BALANCE_DESTROY_REBATES_FUNCTION_NAME: &IdentStr = ident_str!("destroy_storage_rebates"); diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs index 32f82e64f1b2d..17f5eaf221b2f 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs @@ -24,7 +24,7 @@ use crate::{ use move_binary_format::{ CompiledModule, errors::{Location, PartialVMError, VMError}, - file_format::{AbilitySet, TypeParameterIndex}, + file_format::{Ability, AbilitySet, TypeParameterIndex}, }; use move_core_types::{ annotated_value, @@ -39,6 +39,7 @@ use sui_protocol_config::ProtocolConfig; use sui_types::{ Identifier, TypeTag, base_types::{ObjectID, TxContext}, + coin::RESOLVED_COIN_STRUCT, error::{ExecutionError, ExecutionErrorKind}, execution_status::TypeArgumentError, gas_coin::GasCoin, @@ -293,6 +294,21 @@ impl<'pc, 'vm, 'state, 'linkage> Env<'pc, 'vm, 'state, 'linkage> { get_or_init_ty!(self, tx_context_type, TxContext::type_()) } + pub fn coin_type(&self, inner_type: Type) -> Result { + let Some(abilities) = AbilitySet::from_u8((Ability::Key as u8) | (Ability::Store as u8)) + else { + invariant_violation!("Unable to create coin abilities"); + }; + let (a, m, n) = RESOLVED_COIN_STRUCT; + let module = ModuleId::new(*a, m.to_owned()); + Ok(Type::Datatype(Rc::new(Datatype { + abilities, + module, + name: n.to_owned(), + type_arguments: vec![inner_type], + }))) + } + pub fn vector_type(&self, element_type: Type) -> Result { let abilities = AbilitySet::polymorphic_abilities( AbilitySet::VECTOR, diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index f4f4ba04626d8..55309400bf399 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -15,6 +15,7 @@ use crate::{ use indexmap::{IndexMap, IndexSet}; use std::{collections::BTreeMap, rc::Rc}; use sui_types::{ + balance::RESOLVED_BALANCE_STRUCT, base_types::{ObjectRef, TxContextKind}, coin::RESOLVED_COIN_STRUCT, error::{ExecutionError, ExecutionErrorKind, command_argument_error}, @@ -138,6 +139,7 @@ impl Context { pure, receiving, commands, + withdrawal_casts, .. } = self; let objects = objects.into_iter().map(|(_, o)| o).collect(); @@ -148,6 +150,7 @@ impl Context { objects, pure, receiving, + withdrawal_casts, commands, } } @@ -197,10 +200,7 @@ impl Context { fn resolve_location_mask( &mut self, location: T::Location, - ) -> Result, ExecutionError> { - if !self.location_masks.contains_key(&location) { - return Ok(None); - } + ) -> Result { let mut cur = location; let mut visited = IndexSet::new(); loop { @@ -210,7 +210,7 @@ impl Context { "Cycle detected when resolving fixed type for location {location:?}" ); let Some(next) = self.location_masks.get(&cur).copied() else { - return Ok(Some(cur)); + return Ok(cur); }; cur = next; } @@ -220,56 +220,70 @@ impl Context { self.commands.get(i as usize).map(|c| &c.value.result_type) } + fn fixed_location_type( + &mut self, + env: &Env, + location: T::Location, + ) -> Result, ExecutionError> { + Ok(Some(match location { + T::Location::TxContext => env.tx_context_type()?, + T::Location::GasCoin => env.gas_coin_type()?, + T::Location::Result(i, j) => self.result_type(i).unwrap()[j as usize].clone(), + T::Location::ObjectInput(i) => { + let Some((_, object_input)) = self.objects.get_index(i as usize) else { + invariant_violation!("Unbound object input {}", i) + }; + object_input.ty.clone() + } + T::Location::PureInput(_) | T::Location::ReceivingInput(_) => return Ok(None), + })) + } + // Get the fixed type of a location. Returns `None` for Pure and Receiving inputs, fn fixed_type( &mut self, env: &Env, - location: SplatLocation, + splat_location: SplatLocation, ) -> Result, ExecutionError> { - let (location, ty) = match location { - SplatLocation::GasCoin => (T::Location::GasCoin, env.gas_coin_type()?), - SplatLocation::Result(i, j) => ( - T::Location::Result(i, j), - self.result_type(i).unwrap()[j as usize].clone(), - ), + let original_location = match splat_location { + SplatLocation::GasCoin => T::Location::GasCoin, + SplatLocation::Result(i, j) => T::Location::Result(i, j), SplatLocation::Input(i) => match &self.input_resolution[i.0 as usize] { InputKind::Object => { - let Some((object_index, _, object_input)) = self.objects.get_full(&i) else { + let Some(index) = self.objects.get_index_of(&i) else { invariant_violation!("Unbound object input {}", i.0) }; - ( - T::Location::ObjectInput(object_index as u16), - object_input.ty.clone(), - ) + T::Location::ObjectInput(index as u16) } InputKind::Pure | InputKind::Receiving => return Ok(None), }, }; - let Some(new_location) = match location { - SplatLocation::GasCoin => T::Location::GasCoin, - SplatLocation::Result(i, j) => T::Location::Result(i, j), - SplatLocation::Input(i) => T::Location::Input(i), + let location = self.resolve_location_mask(original_location)?; + let Some(ty) = self.fixed_location_type(env, location)? else { + invariant_violation!( + "Location {original_location:?}=>{location:?} does not have a fixed type" + ) }; - let location = self.resolve_splat_location_mask(location)?; - Ok(Some()) + Ok(Some((location, ty))) } fn resolve_location( &mut self, env: &Env, - location: SplatLocation, + splat_location: SplatLocation, expected_ty: &Type, bytes_constraint: BytesConstraint, ) -> Result<(T::Location, Type), ExecutionError> { - let location = self.resolve_splat_location_mask(location)?; - Ok(match location { - SplatLocation::GasCoin | SplatLocation::Result(_, _) => self - .fixed_type(env, location)? - .ok_or_else(|| make_invariant_violation!("Expected fixed type for {location:?}"))?, + let original_location = match splat_location { + SplatLocation::GasCoin => T::Location::GasCoin, + SplatLocation::Result(i, j) => T::Location::Result(i, j), SplatLocation::Input(i) => match &self.input_resolution[i.0 as usize] { - InputKind::Object => self.fixed_type(env, location)?.ok_or_else(|| { - make_invariant_violation!("Expected fixed type for {location:?}") - })?, + InputKind::Object => { + let Some(index) = self.objects.get_index_of(&i) else { + invariant_violation!("Unbound object input {}", i.0) + }; + T::Location::ObjectInput(index as u16) + } InputKind::Pure => { let ty = match expected_ty { Type::Reference(_, inner) => (**inner).clone(), @@ -289,7 +303,7 @@ impl Context { self.pure.insert(k.clone(), pure); } let byte_index = self.pure.get_index_of(&k).unwrap(); - (T::Location::PureInput(byte_index as u16), ty) + return Ok((T::Location::PureInput(byte_index as u16), ty)); } InputKind::Receiving => { let ty = match expected_ty { @@ -310,10 +324,17 @@ impl Context { self.receiving.insert(k.clone(), receiving); } let byte_index = self.receiving.get_index_of(&k).unwrap(); - (T::Location::ReceivingInput(byte_index as u16), ty) + return Ok((T::Location::ReceivingInput(byte_index as u16), ty)); } }, - }) + }; + let location = self.resolve_location_mask(original_location)?; + let Some(ty) = self.fixed_location_type(env, location)? else { + invariant_violation!( + "Location {original_location:?}=>{location:?} does not have a fixed type" + ) + }; + Ok((location, ty)) } } @@ -661,8 +682,16 @@ fn argument_( command: current_command, argument: command_arg_idx as u16, }; - let (location, actual_ty): (T::Location, Type) = + let (location, actual_ty) = context.resolve_location(env, location, expected_ty, bytes_constraint)?; + let (location, actual_ty) = apply_conversion( + env, + context, + command_arg_idx, + location, + actual_ty, + expected_ty, + )?; Ok(match (actual_ty, expected_ty) { // Reference location types (Type::Reference(a_is_mut, a), Type::Reference(b_is_mut, b)) => { @@ -676,7 +705,7 @@ fn argument_( }; debug_assert!(expected_ty.abilities().has_copy()); // unused since the type is fixed - let location = check_type(context, location, &a, b)?; + check_type(&a, b)?; if needs_freeze { T::Argument__::Freeze(T::Usage::new_copy(location)) } else { @@ -684,7 +713,7 @@ fn argument_( } } (Type::Reference(_, a), b) => { - let location = check_type(context, location, &a, b)?; + check_type(&a, b)?; if !b.abilities().has_copy() { // TODO this should be a different error for missing copy return Err(CommandArgumentError::TypeMismatch.into()); @@ -694,11 +723,11 @@ fn argument_( // Non reference location types (actual_ty, Type::Reference(is_mut, inner)) => { - let location = check_type(context, location, &actual_ty, inner)?; + check_type(&actual_ty, inner)?; T::Argument__::Borrow(/* mut */ *is_mut, location) } (actual_ty, _) => { - let location = check_type(context, location, &actual_ty, expected_ty)?; + check_type(&actual_ty, expected_ty)?; T::Argument__::Use(if expected_ty.abilities().has_copy() { T::Usage::new_copy(location) } else { @@ -708,20 +737,9 @@ fn argument_( }) } -fn check_type( - context: &mut Context, - location: T::Location, - actual_ty: &Type, - expected_ty: &Type, -) -> Result { +fn check_type(actual_ty: &Type, expected_ty: &Type) -> Result<(), CommandArgumentError> { if actual_ty == expected_ty { - Ok(location) - } else if let Some(expected_inner_t) = coin_inner_type(expected_ty) - && let Some(actual_inner_t) = withdrawal_inner_type(actual_ty) - && actual_inner_t == expected_inner_t - { - let cast_result = cast_withdrawl_to_coin(context, location, actual_inner_t); - Ok(cast_result) + Ok(()) } else { Err(CommandArgumentError::TypeMismatch) } @@ -857,6 +875,37 @@ fn check_coin_type(ty: &Type) -> Result<(), EitherError> { } } +fn apply_conversion( + env: &Env, + context: &mut Context, + command_arg_idx: usize, + location: T::Location, + actual_ty: Type, + expected_ty: &Type, +) -> Result<(T::Location, Type), ExecutionError> { + // check for withdrawal to coin conversion + let expected_ty = match expected_ty { + Type::Reference(_, inner) => &**inner, + ty => ty, + }; + if let Some(expected_inner) = coin_inner_type(expected_ty) + && let Some(actual_withdrawal_inner) = withdrawal_inner_type(&actual_ty) + && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) + && actual_inner == expected_inner + { + cast_withdrawal_to_coin( + env, + context, + command_arg_idx, + location, + actual_ty, + expected_inner, + ) + } else { + Ok((location, actual_ty)) + } +} + /// Returns the inner type `T` if the type is `sui::coin::Coin`, else `None` fn coin_inner_type(ty: &Type) -> Option<&Type> { let Type::Datatype(dt) = ty else { @@ -873,6 +922,22 @@ fn coin_inner_type(ty: &Type) -> Option<&Type> { } } +/// Returns the inner type `T` if the type is `sui::balance::Balance`, else `None` +fn balance_inner_type(ty: &Type) -> Option<&Type> { + let Type::Datatype(dt) = ty else { + return None; + }; + if dt.type_arguments.len() != 1 { + return None; + } + let resolved = dt.qualified_ident(); + if resolved == RESOLVED_BALANCE_STRUCT { + Some(&dt.type_arguments[0]) + } else { + None + } +} + /// Returns the inner type `T` if the type is `sui::funds_accumulator::Withdrawal`, else `None` fn withdrawal_inner_type(ty: &Type) -> Option<&Type> { let Type::Datatype(dt) = ty else { @@ -892,11 +957,12 @@ fn withdrawal_inner_type(ty: &Type) -> Option<&Type> { fn cast_withdrawal_to_coin( env: &Env, context: &mut Context, + command_arg_idx: usize, location: T::Location, - withdrawal_type: &Type, + withdrawal_type: Type, inner_ty: &Type, -) -> Result { - let idx = todo!("actual arg index"); +) -> Result<(T::Location, Type), ExecutionError> { + let idx = command_arg_idx as u16; // first store the owner of the withdrawal let withdrawal_borrow_ = T::Argument__::Borrow(false, location); let withdrawl_ref_ty = Type::Reference(false, Rc::new(withdrawal_type.clone())); @@ -914,7 +980,7 @@ fn cast_withdrawal_to_coin( let owner_idx = context.push_generated_command(owner_command_)?; // we need to insert a cast command before the current command let withdrawal_arg_ = T::Argument__::new_move(location); - let withdrawal_arg = sp(idx, (withdrawal_arg_, withdrawal_type.clone())); + let withdrawal_arg = sp(idx, (withdrawal_arg_, withdrawal_type)); let ctx_arg_ = T::Argument__::Borrow(true, T::Location::TxContext); let ctx_ty = Type::Reference(true, Rc::new(env.tx_context_type()?)); let ctx_arg = sp(idx, (ctx_arg_, ctx_ty)); @@ -922,6 +988,7 @@ fn cast_withdrawal_to_coin( function: todo!("call sui::coin::redeem_funds"), arguments: vec![withdrawal_arg, ctx_arg], })); + let coin_type = env.coin_type(inner_ty.clone())?; let cast_command_ = T::Command_ { command: cast_command__, result_type: vec![env.coin_type(inner_ty.clone())?], @@ -930,29 +997,20 @@ fn cast_withdrawal_to_coin( }; let cast_idx = context.push_generated_command(cast_command_)?; // add mask - let splat_location = match location { - T::Location::ObjectInput(_) => todo!(), - T::Location::Result(_, _) => todo!(), - T::Location::TxContext => todo!(), - T::Location::GasCoin => todo!(), - T::Location::ReceivingInput(_) => todo!(), - T::Location::PureInput(_) => todo!(), - }; context .location_masks - .insert(splat_location, SplatLocation::Result(cast_idx, 0)); + .insert(location, T::Location::Result(cast_idx, 0)); // manage metadata context.withdrawal_casts.insert( location, T::WithdrawalCast { - owner_idx, - cast_idx, + owner_result: owner_idx, + cast_result: cast_idx, cast_kind: T::WithdrawalCastKind::ToCoin, }, ); - // the result of the cast is at (cast_idx, 0) - T::Location::Result(cast_idx, 0) + Ok((T::Location::Result(cast_idx, 0), coin_type)) } //************************************************************************************************** @@ -1125,6 +1183,7 @@ mod consumed_shared_objects { objects, pure: _, receiving: _, + withdrawal_casts: _, commands: _, } = ast; let inputs = objects From 23784ea0dc607f02ac24e21e3a2ec1dd1a567368 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Wed, 19 Nov 2025 13:24:28 -0800 Subject: [PATCH 04/10] drop return --- .../execution/interpreter.rs | 1 + .../typing/ast.rs | 14 +- .../typing/invariant_checks/memory_safety.rs | 2 + .../typing/invariant_checks/type_check.rs | 1 + .../typing/translate.rs | 38 +++--- .../typing/verify/drop_safety.rs | 127 +++++++++++++++--- .../typing/verify/input_arguments.rs | 1 + 7 files changed, 139 insertions(+), 45 deletions(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/execution/interpreter.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/execution/interpreter.rs index edd50deaf229c..12a5b2097239a 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/execution/interpreter.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/execution/interpreter.rs @@ -71,6 +71,7 @@ where objects, pure, receiving, + withdrawal_conversions: _, commands, } = ast; let mut context = Context::new( diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs index 6ed441264d87a..5c505685c9a8c 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/ast.rs @@ -23,7 +23,7 @@ pub struct Transaction { pub pure: Vec, /// All receiving inputs pub receiving: Vec, - pub withdrawal_casts: IndexMap, + pub withdrawal_conversions: IndexMap, pub commands: Commands, } @@ -61,17 +61,17 @@ pub struct ReceivingInput { pub constraint: BytesConstraint, } -#[derive(Debug)] -pub struct WithdrawalCast { +#[derive(Debug, Clone, Copy)] +pub struct WithdrawalConversion { // Result index to a call to `sui::funds_accumulator::withdrawal_owner` pub owner_result: u16, - // Result index to cast call - pub cast_result: u16, - pub cast_kind: WithdrawalCastKind, + // Result index to conversion call + pub conversion_result: u16, + pub conversion_kind: WithdrawalConversionKind, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum WithdrawalCastKind { +pub enum WithdrawalConversionKind { ToCoin, } diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/memory_safety.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/memory_safety.rs index 8acab67977fb7..ba6d577cffb30 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/memory_safety.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/memory_safety.rs @@ -295,6 +295,7 @@ impl Context { objects, pure, receiving, + withdrawal_conversions: _, commands: _, } = txn; let tx_context = Location::non_ref(T::Location::TxContext); @@ -508,6 +509,7 @@ fn verify_(txn: &T::Transaction) -> anyhow::Result<()> { objects: _, pure: _, receiving: _, + withdrawal_conversions: _, commands, } = txn; for c in commands { diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/type_check.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/type_check.rs index 953b665ab5127..359bb1eda5cb1 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/type_check.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/invariant_checks/type_check.rs @@ -53,6 +53,7 @@ fn verify_(env: &Env, txn: &T::Transaction) -> anyhow::Resu objects, pure, receiving, + withdrawal_conversions: _, commands, } = txn; for obj in objects { diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index 55309400bf399..43316f4c49772 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -48,7 +48,7 @@ struct Context { objects: IndexMap, pure: IndexMap<(T::InputIndex, Type), T::PureInput>, receiving: IndexMap<(T::InputIndex, Type), T::ReceivingInput>, - withdrawal_casts: IndexMap, + withdrawal_conversions: IndexMap, commands: Vec, // map from original result to the lifted result // the map should only be queried via a range, up to the current command index @@ -67,7 +67,7 @@ impl Context { receiving_refs: IndexMap::new(), objects: IndexMap::new(), pure: IndexMap::new(), - withdrawal_casts: IndexMap::new(), + withdrawal_conversions: IndexMap::new(), receiving: IndexMap::new(), commands: vec![], result_index_lift: BTreeMap::new(), @@ -139,7 +139,7 @@ impl Context { pure, receiving, commands, - withdrawal_casts, + withdrawal_conversions, .. } = self; let objects = objects.into_iter().map(|(_, o)| o).collect(); @@ -150,7 +150,7 @@ impl Context { objects, pure, receiving, - withdrawal_casts, + withdrawal_conversions, commands, } } @@ -893,7 +893,7 @@ fn apply_conversion( && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) && actual_inner == expected_inner { - cast_withdrawal_to_coin( + convert_withdrawal_to_coin( env, context, command_arg_idx, @@ -954,7 +954,7 @@ fn withdrawal_inner_type(ty: &Type) -> Option<&Type> { } } -fn cast_withdrawal_to_coin( +fn convert_withdrawal_to_coin( env: &Env, context: &mut Context, command_arg_idx: usize, @@ -978,39 +978,39 @@ fn cast_withdrawal_to_coin( consumed_shared_objects: vec![], }; let owner_idx = context.push_generated_command(owner_command_)?; - // we need to insert a cast command before the current command + // we need to insert a conversion command before the current command let withdrawal_arg_ = T::Argument__::new_move(location); let withdrawal_arg = sp(idx, (withdrawal_arg_, withdrawal_type)); let ctx_arg_ = T::Argument__::Borrow(true, T::Location::TxContext); let ctx_ty = Type::Reference(true, Rc::new(env.tx_context_type()?)); let ctx_arg = sp(idx, (ctx_arg_, ctx_ty)); - let cast_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { + let conversion_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { function: todo!("call sui::coin::redeem_funds"), arguments: vec![withdrawal_arg, ctx_arg], })); let coin_type = env.coin_type(inner_ty.clone())?; - let cast_command_ = T::Command_ { - command: cast_command__, + let conversion_command_ = T::Command_ { + command: conversion_command__, result_type: vec![env.coin_type(inner_ty.clone())?], drop_values: vec![], consumed_shared_objects: vec![], }; - let cast_idx = context.push_generated_command(cast_command_)?; + let conversion_idx = context.push_generated_command(conversion_command_)?; // add mask context .location_masks - .insert(location, T::Location::Result(cast_idx, 0)); + .insert(location, T::Location::Result(conversion_idx, 0)); // manage metadata - context.withdrawal_casts.insert( + context.withdrawal_conversions.insert( location, - T::WithdrawalCast { + T::WithdrawalConversion { owner_result: owner_idx, - cast_result: cast_idx, - cast_kind: T::WithdrawalCastKind::ToCoin, + conversion_result: conversion_idx, + conversion_kind: T::WithdrawalConversionKind::ToCoin, }, ); - // the result of the cast is at (cast_idx, 0) - Ok((T::Location::Result(cast_idx, 0), coin_type)) + // the result of the conversion is at (conversion_idx, 0) + Ok((T::Location::Result(conversion_idx, 0), coin_type)) } //************************************************************************************************** @@ -1183,7 +1183,7 @@ mod consumed_shared_objects { objects, pure: _, receiving: _, - withdrawal_casts: _, + withdrawal_conversions: _, commands: _, } = ast; let inputs = objects diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs index 7d11affbbedb3..2440dd7f478ee 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs @@ -22,71 +22,160 @@ pub fn refine_and_verify( mod refine { use crate::{ sp, - static_programmable_transactions::typing::ast::{self as T}, + static_programmable_transactions::{ + spanned::sp, + typing::ast::{self as T, Type}, + }, }; use std::collections::BTreeSet; + struct Context { + // All locations that were used + used: BTreeSet, + // All locations that were used via a Move + moved: BTreeSet, + } + + impl Context { + fn new() -> Self { + Self { + used: BTreeSet::new(), + moved: BTreeSet::new(), + } + } + } + /// After memory safety, we can switch the last usage of a `Copy` to a `Move` if it is not /// borrowed at the time of the last usage. pub fn transaction(ast: &mut T::Transaction) { - let mut used: BTreeSet = BTreeSet::new(); + let mut context = Context::new(); for c in ast.commands.iter_mut().rev() { - command(&mut used, c); + command(&mut context, c); } } - fn command(used: &mut BTreeSet, sp!(_, c): &mut T::Command) { + fn command(context: &mut Context, sp!(_, c): &mut T::Command) { match &mut c.command { - T::Command__::MoveCall(mc) => arguments(used, &mut mc.arguments), + T::Command__::MoveCall(mc) => arguments(context, &mut mc.arguments), T::Command__::TransferObjects(objects, recipient) => { - argument(used, recipient); - arguments(used, objects); + argument(context, recipient); + arguments(context, objects); } T::Command__::SplitCoins(_, coin, amounts) => { - arguments(used, amounts); - argument(used, coin); + arguments(context, amounts); + argument(context, coin); } T::Command__::MergeCoins(_, target, coins) => { - arguments(used, coins); - argument(used, target); + arguments(context, coins); + argument(context, target); } - T::Command__::MakeMoveVec(_, xs) => arguments(used, xs), + T::Command__::MakeMoveVec(_, xs) => arguments(context, xs), T::Command__::Publish(_, _, _) => (), - T::Command__::Upgrade(_, _, _, x, _) => argument(used, x), + T::Command__::Upgrade(_, _, _, x, _) => argument(context, x), } } - fn arguments(used: &mut BTreeSet, args: &mut [T::Argument]) { + fn arguments(context: &mut Context, args: &mut [T::Argument]) { for arg in args.iter_mut().rev() { - argument(used, arg) + argument(context, arg) } } - fn argument(used: &mut BTreeSet, arg: &mut T::Argument) { + fn argument(context: &mut Context, arg: &mut T::Argument) { let usage = match &mut arg.value.0 { T::Argument__::Use(u) | T::Argument__::Read(u) | T::Argument__::Freeze(u) => u, T::Argument__::Borrow(_, loc) => { // mark location as used - used.insert(*loc); + context.used.insert(*loc); return; } }; match &usage { T::Usage::Move(loc) => { // mark location as used - used.insert(*loc); + context.used.insert(*loc); + context.moved.insert(*loc); } T::Usage::Copy { location, borrowed } => { // we are at the last usage of a reference result if it was not yet added to the set let location = *location; - let last_usage = used.insert(location); + let last_usage = context.used.insert(location); if last_usage && !borrowed.get().unwrap() { // if it was the last usage, we need to change the Copy to a Move *usage = T::Usage::Move(location); + context.moved.insert(location); } } } } + + /// For any withdrawal conversion where the value was not moved, send it back to the original + /// owner + fn return_unused_withdrawal_conversions( + ast: &mut T::Transaction, + moved_locations: &BTreeSet, + ) { + for (unmoved_loc, conversion_info) in ast + .withdrawal_conversions + .iter() + .filter(|(l, _)| !moved_locations.contains(l)) + { + let Some(cur_command) = ast.commands.len().checked_sub(1) else { + invariant_violation!("cannot be zero commands with a conversion") + }; + let cur_command = cur_command as u16; + let T::WithdrawalConversion { + owner_result, + conversion_result, + conversion_kind, + } = *conversion_info; + // set owner result as used + let owner_command = &mut ast.commands[owner_result as usize]; + assert_invariant!( + owner_command.value.drop_values.as_slice() == &[true], + "owner result should be unused thus far and should be dropped" + ); + owner_command.value.drop_values[0] = false; + let owner_command = &*owner_command; + let conversion_command = &ast.commands[conversion_result as usize]; + assert_invariant!( + conversion_command.value.result_type.len() == 1, + "conversion should have one result" + ); + assert_invariant!( + owner_command.value.result_type.len() == 1, + "owner command should have one result" + ); + assert_invariant!( + owner_command.value.result_type[0] == T::Type::Address, + "owner command should return an address" + ); + assert_invariant!( + matches!(conversion_kind, T::WithdrawalConversionKind::ToCoin), + "only coin conversion supported" + ); + let conversion_ty = &conversion_command.value.result_type[0]; + let move_result_ = T::Argument__::new_move(T::Location::Result(conversion_result, 0)); + let move_result = sp(cur_command, (move_result_, conversion_ty.clone())); + let owner_ty = Type::Address; + let owner_arg_ = T::Argument__::new_move(T::Location::Result(owner_result, 0)); + let owner_arg = sp(cur_command, (owner_arg_, owner_ty)); + let return_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { + function: todo!("call sui::coin::send_funds"), + arguments: vec![move_result, owner_arg], + })); + let return_command = sp( + cur_command, + T::Command_ { + command: return_command__, + result_type: vec![], + drop_values: vec![], + consumed_shared_objects: vec![], + }, + ); + ast.commands.push(return_command); + } + } } mod verify { diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/input_arguments.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/input_arguments.rs index bce5d6a99e941..3f11bd13ceb10 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/input_arguments.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/input_arguments.rs @@ -75,6 +75,7 @@ pub fn verify(_env: &Env, txn: &T::Transaction) -> Result<( objects: _, pure, receiving, + withdrawal_conversions: _, commands, } = txn; for pure in pure { From bc7dffb4fdecfc88144681ac5aa49aeaf7eb6460 Mon Sep 17 00:00:00 2001 From: Tim Zakian <2895723+tzakian@users.noreply.github.com> Date: Wed, 19 Nov 2025 15:55:22 -0800 Subject: [PATCH 05/10] Add resolution of framework calls (#24348) ## Description Describe the changes or additions included in this PR. ## Test plan How did you test the new or updated feature? --- ## 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): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework: --- crates/sui-types/src/coin.rs | 2 ++ crates/sui-types/src/funds_accumulator.rs | 1 + .../static_programmable_transactions/env.rs | 21 +++++++++++++- .../linkage/analysis.rs | 29 ++++++++++++++++--- .../typing/translate.rs | 22 ++++++++++---- .../typing/verify/drop_safety.rs | 25 ++++++++++++++-- 6 files changed, 87 insertions(+), 13 deletions(-) diff --git a/crates/sui-types/src/coin.rs b/crates/sui-types/src/coin.rs index 4d8699968ad8f..96118a1b91b97 100644 --- a/crates/sui-types/src/coin.rs +++ b/crates/sui-types/src/coin.rs @@ -35,6 +35,8 @@ pub const PAY_MODULE_NAME: &IdentStr = ident_str!("pay"); pub const PAY_JOIN_FUNC_NAME: &IdentStr = ident_str!("join"); pub const PAY_SPLIT_N_FUNC_NAME: &IdentStr = ident_str!("divide_and_keep"); pub const PAY_SPLIT_VEC_FUNC_NAME: &IdentStr = ident_str!("split_vec"); +pub const REDEEM_FUNDS_FUNC_NAME: &IdentStr = ident_str!("redeem_funds"); +pub const SEND_FUNDS_FUNC_NAME: &IdentStr = ident_str!("send_funds"); // Rust version of the Move sui::coin::Coin type #[derive(Debug, Serialize, Deserialize, Clone, JsonSchema, Eq, PartialEq)] diff --git a/crates/sui-types/src/funds_accumulator.rs b/crates/sui-types/src/funds_accumulator.rs index 67fd0070fb8eb..082f6d0cb58a2 100644 --- a/crates/sui-types/src/funds_accumulator.rs +++ b/crates/sui-types/src/funds_accumulator.rs @@ -21,6 +21,7 @@ pub const RESOLVED_WITHDRAWAL_STRUCT: (&AccountAddress, &IdentStr, &IdentStr) = FUNDS_ACCUMULATOR_MODULE_NAME, WITHDRAWAL_STRUCT_NAME, ); +pub const WITHDRAWAL_OWNER_FUNC_NAME: &IdentStr = ident_str!("withdrawal_owner"); /// Rust bindings for the Move struct `sui::funds_accumulator::Withdrawal`. #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs index 17f5eaf221b2f..aa0c4d020f174 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs @@ -28,6 +28,7 @@ use move_binary_format::{ }; use move_core_types::{ annotated_value, + identifier::IdentStr, language_storage::{ModuleId, StructTag}, runtime_value::{self, MoveTypeLayout}, vm_status::StatusCode, @@ -37,7 +38,7 @@ use move_vm_types::{data_store::DataStore, loaded_data::runtime_types as vm_runt use std::{cell::OnceCell, rc::Rc, sync::Arc}; use sui_protocol_config::ProtocolConfig; use sui_types::{ - Identifier, TypeTag, + Identifier, SUI_FRAMEWORK_PACKAGE_ID, TypeTag, base_types::{ObjectID, TxContext}, coin::RESOLVED_COIN_STRUCT, error::{ExecutionError, ExecutionErrorKind}, @@ -162,6 +163,24 @@ impl<'pc, 'vm, 'state, 'linkage> Env<'pc, 'vm, 'state, 'linkage> { .map_err(|e| self.convert_vm_error(e)) } + pub fn load_framework_function( + &self, + module: &IdentStr, + function: &IdentStr, + type_arguments: Vec, + ) -> Result { + let call_linkage = self + .linkage_analysis + .framework_call_linkage(&type_arguments, self.linkable_store)?; + self.load_function( + SUI_FRAMEWORK_PACKAGE_ID, + module.to_string(), + function.to_string(), + type_arguments, + call_linkage, + ) + } + pub fn load_function( &self, package: ObjectID, diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/linkage/analysis.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/linkage/analysis.rs index 6322d013ba641..4d954888066fe 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/linkage/analysis.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/linkage/analysis.rs @@ -1,13 +1,18 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +use std::rc::Rc; + use crate::{ data_store::PackageStore, execution_mode::ExecutionMode, - static_programmable_transactions::linkage::{ - config::{LinkageConfig, ResolutionConfig}, - resolution::{ResolutionTable, VersionConstraint, add_and_unify, get_package}, - resolved_linkage::ResolvedLinkage, + static_programmable_transactions::{ + linkage::{ + config::{LinkageConfig, ResolutionConfig}, + resolution::{ResolutionTable, VersionConstraint, add_and_unify, get_package}, + resolved_linkage::{ResolvedLinkage, RootedLinkage}, + }, + loading::ast::Type, }, }; use sui_protocol_config::ProtocolConfig; @@ -57,6 +62,22 @@ impl LinkageAnalyzer { &self.internal } + pub fn framework_call_linkage( + &self, + // Type arguments do not need to be included in the linkage in the current VM. + _type_args: &[Type], + store: &dyn PackageStore, + ) -> Result { + Ok(RootedLinkage { + link_context: *sui_types::SUI_FRAMEWORK_PACKAGE_ID, + resolved_linkage: Rc::new(ResolvedLinkage::from_resolution_table( + self.internal + .linkage_config + .resolution_table_with_native_packages(store)?, + )), + }) + } + fn compute_call_linkage_( &self, move_call: &P::ProgrammableMoveCall, diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index 43316f4c49772..7885b27e90546 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -7,6 +7,7 @@ use crate::{ programmable_transactions::context::EitherError, sp, static_programmable_transactions::{ + linkage::resolved_linkage::ResolvedLinkage, loading::ast::{self as L, Datatype, Type}, spanned::sp, typing::ast::BytesConstraint, @@ -15,12 +16,15 @@ use crate::{ use indexmap::{IndexMap, IndexSet}; use std::{collections::BTreeMap, rc::Rc}; use sui_types::{ + SUI_FRAMEWORK_PACKAGE_ID, TypeTag, balance::RESOLVED_BALANCE_STRUCT, base_types::{ObjectRef, TxContextKind}, - coin::RESOLVED_COIN_STRUCT, + coin::{COIN_MODULE_NAME, REDEEM_FUNDS_FUNC_NAME, RESOLVED_COIN_STRUCT}, error::{ExecutionError, ExecutionErrorKind, command_argument_error}, execution_status::CommandArgumentError, - funds_accumulator::RESOLVED_WITHDRAWAL_STRUCT, + funds_accumulator::{ + FUNDS_ACCUMULATOR_MODULE_NAME, RESOLVED_WITHDRAWAL_STRUCT, WITHDRAWAL_OWNER_FUNC_NAME, + }, }; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -907,7 +911,7 @@ fn apply_conversion( } /// Returns the inner type `T` if the type is `sui::coin::Coin`, else `None` -fn coin_inner_type(ty: &Type) -> Option<&Type> { +pub(crate) fn coin_inner_type(ty: &Type) -> Option<&Type> { let Type::Datatype(dt) = ty else { return None; }; @@ -968,7 +972,11 @@ fn convert_withdrawal_to_coin( let withdrawl_ref_ty = Type::Reference(false, Rc::new(withdrawal_type.clone())); let withdrawal_borrow = sp(idx, (withdrawal_borrow_, withdrawl_ref_ty)); let owner_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { - function: todo!("call sui::funds_accumulator::withdrawal_owner"), + function: env.load_framework_function( + FUNDS_ACCUMULATOR_MODULE_NAME, + WITHDRAWAL_OWNER_FUNC_NAME, + vec![inner_ty.clone()], + )?, arguments: vec![withdrawal_borrow], })); let owner_command_ = T::Command_ { @@ -985,7 +993,11 @@ fn convert_withdrawal_to_coin( let ctx_ty = Type::Reference(true, Rc::new(env.tx_context_type()?)); let ctx_arg = sp(idx, (ctx_arg_, ctx_ty)); let conversion_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { - function: todo!("call sui::coin::redeem_funds"), + function: env.load_framework_function( + COIN_MODULE_NAME, + REDEEM_FUNDS_FUNC_NAME, + vec![inner_ty.clone()], + )?, arguments: vec![withdrawal_arg, ctx_arg], })); let coin_type = env.coin_type(inner_ty.clone())?; diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs index 2440dd7f478ee..241bc2579e3f8 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs @@ -20,11 +20,21 @@ pub fn refine_and_verify( } mod refine { + use sui_types::{ + SUI_FRAMEWORK_PACKAGE_ID, + coin::{COIN_MODULE_NAME, SEND_FUNDS_FUNC_NAME}, + error::ExecutionError, + }; + use crate::{ sp, static_programmable_transactions::{ + env::Env, spanned::sp, - typing::ast::{self as T, Type}, + typing::{ + ast::{self as T, Type}, + translate::coin_inner_type, + }, }, }; use std::collections::BTreeSet; @@ -112,9 +122,10 @@ mod refine { /// For any withdrawal conversion where the value was not moved, send it back to the original /// owner fn return_unused_withdrawal_conversions( + env: &Env, ast: &mut T::Transaction, moved_locations: &BTreeSet, - ) { + ) -> Result<(), ExecutionError> { for (unmoved_loc, conversion_info) in ast .withdrawal_conversions .iter() @@ -155,13 +166,20 @@ mod refine { "only coin conversion supported" ); let conversion_ty = &conversion_command.value.result_type[0]; + let Some(inner_ty) = coin_inner_type(conversion_ty) else { + invariant_violation!("conversion result should be a coin type") + }; let move_result_ = T::Argument__::new_move(T::Location::Result(conversion_result, 0)); let move_result = sp(cur_command, (move_result_, conversion_ty.clone())); let owner_ty = Type::Address; let owner_arg_ = T::Argument__::new_move(T::Location::Result(owner_result, 0)); let owner_arg = sp(cur_command, (owner_arg_, owner_ty)); let return_command__ = T::Command__::MoveCall(Box::new(T::MoveCall { - function: todo!("call sui::coin::send_funds"), + function: env.load_framework_function( + COIN_MODULE_NAME, + SEND_FUNDS_FUNC_NAME, + vec![inner_ty.clone()], + )?, arguments: vec![move_result, owner_arg], })); let return_command = sp( @@ -175,6 +193,7 @@ mod refine { ); ast.commands.push(return_command); } + Ok(()) } } From dc639cb8b7969ede648aa5039fbd9db687426787 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 24 Nov 2025 16:48:38 -0800 Subject: [PATCH 06/10] fix rebase --- .../static_programmable_transactions/env.rs | 2 +- .../typing/translate.rs | 56 +++++++++---------- .../typing/verify/drop_safety.rs | 19 +++---- 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs index 6a942a8d87639..e7170437c24c1 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs @@ -38,7 +38,7 @@ use move_vm_types::{data_store::DataStore, loaded_data::runtime_types as vm_runt use std::{cell::OnceCell, rc::Rc, sync::Arc}; use sui_protocol_config::ProtocolConfig; use sui_types::{ - Identifier, Identifier, SUI_FRAMEWORK_PACKAGE_ID, TypeTag, TypeTag, + Identifier, SUI_FRAMEWORK_PACKAGE_ID, TypeTag, balance::RESOLVED_BALANCE_STRUCT, base_types::{ObjectID, TxContext}, coin::RESOLVED_COIN_STRUCT, diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index 6fdf0b9343460..b49eb8ee09013 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -5,10 +5,8 @@ use super::{ast as T, env::Env}; use crate::{ execution_mode::ExecutionMode, programmable_transactions::context::EitherError, - sp, static_programmable_transactions::{ - linkage::resolved_linkage::ResolvedLinkage, - loading::ast::{self as L, Datatype, Type}, + loading::ast::{self as L, Type}, spanned::sp, typing::ast::BytesConstraint, }, @@ -16,7 +14,6 @@ use crate::{ use indexmap::{IndexMap, IndexSet}; use std::{collections::BTreeMap, rc::Rc}; use sui_types::{ - SUI_FRAMEWORK_PACKAGE_ID, TypeTag, balance::RESOLVED_BALANCE_STRUCT, base_types::{ObjectRef, TxContextKind}, coin::{COIN_MODULE_NAME, REDEEM_FUNDS_FUNC_NAME, RESOLVED_COIN_STRUCT}, @@ -200,7 +197,7 @@ impl Context { let Some(cur_lift) = prev_lift.checked_add(1) else { invariant_violation!("Cannot increment lift value of {prev_lift}") }; - self.result_index_lift.insert(cur, cur_lift + 1); + self.result_index_lift.insert(cur, cur_lift); self.commands.push(sp(self.current_command, command)); Ok(cur) } @@ -257,6 +254,12 @@ impl Context { }; object_input.ty.clone() } + T::Location::WithdrawalInput(i) => { + let Some((_, withdrawal_input)) = self.withdrawals.get_index(i as usize) else { + invariant_violation!("Unbound withdrawal input {}", i) + }; + withdrawal_input.ty.clone() + } T::Location::PureInput(_) | T::Location::ReceivingInput(_) => return Ok(None), })) } @@ -278,15 +281,10 @@ impl Context { T::Location::ObjectInput(index as u16) } InputKind::Withdrawal => { - let Some((withdrawal_index, _, withdrawal_input)) = - self.withdrawals.get_full(&i) - else { + let Some(withdrawal_index) = self.withdrawals.get_index_of(&i) else { invariant_violation!("Unbound withdrawal input {}", i.0) }; - ( - T::Location::WithdrawalInput(withdrawal_index as u16), - withdrawal_input.ty.clone(), - ) + T::Location::WithdrawalInput(withdrawal_index as u16) } InputKind::Pure | InputKind::Receiving => return Ok(None), }, @@ -383,25 +381,21 @@ pub fn transaction( ) -> Result { let L::Transaction { inputs, commands } = lt; let mut context = Context::new(inputs)?; - commands - .into_iter() - .enumerate() - .map(|(i, c)| { - let idx = i as u16; - context.current_command = idx; - let (c_, tys) = - command::(env, &mut context, c).map_err(|e| e.with_command_index(i))?; - let c = T::Command_ { - command: c_, - result_type: tys, - // computed later - drop_values: vec![], - // computed later - consumed_shared_objects: vec![], - }; - context.push_result(c) - }) - .collect::>()?; + for (i, c) in commands.into_iter().enumerate() { + let idx = i as u16; + context.current_command = idx; + let (c_, tys) = + command::(env, &mut context, c).map_err(|e| e.with_command_index(i))?; + let c = T::Command_ { + command: c_, + result_type: tys, + // computed later + drop_values: vec![], + // computed later + consumed_shared_objects: vec![], + }; + context.push_result(c)? + } let mut ast = context.finish(); // mark the last usage of references as Move instead of Copy scope_references::transaction(&mut ast); diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs index 821658fb8d182..e6b83f8c5b18b 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs @@ -14,14 +14,13 @@ pub fn refine_and_verify( env: &Env, ast: &mut T::Transaction, ) -> Result<(), ExecutionError> { - refine::transaction(ast); + refine::transaction(env, ast)?; verify::transaction::(env, ast)?; Ok(()) } mod refine { use sui_types::{ - SUI_FRAMEWORK_PACKAGE_ID, coin::{COIN_MODULE_NAME, SEND_FUNDS_FUNC_NAME}, error::ExecutionError, }; @@ -57,11 +56,12 @@ mod refine { /// After memory safety, we can switch the last usage of a `Copy` to a `Move` if it is not /// borrowed at the time of the last usage. - pub fn transaction(ast: &mut T::Transaction) { + pub fn transaction(env: &Env, ast: &mut T::Transaction) -> Result<(), ExecutionError> { let mut context = Context::new(); for c in ast.commands.iter_mut().rev() { command(&mut context, c); } + return_unused_withdrawal_conversions(env, ast, &context.moved) } fn command(context: &mut Context, sp!(_, c): &mut T::Command) { @@ -126,11 +126,10 @@ mod refine { ast: &mut T::Transaction, moved_locations: &BTreeSet, ) -> Result<(), ExecutionError> { - for (unmoved_loc, conversion_info) in ast - .withdrawal_conversions - .iter() - .filter(|(l, _)| !moved_locations.contains(l)) - { + for conversion_info in ast.withdrawal_conversions.values().filter(|conversion| { + let conversion_location = T::Location::Result(conversion.conversion_result, 0); + !moved_locations.contains(&conversion_location) + }) { let Some(cur_command) = ast.commands.len().checked_sub(1) else { invariant_violation!("cannot be zero commands with a conversion") }; @@ -143,11 +142,11 @@ mod refine { // set owner result as used let owner_command = &mut ast.commands[owner_result as usize]; assert_invariant!( - owner_command.value.drop_values.as_slice() == &[true], + owner_command.value.drop_values.as_slice() == [true], "owner result should be unused thus far and should be dropped" ); owner_command.value.drop_values[0] = false; - let owner_command = &*owner_command; + let owner_command = &ast.commands[owner_result as usize]; let conversion_command = &ast.commands[conversion_result as usize]; assert_invariant!( conversion_command.value.result_type.len() == 1, From 08b567fdc435a1ca153a822df56c8e6cc9b7c816 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 24 Nov 2025 16:52:59 -0800 Subject: [PATCH 07/10] add additional asserts --- .../static_programmable_transactions/loading/translate.rs | 4 ++++ .../src/static_programmable_transactions/typing/translate.rs | 3 ++- .../typing/verify/drop_safety.rs | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/loading/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/loading/translate.rs index c3750b59943d9..4b618c3e693ad 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/loading/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/loading/translate.rs @@ -95,6 +95,10 @@ fn input( ) } CallArg::FundsWithdrawal(f) => { + assert_invariant!( + env.protocol_config.enable_accumulators(), + "Withdrawals should be rejected at signing if accumulators are not enabled" + ); let FundsWithdrawalArg { reservation, type_arg, diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index b49eb8ee09013..a18b6deb0694d 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -935,7 +935,8 @@ fn apply_conversion( Type::Reference(_, inner) => &**inner, ty => ty, }; - if let Some(expected_inner) = coin_inner_type(expected_ty) + if env.protocol_config.enable_accumulators() + && let Some(expected_inner) = coin_inner_type(expected_ty) && let Some(actual_withdrawal_inner) = withdrawal_inner_type(&actual_ty) && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) && actual_inner == expected_inner diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs index e6b83f8c5b18b..adf0bb2851422 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/verify/drop_safety.rs @@ -126,6 +126,11 @@ mod refine { ast: &mut T::Transaction, moved_locations: &BTreeSet, ) -> Result<(), ExecutionError> { + // withdrawal conversions not empty ==> accumulators enabled + assert_invariant!( + ast.withdrawal_conversions.is_empty() || env.protocol_config.enable_accumulators(), + "Withdrawal conversions should be empty if accumulators are not enabled" + ); for conversion_info in ast.withdrawal_conversions.values().filter(|conversion| { let conversion_location = T::Location::Result(conversion.conversion_result, 0); !moved_locations.contains(&conversion_location) From 46a9ab2d3180ab201021efa04e32eafa39a458f0 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 1 Dec 2025 14:26:10 -0800 Subject: [PATCH 08/10] convert withdrawal coin mut ref --- .../typing/translate.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index a18b6deb0694d..4dd24a2a38b46 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -881,7 +881,7 @@ fn coin_mut_ref_argument( command_arg_idx: usize, location: SplatLocation, ) -> Result { - let arg_ = coin_mut_ref_argument_(env, context, location) + let arg_ = coin_mut_ref_argument_(env, context, command_arg_idx, location) .map_err(|e| e.into_execution_error(command_arg_idx))?; Ok(sp(command_arg_idx as u16, arg_)) } @@ -889,6 +889,7 @@ fn coin_mut_ref_argument( fn coin_mut_ref_argument_( env: &Env, context: &mut Context, + command_arg_idx: usize, location: SplatLocation, ) -> Result { let Some((location, actual_ty)) = context.fixed_type(env, location)? else { @@ -896,6 +897,21 @@ fn coin_mut_ref_argument_( // inference not currently supported return Err(CommandArgumentError::TypeMismatch.into()); }; + let (location, actual_ty) = if env.protocol_config.enable_accumulators() + && let Some(actual_withdrawal_inner) = withdrawal_inner_type(&actual_ty) + && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) + { + convert_withdrawal_to_coin( + env, + context, + command_arg_idx, + location, + actual_ty.clone(), + actual_inner, + )? + } else { + (location, actual_ty) + }; Ok(match &actual_ty { Type::Reference(is_mut, ty) if *is_mut => { check_coin_type(ty)?; From 9d46c28a5f16e708e084de2c462d5d644217b47d Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 1 Dec 2025 20:15:20 -0800 Subject: [PATCH 09/10] protocol config and cleanup --- crates/sui-protocol-config/src/lib.rs | 12 +++ .../move-binary-format/src/file_format.rs | 28 ++--- .../typing/translate.rs | 101 +++++++++--------- 3 files changed, 76 insertions(+), 65 deletions(-) diff --git a/crates/sui-protocol-config/src/lib.rs b/crates/sui-protocol-config/src/lib.rs index 0f92dc7c800c7..c81635b1a50cc 100644 --- a/crates/sui-protocol-config/src/lib.rs +++ b/crates/sui-protocol-config/src/lib.rs @@ -894,6 +894,10 @@ struct FeatureFlags { // If true, skip GC'ed accept votes in CommitFinalizer. #[serde(skip_serializing_if = "is_false")] consensus_skip_gced_accept_votes: bool, + + // If true, automatically convert withdrawal PTB arguments to coin or balance when needed. + #[serde(skip_serializing_if = "is_false")] + convert_withdrawal_ptb_arguments: bool, } fn is_false(b: &bool) -> bool { @@ -2408,6 +2412,10 @@ impl ProtocolConfig { pub fn consensus_skip_gced_accept_votes(&self) -> bool { self.feature_flags.consensus_skip_gced_accept_votes } + + pub fn convert_withdrawal_ptb_arguments(&self) -> bool { + self.feature_flags.convert_withdrawal_ptb_arguments + } } #[cfg(not(msim))] @@ -4646,6 +4654,10 @@ impl ProtocolConfig { pub fn set_consensus_skip_gced_accept_votes_for_testing(&mut self, val: bool) { self.feature_flags.consensus_skip_gced_accept_votes = val; } + + pub fn enable_withdrawal_ptb_argument_conversion_for_testing(&mut self, val: bool) { + self.feature_flags.convert_withdrawal_ptb_arguments = val; + } } type OverrideFn = dyn Fn(ProtocolVersion, ProtocolConfig) -> ProtocolConfig + Send; diff --git a/external-crates/move/crates/move-binary-format/src/file_format.rs b/external-crates/move/crates/move-binary-format/src/file_format.rs index c8c82c107666d..f13745276dcc1 100644 --- a/external-crates/move/crates/move-binary-format/src/file_format.rs +++ b/external-crates/move/crates/move-binary-format/src/file_format.rs @@ -824,53 +824,53 @@ impl AbilitySet { | (Ability::Key as u8), ); - pub fn singleton(ability: Ability) -> Self { + pub const fn singleton(ability: Ability) -> Self { Self(ability as u8) } - pub fn has_ability(self, ability: Ability) -> bool { + pub const fn has_ability(self, ability: Ability) -> bool { let a = ability as u8; (a & self.0) == a } - pub fn has_copy(self) -> bool { + pub const fn has_copy(self) -> bool { self.has_ability(Ability::Copy) } - pub fn has_drop(self) -> bool { + pub const fn has_drop(self) -> bool { self.has_ability(Ability::Drop) } - pub fn has_store(self) -> bool { + pub const fn has_store(self) -> bool { self.has_ability(Ability::Store) } - pub fn has_key(self) -> bool { + pub const fn has_key(self) -> bool { self.has_ability(Ability::Key) } - pub fn remove(self, ability: Ability) -> Self { + pub const fn remove(self, ability: Ability) -> Self { self.difference(Self::singleton(ability)) } - pub fn intersect(self, other: Self) -> Self { + pub const fn intersect(self, other: Self) -> Self { Self(self.0 & other.0) } - pub fn union(self, other: Self) -> Self { + pub const fn union(self, other: Self) -> Self { Self(self.0 | other.0) } - pub fn difference(self, other: Self) -> Self { + pub const fn difference(self, other: Self) -> Self { Self(self.0 & !other.0) } #[inline] - fn is_subset_bits(sub: u8, sup: u8) -> bool { + const fn is_subset_bits(sub: u8, sup: u8) -> bool { (sub & sup) == sub } - pub fn is_subset(self, other: Self) -> bool { + pub const fn is_subset(self, other: Self) -> bool { Self::is_subset_bits(self.0, other.0) } @@ -922,7 +922,7 @@ impl AbilitySet { Ok(abs) } - pub fn from_u8(byte: u8) -> Option { + pub const fn from_u8(byte: u8) -> Option { // If there is a bit set in the read `byte`, that bit must be set in the // `AbilitySet` containing all `Ability`s // This corresponds the byte being a bit set subset of ALL @@ -934,7 +934,7 @@ impl AbilitySet { } } - pub fn into_u8(self) -> u8 { + pub const fn into_u8(self) -> u8 { self.0 } } diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs index 4dd24a2a38b46..99857265fbd47 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/typing/translate.rs @@ -12,6 +12,7 @@ use crate::{ }, }; use indexmap::{IndexMap, IndexSet}; +use move_binary_format::file_format::{Ability, AbilitySet}; use std::{collections::BTreeMap, rc::Rc}; use sui_types::{ balance::RESOLVED_BALANCE_STRUCT, @@ -472,6 +473,8 @@ fn command( ) } L::Command::TransferObjects(lobjects, laddress) => { + const TRANSFER_OBJECTS_CONSTRAINT: AbilitySet = + AbilitySet::singleton(Ability::Store).union(AbilitySet::singleton(Ability::Key)); let object_locs = locations(context, 0, lobjects)?; let address_loc = one_location(context, object_locs.len(), laddress)?; let objects = constrained_arguments( @@ -479,10 +482,7 @@ fn command( context, 0, object_locs, - |ty| { - let abilities = ty.abilities(); - Ok(abilities.has_store() && abilities.has_key()) - }, + TRANSFER_OBJECTS_CONSTRAINT, CommandArgumentError::InvalidTransferObject, )?; let address = argument(env, context, objects.len(), address_loc, Type::Address)?; @@ -538,6 +538,7 @@ fn command( ) } L::Command::MakeMoveVec(None, lelems) => { + const MAKE_MOVE_VEC_OBJECT_CONSTRAINT: AbilitySet = AbilitySet::singleton(Ability::Key); let mut lelems = lelems.into_iter(); let Some(lfirst) = lelems.next() else { // TODO maybe this should be a different errors for CLI usage @@ -551,7 +552,7 @@ fn command( context, 0, first_loc, - |ty| Ok(ty.abilities().has_key()), + MAKE_MOVE_VEC_OBJECT_CONSTRAINT, CommandArgumentError::InvalidMakeMoveVecNonObjectArgument, )?; let first_ty = first_arg.value.1.clone(); @@ -789,15 +790,14 @@ fn check_type(actual_ty: &Type, expected_ty: &Type) -> Result<(), CommandArgumen } } -fn constrained_arguments Result>( +fn constrained_arguments( env: &Env, context: &mut Context, start_idx: usize, locations: Vec, - mut is_valid: P, + constraint: AbilitySet, err_case: CommandArgumentError, ) -> Result, ExecutionError> { - let is_valid = &mut is_valid; locations .into_iter() .enumerate() @@ -805,50 +805,42 @@ fn constrained_arguments Result>( let Some(idx) = start_idx.checked_add(i) else { invariant_violation!("usize overflow when calculating argument index"); }; - constrained_argument_(env, context, idx, location, is_valid, err_case) + constrained_argument(env, context, idx, location, constraint, err_case) }) .collect() } -fn constrained_argument Result>( +fn constrained_argument( env: &Env, context: &mut Context, command_arg_idx: usize, location: SplatLocation, - mut is_valid: P, + constraint: AbilitySet, err_case: CommandArgumentError, ) -> Result { - constrained_argument_( + let arg_ = constrained_argument_( env, context, command_arg_idx, location, - &mut is_valid, + constraint, err_case, ) -} - -fn constrained_argument_ Result>( - env: &Env, - context: &mut Context, - command_arg_idx: usize, - location: SplatLocation, - is_valid: &mut P, - err_case: CommandArgumentError, -) -> Result { - let arg_ = constrained_argument__(env, context, location, is_valid, err_case) - .map_err(|e| e.into_execution_error(command_arg_idx))?; + .map_err(|e| e.into_execution_error(command_arg_idx))?; Ok(sp(command_arg_idx as u16, arg_)) } -fn constrained_argument__ Result>( +fn constrained_argument_( env: &Env, context: &mut Context, + command_arg_idx: usize, location: SplatLocation, - is_valid: &mut P, + constraint: AbilitySet, err_case: CommandArgumentError, ) -> Result { - if let Some((location, ty)) = constrained_type(env, context, location, is_valid)? { + if let Some((location, ty)) = + constrained_type(env, context, command_arg_idx, location, constraint)? + { if ty.abilities().has_copy() { Ok((T::Argument__::new_copy(location), ty)) } else { @@ -859,16 +851,27 @@ fn constrained_argument__ Result>( } } -fn constrained_type<'a, P: FnMut(&Type) -> Result>( +fn constrained_type<'a>( env: &'a Env, context: &'a mut Context, + command_arg_idx: usize, location: SplatLocation, - mut is_valid: P, + constraint: AbilitySet, ) -> Result, ExecutionError> { let Some((location, ty)) = context.fixed_type(env, location)? else { return Ok(None); }; - Ok(if is_valid(&ty)? { + // if the argument is a balance withdrawal, and an object is needed, convert it to a coin + let (location, ty) = if env.protocol_config.convert_withdrawal_ptb_arguments() + && constraint.has_key() + && let Some(ty_withdrawal_inner) = withdrawal_inner_type(&ty) + && balance_inner_type(ty_withdrawal_inner).is_some() + { + convert_withdrawal_to_coin(env, context, command_arg_idx, location, ty)? + } else { + (location, ty) + }; + Ok(if constraint.is_subset(ty.abilities()) { Some((location, ty)) } else { None @@ -897,18 +900,12 @@ fn coin_mut_ref_argument_( // inference not currently supported return Err(CommandArgumentError::TypeMismatch.into()); }; - let (location, actual_ty) = if env.protocol_config.enable_accumulators() + // if the argument is a balance withdrawal, convert it to a coin + let (location, actual_ty) = if env.protocol_config.convert_withdrawal_ptb_arguments() && let Some(actual_withdrawal_inner) = withdrawal_inner_type(&actual_ty) - && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) + && balance_inner_type(actual_withdrawal_inner).is_some() { - convert_withdrawal_to_coin( - env, - context, - command_arg_idx, - location, - actual_ty.clone(), - actual_inner, - )? + convert_withdrawal_to_coin(env, context, command_arg_idx, location, actual_ty)? } else { (location, actual_ty) }; @@ -951,20 +948,13 @@ fn apply_conversion( Type::Reference(_, inner) => &**inner, ty => ty, }; - if env.protocol_config.enable_accumulators() + if env.protocol_config.convert_withdrawal_ptb_arguments() && let Some(expected_inner) = coin_inner_type(expected_ty) && let Some(actual_withdrawal_inner) = withdrawal_inner_type(&actual_ty) && let Some(actual_inner) = balance_inner_type(actual_withdrawal_inner) && actual_inner == expected_inner { - convert_withdrawal_to_coin( - env, - context, - command_arg_idx, - location, - actual_ty, - expected_inner, - ) + convert_withdrawal_to_coin(env, context, command_arg_idx, location, actual_ty) } else { Ok((location, actual_ty)) } @@ -1024,8 +1014,17 @@ fn convert_withdrawal_to_coin( command_arg_idx: usize, location: T::Location, withdrawal_type: Type, - inner_ty: &Type, ) -> Result<(T::Location, Type), ExecutionError> { + assert_invariant!( + env.protocol_config.convert_withdrawal_ptb_arguments(), + "convert_withdrawal_to_coin called when conversion is disabled" + ); + let Some(inner_ty) = withdrawal_inner_type(&withdrawal_type) + .and_then(balance_inner_type) + .cloned() + else { + invariant_violation!("convert_withdrawal_to_coin called with non-withdrawal type"); + }; let idx = command_arg_idx as u16; // first store the owner of the withdrawal let withdrawal_borrow_ = T::Argument__::Borrow(false, location); From f86c405d0227a1923ffef4d5f1b2cb942a4ed201 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 1 Dec 2025 20:23:53 -0800 Subject: [PATCH 10/10] cleanup --- .../static_programmable_transactions/env.rs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs index e7170437c24c1..a9afe8eae1f55 100644 --- a/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs +++ b/sui-execution/latest/sui-adapter/src/static_programmable_transactions/env.rs @@ -316,14 +316,12 @@ impl<'pc, 'vm, 'state, 'linkage> Env<'pc, 'vm, 'state, 'linkage> { } pub fn coin_type(&self, inner_type: Type) -> Result { - let Some(abilities) = AbilitySet::from_u8((Ability::Key as u8) | (Ability::Store as u8)) - else { - invariant_violation!("Unable to create coin abilities"); - }; + const COIN_ABILITIES: AbilitySet = + AbilitySet::singleton(Ability::Key).union(AbilitySet::singleton(Ability::Store)); let (a, m, n) = RESOLVED_COIN_STRUCT; let module = ModuleId::new(*a, m.to_owned()); Ok(Type::Datatype(Rc::new(Datatype { - abilities, + abilities: COIN_ABILITIES, module, name: n.to_owned(), type_arguments: vec![inner_type], @@ -331,13 +329,11 @@ impl<'pc, 'vm, 'state, 'linkage> Env<'pc, 'vm, 'state, 'linkage> { } pub fn balance_type(&self, inner_type: Type) -> Result { - let Some(abilities) = AbilitySet::from_u8(Ability::Store as u8) else { - invariant_violation!("Unable to create balance abilities"); - }; + const BALANCE_ABILITIES: AbilitySet = AbilitySet::singleton(Ability::Store); let (a, m, n) = RESOLVED_BALANCE_STRUCT; let module = ModuleId::new(*a, m.to_owned()); Ok(Type::Datatype(Rc::new(Datatype { - abilities, + abilities: BALANCE_ABILITIES, module, name: n.to_owned(), type_arguments: vec![inner_type], @@ -345,13 +341,11 @@ impl<'pc, 'vm, 'state, 'linkage> Env<'pc, 'vm, 'state, 'linkage> { } pub fn withdrawal_type(&self, inner_type: Type) -> Result { - let Some(abilities) = AbilitySet::from_u8(Ability::Drop as u8) else { - invariant_violation!("Unable to create withdrawal abilities"); - }; + const WITHDRAWAL_ABILITIES: AbilitySet = AbilitySet::singleton(Ability::Drop); let (a, m, n) = RESOLVED_WITHDRAWAL_STRUCT; let module = ModuleId::new(*a, m.to_owned()); Ok(Type::Datatype(Rc::new(Datatype { - abilities, + abilities: WITHDRAWAL_ABILITIES, module, name: n.to_owned(), type_arguments: vec![inner_type],