Skip to content

Commit

Permalink
Rework errors around fetching move packages
Browse files Browse the repository at this point in the history
  • Loading branch information
@stefan-mysten
stefan-mysten committed Feb 27, 2025
1 parent 79ac5af commit 89e3602
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 67 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

97 changes: 50 additions & 47 deletions crates/sui/src/client_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
verifier_meter::{AccumulatingMeter, Accumulator},
};
use std::{
collections::{btree_map::Entry, BTreeMap},
collections::{btree_map::Entry, BTreeMap, BTreeSet},
fmt::{Debug, Display, Formatter, Write},
fs,
path::{Path, PathBuf},
Expand Down Expand Up @@ -67,7 +67,7 @@ use sui_types::{
base_types::{ObjectID, SequenceNumber, SuiAddress},
crypto::{EmptySignInfo, SignatureScheme},
digests::TransactionDigest,
error::SuiError,
error::{SuiError, SuiObjectResponseError},
gas::GasCostSummary,
gas_coin::GasCoin,
message_envelope::Envelope,
Expand Down Expand Up @@ -891,12 +891,9 @@ impl SuiClientCommands {
let sender = context.try_get_object_owner(&opts.gas).await?;
let sender = sender.unwrap_or(context.active_address()?);
let client = context.get_client().await?;
let chain_id = client.read_api().get_chain_identifier().await.ok();
let protocol_version = client
.read_api()
.get_protocol_config(None)
.await?
.protocol_version;
let read_api = client.read_api();
let chain_id = read_api.get_chain_identifier().await.ok();
let protocol_version = read_api.get_protocol_config(None).await?.protocol_version;
let protocol_config = ProtocolConfig::get_for_version(
protocol_version,
match chain_id
Expand All @@ -908,7 +905,7 @@ impl SuiClientCommands {
},
);

check_protocol_version_and_warn(&client).await?;
check_protocol_version_and_warn(read_api).await?;
let package_path =
package_path
.canonicalize()
Expand All @@ -935,7 +932,7 @@ impl SuiClientCommands {
check_dep_verification_flags(skip_dependency_verification, verify_deps)?;

let upgrade_result = upgrade_package(
&client,
read_api,
build_config.clone(),
&package_path,
upgrade_capability,
Expand Down Expand Up @@ -967,7 +964,7 @@ impl SuiClientCommands {

if verify_compatibility {
check_compatibility(
&client,
read_api,
package_id,
compiled_package,
package_path,
Expand Down Expand Up @@ -1040,9 +1037,10 @@ impl SuiClientCommands {
let sender = context.try_get_object_owner(&opts.gas).await?;
let sender = sender.unwrap_or(context.active_address()?);
let client = context.get_client().await?;
let chain_id = client.read_api().get_chain_identifier().await.ok();
let read_api = client.read_api();
let chain_id = read_api.get_chain_identifier().await.ok();

check_protocol_version_and_warn(&client).await?;
check_protocol_version_and_warn(read_api).await?;
let package_path =
package_path
.canonicalize()
Expand All @@ -1064,7 +1062,7 @@ impl SuiClientCommands {
check_dep_verification_flags(skip_dependency_verification, verify_deps)?;

let compile_result = compile_package(
&client,
read_api,
build_config.clone(),
&package_path,
with_unpublished_dependencies,
Expand Down Expand Up @@ -1123,6 +1121,7 @@ impl SuiClientCommands {
build_config,
} => {
let client = context.get_client().await?;
let read_api = client.read_api();
let protocol_version =
protocol_version.map_or(ProtocolVersion::MAX, ProtocolVersion::new);
let protocol_config =
Expand Down Expand Up @@ -1151,7 +1150,7 @@ impl SuiClientCommands {
(_, package_path) => {
let package_path = package_path.unwrap_or_else(|| PathBuf::from("."));
let package =
compile_package_simple(&client, build_config, &package_path, None)
compile_package_simple(read_api, build_config, &package_path, None)
.await?;
let name = package
.package
Expand Down Expand Up @@ -1767,7 +1766,7 @@ fn check_dep_verification_flags(
}

async fn compile_package_simple(
client: &SuiClient,
read_api: &ReadApi,
build_config: MoveBuildConfig,
package_path: &Path,
chain_id: Option<String>,
Expand All @@ -1781,13 +1780,13 @@ async fn compile_package_simple(
let resolution_graph = config.resolution_graph(package_path, chain_id.clone())?;
let mut compiled_package =
build_from_resolution_graph(resolution_graph, false, false, chain_id)?;
pkg_tree_shake(client, false, &mut compiled_package).await?;
pkg_tree_shake(read_api, false, &mut compiled_package).await?;

Ok(compiled_package)
}

pub(crate) async fn upgrade_package(
client: &SuiClient,
read_api: &ReadApi,
build_config: MoveBuildConfig,
package_path: &Path,
upgrade_capability: ObjectID,
Expand All @@ -1796,15 +1795,20 @@ pub(crate) async fn upgrade_package(
env_alias: Option<String>,
) -> Result<(u8, CompiledPackage), anyhow::Error> {
let mut compiled_package = compile_package(
client,
read_api,
build_config,
package_path,
with_unpublished_dependencies,
skip_dependency_verification,
)
.await?;

pkg_tree_shake(client, with_unpublished_dependencies, &mut compiled_package).await?;
pkg_tree_shake(
read_api,
with_unpublished_dependencies,
&mut compiled_package,
)
.await?;

compiled_package.published_at.as_ref().map_err(|e| match e {
PublishedAtError::NotPresent => {
Expand Down Expand Up @@ -1832,8 +1836,7 @@ pub(crate) async fn upgrade_package(
}
})?;

let resp = client
.read_api()
let resp = read_api
.get_object_with_options(
upgrade_capability,
SuiObjectDataOptions::default().with_bcs().with_owner(),
Expand Down Expand Up @@ -1861,13 +1864,12 @@ pub(crate) async fn upgrade_package(
}

pub(crate) async fn compile_package(
client: &SuiClient,
read_api: &ReadApi,
build_config: MoveBuildConfig,
package_path: &Path,
with_unpublished_dependencies: bool,
skip_dependency_verification: bool,
) -> Result<CompiledPackage, anyhow::Error> {
let read_api = client.read_api();
let config = resolve_lock_file_path(build_config, Some(package_path))?;
let run_bytecode_verifier = true;
let print_diags_to_stderr = true;
Expand All @@ -1891,7 +1893,12 @@ pub(crate) async fn compile_package(
chain_id,
)?;

pkg_tree_shake(client, with_unpublished_dependencies, &mut compiled_package).await?;
pkg_tree_shake(
read_api,
with_unpublished_dependencies,
&mut compiled_package,
)
.await?;

let protocol_config = read_api.get_protocol_config(None).await?;

Expand Down Expand Up @@ -3086,8 +3093,8 @@ pub(crate) async fn prerender_clever_errors(
}

/// Warn the user if the CLI falls behind more than 2 protocol versions.
async fn check_protocol_version_and_warn(client: &SuiClient) -> Result<(), anyhow::Error> {
let protocol_cfg = client.read_api().get_protocol_config(None).await?;
async fn check_protocol_version_and_warn(read_api: &ReadApi) -> Result<(), anyhow::Error> {
let protocol_cfg = read_api.get_protocol_config(None).await?;
let on_chain_protocol_version = protocol_cfg.protocol_version.as_u64();
let cli_protocol_version = ProtocolVersion::MAX.as_u64();
if (cli_protocol_version + 2) < on_chain_protocol_version {
Expand All @@ -3111,43 +3118,42 @@ async fn check_protocol_version_and_warn(client: &SuiClient) -> Result<(), anyho

/// Fetch move packages based on the provided package IDs.
async fn fetch_move_packages(
client: &SuiClient,
read_api: &ReadApi,
package_ids: &[ObjectID],
pkg_id_to_name: &BTreeMap<&ObjectID, &Symbol>,
) -> Result<Vec<MovePackage>, anyhow::Error> {
let objects = client
.read_api()
let objects = read_api
.multi_get_object_with_options(package_ids.to_vec(), SuiObjectDataOptions::bcs_lossless())
.await?;

objects
.into_iter()
.map(|o| {
let o = o.into_object().map_err(|e| match e {
sui_types::error::SuiObjectResponseError::NotExists { object_id } => {
SuiObjectResponseError::NotExists { object_id } => {
anyhow!(
"Package {:?} with object ID {object_id} does not exist",
pkg_id_to_name.get(&object_id)
"Package {} with object ID {object_id} does not exist",
pkg_id_to_name.get(&object_id).unwrap()
)
}
sui_types::error::SuiObjectResponseError::Deleted {
SuiObjectResponseError::Deleted {
object_id,
version,
digest,
} => {
anyhow!(
"Package {:?} with object ID {object_id} was deleted at version {version} \
"Package {} with object ID {object_id} was deleted at version {version} \
with digest {digest}",
pkg_id_to_name.get(&object_id)
pkg_id_to_name.get(&object_id).unwrap()
)
}
_ => anyhow!("Error fetching package: {e}"),
_ => anyhow!("Cannot convert data into an object: {e}"),
})?;

let Some(SuiRawData::Package(p)) = o.bcs else {
bail!(
"Expected package {:?} with object ID {} but got something else",
pkg_id_to_name.get(&o.object_id),
"Expected package {} with object ID {} but got something else",
pkg_id_to_name.get(&o.object_id).unwrap(),
o.object_id
);
};
Expand All @@ -3159,7 +3165,7 @@ async fn fetch_move_packages(

/// Filter out a package's dependencies which are not referenced in the source code.
pub(crate) async fn pkg_tree_shake(
client: &SuiClient,
read_api: &ReadApi,
with_unpublished_dependencies: bool,
compiled_package: &mut CompiledPackage,
) -> Result<(), anyhow::Error> {
Expand All @@ -3177,15 +3183,12 @@ pub(crate) async fn pkg_tree_shake(
.map(|(pkg_name, module)| (pkg_name, ObjectID::from(module.unit.address.into_inner())))
.collect();

let published_deps_packages = fetch_move_packages(client, &pkg_ids, &pkg_id_to_name).await?;
let published_deps_packages = fetch_move_packages(read_api, &pkg_ids, &pkg_id_to_name).await?;

let linkage_table_ids = published_deps_packages
let linkage_table_ids: BTreeSet<_> = published_deps_packages
.iter()
.flat_map(|pkg| {
let linkage_table = pkg.linkage_table();
linkage_table.keys().cloned().collect::<Vec<_>>()
})
.collect::<Vec<_>>();
.flat_map(|pkg| pkg.linkage_table().keys())
.collect();

compiled_package
.dependency_ids
Expand Down
22 changes: 10 additions & 12 deletions crates/sui/src/client_ptb/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use std::{collections::BTreeMap, path::Path};
use sui_json::{is_receiving_argument, primitive_type};
use sui_json_rpc_types::{SuiObjectData, SuiObjectDataOptions, SuiRawData};
use sui_move::manage_package::resolve_lock_file_path;
use sui_sdk::SuiClient;
use sui_sdk::apis::ReadApi;
use sui_types::{
base_types::{is_primitive_type_tag, ObjectID, TxContext, TxContextKind},
move_package::MovePackage,
Expand Down Expand Up @@ -223,7 +223,7 @@ pub struct PTBBuilder<'a> {
/// transaction arguments.
resolved_arguments: BTreeMap<String, Tx::Argument>,
/// Read API for reading objects from chain. Needed for object resolution.
client: &'a SuiClient,
reader: &'a ReadApi,
/// The last command that we have added. This is used to support assignment commands.
last_command: Option<Tx::Argument>,
/// The actual PTB that we are building up.
Expand Down Expand Up @@ -275,14 +275,14 @@ impl ArgWithHistory {
}

impl<'a> PTBBuilder<'a> {
pub fn new(starting_env: BTreeMap<String, AccountAddress>, client: &'a SuiClient) -> Self {
pub fn new(starting_env: BTreeMap<String, AccountAddress>, reader: &'a ReadApi) -> Self {
Self {
addresses: starting_env,
identifiers: BTreeMap::new(),
arguments_to_resolve: BTreeMap::new(),
resolved_arguments: BTreeMap::new(),
ptb: ProgrammableTransactionBuilder::new(),
client,
reader,
last_command: None,
errors: Vec::new(),
}
Expand Down Expand Up @@ -412,8 +412,7 @@ impl<'a> PTBBuilder<'a> {
loc: Span,
) -> PTBResult<MovePackage> {
let object = self
.client
.read_api()
.reader
.get_object_with_options(package_id, SuiObjectDataOptions::bcs_lossless())
.await
.map_err(|e| err!(loc, "{e}"))?
Expand Down Expand Up @@ -764,8 +763,7 @@ impl<'a> PTBBuilder<'a> {
/// Fetch the `SuiObjectData` for an object ID -- this is used for object resolution.
async fn get_object(&self, object_id: ObjectID, obj_loc: Span) -> PTBResult<SuiObjectData> {
let res = self
.client
.read_api()
.reader
.get_object_with_options(
object_id,
SuiObjectDataOptions::new().with_type().with_owner(),
Expand Down Expand Up @@ -933,7 +931,7 @@ impl<'a> PTBBuilder<'a> {
self.last_command = Some(res);
}
ParsedPTBCommand::Publish(sp!(pkg_loc, package_path)) => {
let chain_id = self.client.read_api().get_chain_identifier().await.ok();
let chain_id = self.reader.get_chain_identifier().await.ok();
let build_config = BuildConfig::default();
let package_path = Path::new(&package_path);
let build_config = resolve_lock_file_path(build_config.clone(), Some(package_path))
Expand All @@ -960,7 +958,7 @@ impl<'a> PTBBuilder<'a> {
.map_err(|e| err!(pkg_loc, "{e}"))?;
}
let compiled_package = compile_package(
self.client,
self.reader,
build_config.clone(),
package_path,
false, /* with_unpublished_dependencies */
Expand Down Expand Up @@ -1001,7 +999,7 @@ impl<'a> PTBBuilder<'a> {
)
.await?;

let chain_id = self.client.read_api().get_chain_identifier().await.ok();
let chain_id = self.reader.get_chain_identifier().await.ok();
let build_config = BuildConfig::default();
let package_path = Path::new(&package_path);
let build_config = resolve_lock_file_path(build_config.clone(), Some(package_path))
Expand Down Expand Up @@ -1029,7 +1027,7 @@ impl<'a> PTBBuilder<'a> {
}

let (upgrade_policy, compiled_package) = upgrade_package(
self.client,
self.reader,
build_config.clone(),
package_path,
ObjectID::from_address(upgrade_cap_id.into_inner()),
Expand Down
2 changes: 1 addition & 1 deletion crates/sui/src/client_ptb/ptb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl PTB {
.into_iter()
.map(|(sa, alias)| (alias.alias.clone(), AccountAddress::from(*sa)))
.collect();
let builder = PTBBuilder::new(starting_addresses, &client);
let builder = PTBBuilder::new(starting_addresses, client.read_api());
builder.build(program).await
}

Expand Down
Loading

0 comments on commit 89e3602

Please sign in to comment.