From 7bc50b1f5ff6d47cd2c769c16964baf1121794fc Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Wed, 8 Jan 2025 16:29:38 -0800 Subject: [PATCH 1/3] unify packages in ptb logic --- crates/sui-types/src/move_package.rs | 24 +- sui-execution/latest/sui-adapter/src/lib.rs | 1 + .../sui-adapter/src/package_unification.rs | 379 ++++++++++++++++++ .../programmable_transactions/execution.rs | 8 + 4 files changed, 404 insertions(+), 8 deletions(-) create mode 100644 sui-execution/latest/sui-adapter/src/package_unification.rs diff --git a/crates/sui-types/src/move_package.rs b/crates/sui-types/src/move_package.rs index 49e8798feedce..ee251da6569a1 100644 --- a/crates/sui-types/src/move_package.rs +++ b/crates/sui-types/src/move_package.rs @@ -96,6 +96,7 @@ pub struct UpgradeInfo { // serde_bytes::ByteBuf is an analog of Vec with built-in fast serialization. #[serde_as] #[derive(Eq, PartialEq, Debug, Clone, Deserialize, Serialize, Hash)] +#[serde(rename_all = "camelCase")] pub struct MovePackage { id: ObjectID, /// Most move packages are uniquely identified by their ID (i.e. there is only one version per @@ -111,7 +112,7 @@ pub struct MovePackage { version: SequenceNumber, // TODO use session cache #[serde_as(as = "BTreeMap<_, Bytes>")] - module_map: BTreeMap>, + pub module_map: BTreeMap>, /// Maps struct/module to a package version where it was first defined, stored as a vector for /// simple serialization and deserialization. @@ -606,18 +607,25 @@ where { let mut normalized_modules = BTreeMap::new(); for bytecode in modules { - let module = - CompiledModule::deserialize_with_config(bytecode, binary_config).map_err(|error| { - SuiError::ModuleDeserializationFailure { - error: error.to_string(), - } - })?; - let normalized_module = normalized::Module::new(&module); + let normalized_module = normalize_module(bytecode, binary_config)?; normalized_modules.insert(normalized_module.name.to_string(), normalized_module); } Ok(normalized_modules) } +pub fn normalize_module<'a>( + bytecode: &'a Vec, + binary_config: &BinaryConfig, +) -> SuiResult { + let module = + CompiledModule::deserialize_with_config(bytecode, binary_config).map_err(|error| { + SuiError::ModuleDeserializationFailure { + error: error.to_string(), + } + })?; + Ok(normalized::Module::new(&module)) +} + pub fn normalize_deserialized_modules<'a, I>(modules: I) -> BTreeMap where I: Iterator, diff --git a/sui-execution/latest/sui-adapter/src/lib.rs b/sui-execution/latest/sui-adapter/src/lib.rs index 51651cb5bc339..4171a227c154d 100644 --- a/sui-execution/latest/sui-adapter/src/lib.rs +++ b/sui-execution/latest/sui-adapter/src/lib.rs @@ -10,6 +10,7 @@ pub mod execution_engine; pub mod execution_mode; pub mod execution_value; pub mod gas_charger; +pub mod package_unification; pub mod programmable_transactions; pub mod temporary_store; pub mod type_layout_resolver; diff --git a/sui-execution/latest/sui-adapter/src/package_unification.rs b/sui-execution/latest/sui-adapter/src/package_unification.rs new file mode 100644 index 0000000000000..8904c1fda6806 --- /dev/null +++ b/sui-execution/latest/sui-adapter/src/package_unification.rs @@ -0,0 +1,379 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use move_binary_format::{binary_config::BinaryConfig, file_format::Visibility}; +use sui_types::{ + base_types::{ObjectID, SequenceNumber}, + move_package::{normalize_module, MovePackage}, + storage::BackingPackageStore, + transaction::{Command, ProgrammableTransaction}, + type_input::TypeInput, + Identifier, +}; + +pub struct PTBLinkageMetadata { + pub types: Vec, + pub entry_functions: Vec, + pub non_entry_functions: Vec, + pub publication_linkages: Vec, + pub upgrading_packages: Vec, + pub all_packages: BTreeMap, +} + +pub struct LinkageConfig { + pub fix_top_level_functions: bool, + pub fix_types: bool, +} + +#[derive(Debug)] +pub enum ConflictResolution { + Exact(SequenceNumber, ObjectID), + AtLeast(SequenceNumber, ObjectID), + Never(ObjectID), +} + +impl LinkageConfig { + pub fn strict() -> Self { + Self { + fix_top_level_functions: true, + fix_types: true, + } + } + + pub fn loose() -> Self { + Self { + fix_top_level_functions: false, + fix_types: false, + } + } +} + +impl ConflictResolution { + pub fn unify(&mut self, other: &ConflictResolution) -> anyhow::Result<()> { + match (&self, other) { + // If we ever try to unify with a Never we fail. + (ConflictResolution::Never(_), _) | (_, ConflictResolution::Never(_)) => { + return Err(anyhow::anyhow!( + "Cannot unify with Never: {:?} and {:?}", + self, + other + )); + } + // If we have two exact resolutions, they must be the same. + (ConflictResolution::Exact(sv, self_id), ConflictResolution::Exact(ov, other_id)) => { + if self_id != other_id { + return Err(anyhow::anyhow!( + "Exact/exact conflicting resolutions for packages: {self_id}@{sv} and {other_id}@{ov}", + )); + } + } + // Take the max if you have two at least resolutions. + ( + ConflictResolution::AtLeast(self_version, sid), + ConflictResolution::AtLeast(other_version, oid), + ) => { + let id = if self_version > other_version { + *sid + } else { + *oid + }; + + *self = ConflictResolution::AtLeast(*self_version.max(other_version), id); + } + // If you unify an exact and an at least, the exact must be greater than or equal to + // the at least. It unifies to an exact. + ( + ConflictResolution::Exact(exact_version, self_id), + ConflictResolution::AtLeast(at_least_version, oid), + ) + | ( + ConflictResolution::AtLeast(at_least_version, oid), + ConflictResolution::Exact(exact_version, self_id), + ) => { + if exact_version < at_least_version { + return Err(anyhow::anyhow!( + "Exact/at least Conflicting resolutions for packages: Exact {self_id}@{exact_version} and {oid}@{at_least_version}", + )); + } + + *self = ConflictResolution::Exact(*exact_version, *self_id); + } + } + + Ok(()) + } +} + +impl PTBLinkageMetadata { + pub fn from_ptb( + ptb: &ProgrammableTransaction, + store: &dyn BackingPackageStore, + ) -> anyhow::Result { + let mut linkage = PTBLinkageMetadata { + types: Vec::new(), + entry_functions: Vec::new(), + non_entry_functions: Vec::new(), + publication_linkages: Vec::new(), + upgrading_packages: Vec::new(), + all_packages: BTreeMap::new(), + }; + + for command in &ptb.commands { + linkage.add_command(command, store)?; + } + + Ok(linkage) + } + + fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> anyhow::Result<()> { + match command { + Command::MoveCall(programmable_move_call) => { + let pkg = self.get_package(&programmable_move_call.package, store)?; + + // TODO/XXX: Make this work without needing to normalize the module. + let module = normalize_module( + pkg.serialized_module_map() + .get(&programmable_move_call.module) + .ok_or_else(|| { + anyhow::anyhow!( + "Module {} not found in package {}", + programmable_move_call.module, + pkg.id() + ) + })?, + &BinaryConfig::standard(), + )?; + let function = module + .functions + .get(&Identifier::new(programmable_move_call.function.clone())?) + .ok_or_else(|| { + anyhow::anyhow!( + "Function {} not found in module {}", + programmable_move_call.function, + module.module_id(), + ) + })?; + + let pkg_id = pkg.id(); + let transitive_deps = pkg + .linkage_table() + .values() + .map(|info| info.upgraded_id) + .collect::>(); + + // load transitive deps + for id in transitive_deps { + self.get_package(&id, store)?; + } + + // Register function entrypoint + if function.is_entry && function.visibility != Visibility::Public { + self.entry_functions.push(pkg_id); + } else { + self.non_entry_functions.push(pkg_id); + } + + for ty in &programmable_move_call.type_arguments { + self.add_type(ty, store)?; + } + } + Command::MakeMoveVec(type_input, _) => { + if let Some(ty) = type_input { + self.add_type(ty, store)?; + } + } + Command::Upgrade(_, transitive_deps, upgrading_object_id, _) => { + self.upgrading_packages.push(*upgrading_object_id); + self.get_package(upgrading_object_id, store)?; + + self.publication_linkages.extend_from_slice(transitive_deps); + for object_id in transitive_deps { + self.get_package(object_id, store)?; + } + } + Command::Publish(_, transitive_deps) => { + self.publication_linkages.extend_from_slice(transitive_deps); + for object_id in transitive_deps { + self.get_package(object_id, store)?; + } + } + Command::TransferObjects(_, _) => (), + Command::SplitCoins(_, _) => (), + Command::MergeCoins(_, _) => (), + }; + + Ok(()) + } + + fn add_type(&mut self, ty: &TypeInput, store: &dyn BackingPackageStore) -> anyhow::Result<()> { + let mut stack = vec![ty]; + while let Some(ty) = stack.pop() { + match ty { + TypeInput::Bool + | TypeInput::U8 + | TypeInput::U64 + | TypeInput::U128 + | TypeInput::Address + | TypeInput::Signer + | TypeInput::U16 + | TypeInput::U32 + | TypeInput::U256 => (), + TypeInput::Vector(type_input) => { + stack.push(&**type_input); + } + TypeInput::Struct(struct_input) => { + let pkg = self + .get_package(&ObjectID::from(struct_input.address), store)? + .id(); + self.types.push(pkg); + for ty in struct_input.type_params.iter() { + stack.push(ty); + } + } + } + } + Ok(()) + } + + // Gather and dedup all packages loaded by the PTB. + // Also gathers the versions of each package loaded by the PTB at the same time. + fn get_package( + &mut self, + object_id: &ObjectID, + store: &dyn BackingPackageStore, + ) -> anyhow::Result<&MovePackage> { + if !self.all_packages.contains_key(object_id) { + let package = store + .get_package_object(object_id)? + .ok_or_else(|| anyhow::anyhow!("Object {} not found in any package", object_id))? + .move_package() + .clone(); + self.all_packages.insert(*object_id, package); + } + + Ok(self + .all_packages + .get(object_id) + .expect("Guaranteed to exist")) + } +} + +impl PTBLinkageMetadata { + pub fn try_compute_unified_linkage( + &self, + linking_config: LinkageConfig, + ) -> anyhow::Result> { + let mut unification_table = BTreeMap::new(); + + // Any packages that are being upgraded cannot be called in the transaction + for object_id in self.upgrading_packages.iter() { + Self::add_and_unify( + &mut unification_table, + &self.all_packages, + object_id, + |pkg| ConflictResolution::Never(pkg.id()), + )?; + } + + // Linkages for packages that are to be published must be exact. + for object_id in self.publication_linkages.iter() { + Self::add_and_unify( + &mut unification_table, + &self.all_packages, + object_id, + |pkg| ConflictResolution::Exact(pkg.version(), pkg.id()), + )?; + } + + for object_id in self.entry_functions.iter() { + Self::add_and_unify( + &mut unification_table, + &self.all_packages, + object_id, + |pkg| ConflictResolution::Exact(pkg.version(), pkg.id()), + )?; + + // transitive closure of entry functions are fixed + for dep_id in self.all_packages[object_id] + .linkage_table() + .values() + .map(|info| &info.upgraded_id) + { + Self::add_and_unify(&mut unification_table, &self.all_packages, dep_id, |pkg| { + ConflictResolution::Exact(pkg.version(), pkg.id()) + })?; + } + } + + // Types can be fixed or not depending on config. + for object_id in self.types.iter() { + Self::add_and_unify( + &mut unification_table, + &self.all_packages, + object_id, + |pkg| { + if linking_config.fix_types { + ConflictResolution::Exact(pkg.version(), pkg.id()) + } else { + ConflictResolution::AtLeast(pkg.version(), *object_id) + } + }, + )?; + } + + // Top level functions can be fixed or not depending on config. But they won't ever + // transitively fix their dependencies. + for object_id in self.non_entry_functions.iter() { + Self::add_and_unify( + &mut unification_table, + &self.all_packages, + object_id, + |pkg| { + if linking_config.fix_top_level_functions { + ConflictResolution::Exact(pkg.version(), pkg.id()) + } else { + ConflictResolution::AtLeast(pkg.version(), *object_id) + } + }, + )?; + } + + Ok(unification_table + .into_values() + .flat_map(|unifier| match unifier { + ConflictResolution::Exact(_, object_id) => Some(object_id), + ConflictResolution::AtLeast(_, object_id) => Some(object_id), + ConflictResolution::Never(_) => None, + }) + .collect()) + } + + // Add a package to the unification table, unifying it with any existing package in the table. + // Errors if the packages cannot be unified (e.g., if one is exact and the other is not). + fn add_and_unify( + unification_table: &mut BTreeMap, + loaded_top_level_packages: &BTreeMap, + object_id: &ObjectID, + resolution_fn: impl Fn(&MovePackage) -> ConflictResolution, + ) -> anyhow::Result<()> { + let package = loaded_top_level_packages + .get(object_id) + .ok_or_else(|| anyhow::anyhow!("Object {} not found in any package", object_id))?; + + let resolution = resolution_fn(package); + + if unification_table.contains_key(&package.original_package_id()) { + let existing_unifier = unification_table + .get_mut(&package.original_package_id()) + .expect("Guaranteed to exist"); + existing_unifier.unify(&resolution)?; + } else { + unification_table.insert(package.original_package_id(), resolution); + } + + Ok(()) + } +} diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs index e6822854aad01..afcc3c7b5b16a 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs @@ -10,6 +10,7 @@ mod checked { CommandKind, ExecutionState, ObjectContents, ObjectValue, RawValueType, Value, }; use crate::gas_charger::GasCharger; + use crate::package_unification::{LinkageConfig, PTBLinkageMetadata}; use move_binary_format::{ compatibility::{Compatibility, InclusionCheck}, errors::{Location, PartialVMResult, VMResult}, @@ -75,6 +76,13 @@ mod checked { gas_charger: &mut GasCharger, pt: ProgrammableTransaction, ) -> Result { + // let linking_metadata = + // PTBLinkageMetadata::from_ptb(&pt, state_view.as_backing_package_store()) + // .expect("Unable to create linking metadata"); + // linking_metadata + // .try_compute_unified_linkage(LinkageConfig::loose()) + // .expect("Unable to unify linkage across ptb"); + let ProgrammableTransaction { inputs, commands } = pt; let mut context = ExecutionContext::new( protocol_config, From d82b182e0c00ed727b2846108030f71b2269eea1 Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Mon, 13 Jan 2025 15:22:53 -0800 Subject: [PATCH 2/3] Update unification logic to be one-pass + fix issue --- crates/sui-types/src/move_package.rs | 20 +- .../sui-adapter/src/package_unification.rs | 324 +++++++----------- .../programmable_transactions/execution.rs | 7 - 3 files changed, 137 insertions(+), 214 deletions(-) diff --git a/crates/sui-types/src/move_package.rs b/crates/sui-types/src/move_package.rs index ee251da6569a1..ae26ba89602d3 100644 --- a/crates/sui-types/src/move_package.rs +++ b/crates/sui-types/src/move_package.rs @@ -510,14 +510,22 @@ impl MovePackage { &self, module: &Identifier, binary_config: &BinaryConfig, + ) -> SuiResult { + self.deserialize_module_by_name(module.as_str(), binary_config) + } + + pub fn deserialize_module_by_name( + &self, + module: &str, + binary_config: &BinaryConfig, ) -> SuiResult { // TODO use the session's cache - let bytes = self - .serialized_module_map() - .get(module.as_str()) - .ok_or_else(|| SuiError::ModuleNotFound { - module_name: module.to_string(), - })?; + let bytes = + self.serialized_module_map() + .get(module) + .ok_or_else(|| SuiError::ModuleNotFound { + module_name: module.to_string(), + })?; CompiledModule::deserialize_with_config(bytes, binary_config).map_err(|error| { SuiError::ModuleDeserializationFailure { error: error.to_string(), diff --git a/sui-execution/latest/sui-adapter/src/package_unification.rs b/sui-execution/latest/sui-adapter/src/package_unification.rs index 8904c1fda6806..36a2b9ba0d082 100644 --- a/sui-execution/latest/sui-adapter/src/package_unification.rs +++ b/sui-execution/latest/sui-adapter/src/package_unification.rs @@ -1,69 +1,90 @@ -use std::collections::{BTreeMap, BTreeSet}; - use move_binary_format::{binary_config::BinaryConfig, file_format::Visibility}; +use std::collections::BTreeMap; use sui_types::{ base_types::{ObjectID, SequenceNumber}, - move_package::{normalize_module, MovePackage}, + move_package::MovePackage, storage::BackingPackageStore, transaction::{Command, ProgrammableTransaction}, type_input::TypeInput, - Identifier, }; +#[derive(Debug)] pub struct PTBLinkageMetadata { - pub types: Vec, - pub entry_functions: Vec, - pub non_entry_functions: Vec, - pub publication_linkages: Vec, - pub upgrading_packages: Vec, + pub config: LinkageConfig, + pub unification_table: BTreeMap, pub all_packages: BTreeMap, } +#[derive(Debug)] pub struct LinkageConfig { pub fix_top_level_functions: bool, pub fix_types: bool, + pub exact_entry_transitive_deps: bool, } #[derive(Debug)] pub enum ConflictResolution { Exact(SequenceNumber, ObjectID), AtLeast(SequenceNumber, ObjectID), - Never(ObjectID), } impl LinkageConfig { - pub fn strict() -> Self { + pub fn loose() -> Self { Self { fix_top_level_functions: true, - fix_types: true, + fix_types: false, + exact_entry_transitive_deps: false, } } - pub fn loose() -> Self { - Self { - fix_top_level_functions: false, - fix_types: false, + pub fn generate_top_level_fn_constraint( + &self, + ) -> for<'a> fn(&'a MovePackage) -> ConflictResolution { + if self.fix_top_level_functions { + ConflictResolution::exact + } else { + ConflictResolution::at_least + } + } + + pub fn generate_type_constraint(&self) -> for<'a> fn(&'a MovePackage) -> ConflictResolution { + if self.fix_types { + ConflictResolution::exact + } else { + ConflictResolution::at_least + } + } + + pub fn generate_entry_transitive_dep_constraint( + &self, + ) -> for<'a> fn(&'a MovePackage) -> ConflictResolution { + if self.exact_entry_transitive_deps { + ConflictResolution::exact + } else { + ConflictResolution::at_least } } } impl ConflictResolution { - pub fn unify(&mut self, other: &ConflictResolution) -> anyhow::Result<()> { + pub fn exact<'a>(pkg: &MovePackage) -> ConflictResolution { + ConflictResolution::Exact(pkg.version(), pkg.id()) + } + + pub fn at_least<'a>(pkg: &MovePackage) -> ConflictResolution { + ConflictResolution::AtLeast(pkg.version(), pkg.id()) + } + + pub fn unify(&self, other: &ConflictResolution) -> anyhow::Result { match (&self, other) { - // If we ever try to unify with a Never we fail. - (ConflictResolution::Never(_), _) | (_, ConflictResolution::Never(_)) => { - return Err(anyhow::anyhow!( - "Cannot unify with Never: {:?} and {:?}", - self, - other - )); - } // If we have two exact resolutions, they must be the same. (ConflictResolution::Exact(sv, self_id), ConflictResolution::Exact(ov, other_id)) => { - if self_id != other_id { + if self_id != other_id || sv != ov { return Err(anyhow::anyhow!( - "Exact/exact conflicting resolutions for packages: {self_id}@{sv} and {other_id}@{ov}", + "UNIFICATION ERROR: Exact/exact conflicting resolutions for packages: {self_id}@{sv} and {other_id}@{ov}", )); + } else { + Ok(ConflictResolution::Exact(*sv, *self_id)) } } // Take the max if you have two at least resolutions. @@ -77,29 +98,30 @@ impl ConflictResolution { *oid }; - *self = ConflictResolution::AtLeast(*self_version.max(other_version), id); + Ok(ConflictResolution::AtLeast( + *self_version.max(other_version), + id, + )) } // If you unify an exact and an at least, the exact must be greater than or equal to // the at least. It unifies to an exact. ( - ConflictResolution::Exact(exact_version, self_id), - ConflictResolution::AtLeast(at_least_version, oid), + ConflictResolution::Exact(exact_version, exact_id), + ConflictResolution::AtLeast(at_least_version, at_least_id), ) | ( - ConflictResolution::AtLeast(at_least_version, oid), - ConflictResolution::Exact(exact_version, self_id), + ConflictResolution::AtLeast(at_least_version, at_least_id), + ConflictResolution::Exact(exact_version, exact_id), ) => { if exact_version < at_least_version { return Err(anyhow::anyhow!( - "Exact/at least Conflicting resolutions for packages: Exact {self_id}@{exact_version} and {oid}@{at_least_version}", + "UNIFICATION ERROR: Exact/at least Conflicting resolutions for packages: Exact {exact_id}@{exact_version} and {at_least_id}@{at_least_version}", )); } - *self = ConflictResolution::Exact(*exact_version, *self_id); + Ok(ConflictResolution::Exact(*exact_version, *exact_id)) } } - - Ok(()) } } @@ -107,18 +129,17 @@ impl PTBLinkageMetadata { pub fn from_ptb( ptb: &ProgrammableTransaction, store: &dyn BackingPackageStore, + config: LinkageConfig, + binary_config: &BinaryConfig, ) -> anyhow::Result { let mut linkage = PTBLinkageMetadata { - types: Vec::new(), - entry_functions: Vec::new(), - non_entry_functions: Vec::new(), - publication_linkages: Vec::new(), - upgrading_packages: Vec::new(), + unification_table: BTreeMap::new(), all_packages: BTreeMap::new(), + config, }; for command in &ptb.commands { - linkage.add_command(command, store)?; + linkage.add_command(command, store, binary_config)?; } Ok(linkage) @@ -128,34 +149,26 @@ impl PTBLinkageMetadata { &mut self, command: &Command, store: &dyn BackingPackageStore, + binary_config: &BinaryConfig, ) -> anyhow::Result<()> { match command { Command::MoveCall(programmable_move_call) => { let pkg = self.get_package(&programmable_move_call.package, store)?; - // TODO/XXX: Make this work without needing to normalize the module. - let module = normalize_module( - pkg.serialized_module_map() - .get(&programmable_move_call.module) - .ok_or_else(|| { - anyhow::anyhow!( - "Module {} not found in package {}", - programmable_move_call.module, - pkg.id() - ) - })?, - &BinaryConfig::standard(), - )?; - let function = module - .functions - .get(&Identifier::new(programmable_move_call.function.clone())?) - .ok_or_else(|| { - anyhow::anyhow!( - "Function {} not found in module {}", - programmable_move_call.function, - module.module_id(), - ) - })?; + let m = pkg + .deserialize_module_by_name(&programmable_move_call.module, binary_config) + .map_err(|e| anyhow::anyhow!("Error deserializing module: {:?}", e))?; + let Some(fdef) = m.function_defs().into_iter().find(|f| { + m.identifier_at(m.function_handle_at(f.function).name) + .as_str() + == &programmable_move_call.function + }) else { + return Err(anyhow::anyhow!( + "Function {} not found in module {}", + programmable_move_call.function, + programmable_move_call.module + )); + }; let pkg_id = pkg.id(); let transitive_deps = pkg @@ -164,20 +177,33 @@ impl PTBLinkageMetadata { .map(|info| info.upgraded_id) .collect::>(); - // load transitive deps - for id in transitive_deps { - self.get_package(&id, store)?; + for ty in &programmable_move_call.type_arguments { + self.add_type(ty, store)?; } // Register function entrypoint - if function.is_entry && function.visibility != Visibility::Public { - self.entry_functions.push(pkg_id); + if fdef.is_entry && fdef.visibility != Visibility::Public { + self.add_and_unify(&pkg_id, store, ConflictResolution::exact)?; + + // transitive closure of entry functions are fixed + for object_id in transitive_deps.iter() { + self.add_and_unify( + object_id, + store, + self.config.generate_entry_transitive_dep_constraint(), + )?; + } } else { - self.non_entry_functions.push(pkg_id); - } - - for ty in &programmable_move_call.type_arguments { - self.add_type(ty, store)?; + self.add_and_unify( + &pkg_id, + store, + self.config.generate_top_level_fn_constraint(), + )?; + + // transitive closure of non-entry functions are at-least + for object_id in transitive_deps.iter() { + self.add_and_unify(object_id, store, ConflictResolution::at_least)?; + } } } Command::MakeMoveVec(type_input, _) => { @@ -185,24 +211,12 @@ impl PTBLinkageMetadata { self.add_type(ty, store)?; } } - Command::Upgrade(_, transitive_deps, upgrading_object_id, _) => { - self.upgrading_packages.push(*upgrading_object_id); - self.get_package(upgrading_object_id, store)?; - - self.publication_linkages.extend_from_slice(transitive_deps); - for object_id in transitive_deps { - self.get_package(object_id, store)?; - } - } - Command::Publish(_, transitive_deps) => { - self.publication_linkages.extend_from_slice(transitive_deps); - for object_id in transitive_deps { - self.get_package(object_id, store)?; - } - } - Command::TransferObjects(_, _) => (), - Command::SplitCoins(_, _) => (), - Command::MergeCoins(_, _) => (), + // Upgrades and Publishes don't count toward the global linkage determination. + Command::Upgrade(_, _, _, _) + | Command::Publish(_, _) + | Command::TransferObjects(_, _) + | Command::SplitCoins(_, _) + | Command::MergeCoins(_, _) => (), }; Ok(()) @@ -225,10 +239,11 @@ impl PTBLinkageMetadata { stack.push(&**type_input); } TypeInput::Struct(struct_input) => { - let pkg = self - .get_package(&ObjectID::from(struct_input.address), store)? - .id(); - self.types.push(pkg); + self.add_and_unify( + &ObjectID::from(struct_input.address), + store, + self.config.generate_type_constraint(), + )?; for ty in struct_input.type_params.iter() { stack.push(ty); } @@ -238,8 +253,6 @@ impl PTBLinkageMetadata { Ok(()) } - // Gather and dedup all packages loaded by the PTB. - // Also gathers the versions of each package loaded by the PTB at the same time. fn get_package( &mut self, object_id: &ObjectID, @@ -259,119 +272,28 @@ impl PTBLinkageMetadata { .get(object_id) .expect("Guaranteed to exist")) } -} - -impl PTBLinkageMetadata { - pub fn try_compute_unified_linkage( - &self, - linking_config: LinkageConfig, - ) -> anyhow::Result> { - let mut unification_table = BTreeMap::new(); - - // Any packages that are being upgraded cannot be called in the transaction - for object_id in self.upgrading_packages.iter() { - Self::add_and_unify( - &mut unification_table, - &self.all_packages, - object_id, - |pkg| ConflictResolution::Never(pkg.id()), - )?; - } - - // Linkages for packages that are to be published must be exact. - for object_id in self.publication_linkages.iter() { - Self::add_and_unify( - &mut unification_table, - &self.all_packages, - object_id, - |pkg| ConflictResolution::Exact(pkg.version(), pkg.id()), - )?; - } - - for object_id in self.entry_functions.iter() { - Self::add_and_unify( - &mut unification_table, - &self.all_packages, - object_id, - |pkg| ConflictResolution::Exact(pkg.version(), pkg.id()), - )?; - - // transitive closure of entry functions are fixed - for dep_id in self.all_packages[object_id] - .linkage_table() - .values() - .map(|info| &info.upgraded_id) - { - Self::add_and_unify(&mut unification_table, &self.all_packages, dep_id, |pkg| { - ConflictResolution::Exact(pkg.version(), pkg.id()) - })?; - } - } - - // Types can be fixed or not depending on config. - for object_id in self.types.iter() { - Self::add_and_unify( - &mut unification_table, - &self.all_packages, - object_id, - |pkg| { - if linking_config.fix_types { - ConflictResolution::Exact(pkg.version(), pkg.id()) - } else { - ConflictResolution::AtLeast(pkg.version(), *object_id) - } - }, - )?; - } - - // Top level functions can be fixed or not depending on config. But they won't ever - // transitively fix their dependencies. - for object_id in self.non_entry_functions.iter() { - Self::add_and_unify( - &mut unification_table, - &self.all_packages, - object_id, - |pkg| { - if linking_config.fix_top_level_functions { - ConflictResolution::Exact(pkg.version(), pkg.id()) - } else { - ConflictResolution::AtLeast(pkg.version(), *object_id) - } - }, - )?; - } - - Ok(unification_table - .into_values() - .flat_map(|unifier| match unifier { - ConflictResolution::Exact(_, object_id) => Some(object_id), - ConflictResolution::AtLeast(_, object_id) => Some(object_id), - ConflictResolution::Never(_) => None, - }) - .collect()) - } // Add a package to the unification table, unifying it with any existing package in the table. // Errors if the packages cannot be unified (e.g., if one is exact and the other is not). fn add_and_unify( - unification_table: &mut BTreeMap, - loaded_top_level_packages: &BTreeMap, + &mut self, object_id: &ObjectID, - resolution_fn: impl Fn(&MovePackage) -> ConflictResolution, + store: &dyn BackingPackageStore, + resolution_fn: fn(&MovePackage) -> ConflictResolution, ) -> anyhow::Result<()> { - let package = loaded_top_level_packages - .get(object_id) - .ok_or_else(|| anyhow::anyhow!("Object {} not found in any package", object_id))?; + let package = self.get_package(object_id, store)?; let resolution = resolution_fn(package); + let original_pkg_id = package.original_package_id(); - if unification_table.contains_key(&package.original_package_id()) { - let existing_unifier = unification_table - .get_mut(&package.original_package_id()) + if self.unification_table.contains_key(&original_pkg_id) { + let existing_unifier = self + .unification_table + .get_mut(&original_pkg_id) .expect("Guaranteed to exist"); - existing_unifier.unify(&resolution)?; + *existing_unifier = existing_unifier.unify(&resolution)?; } else { - unification_table.insert(package.original_package_id(), resolution); + self.unification_table.insert(original_pkg_id, resolution); } Ok(()) diff --git a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs index afcc3c7b5b16a..46e1dc2653f98 100644 --- a/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs +++ b/sui-execution/latest/sui-adapter/src/programmable_transactions/execution.rs @@ -76,13 +76,6 @@ mod checked { gas_charger: &mut GasCharger, pt: ProgrammableTransaction, ) -> Result { - // let linking_metadata = - // PTBLinkageMetadata::from_ptb(&pt, state_view.as_backing_package_store()) - // .expect("Unable to create linking metadata"); - // linking_metadata - // .try_compute_unified_linkage(LinkageConfig::loose()) - // .expect("Unable to unify linkage across ptb"); - let ProgrammableTransaction { inputs, commands } = pt; let mut context = ExecutionContext::new( protocol_config, From 3ef54f8e856a6b730d2eb027f47e64d59a9c92a3 Mon Sep 17 00:00:00 2001 From: Timothy Zakian Date: Wed, 15 Jan 2025 12:27:22 -0800 Subject: [PATCH 3/3] fixup! Update unification logic to be one-pass + fix issue --- crates/sui-types/src/execution_status.rs | 3 + .../sui-adapter/src/package_unification.rs | 336 ++++++++++++++---- 2 files changed, 274 insertions(+), 65 deletions(-) diff --git a/crates/sui-types/src/execution_status.rs b/crates/sui-types/src/execution_status.rs index 8b04b57efa745..905612b139917 100644 --- a/crates/sui-types/src/execution_status.rs +++ b/crates/sui-types/src/execution_status.rs @@ -216,6 +216,9 @@ pub enum ExecutionFailureStatus { #[error("Certificate is cancelled because randomness could not be generated this epoch")] ExecutionCancelledDueToRandomnessUnavailable, + + #[error("A valid unified linkage is unable to be created for the transaction")] + InvalidUnifiedLinkage, // NOTE: if you want to add a new enum, // please add it at the end for Rust SDK backward compatibility. } diff --git a/sui-execution/latest/sui-adapter/src/package_unification.rs b/sui-execution/latest/sui-adapter/src/package_unification.rs index 36a2b9ba0d082..3fb4860182d25 100644 --- a/sui-execution/latest/sui-adapter/src/package_unification.rs +++ b/sui-execution/latest/sui-adapter/src/package_unification.rs @@ -1,39 +1,121 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + use move_binary_format::{binary_config::BinaryConfig, file_format::Visibility}; use std::collections::BTreeMap; use sui_types::{ base_types::{ObjectID, SequenceNumber}, + error::{ExecutionError, ExecutionErrorKind, SuiResult, UserInputError}, move_package::MovePackage, storage::BackingPackageStore, transaction::{Command, ProgrammableTransaction}, type_input::TypeInput, }; +/// Max number of packages to cache in the PTBLinkageMetadata. If we have more than this, we'll +/// drop the cache and restart the cache. +const MAX_CACHED_PACKAGES: usize = 200; + +/// Metadata for the PTB linkage analysis. #[derive(Debug)] pub struct PTBLinkageMetadata { - pub config: LinkageConfig, - pub unification_table: BTreeMap, + /// Config to use for the linkage analysis. + pub linkage_config: LinkageConfig, + /// Config to use for the binary analysis (needed for deserialization to determine if a + /// function is a non-public entry function). + pub binary_config: BinaryConfig, + /// Cache for packages that we've loaded so far. Note: We may drop this cache if it grows too + /// large. pub all_packages: BTreeMap, } +/// Configuration for the linkage analysis. #[derive(Debug)] pub struct LinkageConfig { + /// Do we introduce an `Exact()` for each top-level function ::mname::fname? pub fix_top_level_functions: bool, + /// Do we introduce an `Exact()` for each type ::mname::tname? pub fix_types: bool, + /// Do we introduce an `Exact()` for each transitive dependency of a non-public entry function? pub exact_entry_transitive_deps: bool, + /// Do we introduce an `Exact()` for each transitive dependency of a + /// function? + pub exact_transitive_deps: bool, } +/// Unifiers. These are used to determine how to unify two packages. #[derive(Debug)] pub enum ConflictResolution { + /// An exact constraint unifies as follows: + /// 1. Exact(a) ~ Exact(b) ==> Exact(a), iff a == b + /// 2. Exact(a) ~ AtLeast(b) ==> Exact(a), iff a >= b Exact(SequenceNumber, ObjectID), + /// An at least constraint unifies as follows: + /// * AtLeast(a, a_version) ~ AtLeast(b, b_version) ==> AtLeast(x, max(a_version, b_version)), + /// where x is the package id of either a or b (the one with the greatest version). AtLeast(SequenceNumber, ObjectID), } +pub type ResolvedLinkage = BTreeMap; + +#[derive(Debug)] +pub struct PerCommandLinkage { + internal: PTBLinkageMetadata, +} + +#[derive(Debug)] +pub struct UnifiedLinkage { + /// Current unification table we have for packages. This is a mapping from the original + /// package ID for a package to its current resolution. This is the "constraint set" that we + /// are building/solving as we progress across the PTB. + unification_table: BTreeMap, + internal: PTBLinkageMetadata, +} + +pub trait LinkageAnalysis { + fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> SuiResult; +} + +impl LinkageAnalysis for PerCommandLinkage { + fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> SuiResult { + self.add_command(command, store) + } +} + +impl LinkageAnalysis for UnifiedLinkage { + fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> SuiResult { + self.add_command(command, store) + } +} + impl LinkageConfig { - pub fn loose() -> Self { + pub fn unified_linkage_settings() -> Self { Self { fix_top_level_functions: true, fix_types: false, exact_entry_transitive_deps: false, + exact_transitive_deps: false, + } + } + + pub fn per_command_linkage_settings() -> Self { + Self { + fix_top_level_functions: true, + fix_types: false, + exact_entry_transitive_deps: true, + exact_transitive_deps: true, } } @@ -64,6 +146,16 @@ impl LinkageConfig { ConflictResolution::at_least } } + + pub fn generate_transitive_dep_constraint( + &self, + ) -> for<'a> fn(&'a MovePackage) -> ConflictResolution { + if self.exact_transitive_deps { + ConflictResolution::exact + } else { + ConflictResolution::at_least + } + } } impl ConflictResolution { @@ -75,14 +167,21 @@ impl ConflictResolution { ConflictResolution::AtLeast(pkg.version(), pkg.id()) } - pub fn unify(&self, other: &ConflictResolution) -> anyhow::Result { + pub fn unify(&self, other: &ConflictResolution) -> SuiResult { match (&self, other) { // If we have two exact resolutions, they must be the same. (ConflictResolution::Exact(sv, self_id), ConflictResolution::Exact(ov, other_id)) => { if self_id != other_id || sv != ov { - return Err(anyhow::anyhow!( - "UNIFICATION ERROR: Exact/exact conflicting resolutions for packages: {self_id}@{sv} and {other_id}@{ov}", - )); + return Err( + ExecutionError::new_with_source( + ExecutionErrorKind::InvalidUnifiedLinkage, + format!( + "exact/exact conflicting resolutions for package: linkage requires the same package \ + at different versions. Linkage requires exactly {self_id} (version {sv}) and \ + {other_id} (version {ov}) to be used in the same transaction" + ) + ).into() + ); } else { Ok(ConflictResolution::Exact(*sv, *self_id)) } @@ -114,9 +213,17 @@ impl ConflictResolution { ConflictResolution::Exact(exact_version, exact_id), ) => { if exact_version < at_least_version { - return Err(anyhow::anyhow!( - "UNIFICATION ERROR: Exact/at least Conflicting resolutions for packages: Exact {exact_id}@{exact_version} and {at_least_id}@{at_least_version}", - )); + return Err( + ExecutionError::new_with_source( + ExecutionErrorKind::InvalidUnifiedLinkage, + format!( + "Exact/AtLeast conflicting resolutions for package: linkage requires exactly this \ + package {exact_id} (version {exact_version}) and also at least the following \ + version of the package {at_least_id} at version {at_least_version}. However \ + {exact_id} is at version {exact_version} which is less than {at_least_version}." + ) + ).into() + ); } Ok(ConflictResolution::Exact(*exact_version, *exact_id)) @@ -125,90 +232,171 @@ impl ConflictResolution { } } -impl PTBLinkageMetadata { - pub fn from_ptb( +impl PerCommandLinkage { + pub fn new(binary_config: BinaryConfig) -> Self { + Self { + internal: PTBLinkageMetadata { + all_packages: BTreeMap::new(), + linkage_config: LinkageConfig::per_command_linkage_settings(), + binary_config, + }, + } + } + + pub fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> SuiResult { + let mut unification_table = BTreeMap::new(); + self.internal + .add_command(command, store, &mut unification_table) + } + + pub fn from_ptb_for_testing_only( ptb: &ProgrammableTransaction, store: &dyn BackingPackageStore, - config: LinkageConfig, - binary_config: &BinaryConfig, - ) -> anyhow::Result { - let mut linkage = PTBLinkageMetadata { + binary_config: BinaryConfig, + ) -> SuiResult<()> { + let mut u = Self::new(binary_config); + + for command in &ptb.commands { + u.add_command(command, store)?; + } + + Ok(()) + } +} + +impl UnifiedLinkage { + pub fn new(binary_config: BinaryConfig) -> Self { + Self { + internal: PTBLinkageMetadata { + all_packages: BTreeMap::new(), + linkage_config: LinkageConfig::unified_linkage_settings(), + binary_config, + }, unification_table: BTreeMap::new(), - all_packages: BTreeMap::new(), - config, - }; + } + } + + pub fn add_command( + &mut self, + command: &Command, + store: &dyn BackingPackageStore, + ) -> SuiResult { + self.internal + .add_command(command, store, &mut self.unification_table) + } + + pub fn from_ptb_for_testing_only( + ptb: &ProgrammableTransaction, + store: &dyn BackingPackageStore, + binary_config: BinaryConfig, + ) -> SuiResult<()> { + let mut u = Self::new(binary_config); for command in &ptb.commands { - linkage.add_command(command, store, binary_config)?; + u.add_command(command, store)?; } - Ok(linkage) + Ok(()) } +} - fn add_command( +impl PTBLinkageMetadata { + pub fn new(linkage_config: LinkageConfig, binary_config: BinaryConfig) -> SuiResult { + Ok(Self { + all_packages: BTreeMap::new(), + linkage_config, + binary_config, + }) + } + + pub(crate) fn add_command( &mut self, command: &Command, store: &dyn BackingPackageStore, - binary_config: &BinaryConfig, - ) -> anyhow::Result<()> { + unification_table: &mut BTreeMap, + ) -> SuiResult { match command { Command::MoveCall(programmable_move_call) => { - let pkg = self.get_package(&programmable_move_call.package, store)?; + let pkg = Self::get_package( + &mut self.all_packages, + &programmable_move_call.package, + store, + )?; + let pkg_id = pkg.id(); + let transitive_deps = pkg + .linkage_table() + .values() + .map(|info| info.upgraded_id) + .collect::>(); - let m = pkg - .deserialize_module_by_name(&programmable_move_call.module, binary_config) - .map_err(|e| anyhow::anyhow!("Error deserializing module: {:?}", e))?; + let m = pkg.deserialize_module_by_name( + &programmable_move_call.module, + &self.binary_config, + )?; let Some(fdef) = m.function_defs().into_iter().find(|f| { m.identifier_at(m.function_handle_at(f.function).name) .as_str() == &programmable_move_call.function }) else { - return Err(anyhow::anyhow!( - "Function {} not found in module {}", - programmable_move_call.function, - programmable_move_call.module - )); + return Err(ExecutionError::new_with_source( + ExecutionErrorKind::FunctionNotFound, + format!( + "Function {} not found in module {}", + programmable_move_call.function, programmable_move_call.module + ), + ) + .into()); }; - let pkg_id = pkg.id(); - let transitive_deps = pkg - .linkage_table() - .values() - .map(|info| info.upgraded_id) - .collect::>(); - for ty in &programmable_move_call.type_arguments { - self.add_type(ty, store)?; + self.add_type(ty, store, unification_table)?; } // Register function entrypoint if fdef.is_entry && fdef.visibility != Visibility::Public { - self.add_and_unify(&pkg_id, store, ConflictResolution::exact)?; + self.add_and_unify( + &pkg_id, + store, + unification_table, + ConflictResolution::exact, + )?; // transitive closure of entry functions are fixed for object_id in transitive_deps.iter() { self.add_and_unify( object_id, store, - self.config.generate_entry_transitive_dep_constraint(), + unification_table, + self.linkage_config + .generate_entry_transitive_dep_constraint(), )?; } } else { self.add_and_unify( &pkg_id, store, - self.config.generate_top_level_fn_constraint(), + unification_table, + self.linkage_config.generate_top_level_fn_constraint(), )?; // transitive closure of non-entry functions are at-least for object_id in transitive_deps.iter() { - self.add_and_unify(object_id, store, ConflictResolution::at_least)?; + self.add_and_unify( + object_id, + store, + unification_table, + self.linkage_config.generate_transitive_dep_constraint(), + )?; } } } Command::MakeMoveVec(type_input, _) => { if let Some(ty) = type_input { - self.add_type(ty, store)?; + self.add_type(ty, store, unification_table)?; } } // Upgrades and Publishes don't count toward the global linkage determination. @@ -219,10 +407,21 @@ impl PTBLinkageMetadata { | Command::MergeCoins(_, _) => (), }; - Ok(()) + Ok(unification_table + .iter() + .map(|(k, v)| match v { + ConflictResolution::Exact(_, object_id) + | ConflictResolution::AtLeast(_, object_id) => (*k, *object_id), + }) + .collect()) } - fn add_type(&mut self, ty: &TypeInput, store: &dyn BackingPackageStore) -> anyhow::Result<()> { + fn add_type( + &mut self, + ty: &TypeInput, + store: &dyn BackingPackageStore, + unification_table: &mut BTreeMap, + ) -> SuiResult<()> { let mut stack = vec![ty]; while let Some(ty) = stack.pop() { match ty { @@ -242,7 +441,8 @@ impl PTBLinkageMetadata { self.add_and_unify( &ObjectID::from(struct_input.address), store, - self.config.generate_type_constraint(), + unification_table, + self.linkage_config.generate_type_constraint(), )?; for ty in struct_input.type_params.iter() { stack.push(ty); @@ -253,24 +453,30 @@ impl PTBLinkageMetadata { Ok(()) } - fn get_package( - &mut self, + fn get_package<'a>( + all_packages: &'a mut BTreeMap, object_id: &ObjectID, store: &dyn BackingPackageStore, - ) -> anyhow::Result<&MovePackage> { - if !self.all_packages.contains_key(object_id) { + ) -> SuiResult<&'a MovePackage> { + // If we've cached too many packages, clear the cache. We'll windup pulling in any more + // that we need if we need them again. + if all_packages.len() > MAX_CACHED_PACKAGES { + all_packages.clear(); + } + + if !all_packages.contains_key(object_id) { let package = store .get_package_object(object_id)? - .ok_or_else(|| anyhow::anyhow!("Object {} not found in any package", object_id))? + .ok_or_else(|| UserInputError::ObjectNotFound { + object_id: *object_id, + version: None, + })? .move_package() .clone(); - self.all_packages.insert(*object_id, package); + all_packages.insert(*object_id, package); } - Ok(self - .all_packages - .get(object_id) - .expect("Guaranteed to exist")) + Ok(all_packages.get(object_id).expect("Guaranteed to exist")) } // Add a package to the unification table, unifying it with any existing package in the table. @@ -279,21 +485,21 @@ impl PTBLinkageMetadata { &mut self, object_id: &ObjectID, store: &dyn BackingPackageStore, + unification_table: &mut BTreeMap, resolution_fn: fn(&MovePackage) -> ConflictResolution, - ) -> anyhow::Result<()> { - let package = self.get_package(object_id, store)?; + ) -> SuiResult<()> { + let package = Self::get_package(&mut self.all_packages, object_id, store)?; let resolution = resolution_fn(package); let original_pkg_id = package.original_package_id(); - if self.unification_table.contains_key(&original_pkg_id) { - let existing_unifier = self - .unification_table + if unification_table.contains_key(&original_pkg_id) { + let existing_unifier = unification_table .get_mut(&original_pkg_id) .expect("Guaranteed to exist"); *existing_unifier = existing_unifier.unify(&resolution)?; } else { - self.unification_table.insert(original_pkg_id, resolution); + unification_table.insert(original_pkg_id, resolution); } Ok(())