Remove need for owner when fetching objects (MystenLabs#512)

* Remove need for owner when fetching objects

sui/src/config.rs

@@ -2,8 +2,12 @@
 // Copyright (c) 2022, Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+use sui_core::authority_client::AuthorityClient;
+use sui_network::network::NetworkClient;
 use sui_types::base_types::*;
+use sui_types::committee::Committee;
 use sui_types::crypto::{get_key_pair, KeyPair};
+use sui_types::error::SuiError;
 
 use crate::utils::optional_address_as_hex;
 use crate::utils::optional_address_from_hex;
@@ -12,6 +16,7 @@ use anyhow::anyhow;
 use once_cell::sync::Lazy;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
+use std::collections::BTreeMap;
 use std::fmt::{Display, Formatter};
 use std::path::{Path, PathBuf};
 use std::sync::Mutex;
@@ -136,6 +141,38 @@ impl Config for WalletConfig {
     }
 }
 
+impl WalletConfig {
+    pub fn make_committee(&self) -> Committee {
+        let voting_rights = self
+            .authorities
+            .iter()
+            .map(|authority| (authority.name, 1))
+            .collect();
+        Committee::new(voting_rights)
+    }
+
+    pub fn make_authority_clients(&self) -> BTreeMap<AuthorityName, AuthorityClient> {
+        let mut authority_clients = BTreeMap::new();
+        for authority in &self.authorities {
+            let client = AuthorityClient::new(NetworkClient::new(
+                authority.host.clone(),
+                authority.base_port,
+                self.buffer_size,
+                self.send_timeout,
+                self.recv_timeout,
+            ));
+            authority_clients.insert(authority.name, client);
+        }
+        authority_clients
+    }
+    pub fn get_account_cfg_info(&self, address: &SuiAddress) -> Result<&AccountInfo, SuiError> {
+        self.accounts
+            .iter()
+            .find(|info| &info.address == address)
+            .ok_or(SuiError::AccountNotFound)
+    }
+}
+
 impl Display for WalletConfig {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(

sui/src/unit_tests/cli_tests.rs

@@ -264,3 +264,58 @@ async fn test_custom_genesis_with_custom_move_package() -> Result<(), anyhow::Er
     }
     Ok(())
 }
+
+#[traced_test]
+#[tokio::test]
+async fn test_object_info_get_command() -> Result<(), anyhow::Error> {
+    let working_dir = tempfile::tempdir()?;
+    let mut config = NetworkConfig::read_or_create(&working_dir.path().join("network.conf"))?;
+
+    SuiCommand::Genesis { config: None }
+        .execute(&mut config)
+        .await?;
+
+    // Start network
+    let network = task::spawn(async move { SuiCommand::Start.execute(&mut config).await });
+
+    // Wait for authorities to come alive.
+    retry_assert!(
+        logs_contain("Listening to TCP traffic on 127.0.0.1"),
+        Duration::from_millis(5000)
+    );
+
+    // Create Wallet context.
+    let wallet_conf = WalletConfig::read_or_create(&working_dir.path().join("wallet.conf"))?;
+    let address = wallet_conf.accounts.first().unwrap().address;
+    let mut context = WalletContext::new(wallet_conf)?;
+
+    // Sync client to retrieve objects from the network.
+    WalletCommands::SyncClientState { address }
+        .execute(&mut context)
+        .await?;
+
+    let state = context
+        .address_manager
+        .get_managed_address_states()
+        .get(&address)
+        .unwrap();
+
+    // Check log output contains all object ids.
+    let object_id = state.object_refs().next().unwrap().0;
+
+    WalletCommands::Object {
+        id: object_id,
+        deep: false,
+    }
+    .execute(&mut context)
+    .await?;
+    let obj_owner = format!("{:?}", address);
+
+    retry_assert!(
+        logs_contain(obj_owner.as_str()),
+        Duration::from_millis(5000)
+    );
+
+    network.abort();
+    Ok(())
+}

sui/src/wallet_commands.rs

@@ -3,11 +3,7 @@
 use crate::config::{AccountInfo, WalletConfig};
 use sui_core::authority_client::AuthorityClient;
 use sui_core::client::{Client, ClientAddressManager, ClientState};
-use sui_network::network::NetworkClient;
-use sui_types::base_types::{
-    decode_bytes_hex, encode_bytes_hex, AuthorityName, ObjectID, SuiAddress,
-};
-use sui_types::committee::Committee;
+use sui_types::base_types::{decode_bytes_hex, encode_bytes_hex, ObjectID, SuiAddress};
 use sui_types::crypto::get_key_pair;
 use sui_types::messages::ExecutionStatus;
 
@@ -17,7 +13,6 @@ use move_core_types::identifier::Identifier;
 use move_core_types::language_storage::TypeTag;
 use move_core_types::parser::{parse_transaction_argument, parse_type_tag};
 use move_core_types::transaction_argument::{convert_txn_args, TransactionArgument};
-use std::collections::BTreeMap;
 use std::time::Instant;
 use structopt::clap::AppSettings;
 use structopt::StructOpt;
@@ -35,10 +30,6 @@ pub enum WalletCommands {
     /// Get obj info
     #[structopt(name = "object")]
     Object {
-        /// Owner address
-        #[structopt(long, parse(try_from_str = decode_bytes_hex))]
-        owner: SuiAddress,
-
         /// Object ID of the object to fetch
         #[structopt(long)]
         id: ObjectID,
@@ -51,10 +42,6 @@ pub enum WalletCommands {
     /// Publish Move modules
     #[structopt(name = "publish")]
     Publish {
-        /// Sender address
-        #[structopt(long, parse(try_from_str = decode_bytes_hex))]
-        sender: SuiAddress,
-
         /// Path to directory containing a Move package
         #[structopt(long)]
         path: String,
@@ -71,9 +58,6 @@ pub enum WalletCommands {
     /// Call Move function
     #[structopt(name = "call")]
     Call {
-        /// Sender address
-        #[structopt(long, parse(try_from_str = decode_bytes_hex))]
-        sender: SuiAddress,
         /// Object ID of the package, which contains the module
         #[structopt(long)]
         package: ObjectID,
@@ -106,10 +90,6 @@ pub enum WalletCommands {
     /// Transfer an object
     #[structopt(name = "transfer")]
     Transfer {
-        /// Sender address
-        #[structopt(long, parse(try_from_str = decode_bytes_hex))]
-        from: SuiAddress,
-
         /// Recipient address
         #[structopt(long, parse(try_from_str = decode_bytes_hex))]
         to: SuiAddress,
@@ -150,12 +130,12 @@ impl WalletCommands {
     pub async fn execute(&mut self, context: &mut WalletContext) -> Result<(), anyhow::Error> {
         match self {
             WalletCommands::Publish {
-                sender,
                 path,
                 gas,
                 gas_budget,
             } => {
                 // Find owner of gas object
+                let sender = &context.address_manager.get_object_owner(*gas).await?;
                 let client_state = context.get_or_create_client_state(sender)?;
                 let gas_obj_ref = client_state.object_ref(*gas)?;
 
@@ -169,10 +149,9 @@ impl WalletCommands {
                 info!("{}", effects);
             }
 
-            WalletCommands::Object { id, deep, owner } => {
+            WalletCommands::Object { id, deep } => {
                 // Fetch the object ref
-                let client_state = context.get_or_create_client_state(owner)?;
-                let object_read = client_state.get_object_info(*id).await?;
+                let object_read = context.address_manager.get_object_info(*id).await?;
                 let object = object_read.object()?;
                 if *deep {
                     let layout = object_read.layout()?;
@@ -182,7 +161,6 @@ impl WalletCommands {
                 }
             }
             WalletCommands::Call {
-                sender,
                 package,
                 module,
                 function,
@@ -192,6 +170,7 @@ impl WalletCommands {
                 gas,
                 gas_budget,
             } => {
+                let sender = &context.address_manager.get_object_owner(*gas).await?;
                 let client_state = context.get_or_create_client_state(sender)?;
 
                 let package_obj_info = client_state.get_object_info(*package).await?;
@@ -223,12 +202,8 @@ impl WalletCommands {
                 info!("{}", effects);
             }
 
-            WalletCommands::Transfer {
-                to,
-                object_id,
-                gas,
-                from,
-            } => {
+            WalletCommands::Transfer { to, object_id, gas } => {
+                let from = &context.address_manager.get_object_owner(*gas).await?;
                 let client_state = context.get_or_create_client_state(from)?;
                 info!("Starting transfer");
                 let time_start = Instant::now();
@@ -295,9 +270,12 @@ impl WalletContext {
             .iter()
             .map(|info| info.address)
             .collect::<Vec<_>>();
+
+        let committee = config.make_committee();
+        let authority_clients = config.make_authority_clients();
         let mut context = Self {
             config,
-            address_manager: ClientAddressManager::new(path),
+            address_manager: ClientAddressManager::new(path, committee, authority_clients),
         };
         // Pre-populate client state for each address in the config.
         for address in addresses {
@@ -310,39 +288,7 @@ impl WalletContext {
         &mut self,
         owner: &SuiAddress,
     ) -> Result<&mut ClientState<AuthorityClient>, SuiError> {
-        let kp = Box::pin(self.get_account_cfg_info(owner)?.key_pair.copy());
-        let voting_rights = self
-            .config
-            .authorities
-            .iter()
-            .map(|authority| (authority.name, 1))
-            .collect();
-        let committee = Committee::new(voting_rights);
-        let authority_clients = self.make_authority_clients();
-        self.address_manager
-            .get_or_create_state_mut(*owner, kp, committee, authority_clients)
-    }
-
-    fn make_authority_clients(&self) -> BTreeMap<AuthorityName, AuthorityClient> {
-        let mut authority_clients = BTreeMap::new();
-        for authority in &self.config.authorities {
-            let client = AuthorityClient::new(NetworkClient::new(
-                authority.host.clone(),
-                authority.base_port,
-                self.config.buffer_size,
-                self.config.send_timeout,
-                self.config.recv_timeout,
-            ));
-            authority_clients.insert(authority.name, client);
-        }
-        authority_clients
-    }
-
-    pub fn get_account_cfg_info(&self, address: &SuiAddress) -> Result<&AccountInfo, SuiError> {
-        self.config
-            .accounts
-            .iter()
-            .find(|info| &info.address == address)
-            .ok_or(SuiError::AccountNotFound)
+        let kp = Box::pin(self.config.get_account_cfg_info(owner)?.key_pair.copy());
+        self.address_manager.get_or_create_state_mut(*owner, kp)
     }
 }

sui_core/src/authority_aggregator.rs

@@ -990,7 +990,7 @@ where
     }
 
     pub async fn get_object_info_execute(
-        &mut self,
+        &self,
         object_id: ObjectID,
     ) -> Result<ObjectRead, anyhow::Error> {
         let (object_map, cert_map) = self

sui_core/src/client.rs

@@ -40,15 +40,28 @@ pub mod client_store;
 pub type AsyncResult<'a, T, E> = future::BoxFuture<'a, Result<T, E>>;
 
 pub struct ClientAddressManager<A> {
+    committee: Committee,
+    authority_clients: BTreeMap<AuthorityName, A>,
+    authorities: AuthorityAggregator<A>,
     store: client_store::ClientAddressManagerStore,
     address_states: BTreeMap<SuiAddress, ClientState<A>>,
 }
-impl<A> ClientAddressManager<A> {
+impl<A> ClientAddressManager<A>
+where
+    A: AuthorityAPI + Send + Sync + 'static + Clone,
+{
     /// Create a new manager which stores its managed addresses at `path`
-    pub fn new(path: PathBuf) -> Self {
+    pub fn new(
+        path: PathBuf,
+        committee: Committee,
+        authority_clients: BTreeMap<AuthorityName, A>,
+    ) -> Self {
         Self {
             store: client_store::ClientAddressManagerStore::open(path),
+            authority_clients: authority_clients.clone(),
+            authorities: AuthorityAggregator::new(committee.clone(), authority_clients),
             address_states: BTreeMap::new(),
+            committee,
         }
     }
 
@@ -57,8 +70,6 @@ impl<A> ClientAddressManager<A> {
         &mut self,
         address: SuiAddress,
         secret: StableSyncSigner,
-        committee: Committee,
-        authority_clients: BTreeMap<AuthorityName, A>,
     ) -> Result<&mut ClientState<A>, SuiError> {
         #[allow(clippy::map_entry)]
         // the fallible store creation complicates the use of the entry API
@@ -73,8 +84,8 @@ impl<A> ClientAddressManager<A> {
                 ClientState::new_for_manager(
                     address,
                     secret,
-                    committee,
-                    authority_clients,
+                    self.committee.clone(),
+                    self.authority_clients.clone(),
                     single_store,
                 ),
             );
@@ -87,6 +98,17 @@ impl<A> ClientAddressManager<A> {
     pub fn get_managed_address_states(&self) -> &BTreeMap<SuiAddress, ClientState<A>> {
         &self.address_states
     }
+
+    /// Get the object info
+    pub async fn get_object_info(&self, object_id: ObjectID) -> Result<ObjectRead, anyhow::Error> {
+        self.authorities.get_object_info_execute(object_id).await
+    }
+
+    /// Get the current owner of the given object
+    pub async fn get_object_owner(&self, object_id: ObjectID) -> Result<SuiAddress, anyhow::Error> {
+        let obj_read = self.authorities.get_object_info_execute(object_id).await?;
+        Ok(obj_read.object()?.owner)
+    }
 }
 
 pub struct ClientState<A> {
@@ -170,7 +192,8 @@ pub trait Client {
     ) -> Result<MergeCoinResponse, anyhow::Error>;
 
     /// Get the object information
-    async fn get_object_info(&mut self, object_id: ObjectID) -> Result<ObjectRead, anyhow::Error>;
+    /// TODO: move this out to AddressManager
+    async fn get_object_info(&self, object_id: ObjectID) -> Result<ObjectRead, anyhow::Error>;
 
     /// Get all object we own.
     fn get_owned_objects(&self) -> Vec<ObjectID>;
@@ -767,7 +790,7 @@ where
         })
     }
 
-    async fn get_object_info(&mut self, object_id: ObjectID) -> Result<ObjectRead, anyhow::Error> {
+    async fn get_object_info(&self, object_id: ObjectID) -> Result<ObjectRead, anyhow::Error> {
         self.authorities.get_object_info_execute(object_id).await
     }