Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[move-stdlib] Implement bcs::constant_serialized_size<T>(): Option<u64> #14984

Merged
merged 3 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

3 changes: 3 additions & 0 deletions aptos-move/aptos-gas-schedule/src/gas_schedule/move_stdlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
};
use aptos_gas_algebra::{
InternalGas, InternalGasPerAbstractValueUnit, InternalGasPerArg, InternalGasPerByte,
InternalGasPerTypeNode,
};

crate::gas_schedule::macros::define_gas_parameters!(
Expand Down Expand Up @@ -41,6 +42,8 @@ crate::gas_schedule::macros::define_gas_parameters!(
[bcs_serialized_size_base: InternalGas, { RELEASE_V1_18.. => "bcs.serialized_size.base" }, 735],
[bcs_serialized_size_per_byte_serialized: InternalGasPerByte, { RELEASE_V1_18.. => "bcs.serialized_size.per_byte_serialized" }, 36],
[bcs_serialized_size_failure: InternalGas, { RELEASE_V1_18.. => "bcs.serialized_size.failure" }, 3676],
[bcs_constant_serialized_size_base: InternalGas, { RELEASE_V1_24.. => "bcs.constant_serialized_size.base" }, 735],
[bcs_constant_serialized_size_per_type_node: InternalGasPerTypeNode, { RELEASE_V1_24.. => "bcs.constant_serialized_size.per_type_node" }, 40],

[cmp_compare_base: InternalGas, { RELEASE_V1_24.. => "cmp.compare.base" }, 367],
[cmp_compare_per_abs_val_unit: InternalGasPerAbstractValueUnit, { RELEASE_V1_24.. => "cmp.compare.per_abs_val_unit"}, 14],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ module admin::transaction_context_test {
), type_info::type_name<T3>()],
13
);

assert!(option::some(option::destroy_some(payload_opt)) == transaction_context::entry_function_payload(), 13);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated, but done while working on this PR - added a test that move correctly returns Option from natives, which this PR also does

} else {
assert!(option::none() == payload_opt, 14);
}
}

Expand All @@ -128,7 +132,10 @@ module admin::transaction_context_test {
store.function_name = transaction_context::function_name(entry);
store.type_arg_names = transaction_context::type_arg_names(entry);
store.args = transaction_context::args(entry);
}
};
assert!(option::some(option::destroy_some(multisig_opt)) == transaction_context::multisig_payload(), 1);
} else {
assert!(option::none() == multisig_opt, 2);
}
}

Expand Down
1 change: 1 addition & 0 deletions aptos-move/framework/move-stdlib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ publish = false
aptos-gas-schedule = { workspace = true }
aptos-native-interface = { workspace = true }
aptos-types = { workspace = true }
bcs = { workspace = true }
move-core-types = { path = "../../../third_party/move/move-core/types" }
move-vm-runtime = { path = "../../../third_party/move/move-vm/runtime" }
move-vm-types = { path = "../../../third_party/move/move-vm/types" }
Expand Down
36 changes: 35 additions & 1 deletion aptos-move/framework/move-stdlib/doc/bcs.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ details on BCS.

- [Function `to_bytes`](#0x1_bcs_to_bytes)
- [Function `serialized_size`](#0x1_bcs_serialized_size)
- [Function `constant_serialized_size`](#0x1_bcs_constant_serialized_size)
- [Specification](#@Specification_0)
- [Function `serialized_size`](#@Specification_0_serialized_size)


<pre><code></code></pre>
<pre><code><b>use</b> <a href="option.md#0x1_option">0x1::option</a>;
</code></pre>



Expand Down Expand Up @@ -65,6 +67,38 @@ Aborts with <code>0x1c5</code> error code if there is a failure when calculating



</details>

<a id="0x1_bcs_constant_serialized_size"></a>

## Function `constant_serialized_size`

If the type has known constant (always the same, independent of instance) serialized size
in BCS (Binary Canonical Serialization) format, returns it, otherwise returns None.
Aborts with <code>0x1c5</code> error code if there is a failure when calculating serialized size.

Note:
For some types it might not be known they have constant size, and function might return None.
For example, signer appears to have constant size, but it's size might change.
If this function returned Some() for some type before - it is guaranteed to continue returning Some().
On the other hand, if function has returned None for some type,
it might change in the future to return Some() instead, if size becomes "known".


<pre><code><b>public</b>(<b>friend</b>) <b>fun</b> <a href="bcs.md#0x1_bcs_constant_serialized_size">constant_serialized_size</a>&lt;MoveValue&gt;(): <a href="option.md#0x1_option_Option">option::Option</a>&lt;u64&gt;
</code></pre>



<details>
<summary>Implementation</summary>


<pre><code><b>native</b> <b>public</b>(<b>friend</b>) <b>fun</b> <a href="bcs.md#0x1_bcs_constant_serialized_size">constant_serialized_size</a>&lt;MoveValue&gt;(): Option&lt;u64&gt;;
</code></pre>



</details>

<a id="@Specification_0"></a>
Expand Down
19 changes: 19 additions & 0 deletions aptos-move/framework/move-stdlib/sources/bcs.move
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
/// published on-chain. See https://github.com/aptos-labs/bcs#binary-canonical-serialization-bcs for more
/// details on BCS.
module std::bcs {
use std::option::Option;

/// Returns the binary representation of `v` in BCS (Binary Canonical Serialization) format.
/// Aborts with `0x1c5` error code if serialization fails.
native public fun to_bytes<MoveValue>(v: &MoveValue): vector<u8>;
Expand All @@ -11,6 +13,23 @@ module std::bcs {
/// Aborts with `0x1c5` error code if there is a failure when calculating serialized size.
native public fun serialized_size<MoveValue>(v: &MoveValue): u64;

// TODO - function `constant_serialized_size1 is `public(friend)` here for one release,
// and to be changed to `public` one release later.
#[test_only]
friend std::bcs_tests;

/// If the type has known constant (always the same, independent of instance) serialized size
/// in BCS (Binary Canonical Serialization) format, returns it, otherwise returns None.
/// Aborts with `0x1c5` error code if there is a failure when calculating serialized size.
///
/// Note:
/// For some types it might not be known they have constant size, and function might return None.
/// For example, signer appears to have constant size, but it's size might change.
/// If this function returned Some() for some type before - it is guaranteed to continue returning Some().
/// On the other hand, if function has returned None for some type,
/// it might change in the future to return Some() instead, if size becomes "known".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can this happen - for a size of a type to become known in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what I meant here was with a future release, size goes from None to Some.

for example if we start supporting bcs serialization and size for delayed fields.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KK, that makes sense.

native public(friend) fun constant_serialized_size<MoveValue>(): Option<u64>;

// ==============================
// Module Specification
spec module {} // switch to module documentation context
Expand Down
116 changes: 113 additions & 3 deletions aptos-move/framework/move-stdlib/src/natives/bcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,26 @@ use aptos_native_interface::{
SafeNativeResult,
};
use move_core_types::{
gas_algebra::NumBytes, vm_status::sub_status::NFE_BCS_SERIALIZATION_FAILURE,
account_address::AccountAddress,
gas_algebra::{NumBytes, NumTypeNodes},
u256,
value::{MoveStructLayout, MoveTypeLayout},
vm_status::{sub_status::NFE_BCS_SERIALIZATION_FAILURE, StatusCode},
};
use move_vm_runtime::native_functions::NativeFunction;
use move_vm_types::{
loaded_data::runtime_types::Type,
natives::function::PartialVMResult,
natives::function::{PartialVMError, PartialVMResult},
value_serde::serialized_size_allowing_delayed_values,
values::{values_impl::Reference, Value},
values::{values_impl::Reference, Struct, Value},
};
use smallvec::{smallvec, SmallVec};
use std::collections::VecDeque;

pub fn create_option_u64(value: Option<u64>) -> Value {
Value::struct_(Struct::pack(vec![Value::vector_u64(value)]))
}

/***************************************************************************************************
* native fun to_bytes
*
Expand Down Expand Up @@ -126,6 +134,107 @@ fn serialized_size_impl(
serialized_size_allowing_delayed_values(&value, &ty_layout)
}

fn native_constant_serialized_size(
context: &mut SafeNativeContext,
mut ty_args: Vec<Type>,
_args: VecDeque<Value>,
) -> SafeNativeResult<SmallVec<[Value; 1]>> {
debug_assert!(ty_args.len() == 1);

context.charge(BCS_CONSTANT_SERIALIZED_SIZE_BASE)?;

let ty = ty_args.pop().unwrap();
georgemitenkov marked this conversation as resolved.
Show resolved Hide resolved
let ty_layout = context.type_to_type_layout(&ty)?;

let (visited_count, serialized_size_result) = constant_serialized_size(&ty_layout);
context
.charge(BCS_CONSTANT_SERIALIZED_SIZE_PER_TYPE_NODE * NumTypeNodes::new(visited_count))?;

let result = match serialized_size_result {
Ok(value) => create_option_u64(value.map(|v| v as u64)),
Err(_) => {
context.charge(BCS_SERIALIZED_SIZE_FAILURE)?;

// Re-use the same abort code as bcs::to_bytes.
return Err(SafeNativeError::Abort {
abort_code: NFE_BCS_SERIALIZATION_FAILURE,
});
},
};

Ok(smallvec![result])
}

/// If given type has a constant serialized size (irrespective of the instance), it returns the serialized
/// size in bytes any value would have.
/// Otherwise it returns None.
/// First element of the returned tuple represents number of visited nodes, used to charge gas.
fn constant_serialized_size(ty_layout: &MoveTypeLayout) -> (u64, PartialVMResult<Option<usize>>) {
let mut visited_count = 1;
let bcs_size_result = match ty_layout {
MoveTypeLayout::Bool => bcs::serialized_size(&false).map(Some),
MoveTypeLayout::U8 => bcs::serialized_size(&0u8).map(Some),
MoveTypeLayout::U16 => bcs::serialized_size(&0u16).map(Some),
MoveTypeLayout::U32 => bcs::serialized_size(&0u32).map(Some),
MoveTypeLayout::U64 => bcs::serialized_size(&0u64).map(Some),
MoveTypeLayout::U128 => bcs::serialized_size(&0u128).map(Some),
MoveTypeLayout::U256 => bcs::serialized_size(&u256::U256::zero()).map(Some),
MoveTypeLayout::Address => bcs::serialized_size(&AccountAddress::ZERO).map(Some),
// signer's size is VM implementation detail, and can change at will.
MoveTypeLayout::Signer => Ok(None),
// vectors have no constant size
MoveTypeLayout::Vector(_) => Ok(None),
// enums have no constant size
MoveTypeLayout::Struct(
MoveStructLayout::RuntimeVariants(_) | MoveStructLayout::WithVariants(_),
) => Ok(None),
MoveTypeLayout::Struct(MoveStructLayout::Runtime(fields)) => {
let mut total = Some(0);
for field in fields {
let (cur_visited_count, cur) = constant_serialized_size(field);
visited_count += cur_visited_count;
match cur {
Err(e) => return (visited_count, Err(e)),
Ok(Some(cur_value)) => total = total.map(|v| v + cur_value),
Ok(None) => {
total = None;
break;
},
}
}
Ok(total)
},
MoveTypeLayout::Struct(MoveStructLayout::WithFields(_))
| MoveTypeLayout::Struct(MoveStructLayout::WithTypes { .. }) => {
return (
visited_count,
Err(
PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(
"Only runtime types expected, but found WithFields/WithTypes".to_string(),
),
),
)
},
MoveTypeLayout::Native(_, inner) => {
let (cur_visited_count, cur) = constant_serialized_size(inner);
visited_count += cur_visited_count;
match cur {
Err(e) => return (visited_count, Err(e)),
Ok(v) => Ok(v),
}
},
};
(
visited_count,
bcs_size_result.map_err(|e| {
PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!(
"failed to compute serialized size of a value: {:?}",
e
))
}),
)
}

/***************************************************************************************************
* module
**************************************************************************************************/
Expand All @@ -135,6 +244,7 @@ pub fn make_all(
let funcs = [
("to_bytes", native_to_bytes as RawSafeNative),
("serialized_size", native_serialized_size),
("constant_serialized_size", native_constant_serialized_size),
];

builder.make_named_natives(funcs)
Expand Down
40 changes: 40 additions & 0 deletions aptos-move/framework/move-stdlib/tests/bcs_tests.move
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
module std::bcs_tests {
use std::bcs;
use std::vector;
use std::option;
use std::signer;

struct Box<T> has copy, drop, store { x: T }
struct Box3<T> has copy, drop, store { x: Box<Box<T>> }
Expand All @@ -20,6 +22,8 @@ module std::bcs_tests {
let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&true);
assert!(actual_size == expected_size, 1);

assert!(option::some(actual_size) == bcs::constant_serialized_size<bool>(), 2);
}

#[test]
Expand All @@ -31,6 +35,8 @@ module std::bcs_tests {
let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&1u8);
assert!(actual_size == expected_size, 1);

assert!(option::some(actual_size) == bcs::constant_serialized_size<u8>(), 2);
}

#[test]
Expand All @@ -42,6 +48,8 @@ module std::bcs_tests {
let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&1);
assert!(actual_size == expected_size, 1);

assert!(option::some(actual_size) == bcs::constant_serialized_size<u64>(), 2);
}

#[test]
Expand All @@ -53,6 +61,8 @@ module std::bcs_tests {
let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&1u128);
assert!(actual_size == expected_size, 1);

assert!(option::some(actual_size) == bcs::constant_serialized_size<u128>(), 2);
}

#[test]
Expand All @@ -66,6 +76,23 @@ module std::bcs_tests {
let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&v);
assert!(actual_size == expected_size, 1);

assert!(option::none() == bcs::constant_serialized_size<vector<u8>>(), 2);
}

#[test(creator = @0xcafe)]
fun bcs_address(creator: &signer) {
let v = signer::address_of(creator);

let expected_bytes = x"000000000000000000000000000000000000000000000000000000000000CAFE";
let actual_bytes = bcs::to_bytes(&v);
assert!(actual_bytes == expected_bytes, 0);

let expected_size = vector::length(&actual_bytes);
let actual_size = bcs::serialized_size(&v);
assert!(actual_size == expected_size, 1);

assert!(option::some(actual_size) == bcs::constant_serialized_size<address>(), 2);
}

fun box3<T>(x: T): Box3<T> {
Expand Down Expand Up @@ -101,5 +128,18 @@ module std::bcs_tests {

let actual_size = bcs::serialized_size(&box);
assert!(actual_size == expected_size, 0);

assert!(option::some(actual_size) == bcs::constant_serialized_size<Box127<bool>>(), 1);
assert!(option::none() == bcs::constant_serialized_size<Box63<vector<bool>>>(), 2);
assert!(option::none() == bcs::constant_serialized_size<Box63<option::Option<bool>>>(), 3);
}

// enum Singleton {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This for Move 2 tests only? You have tested it locally? Or will enable later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will enable this once framework is compiled with move 2

// V1(u64),
// }

// fun encode_enum() {
// assert!(option::none() == bcs::constant_serialized_size<Singleton>());
// assert!(option::none() == bcs::constant_serialized_size<Box3<Singleton>>());
// }
}
1 change: 1 addition & 0 deletions types/src/on_chain_config/aptos_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub enum FeatureFlag {
TRANSACTION_SIMULATION_ENHANCEMENT = 78,
COLLECTION_OWNER = 79,
/// covers mem::swap and vector::move_range
/// AIP-105 (https://github.com/aptos-foundation/AIPs/blob/main/aips/aip-105.md)
NATIVE_MEMORY_OPERATIONS = 80,
ENABLE_LOADER_V2 = 81,
}
Expand Down
Loading