From 3ae6a87ce89d2cdd250cbca38aa7b740bf39b913 Mon Sep 17 00:00:00 2001 From: runtianz Date: Wed, 4 Sep 2024 01:55:22 -0700 Subject: [PATCH] Implement rust logics for permissioned signer --- aptos-move/e2e-move-tests/src/tests/gas.rs | 10 +- aptos-move/framework/src/natives/mod.rs | 5 + .../src/natives/permissioned_signer.rs | 103 ++++++++++++++++++ .../framework/src/natives/string_utils.rs | 2 +- .../move/move-compiler-v2/src/plan_builder.rs | 9 +- .../src/unit_test/plan_builder.rs | 16 ++- .../types/src/unit_tests/value_test.rs | 10 ++ third_party/move/move-core/types/src/value.rs | 19 +++- .../move/move-stdlib/src/natives/signer.rs | 6 +- .../move/move-vm/runtime/src/interpreter.rs | 4 +- .../move/move-vm/types/src/value_serde.rs | 5 + .../types/src/values/serialization_tests.rs | 14 +++ .../move-vm/types/src/values/values_impl.rs | 77 ++++++++++--- 13 files changed, 250 insertions(+), 30 deletions(-) create mode 100644 aptos-move/framework/src/natives/permissioned_signer.rs diff --git a/aptos-move/e2e-move-tests/src/tests/gas.rs b/aptos-move/e2e-move-tests/src/tests/gas.rs index 4ccbeb674171e..8bc84943d52c7 100644 --- a/aptos-move/e2e-move-tests/src/tests/gas.rs +++ b/aptos-move/e2e-move-tests/src/tests/gas.rs @@ -30,7 +30,7 @@ use aptos_types::{ transaction::{EntryFunction, TransactionPayload}, vm::configs::set_paranoid_type_checks, }; -use move_core_types::{identifier::Identifier, language_storage::ModuleId}; +use move_core_types::{identifier::Identifier, language_storage::ModuleId, value::MoveValue}; use rand::{rngs::StdRng, SeedableRng}; use sha3::{Digest, Sha3_512}; use std::path::Path; @@ -57,7 +57,9 @@ fn test_modify_gas_schedule_check_hash() { "set_for_next_epoch_check_hash", vec![], vec![ - bcs::to_bytes(&CORE_CODE_ADDRESS).unwrap(), + MoveValue::Signer(CORE_CODE_ADDRESS) + .simple_serialize() + .unwrap(), bcs::to_bytes(&old_hash).unwrap(), bcs::to_bytes(&bcs::to_bytes(&gas_schedule).unwrap()).unwrap(), ], @@ -66,7 +68,9 @@ fn test_modify_gas_schedule_check_hash() { harness .executor .exec("reconfiguration_with_dkg", "finish", vec![], vec![ - bcs::to_bytes(&CORE_CODE_ADDRESS).unwrap(), + MoveValue::Signer(CORE_CODE_ADDRESS) + .simple_serialize() + .unwrap(), ]); let (_, gas_params) = harness.get_gas_params(); diff --git a/aptos-move/framework/src/natives/mod.rs b/aptos-move/framework/src/natives/mod.rs index dcfc80407f932..956c590b4afc8 100644 --- a/aptos-move/framework/src/natives/mod.rs +++ b/aptos-move/framework/src/natives/mod.rs @@ -15,6 +15,7 @@ pub mod function_info; pub mod hash; pub mod object; pub mod object_code_deployment; +pub mod permissioned_signer; pub mod randomness; pub mod state_storage; pub mod string_utils; @@ -91,6 +92,10 @@ pub fn all_natives( "dispatchable_fungible_asset", dispatchable_fungible_asset::make_all(builder) ); + add_natives_from_module!( + "permissioned_signer", + permissioned_signer::make_all(builder) + ); if inject_create_signer_for_gov_sim { add_natives_from_module!( diff --git a/aptos-move/framework/src/natives/permissioned_signer.rs b/aptos-move/framework/src/natives/permissioned_signer.rs new file mode 100644 index 0000000000000..321046d09f765 --- /dev/null +++ b/aptos-move/framework/src/natives/permissioned_signer.rs @@ -0,0 +1,103 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 +use aptos_native_interface::{ + safely_pop_arg, RawSafeNative, SafeNativeBuilder, SafeNativeContext, SafeNativeError, + SafeNativeResult, +}; +use move_vm_runtime::native_functions::NativeFunction; +use move_vm_types::{ + loaded_data::runtime_types::Type, + values::{SignerRef, Struct, Value}, +}; +use move_core_types::account_address::AccountAddress; +use smallvec::{smallvec, SmallVec}; +use std::collections::VecDeque; + +/*************************************************************************************************** + * native fun is_permissioned_signer + * + * Returns true if the signer passed in is a permissioned signer + * gas cost: base_cost + * + **************************************************************************************************/ +fn native_is_permissioned_signer( + _context: &mut SafeNativeContext, + _ty_args: Vec, + mut arguments: VecDeque, +) -> SafeNativeResult> { + debug_assert!(arguments.len() == 1); + + let s_arg = safely_pop_arg!(arguments, SignerRef); + + // context.charge()?; + let result = s_arg.is_permissioned()?; + + Ok(smallvec![Value::bool(result)]) +} + +/*************************************************************************************************** + * native fun permission_signer + * + * Returns the permission signer if the signer passed in is a permissioned signer + * gas cost: base_cost + * + **************************************************************************************************/ +fn native_permission_signer( + _context: &mut SafeNativeContext, + _ty_args: Vec, + mut arguments: VecDeque, +) -> SafeNativeResult> { + debug_assert!(arguments.len() == 1); + + let s_arg = safely_pop_arg!(arguments, SignerRef); + + // context.charge()?; + if !s_arg.is_permissioned()? { + return Err(SafeNativeError::Abort { abort_code: 3 }); + } + + Ok(smallvec![s_arg.permissioned_signer()?]) +} + +/*************************************************************************************************** + * native fun signer_from_permissioned + * + * Returns the permission signer from a master signer. + * gas cost: base_cost + * + **************************************************************************************************/ +fn native_signer_from_permissioned( + _context: &mut SafeNativeContext, + _ty_args: Vec, + mut arguments: VecDeque, +) -> SafeNativeResult> { + debug_assert!(arguments.len() == 2); + + let permission_addr = safely_pop_arg!(arguments, AccountAddress); + let master_addr = safely_pop_arg!(arguments, AccountAddress); + // context.charge()?; + + Ok(smallvec![Value::struct_(Struct::pack_variant(1, vec![Value::address(master_addr), Value::address(permission_addr)]))]) +} + +/*************************************************************************************************** + * module + * + **************************************************************************************************/ +pub fn make_all( + builder: &SafeNativeBuilder, +) -> impl Iterator + '_ { + let natives = [ + ( + "is_permissioned_signer", + native_is_permissioned_signer as RawSafeNative, + ), + ("permission_signer", native_permission_signer), + ( + "signer_from_permissioned_impl", + native_signer_from_permissioned, + ), + ]; + + builder.make_named_natives(natives) +} diff --git a/aptos-move/framework/src/natives/string_utils.rs b/aptos-move/framework/src/natives/string_utils.rs index 0a4c7c71583f8..3cb87c669b239 100644 --- a/aptos-move/framework/src/natives/string_utils.rs +++ b/aptos-move/framework/src/natives/string_utils.rs @@ -188,7 +188,7 @@ fn native_format_impl( let addr = if fix_enabled { val.value_as::()? .unpack()? - .next() + .nth(1) .unwrap() .value_as::()? } else { diff --git a/third_party/move/move-compiler-v2/src/plan_builder.rs b/third_party/move/move-compiler-v2/src/plan_builder.rs index 804ba329bd0a4..4906ca1cba5f3 100644 --- a/third_party/move/move-compiler-v2/src/plan_builder.rs +++ b/third_party/move/move-compiler-v2/src/plan_builder.rs @@ -162,9 +162,16 @@ fn build_test_info( let mut arguments = Vec::new(); for param in function.get_parameters_ref() { - let Parameter(var, _ty, var_loc) = ¶m; + let Parameter(var, ty, var_loc) = ¶m; match test_annotation_params.get(var) { + Some(MoveValue::Address(addr)) => arguments.push(match ty { + Type::Primitive(PrimitiveType::Signer) => MoveValue::Signer(*addr), + Type::Reference(_, inner) if **inner == Type::Primitive(PrimitiveType::Signer) => { + MoveValue::Signer(*addr) + }, + _ => MoveValue::Address(*addr), + }), Some(value) => arguments.push(value.clone()), None => { let missing_param_msg = "Missing test parameter assignment in test. Expected a \ diff --git a/third_party/move/move-compiler/src/unit_test/plan_builder.rs b/third_party/move/move-compiler/src/unit_test/plan_builder.rs index c53926366cbd9..56b3f5140fd74 100644 --- a/third_party/move/move-compiler/src/unit_test/plan_builder.rs +++ b/third_party/move/move-compiler/src/unit_test/plan_builder.rs @@ -8,6 +8,7 @@ use crate::{ expansion::ast::{ self as E, Address, Attribute, AttributeValue, ModuleAccess_, ModuleIdent, ModuleIdent_, }, + hlir::ast::{BaseType_, SingleType_}, parser::ast::ConstantName, shared::{ known_attributes::{AttributeKind, KnownAttribute, TestingAttribute}, @@ -159,10 +160,19 @@ fn build_test_info<'func>( let test_annotation_params = parse_test_attribute(context, test_attribute, 0); let mut arguments = Vec::new(); - for (var, _) in &function.signature.parameters { + for (var, ty) in &function.signature.parameters { match test_annotation_params.get(&var.value()) { - Some(value) => arguments.push(value.clone()), - None => { + Some(MoveValue::Address(addr)) => match &ty.value { + SingleType_::Base(ty) => arguments.push( + if ty == &BaseType_::address(ty.loc) { + MoveValue::Address(*addr) + } else { + MoveValue::Signer(*addr) + }, + ), + SingleType_::Ref(_, _) => arguments.push(MoveValue::Signer(*addr)), + }, + _ => { let missing_param_msg = "Missing test parameter assignment in test. Expected a \ parameter to be assigned in this attribute"; context.env.add_diag(diag!( diff --git a/third_party/move/move-core/types/src/unit_tests/value_test.rs b/third_party/move/move-core/types/src/unit_tests/value_test.rs index 5c3bb08841503..1519fc7961c57 100644 --- a/third_party/move/move-core/types/src/unit_tests/value_test.rs +++ b/third_party/move/move-core/types/src/unit_tests/value_test.rs @@ -137,3 +137,13 @@ fn nested_typed_struct_deserialization() { }) ); } + +#[test] +fn signer_deserialization() { + let v = MoveValue::Signer(AccountAddress::ZERO); + let bytes = v.simple_serialize().unwrap(); + assert_eq!( + MoveValue::simple_deserialize(&bytes, &crate::value::MoveTypeLayout::Signer).unwrap(), + v + ); +} diff --git a/third_party/move/move-core/types/src/value.rs b/third_party/move/move-core/types/src/value.rs index 49a4d4eada8df..7fb43bcae11ed 100644 --- a/third_party/move/move-core/types/src/value.rs +++ b/third_party/move/move-core/types/src/value.rs @@ -452,6 +452,13 @@ impl MoveStructLayout { }, } } + + pub fn signer() -> Self { + MoveStructLayout::RuntimeVariants(vec![vec![MoveTypeLayout::Address], vec![ + MoveTypeLayout::Address, + MoveTypeLayout::Address, + ]]) + } } impl<'d> serde::de::DeserializeSeed<'d> for &MoveTypeLayout { @@ -473,7 +480,13 @@ impl<'d> serde::de::DeserializeSeed<'d> for &MoveTypeLayout { AccountAddress::deserialize(deserializer).map(MoveValue::Address) }, MoveTypeLayout::Signer => { - AccountAddress::deserialize(deserializer).map(MoveValue::Signer) + let (_, fields) = MoveStructLayout::signer() + .deserialize(deserializer)? + .into_optional_variant_and_fields(); + Ok(MoveValue::Signer(match fields[0] { + MoveValue::Address(addr) => addr, + _ => return Err(D::Error::custom("signer deserialization error")), + })) }, MoveTypeLayout::Struct(ty) => Ok(MoveValue::Struct(ty.deserialize(deserializer)?)), MoveTypeLayout::Vector(layout) => Ok(MoveValue::Vector( @@ -678,7 +691,9 @@ impl serde::Serialize for MoveValue { MoveValue::U128(i) => serializer.serialize_u128(*i), MoveValue::U256(i) => i.serialize(serializer), MoveValue::Address(a) => a.serialize(serializer), - MoveValue::Signer(a) => a.serialize(serializer), + MoveValue::Signer(a) => { + MoveStruct::new_variant(0, vec![MoveValue::Address(*a)]).serialize(serializer) + }, MoveValue::Vector(v) => { let mut t = serializer.serialize_seq(Some(v.len()))?; for val in v { diff --git a/third_party/move/move-stdlib/src/natives/signer.rs b/third_party/move/move-stdlib/src/natives/signer.rs index 3a15343186a0c..5d4c0089f9b23 100644 --- a/third_party/move/move-stdlib/src/natives/signer.rs +++ b/third_party/move/move-stdlib/src/natives/signer.rs @@ -37,10 +37,10 @@ fn native_borrow_address( debug_assert!(arguments.len() == 1); let signer_reference = pop_arg!(arguments, SignerRef); + let out = signer_reference.borrow_signer()?; + println!("borrow_address: {:?}", out); - Ok(NativeResult::ok(gas_params.base, smallvec![ - signer_reference.borrow_signer()? - ])) + Ok(NativeResult::ok(gas_params.base, smallvec![out])) } pub fn make_native_borrow_address(gas_params: BorrowAddressGasParameters) -> NativeFunction { diff --git a/third_party/move/move-vm/runtime/src/interpreter.rs b/third_party/move/move-vm/runtime/src/interpreter.rs index 2baff79cc43df..08dda5d28ea79 100644 --- a/third_party/move/move-vm/runtime/src/interpreter.rs +++ b/third_party/move/move-vm/runtime/src/interpreter.rs @@ -2988,7 +2988,7 @@ impl Frame { let resource = interpreter.operand_stack.pop()?; let signer_reference = interpreter.operand_stack.pop_as::()?; let addr = signer_reference - .borrow_field(0)? + .borrow_field(1)? .value_as::()? .read_ref()? .value_as::()?; @@ -3008,7 +3008,7 @@ impl Frame { let resource = interpreter.operand_stack.pop()?; let signer_reference = interpreter.operand_stack.pop_as::()?; let addr = signer_reference - .borrow_field(0)? + .borrow_field(1)? .value_as::()? .read_ref()? .value_as::()?; diff --git a/third_party/move/move-vm/types/src/value_serde.rs b/third_party/move/move-vm/types/src/value_serde.rs index 5f17bfe9b3848..c7f2c74b0e4d9 100644 --- a/third_party/move/move-vm/types/src/value_serde.rs +++ b/third_party/move/move-vm/types/src/value_serde.rs @@ -104,6 +104,7 @@ impl CustomSerializer for RelaxedCustomSerDe { custom_serializer: None::<&RelaxedCustomSerDe>, layout, value: &value.0, + legacy_signer: false, } .serialize(serializer) } @@ -134,6 +135,7 @@ pub fn serialize_and_allow_delayed_values( custom_serializer: Some(&native_serializer), layout, value: &value.0, + legacy_signer: false, }; bcs::to_bytes(&value) .ok() @@ -163,6 +165,7 @@ pub fn serialized_size_allowing_delayed_values( custom_serializer: Some(&native_serializer), layout, value: &value.0, + legacy_signer: true, }; bcs::serialized_size(&value).map_err(|e| { PartialVMError::new(StatusCode::VALUE_SERIALIZATION_ERROR).with_message(format!( @@ -232,6 +235,7 @@ impl<'a, I: From + ExtractWidth + ExtractUniqueIndex> CustomSerializer custom_serializer: None::<&RelaxedCustomSerDe>, layout, value: &value.0, + legacy_signer: false, } .serialize(serializer) } @@ -291,6 +295,7 @@ pub fn serialize_and_replace_ids_with_values + ExtractWidth + Extra custom_serializer: Some(&custom_serializer), layout, value: &value.0, + legacy_signer: false, }; bcs::to_bytes(&value).ok().filter(|_| { // Should never happen, it should always fail first in serialize_and_allow_delayed_values diff --git a/third_party/move/move-vm/types/src/values/serialization_tests.rs b/third_party/move/move-vm/types/src/values/serialization_tests.rs index 6085a50183e2c..7b4aef0f14708 100644 --- a/third_party/move/move-vm/types/src/values/serialization_tests.rs +++ b/third_party/move/move-vm/types/src/values/serialization_tests.rs @@ -244,3 +244,17 @@ fn test_serialized_size() { assert_err!(serialized_size_allowing_delayed_values(&value, &layout)); } } + +#[test] +fn signer_round_trip_vm_value() { + let v = MoveValue::Signer(AccountAddress::ZERO); + let bytes = v.simple_serialize().unwrap(); + let vm_value = Value::simple_deserialize(&bytes, &MoveTypeLayout::Signer).unwrap(); + let vm_bytes = serialize_and_allow_delayed_values(&vm_value, &MoveTypeLayout::Signer) + .unwrap() + .unwrap(); + assert_eq!( + v, + MoveValue::simple_deserialize(&vm_bytes, &MoveTypeLayout::Signer).unwrap() + ) +} diff --git a/third_party/move/move-vm/types/src/values/values_impl.rs b/third_party/move/move-vm/types/src/values/values_impl.rs index 006c16d6b9dd1..f7c2ec6214ce1 100644 --- a/third_party/move/move-vm/types/src/values/values_impl.rs +++ b/third_party/move/move-vm/types/src/values/values_impl.rs @@ -283,7 +283,10 @@ impl Container { } fn signer(x: AccountAddress) -> Self { - Container::Struct(Rc::new(RefCell::new(vec![ValueImpl::Address(x)]))) + Container::Struct(Rc::new(RefCell::new(vec![ + ValueImpl::U16(0), + ValueImpl::Address(x), + ]))) } } @@ -1058,7 +1061,31 @@ impl Locals { impl SignerRef { pub fn borrow_signer(&self) -> PartialVMResult { - Ok(Value(self.0.borrow_elem(0)?)) + Ok(Value(self.0.borrow_elem(1)?)) + } + + pub fn is_permissioned(&self) -> PartialVMResult { + match &self.0 { + ContainerRef::Local(Container::Struct(s)) => { + Ok(*s.borrow()[0].as_value_ref::()? == 1) + }, + _ => Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message(format!("unexpected signer value: {:?}", self)), + ), + } + } + + pub fn permissioned_signer(&self) -> PartialVMResult { + match &self.0 { + ContainerRef::Local(Container::Struct(s)) => Ok(Value::signer( + *s.borrow()[1].as_value_ref::()?, + )), + _ => Err( + PartialVMError::new(StatusCode::UNKNOWN_INVARIANT_VIOLATION_ERROR) + .with_message(format!("unexpected signer value: {:?}", self)), + ), + } } } @@ -3123,6 +3150,7 @@ impl Value { custom_serializer: None::<&RelaxedCustomSerDe>, layout, value: &self.0, + legacy_signer: false, }) .ok() } @@ -3142,6 +3170,7 @@ impl Struct { custom_serializer: None::<&RelaxedCustomSerDe>, layout, value: &self.fields, + legacy_signer: false, }) .ok() } @@ -3156,6 +3185,7 @@ pub(crate) struct SerializationReadyValue<'c, 'l, 'v, L, V, C> { pub(crate) layout: &'l L, // Value to serialize. pub(crate) value: &'v V, + pub(crate) legacy_signer: bool, } fn invariant_violation(message: String) -> S::Error { @@ -3187,6 +3217,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize custom_serializer: self.custom_serializer, layout: struct_layout, value: &*r.borrow(), + legacy_signer: self.legacy_signer, }) .serialize(serializer) }, @@ -3211,6 +3242,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize custom_serializer: self.custom_serializer, layout, value, + legacy_signer: self.legacy_signer, })?; } t.end() @@ -3224,19 +3256,25 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize // Signer. (L::Signer, ValueImpl::Container(Container::Struct(r))) => { - let v = r.borrow(); - if v.len() != 1 { - return Err(invariant_violation::(format!( - "cannot serialize container as a signer -- expected 1 field got {}", - v.len() - ))); + if self.legacy_signer { + r.borrow()[1] + .as_value_ref::() + .map_err(|_| { + invariant_violation::(format!( + "cannot serialize container {:?} as {:?}", + self.value, self.layout + )) + })? + .serialize(serializer) + } else { + (SerializationReadyValue { + custom_serializer: self.custom_serializer, + layout: &MoveStructLayout::signer(), + value: &*r.borrow(), + legacy_signer: self.legacy_signer, + }) + .serialize(serializer) } - (SerializationReadyValue { - custom_serializer: self.custom_serializer, - layout: &L::Address, - value: &v[0], - }) - .serialize(serializer) }, // Delayed values. For their serialization, we must have custom @@ -3297,6 +3335,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize custom_serializer: self.custom_serializer, layout: &variant_layouts[0], value: &values[0], + legacy_signer: self.legacy_signer, }, ), _ => { @@ -3311,6 +3350,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize custom_serializer: self.custom_serializer, layout, value, + legacy_signer: self.legacy_signer, })? } t.end() @@ -3330,6 +3370,7 @@ impl<'c, 'l, 'v, C: CustomSerializer> serde::Serialize custom_serializer: self.custom_serializer, layout: field_layout, value, + legacy_signer: self.legacy_signer, })?; } t.end() @@ -3368,7 +3409,13 @@ impl<'d, 'c, C: CustomDeserializer> serde::de::DeserializeSeed<'d> L::U128 => u128::deserialize(deserializer).map(Value::u128), L::U256 => u256::U256::deserialize(deserializer).map(Value::u256), L::Address => AccountAddress::deserialize(deserializer).map(Value::address), - L::Signer => AccountAddress::deserialize(deserializer).map(Value::signer), + L::Signer => { + let seed = DeserializationSeed { + custom_deserializer: self.custom_deserializer, + layout: &MoveStructLayout::signer(), + }; + Ok(Value::struct_(seed.deserialize(deserializer)?)) + }, // Structs. L::Struct(struct_layout) => {