Skip to content

Commit

Permalink
[framework] Consolidate conversions (#3516)
Browse files Browse the repository at this point in the history
This PR introduces as the matching counterpart to `std::bcs` the module `aptos_std::from_bcs`. It seems to be appropriate to tie the byte conversions together under the `bcs` label because that is how we have implemented them.

All safe conversions from primitive types + Strings bytes to values are supported. Naming is as in `from_bcs::to_<type>`. Example:

```
use std::bcs;
use aptos_std::from_bcs;

assert!(from_bcs::to_address(bcs::to_bytes(&@0xabcdef)) == @0xabcdef;
```

Also the friend native `from_bytes` is now defined in `from_bcs` instead of `any`.
  • Loading branch information
wrwg authored Aug 26, 2022
1 parent 26e3aeb commit 6d7b9b6
Show file tree
Hide file tree
Showing 15 changed files with 140 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ module std::string {
}

// Native API
native fun internal_check_utf8(v: &vector<u8>): bool;
public native fun internal_check_utf8(v: &vector<u8>): bool;
native fun internal_is_char_boundary(v: &vector<u8>, i: u64): bool;
native fun internal_sub_string(v: &vector<u8>, i: u64, j: u64): vector<u8>;
native fun internal_index_of(v: &vector<u8>, r: &vector<u8>): u64;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ module std::string {
//}

// Native API
native fun internal_check_utf8(v: &vector<u8>): bool;
public native fun internal_check_utf8(v: &vector<u8>): bool;
native fun internal_is_char_boundary(v: &vector<u8>, i: u64): bool;
native fun internal_sub_string(v: &vector<u8>, i: u64, j: u64): vector<u8>;
native fun internal_index_of(v: &vector<u8>, r: &vector<u8>): u64;
Expand Down
14 changes: 7 additions & 7 deletions aptos-move/framework/aptos-framework/sources/account.move
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ module aptos_framework::account {
use aptos_framework::event::{Self, EventHandle};
use aptos_framework::guid;
use aptos_framework::system_addresses;
use aptos_framework::util;
use aptos_std::table::{Self, Table};
use aptos_std::ed25519;
use aptos_std::from_bcs;

friend aptos_framework::aptos_account;
friend aptos_framework::coin;
Expand Down Expand Up @@ -222,7 +222,7 @@ module aptos_framework::account {
let account_resource = borrow_global_mut<Account>(addr);
assert!(verify_authentication_key_matches_ed25519_public_key(account_resource.authentication_key, curr_pk_bytes), std::error::unauthenticated(EWRONG_CURRENT_PUBLIC_KEY));

let curr_auth_key = util::address_from_bytes(account_resource.authentication_key);
let curr_auth_key = from_bcs::to_address(account_resource.authentication_key);
// Construct a RotationProofChallenge struct
let challenge = RotationProofChallenge {
sequence_number: account_resource.sequence_number,
Expand All @@ -246,7 +246,7 @@ module aptos_framework::account {
// Derive the authentication key of the new PK
vector::push_back(&mut new_pk_bytes, 0);
let new_auth_key = hash::sha3_256(new_pk_bytes);
let new_address = util::address_from_bytes(new_auth_key);
let new_address = from_bcs::to_address(new_auth_key);

// Update the originating address map
table::add(address_map, new_address, addr);
Expand Down Expand Up @@ -317,7 +317,7 @@ module aptos_framework::account {
public fun create_resource_account(source: &signer, seed: vector<u8>): (signer, SignerCapability) {
let bytes = bcs::to_bytes(&signer::address_of(source));
vector::append(&mut bytes, seed);
let addr = util::address_from_bytes(hash::sha3_256(bytes));
let addr = from_bcs::to_address(hash::sha3_256(bytes));

let signer = create_account_unchecked(copy addr);
let signer_cap = SignerCapability { account: addr };
Expand Down Expand Up @@ -476,7 +476,7 @@ module aptos_framework::account {
public entry fun test_invalid_offer_rotation_capability(bob: signer) acquires Account {
let pk_with_scheme = x"f66bf0ce5ceb582b93d6780820c2025b9967aedaa259bdbb9f3d0297eced0e18";
vector::push_back(&mut pk_with_scheme, 0);
let alice_address = util::address_from_bytes(hash::sha3_256(pk_with_scheme));
let alice_address = from_bcs::to_address(hash::sha3_256(pk_with_scheme));
let alice = create_account_unchecked(alice_address);
create_account(signer::address_of(&bob));

Expand All @@ -490,7 +490,7 @@ module aptos_framework::account {
let pk = x"f66bf0ce5ceb582b93d6780820c2025b9967aedaa259bdbb9f3d0297eced0e18";
let pk_with_scheme = copy pk;
vector::push_back(&mut pk_with_scheme, 0);
let alice_address = util::address_from_bytes(hash::sha3_256(pk_with_scheme));
let alice_address = from_bcs::to_address(hash::sha3_256(pk_with_scheme));
let alice = create_account_unchecked(alice_address);
create_account(signer::address_of(&bob));

Expand All @@ -510,7 +510,7 @@ module aptos_framework::account {
let pk = x"f66bf0ce5ceb582b93d6780820c2025b9967aedaa259bdbb9f3d0297eced0e18";
let pk_with_scheme = copy pk;
vector::push_back(&mut pk_with_scheme, 0);
let alice_address = util::address_from_bytes(hash::sha3_256(pk_with_scheme));
let alice_address = from_bcs::to_address(hash::sha3_256(pk_with_scheme));
let alice = create_account_unchecked(alice_address);
create_account(signer::address_of(&bob));
create_account(signer::address_of(&charlie));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ module aptos_framework::aptos_account {
#[test(alice = @0xa11ce, core = @0x1)]
public fun test_transfer(alice: signer, core: signer) {
use std::signer;
use aptos_framework::util;
use aptos_std::from_bcs;

let bob = util::address_from_bytes(x"0000000000000000000000000000000000000000000000000000000000000b0b");
let carol = util::address_from_bytes(x"00000000000000000000000000000000000000000000000000000000000ca501");
let bob = from_bcs::to_address(x"0000000000000000000000000000000000000000000000000000000000000b0b");
let carol = from_bcs::to_address(x"00000000000000000000000000000000000000000000000000000000000ca501");

let (burn_cap, mint_cap) = aptos_framework::aptos_coin::initialize_for_test(&core);
create_account(signer::address_of(&alice));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ module aptos_framework::resource_account {
let seed = x"01";
let bytes = bcs::to_bytes(&user_addr);
vector::append(&mut bytes, copy seed);
let resource_addr = aptos_framework::util::address_from_bytes(hash::sha3_256(bytes));
let resource_addr = aptos_std::from_bcs::to_address(hash::sha3_256(bytes));

create_resource_account(&user, seed, vector::empty());
let container = borrow_global<Container>(user_addr);
Expand Down
12 changes: 3 additions & 9 deletions aptos-move/framework/aptos-stdlib/sources/any.move
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module aptos_std::any {
use aptos_std::type_info;
use std::bcs;
use aptos_std::from_bcs::from_bytes;
use std::bcs::to_bytes;
use std::error;
use std::string::String;

Expand Down Expand Up @@ -30,7 +31,7 @@ module aptos_std::any {
public fun pack<T: drop + store>(x: T): Any {
Any {
type_name: type_info::type_name<T>(),
data: bcs::to_bytes(&x)
data: to_bytes(&x)
}
}

Expand All @@ -45,13 +46,6 @@ module aptos_std::any {
&x.type_name
}

/// Native function to deserialize a type T.
///
/// Note that this function does not put any constraint on `T`. If code uses this function to
/// deserialized a linear value, its their responsibility that the data they deserialize is
/// owned.
public(friend) native fun from_bytes<T>(bytes: vector<u8>): T;

#[test_only]
struct S has store, drop { x: u64 }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module aptos_std::copyable_any {
use aptos_std::type_info;
use aptos_std::any::from_bytes;
use aptos_std::from_bcs::from_bytes;
use std::bcs;
use std::error;
use std::string::String;
Expand Down
75 changes: 75 additions & 0 deletions aptos-move/framework/aptos-stdlib/sources/from_bcs.move
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/// This module provides a number of functions to convert _primitive_ types from their representation in `std::bcs`
/// to values. This is the opposite of `bcs::to_bytes`. Note that it is not safe to define a generic public `from_bytes`
/// function because this can violate implicit struct invariants, therefore only primitive types are offerred. If
/// a general conversion back-and-force is needed, consider the `aptos_std::Any` type which preserves invariants.
///
/// Example:
/// ```
/// use std::bcs;
/// use aptos_std::from_bcs;
///
/// assert!(from_bcs::to_address(bcs::to_bytes(&@0xabcdef)) == @0xabcdef, 0);
/// ```
module aptos_std::from_bcs {
use std::string::{Self, String};

/// UTF8 check failed in conversion from bytes to string
const EINVALID_UTF8: u64 = 0x1;

public fun to_bool(v: vector<u8>): bool {
from_bytes<bool>(v)
}

public fun to_u8(v: vector<u8>): u8 {
from_bytes<u8>(v)
}

public fun to_u64(v: vector<u8>): u64 {
from_bytes<u64>(v)
}

public fun to_u128(v: vector<u8>): u128 {
from_bytes<u128>(v)
}

public fun to_address(v: vector<u8>): address {
from_bytes<address>(v)
}

public fun to_string(v: vector<u8>): String {
// To make this safe, we need to evaluate the utf8 invariant.
let s = from_bytes<String>(v);
assert!(string::internal_check_utf8(string::bytes(&s)), EINVALID_UTF8);
s
}

/// Package private native function to deserialize a type T.
///
/// Note that this function does not put any constraint on `T`. If code uses this function to
/// deserialize a linear value, its their responsibility that the data they deserialize is
/// owned.
public(friend) native fun from_bytes<T>(bytes: vector<u8>): T;
friend aptos_std::any;
friend aptos_std::copyable_any;


#[test_only]
use std::bcs::to_bytes;

#[test]
fun test_address() {
let addr = @0x01;
let addr_vec = x"0000000000000000000000000000000000000000000000000000000000000001";
let addr_out = to_address(addr_vec);
let addr_vec_out = to_bytes(&addr_out);
assert!(addr == addr_out, 0);
assert!(addr_vec == addr_vec_out, 1);
}

#[test]
#[expected_failure(abort_code = 0x10001)]
fun test_address_fail() {
let bad_vec = b"01";
to_address(bad_vec);
}
}
2 changes: 1 addition & 1 deletion aptos-move/framework/move-stdlib/sources/string.move
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ module std::string {


// Native API
native fun internal_check_utf8(v: &vector<u8>): bool;
public native fun internal_check_utf8(v: &vector<u8>): bool;
native fun internal_is_char_boundary(v: &vector<u8>, i: u64): bool;
native fun internal_sub_string(v: &vector<u8>, i: u64, j: u64): vector<u8>;
native fun internal_index_of(v: &vector<u8>, r: &vector<u8>): u64;
Expand Down
13 changes: 0 additions & 13 deletions aptos-move/framework/src/natives/any.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Copyright (c) Aptos
// SPDX-License-Identifier: Apache-2.0

use crate::natives::{util, GasParameters};
use anyhow::bail;
use move_deps::move_vm_runtime::native_functions::NativeFunction;
use serde::de::DeserializeOwned;
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -33,14 +31,3 @@ impl Any {
}
}
}

// The Any module hijacks just one function, from_bytes, from the util module. This
// is a friend function which cannot be used across packages, so we have it both
// in aptos_std and aptos_framework.
pub fn make_all(gas_params: GasParameters) -> impl Iterator<Item = (String, NativeFunction)> {
let natives = [(
"from_bytes",
util::make_native_from_bytes(gas_params.util.from_bytes),
)];
crate::natives::helpers::make_module_natives(natives)
}
2 changes: 1 addition & 1 deletion aptos-move/framework/src/natives/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub fn all_natives(
);
add_natives_from_module!("type_info", type_info::make_all(gas_params.type_info));
add_natives_from_module!("util", util::make_all(gas_params.util.clone()));
add_natives_from_module!("any", util::make_all(gas_params.util));
add_natives_from_module!("from_bcs", util::make_all(gas_params.util));
add_natives_from_module!(
"transaction_context",
transaction_context::make_all(gas_params.transaction_context)
Expand Down
1 change: 1 addition & 0 deletions crates/aptos/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ fn compile_in_temp_dir(
framework_rev,
BTreeMap::new(),
prompt_options,
None,
)?;

// Insert the new script
Expand Down
20 changes: 19 additions & 1 deletion crates/aptos/src/move_tool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ pub struct InitPackage {

#[clap(flatten)]
pub(crate) prompt_options: PromptOptions,

/// For test: use the given local reference to the aptos framework
#[clap(skip)]
pub(crate) for_test_framework: Option<PathBuf>,
}

#[async_trait]
Expand All @@ -140,6 +144,7 @@ impl CliCommand<()> for InitPackage {
Some("main".to_string()),
addresses,
self.prompt_options,
self.for_test_framework,
)
}
}
Expand All @@ -150,6 +155,7 @@ pub fn init_move_dir(
rev: Option<String>,
addresses: BTreeMap<String, ManifestNamedAddress>,
prompt_options: PromptOptions,
for_test_framework: Option<PathBuf>,
) -> CliTypedResult<()> {
let move_toml = package_dir.join(SourcePackageLayout::Manifest.path());
check_if_file_exists(move_toml.as_path(), prompt_options)?;
Expand All @@ -161,7 +167,19 @@ pub fn init_move_dir(

// Add the framework dependency if it's provided
let mut dependencies = BTreeMap::new();
if let Some(rev) = rev {
if let Some(path) = for_test_framework {
dependencies.insert(
"AptosFramework".to_string(),
Dependency {
local: Some(path.display().to_string()),
git: None,
rev: None,
subdir: None,
aptos: None,
address: None,
},
);
} else if let Some(rev) = rev {
dependencies.insert(
"AptosFramework".to_string(),
Dependency {
Expand Down
2 changes: 2 additions & 0 deletions crates/aptos/src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ impl CliTestFramework {
&self,
name: String,
account_strs: BTreeMap<&str, &str>,
framework_dir: Option<PathBuf>,
) -> CliTypedResult<()> {
InitPackage {
name,
Expand All @@ -636,6 +637,7 @@ impl CliTestFramework {
assume_yes: false,
assume_no: true,
},
for_test_framework: framework_dir,
}
.execute()
.await
Expand Down
Loading

0 comments on commit 6d7b9b6

Please sign in to comment.