Skip to content

Commit

Permalink
[types] remove ResourceKey from AccessPath
Browse files Browse the repository at this point in the history
I guess this must have been some legacy code, but we were passing around
an unnecessary ResourceKey everywhere, which just resulted in functional
bloat since there were either scattered helper functions or extra big
function calls.

Ideally this just improves readability, unless the compiler doesn't
treat this as a no-op, in which case, that'd be a cool little bit of
savings...
  • Loading branch information
davidiw committed Jan 16, 2023
1 parent 18367fd commit b362344
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 58 deletions.
12 changes: 5 additions & 7 deletions api/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ use aptos_types::{
state_store::state_key::StateKey,
};
use move_core_types::{
identifier::Identifier,
language_storage::{ResourceKey, StructTag},
move_resource::MoveStructType,
identifier::Identifier, language_storage::StructTag, move_resource::MoveStructType,
value::MoveValue,
};
use poem_openapi::{
Expand Down Expand Up @@ -249,10 +247,10 @@ impl Account {
}

pub fn get_account_resource(&self) -> Result<Vec<u8>, BasicErrorWith404> {
let state_key = StateKey::AccessPath(AccessPath::resource_access_path(ResourceKey::new(
let state_key = StateKey::AccessPath(AccessPath::resource_access_path(
self.address.into(),
AccountResource::struct_tag(),
)));
));

let state_value = self.context.get_state_value_poem(
&state_key,
Expand Down Expand Up @@ -476,10 +474,10 @@ impl Account {
&self,
struct_tag: &StructTag,
) -> Result<Vec<(Identifier, MoveValue)>, BasicErrorWith404> {
let state_key = StateKey::AccessPath(AccessPath::resource_access_path(ResourceKey::new(
let state_key = StateKey::AccessPath(AccessPath::resource_access_path(
self.address.into(),
struct_tag.clone(),
)));
));
let state_value_bytes = self
.context
.db
Expand Down
5 changes: 2 additions & 3 deletions api/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use aptos_types::{
state_store::{state_key::StateKey, table::TableHandle},
};
use aptos_vm::data_cache::AsMoveResolver;
use move_core_types::language_storage::{ModuleId, ResourceKey, StructTag};
use move_core_types::language_storage::{ModuleId, StructTag};
use poem_openapi::{
param::{Path, Query},
payload::Json,
Expand Down Expand Up @@ -244,8 +244,7 @@ impl StateApi {
BasicErrorWith404::bad_request_with_code_no_info(err, AptosErrorCode::InvalidInput)
})?;
let (ledger_info, ledger_version, state_view) = self.preprocess_request(ledger_version)?;
let resource_key = ResourceKey::new(address.into(), resource_type.clone());
let access_path = AccessPath::resource_access_path(resource_key);
let access_path = AccessPath::resource_access_path(address.into(), resource_type.clone());
let state_key = StateKey::AccessPath(access_path);
let bytes = state_view
.get_state_value(&state_key)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use move_compiler::{self, shared::PackagePaths, FullyCompiledProgram};
use move_core_types::{
account_address::AccountAddress,
identifier::{IdentStr, Identifier},
language_storage::{ModuleId, ResourceKey, TypeTag},
language_storage::{ModuleId, TypeTag},
move_resource::MoveStructType,
parser::parse_type_tag,
transaction_argument::{convert_txn_args, TransactionArgument},
Expand Down Expand Up @@ -364,10 +364,8 @@ impl<'a> AptosTestAdapter<'a> {
/// Obtain a Rust representation of the account resource from storage, which is used to derive
/// a few default transaction parameters.
fn fetch_account_resource(&self, signer_addr: &AccountAddress) -> Result<AccountResource> {
let account_access_path = AccessPath::resource_access_path(ResourceKey::new(
*signer_addr,
AccountResource::struct_tag(),
));
let account_access_path =
AccessPath::resource_access_path(*signer_addr, AccountResource::struct_tag());
let account_blob = self
.storage
.get_state_value(&StateKey::AccessPath(account_access_path))
Expand All @@ -385,10 +383,8 @@ impl<'a> AptosTestAdapter<'a> {
fn fetch_account_balance(&self, signer_addr: &AccountAddress) -> Result<u64> {
let aptos_coin_tag = CoinStoreResource::struct_tag();

let coin_access_path = AccessPath::resource_access_path(ResourceKey::new(
*signer_addr,
aptos_coin_tag.clone(),
));
let coin_access_path =
AccessPath::resource_access_path(*signer_addr, aptos_coin_tag.clone());

let balance_blob = self
.storage
Expand Down
3 changes: 1 addition & 2 deletions aptos-move/aptos-vm/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0
//! Scratchpad for on chain values during the execution.

use crate::create_access_path;
#[allow(unused_imports)]
use anyhow::Error;
use aptos_framework::natives::state_storage::StateStorageUsageResolver;
Expand Down Expand Up @@ -55,7 +54,7 @@ impl<'a, S: StateView> ResourceResolver for StorageAdapter<'a, S> {
address: &AccountAddress,
struct_tag: &StructTag,
) -> Result<Option<Vec<u8>>, Self::Error> {
let ap = create_access_path(*address, struct_tag.clone());
let ap = AccessPath::resource_access_path(*address, struct_tag.clone());
self.get(&ap).map_err(|e| e.finish(Location::Undefined))
}
}
Expand Down
11 changes: 0 additions & 11 deletions aptos-move/aptos-vm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,9 @@ mod verifier;
pub use crate::aptos_vm::AptosVM;
use aptos_state_view::StateView;
use aptos_types::{
access_path::AccessPath,
transaction::{SignedTransaction, Transaction, TransactionOutput, VMValidatorResult},
vm_status::VMStatus,
};
use move_core_types::{
account_address::AccountAddress,
language_storage::{ResourceKey, StructTag},
};
use std::marker::Sync;

/// This trait describes the VM's validation interfaces.
Expand All @@ -158,9 +153,3 @@ pub trait VMExecutor: Send + Sync {
state_view: &(impl StateView + Sync),
) -> Result<Vec<TransactionOutput>, VMStatus>;
}

/// Get the AccessPath to a resource stored under `address` with type name `tag`
fn create_access_path(address: AccountAddress, tag: StructTag) -> AccessPath {
let resource_tag = ResourceKey::new(address, tag);
AccessPath::resource_access_path(resource_tag)
}
6 changes: 3 additions & 3 deletions aptos-move/e2e-move-tests/src/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use aptos_types::{
},
};
use move_core_types::{
language_storage::{ResourceKey, StructTag, TypeTag},
language_storage::{StructTag, TypeTag},
move_resource::MoveStructType,
value::MoveValue,
};
Expand Down Expand Up @@ -358,7 +358,7 @@ impl MoveHarness {
addr: &AccountAddress,
struct_tag: StructTag,
) -> Option<Vec<u8>> {
let path = AccessPath::resource_access_path(ResourceKey::new(*addr, struct_tag));
let path = AccessPath::resource_access_path(*addr, struct_tag);
self.read_state_value(&StateKey::AccessPath(path))
}

Expand Down Expand Up @@ -387,7 +387,7 @@ impl MoveHarness {
struct_tag: StructTag,
data: &T,
) {
let path = AccessPath::resource_access_path(ResourceKey::new(addr, struct_tag));
let path = AccessPath::resource_access_path(addr, struct_tag);
let state_key = StateKey::AccessPath(path);
self.executor
.write_state_value(state_key, bcs::to_bytes(data).unwrap());
Expand Down
14 changes: 3 additions & 11 deletions aptos-move/e2e-tests/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,7 @@ use aptos_types::{
write_set::{WriteOp, WriteSet, WriteSetMut},
};
use aptos_vm_genesis::GENESIS_KEYPAIR;
use move_core_types::{
language_storage::{ResourceKey, StructTag},
move_resource::MoveStructType,
};
use move_core_types::move_resource::MoveStructType;

// TTL is 86400s. Initial time was set to 0.
pub const DEFAULT_EXPIRATION_TIME: u64 = 4_000_000;
Expand Down Expand Up @@ -121,19 +118,14 @@ impl Account {
///
/// Use this to retrieve or publish the Account blob.
pub fn make_account_access_path(&self) -> AccessPath {
self.make_access_path(AccountResource::struct_tag())
AccessPath::resource_access_path(self.addr, AccountResource::struct_tag())
}

/// Returns the AccessPath that describes the Account's CoinStore resource instance.
///
/// Use this to retrieve or publish the Account CoinStore blob.
pub fn make_coin_store_access_path(&self) -> AccessPath {
self.make_access_path(CoinStoreResource::struct_tag())
}

pub fn make_access_path(&self, tag: StructTag) -> AccessPath {
let resource_tag = ResourceKey::new(self.addr, tag);
AccessPath::resource_access_path(resource_tag)
AccessPath::resource_access_path(self.addr, CoinStoreResource::struct_tag())
}

/// Changes the keys for this account to the provided ones.
Expand Down
4 changes: 2 additions & 2 deletions aptos-move/e2e-tests/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use aptos_vm_genesis::{generate_genesis_change_set_for_testing_with_count, Genes
use move_core_types::{
account_address::AccountAddress,
identifier::Identifier,
language_storage::{ModuleId, ResourceKey, TypeTag},
language_storage::{ModuleId, TypeTag},
move_resource::MoveResource,
};
use move_vm_types::gas::UnmeteredGasMeter;
Expand Down Expand Up @@ -288,7 +288,7 @@ impl FakeExecutor {
}

pub fn read_resource<T: MoveResource>(&self, addr: &AccountAddress) -> Option<T> {
let ap = AccessPath::resource_access_path(ResourceKey::new(*addr, T::struct_tag()));
let ap = AccessPath::resource_access_path(*addr, T::struct_tag());
let data_blob = TStateView::get_state_value(&self.data_store, &StateKey::AccessPath(ap))
.expect("account must exist in data store")
.unwrap_or_else(|| panic!("Can't fetch {} resource for {}", T::STRUCT_NAME, addr));
Expand Down
18 changes: 11 additions & 7 deletions types/src/access_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
use crate::{account_address::AccountAddress, state_store::state_key::StateKey};
use anyhow::{Error, Result};
use aptos_crypto::hash::HashValue;
use move_core_types::language_storage::{ModuleId, ResourceKey, StructTag, CODE_TAG, RESOURCE_TAG};
use move_core_types::language_storage::{ModuleId, StructTag};
#[cfg(any(test, feature = "fuzzing"))]
use proptest_derive::Arbitrary;
use serde::{Deserialize, Serialize};
Expand All @@ -58,6 +58,11 @@ pub enum Path {
Resource(StructTag),
}

pub enum PathType {
Code,
Resource,
}

impl AccessPath {
pub fn new(address: AccountAddress, path: Vec<u8>) -> Self {
AccessPath { address, path }
Expand All @@ -69,11 +74,10 @@ impl AccessPath {

/// Convert Accesses into a byte offset which would be used by the storage layer to resolve
/// where fields are stored.
pub fn resource_access_path(key: ResourceKey) -> AccessPath {
let path = AccessPath::resource_path_vec(key.type_);
pub fn resource_access_path(address: AccountAddress, type_: StructTag) -> AccessPath {
AccessPath {
address: key.address,
path,
address,
path: AccessPath::resource_path_vec(type_),
}
}

Expand Down Expand Up @@ -128,8 +132,8 @@ impl fmt::Display for AccessPath {
} else {
write!(f, "AccessPath {{ address: {:x}, ", self.address)?;
match self.path[0] {
RESOURCE_TAG => write!(f, "type: Resource, ")?,
CODE_TAG => write!(f, "type: Module, ")?,
p if p == PathType::Resource as u8 => write!(f, "type: Resource, ")?,
p if p == PathType::Code as u8 => write!(f, "type: Module, ")?,
tag => write!(f, "type: {:?}, ", tag)?,
};
write!(
Expand Down
6 changes: 3 additions & 3 deletions types/src/account_config/resources/coin_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use move_core_types::{
account_address::AccountAddress,
ident_str,
identifier::IdentStr,
language_storage::{ResourceKey, TypeTag},
language_storage::TypeTag,
move_resource::{MoveResource, MoveStructType},
};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -111,8 +111,8 @@ impl CoinInfoResource {
/// Returns a writeset corresponding to the creation of CoinInfo in Move.
/// This can be passed to data store for testing total supply.
pub fn to_writeset(&self) -> WriteSet {
let tag = ResourceKey::new(AccountAddress::ONE, CoinInfoResource::struct_tag());
let ap = AccessPath::resource_access_path(tag);
let ap =
AccessPath::resource_access_path(AccountAddress::ONE, CoinInfoResource::struct_tag());

let value_state_key = self
.supply
Expand Down

0 comments on commit b362344

Please sign in to comment.