[Storage] Unify AccountState and AccountView

Closes: aptos-labs#777
My-Joke · May 4, 2022 · ba399a7 · ba399a7
1 parent 9636dd0
commit ba399a7
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 70 deletions.
diff --git a/api/src/accounts.rs b/api/src/accounts.rs
@@ -20,7 +20,7 @@ use aptos_types::{
 };
 
 use anyhow::Result;
-use aptos_types::account_state_blob::AccountStateBlob;
+use aptos_types::{account_state_blob::AccountStateBlob, account_view::AccountView};
 use move_core_types::{
     identifier::Identifier, language_storage::StructTag, move_resource::MoveStructType,
     value::MoveValue,

diff --git a/aptos-move/aptos-validator-interface/src/lib.rs b/aptos-move/aptos-validator-interface/src/lib.rs
@@ -11,6 +11,7 @@ use aptos_types::{
     account_address::AccountAddress,
     account_config,
     account_state::AccountState,
+    account_view::AccountView,
     contract_event::EventWithProof,
     event::EventKey,
     on_chain_config::ValidatorSet,

diff --git a/aptos-move/transaction-replay/src/lib.rs b/aptos-move/transaction-replay/src/lib.rs
@@ -10,6 +10,7 @@ use aptos_types::{
     account_address::AccountAddress,
     account_config::aptos_root_address,
     account_state::AccountState,
+    account_view::AccountView,
     contract_event::{ContractEvent, EventWithProof},
     event::EventKey,
     transaction::{ChangeSet, Transaction, TransactionOutput, Version, WriteSetPayload},

diff --git a/config/management/operational/src/rest_client.rs b/config/management/operational/src/rest_client.rs
@@ -6,7 +6,7 @@ use aptos_management::error::Error;
 use aptos_rest_client::Client;
 use aptos_types::{
     account_address::AccountAddress, account_config, account_config::AccountResource,
-    account_state::AccountState, account_state_blob::AccountStateBlob,
+    account_state::AccountState, account_state_blob::AccountStateBlob, account_view::AccountView,
     transaction::SignedTransaction, validator_config::ValidatorConfig,
     validator_info::ValidatorInfo,
 };

diff --git a/config/seed-peer-generator/src/utils.rs b/config/seed-peer-generator/src/utils.rs
@@ -11,8 +11,9 @@ use aptos_logger::prelude::*;
 use aptos_rest_client::Client;
 use aptos_types::{
     account_config::aptos_root_address, account_state::AccountState,
-    account_state_blob::AccountStateBlob, network_address::NetworkAddress,
-    on_chain_config::ValidatorSet, validator_info::ValidatorInfo, PeerId,
+    account_state_blob::AccountStateBlob, account_view::AccountView,
+    network_address::NetworkAddress, on_chain_config::ValidatorSet, validator_info::ValidatorInfo,
+    PeerId,
 };
 
 /// Retrieve the Fullnode seed peers from JSON-RPC

diff --git a/storage/state-view/src/account_with_state_cache.rs b/storage/state-view/src/account_with_state_cache.rs
@@ -32,8 +32,8 @@ impl<'a> AccountView for AccountWithStateCache<'a> {
             .and_then(|x| x.maybe_bytes.clone()))
     }
 
-    fn get_account_address(&self) -> &AccountAddress {
-        self.account_address
+    fn get_account_address(&self) -> anyhow::Result<Option<AccountAddress>> {
+        Ok(Some(*self.account_address))
     }
 }
 

diff --git a/storage/state-view/src/account_with_state_view.rs b/storage/state-view/src/account_with_state_view.rs
@@ -24,8 +24,8 @@ impl<'a> AccountView for AccountWithStateView<'a> {
         self.state_view.get_state_value(state_key)
     }
 
-    fn get_account_address(&self) -> &AccountAddress {
-        self.account_address
+    fn get_account_address(&self) -> anyhow::Result<Option<AccountAddress>> {
+        Ok(Some(*self.account_address))
     }
 }
 

diff --git a/testsuite/smoke-test/src/operational_tooling.rs b/testsuite/smoke-test/src/operational_tooling.rs
@@ -37,6 +37,7 @@ use aptos_types::{
     account_address::{from_identity_public_key, AccountAddress},
     account_state::AccountState,
     account_state_blob::AccountStateBlob,
+    account_view::AccountView,
     block_info::BlockInfo,
     ledger_info::LedgerInfo,
     network_address::NetworkAddress,

diff --git a/types/src/account_state.rs b/types/src/account_state.rs
@@ -6,9 +6,8 @@ use crate::{
     account_address::AccountAddress,
     account_config::{AccountResource, BalanceResource},
     account_state_blob::AccountStateBlob,
-    on_chain_config::{access_path_for_config, OnChainConfig, ValidatorSet},
+    account_view::AccountView,
     state_store::{state_key::StateKey, state_value::StateValue},
-    validator_config::{ValidatorConfig, ValidatorOperatorConfigResource},
 };
 use anyhow::{anyhow, Error, Result};
 use move_core_types::{language_storage::StructTag, move_resource::MoveResource};
@@ -23,48 +22,10 @@ use std::{
 pub struct AccountState(BTreeMap<Vec<u8>, Vec<u8>>);
 
 impl AccountState {
-    // By design and do not remove
-    pub fn get_account_address(&self) -> Result<Option<AccountAddress>> {
-        self.get_account_resource()
-            .map(|opt_ar| opt_ar.map(|ar| ar.address()))
-    }
-
-    // Return the `AccountResource` for this blob. If the blob doesn't have an `AccountResource`
-    // then it must have a `AptosAccountResource` in which case we convert that to an
-    // `AccountResource`.
-    pub fn get_account_resource(&self) -> Result<Option<AccountResource>> {
-        match self.get_resource::<AccountResource>()? {
-            x @ Some(_) => Ok(x),
-            None => Ok(None),
-        }
-    }
-
-    pub fn get_validator_config_resource(&self) -> Result<Option<ValidatorConfig>> {
-        self.get_resource::<ValidatorConfig>()
-    }
-
-    pub fn get_validator_operator_config_resource(
-        &self,
-    ) -> Result<Option<ValidatorOperatorConfigResource>> {
-        self.get_resource::<ValidatorOperatorConfigResource>()
-    }
-
-    pub fn get_validator_set(&self) -> Result<Option<ValidatorSet>> {
-        self.get_config::<ValidatorSet>()
-    }
-
     pub fn get(&self, key: &[u8]) -> Option<&Vec<u8>> {
         self.0.get(key)
     }
 
-    pub fn get_resource_impl<T: DeserializeOwned>(&self, key: &[u8]) -> Result<Option<T>> {
-        self.0
-            .get(key)
-            .map(|bytes| bcs::from_bytes(bytes))
-            .transpose()
-            .map_err(Into::into)
-    }
-
     pub fn insert(&mut self, key: Vec<u8>, value: Vec<u8>) -> Option<Vec<u8>> {
         self.0.insert(key, value)
     }
@@ -77,14 +38,6 @@ impl AccountState {
         self.0.iter()
     }
 
-    pub fn get_config<T: OnChainConfig>(&self) -> Result<Option<T>> {
-        self.get_resource_impl(&access_path_for_config(T::CONFIG_ID).path)
-    }
-
-    pub fn get_resource<T: MoveResource>(&self) -> Result<Option<T>> {
-        self.get_resource_impl(&T::struct_tag().access_vector())
-    }
-
     /// Return an iterator over the module values stored under this account
     pub fn get_modules(&self) -> impl Iterator<Item = &Vec<u8>> {
         self.0.iter().filter_map(
@@ -144,6 +97,28 @@ impl fmt::Debug for AccountState {
     }
 }
 
+impl AccountView for AccountState {
+    fn get_state_value(&self, _: &StateKey) -> Result<Option<Vec<u8>>> {
+        unimplemented!()
+    }
+
+    fn get_account_address(&self) -> anyhow::Result<Option<AccountAddress>> {
+        match self.get_resource::<AccountResource>()? {
+            x @ Some(_) => Ok(x),
+            None => Ok(None),
+        }
+        .map(|opt_ar| opt_ar.map(|ar| ar.address()))
+    }
+
+    fn get_resource_impl<T: DeserializeOwned>(&self, path: Vec<u8>) -> Result<Option<T>> {
+        self.0
+            .get(&path)
+            .map(|bytes| bcs::from_bytes(bytes))
+            .transpose()
+            .map_err(Into::into)
+    }
+}
+
 impl TryFrom<&StateValue> for AccountState {
     type Error = Error;
 

diff --git a/types/src/account_state_blob.rs b/types/src/account_state_blob.rs
@@ -4,6 +4,7 @@
 use crate::{
     account_config::{AccountResource, BalanceResource},
     account_state::AccountState,
+    account_view::AccountView,
     state_store::state_value::StateValue,
 };
 use anyhow::{anyhow, format_err, Error, Result};

diff --git a/types/src/account_view.rs b/types/src/account_view.rs
@@ -8,15 +8,16 @@ use crate::{
         access_path_for_config, ConfigurationResource, OnChainConfig, ValidatorSet, Version,
     },
     state_store::state_key::StateKey,
-    validator_config::ValidatorConfig,
+    validator_config::{ValidatorConfig, ValidatorOperatorConfigResource},
 };
+use anyhow::anyhow;
 use move_core_types::{account_address::AccountAddress, move_resource::MoveResource};
 use serde::de::DeserializeOwned;
 
 pub trait AccountView {
     fn get_state_value(&self, state_key: &StateKey) -> anyhow::Result<Option<Vec<u8>>>;
 
-    fn get_account_address(&self) -> &AccountAddress;
+    fn get_account_address(&self) -> anyhow::Result<Option<AccountAddress>>;
 
     fn get_validator_set(&self) -> anyhow::Result<Option<ValidatorSet>> {
         self.get_on_chain_config::<ValidatorSet>()
@@ -27,25 +28,29 @@ pub trait AccountView {
     }
 
     fn get_move_resource<T: MoveResource>(&self) -> anyhow::Result<Option<T>> {
-        let state_key = self.get_state_key_for_path(T::struct_tag().access_vector());
-        self.get_resource_impl(&state_key)
+        self.get_resource_impl(T::struct_tag().access_vector())
     }
 
     fn get_validator_config_resource(&self) -> anyhow::Result<Option<ValidatorConfig>> {
         self.get_resource::<ValidatorConfig>()
     }
 
+    fn get_validator_operator_config_resource(
+        &self,
+    ) -> anyhow::Result<Option<ValidatorOperatorConfigResource>> {
+        self.get_resource::<ValidatorOperatorConfigResource>()
+    }
+
     fn get_on_chain_config<T: OnChainConfig>(&self) -> anyhow::Result<Option<T>> {
-        let state_key = self.get_state_key_for_path(access_path_for_config(T::CONFIG_ID).path);
-        self.get_resource_impl(&state_key)
+        self.get_resource_impl(access_path_for_config(T::CONFIG_ID).path)
     }
 
     fn get_version(&self) -> anyhow::Result<Option<Version>> {
         self.get_on_chain_config::<Version>()
     }
 
     fn get_resource<T: MoveResource>(&self) -> anyhow::Result<Option<T>> {
-        self.get_resource_impl(&self.get_state_key_for_path(T::struct_tag().access_vector()))
+        self.get_resource_impl(T::struct_tag().access_vector())
     }
 
     fn get_chain_id_resource(&self) -> anyhow::Result<Option<ChainIdResource>> {
@@ -60,19 +65,23 @@ pub trait AccountView {
         self.get_resource::<BalanceResource>()
     }
 
-    fn get_state_key_for_path(&self, path: Vec<u8>) -> StateKey {
-        StateKey::AccessPath(AccessPath::new(*self.get_account_address(), path))
+    fn get_state_key_for_path(&self, path: Vec<u8>) -> anyhow::Result<StateKey> {
+        let account_address = self
+            .get_account_address()?
+            .ok_or_else(|| anyhow!("Could not fetch account address"))?;
+        Ok(StateKey::AccessPath(AccessPath::new(account_address, path)))
     }
 
     fn get_account_resource(&self) -> anyhow::Result<Option<AccountResource>> {
         self.get_resource::<AccountResource>()
     }
 
-    fn get_resource_impl<T: DeserializeOwned>(
-        &self,
-        state_key: &StateKey,
-    ) -> anyhow::Result<Option<T>> {
-        self.get_state_value(state_key)?
+    fn get_config<T: OnChainConfig>(&self) -> anyhow::Result<Option<T>> {
+        self.get_resource_impl(access_path_for_config(T::CONFIG_ID).path)
+    }
+
+    fn get_resource_impl<T: DeserializeOwned>(&self, path: Vec<u8>) -> anyhow::Result<Option<T>> {
+        self.get_state_value(&self.get_state_key_for_path(path)?)?
             .map(|bytes| bcs::from_bytes(&bytes))
             .transpose()
             .map_err(Into::into)