Skip to content

Commit 58271fd

Browse files
authored
[compiler v2] Resource access control (read-write sets) (aptos-labs#10480)
* [compiler v2] Resource access control (read-write sets) This is an e2e implementation of resource access control for Move, with most parts in place: - Replaces the acquires syntax in a downwards-compatible way - The extended syntax is only available in compiler v2 - One can now specify `acquires`, `reads`, and `writes` - One can specify the address of a resource in dependency of parameters - Multiple levels of wildcards are allowed, e.g. `acquires *(object::address_of(param))` specifies that all resources at the given address are read or written. - Implements parsing->expansion->move model->file format generator - Extends `file_format::FunctionHandle` to carry the new information, introducing bytecode version v7. v7 became the new experimental version only available in test code for now. - TODO: dynamic runtime checking of resource access. Static analysis is also on the horizon, but not needed for an MVP of this feature. - TODO: bytecode verification of access specifiers An AIP for this new feature will be filed soon. As an example, here is some extract from the tests: ```move module 0x42::m { struct S has store {} struct R has store {} struct T has store {} struct G<T> has store {} fun f1() acquires S { } fun f2() reads S { } fun f3() writes S { } fun f4() acquires S(*) { } fun f_multiple() acquires R reads R writes T, S reads G<u64> { } fun f5() acquires 0x42::*::* { } fun f6() acquires 0x42::m::R { } fun f7() acquires *(*) { } fun f8() acquires *(0x42) { } fun f9(a: address) acquires *(a) { } fun f10(x: u64) acquires *(make_up_address(x)) { } fun make_up_address(x: u64): address { @0x42 } } ``` * Addressing reviewer comments * Addressing reviewer comments #2 * Addressing reviewer comments aptos-labs#3 * Addressing reviewer comments aptos-labs#4 * Reviewer comments aptos-labs#5
1 parent b55ad61 commit 58271fd

File tree

80 files changed

+1915
-180
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

80 files changed

+1915
-180
lines changed

aptos-move/aptos-release-builder/src/components/feature_flags.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ pub enum FeatureFlag {
9090
ConcurrentAssets,
9191
LimitMaxIdentifierLength,
9292
OperatorBeneficiaryChange,
93+
VMBinaryFormatV7,
9394
}
9495

9596
fn generate_features_blob(writer: &CodeWriter, data: &[u64]) {
@@ -185,6 +186,7 @@ impl From<FeatureFlag> for AptosFeatureFlag {
185186
},
186187
FeatureFlag::AptosStdChainIdNatives => AptosFeatureFlag::APTOS_STD_CHAIN_ID_NATIVES,
187188
FeatureFlag::VMBinaryFormatV6 => AptosFeatureFlag::VM_BINARY_FORMAT_V6,
189+
FeatureFlag::VMBinaryFormatV7 => AptosFeatureFlag::VM_BINARY_FORMAT_V7,
188190
FeatureFlag::MultiEd25519PkValidateV2Natives => {
189191
AptosFeatureFlag::MULTI_ED25519_PK_VALIDATE_V2_NATIVES
190192
},
@@ -252,6 +254,7 @@ impl From<AptosFeatureFlag> for FeatureFlag {
252254
},
253255
AptosFeatureFlag::APTOS_STD_CHAIN_ID_NATIVES => FeatureFlag::AptosStdChainIdNatives,
254256
AptosFeatureFlag::VM_BINARY_FORMAT_V6 => FeatureFlag::VMBinaryFormatV6,
257+
AptosFeatureFlag::VM_BINARY_FORMAT_V7 => FeatureFlag::VMBinaryFormatV7,
255258
AptosFeatureFlag::MULTI_ED25519_PK_VALIDATE_V2_NATIVES => {
256259
FeatureFlag::MultiEd25519PkValidateV2Natives
257260
},

aptos-move/aptos-vm/src/aptos_vm.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ use crate::{
99
counters::*,
1010
data_cache::{AsMoveResolver, StorageAdapter},
1111
errors::expect_only_successful_execution,
12-
move_vm_ext::{AptosMoveResolver, RespawnedSession, SessionExt, SessionId},
12+
move_vm_ext::{
13+
get_max_binary_format_version, AptosMoveResolver, RespawnedSession, SessionExt, SessionId,
14+
},
1315
sharded_block_executor::{executor_client::ExecutorClient, ShardedBlockExecutor},
1416
system_module_names::*,
1517
transaction_metadata::TransactionMetadata,
@@ -855,15 +857,7 @@ impl AptosVM {
855857

856858
/// Deserialize a module bundle.
857859
fn deserialize_module_bundle(&self, modules: &ModuleBundle) -> VMResult<Vec<CompiledModule>> {
858-
let max_version = if self
859-
.0
860-
.get_features()
861-
.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6)
862-
{
863-
6
864-
} else {
865-
5
866-
};
860+
let max_version = get_max_binary_format_version(self.0.get_features(), None);
867861
let max_identifier_size = if self
868862
.0
869863
.get_features()

aptos-move/aptos-vm/src/data_cache.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl<'e, E: ExecutorView> StorageAdapter<'e, E> {
9191
features: &Features,
9292
maybe_resource_group_view: Option<&'e dyn ResourceGroupView>,
9393
) -> Self {
94-
let max_binary_version = get_max_binary_format_version(features, gas_feature_version);
94+
let max_binary_version = get_max_binary_format_version(features, Some(gas_feature_version));
9595
let max_identifier_size = get_max_identifier_size(features);
9696
let resource_group_adapter = ResourceGroupAdapter::new(
9797
maybe_resource_group_view,
@@ -332,7 +332,8 @@ impl<S: StateView> AsMoveResolver<S> for S {
332332
let config_view = ConfigAdapter(self);
333333
let (_, gas_feature_version) = gas_config(&config_view);
334334
let features = Features::fetch_config(&config_view).unwrap_or_default();
335-
let max_binary_version = get_max_binary_format_version(&features, gas_feature_version);
335+
let max_binary_version =
336+
get_max_binary_format_version(&features, Some(gas_feature_version));
336337
let resource_group_adapter = ResourceGroupAdapter::new(None, self, gas_feature_version);
337338
let max_identifier_size = get_max_identifier_size(&features);
338339
StorageAdapter::new(

aptos-move/aptos-vm/src/move_vm_ext/vm.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use aptos_types::on_chain_config::{FeatureFlag, Features, TimedFeatureFlag, Time
1818
use move_binary_format::{
1919
deserializer::DeserializerConfig,
2020
errors::VMResult,
21+
file_format_common,
2122
file_format_common::{IDENTIFIER_SIZE_MAX, LEGACY_IDENTIFIER_SIZE_MAX},
2223
};
2324
use move_bytecode_verifier::VerifierConfig;
@@ -32,11 +33,21 @@ pub struct MoveVmExt {
3233
features: Arc<Features>,
3334
}
3435

35-
pub fn get_max_binary_format_version(features: &Features, gas_feature_version: u64) -> u32 {
36-
if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6) && gas_feature_version >= 5 {
37-
6
36+
pub fn get_max_binary_format_version(
37+
features: &Features,
38+
gas_feature_version_opt: Option<u64>,
39+
) -> u32 {
40+
// For historical reasons, we support still < gas version 5, but if a new caller don't specify
41+
// the gas version, we default to 5, which was introduced in late '22.
42+
let gas_feature_version = gas_feature_version_opt.unwrap_or(5);
43+
if gas_feature_version < 5 {
44+
file_format_common::VERSION_5
45+
} else if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V7) {
46+
file_format_common::VERSION_7
47+
} else if features.is_enabled(FeatureFlag::VM_BINARY_FORMAT_V6) {
48+
file_format_common::VERSION_6
3849
} else {
39-
5
50+
file_format_common::VERSION_5
4051
}
4152
}
4253

@@ -66,7 +77,7 @@ impl MoveVmExt {
6677
// Therefore it depends on a new version of the gas schedule and cannot be allowed if
6778
// the gas schedule hasn't been updated yet.
6879
let max_binary_format_version =
69-
get_max_binary_format_version(&features, gas_feature_version);
80+
get_max_binary_format_version(&features, Some(gas_feature_version));
7081

7182
let max_identifier_size = get_max_identifier_size(&features);
7283

aptos-move/e2e-move-tests/src/tests/access_path_test.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ fn access_path_panic() {
5252
parameters: SignatureIndex(0),
5353
return_: SignatureIndex(0),
5454
type_parameters: vec![],
55+
access_specifiers: None,
5556
}],
5657
field_handles: vec![],
5758
friend_decls: vec![],

aptos-move/e2e-testsuite/src/tests/scripts.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ fn script_none_existing_module_dep() {
9999
parameters: SignatureIndex(0),
100100
return_: SignatureIndex(0),
101101
type_parameters: vec![],
102+
access_specifiers: None,
102103
};
103104
script.function_handles.push(fun_handle);
104105

@@ -177,6 +178,7 @@ fn script_non_existing_function_dep() {
177178
parameters: SignatureIndex(0),
178179
return_: SignatureIndex(0),
179180
type_parameters: vec![],
181+
access_specifiers: None,
180182
};
181183
script.function_handles.push(fun_handle);
182184

@@ -257,6 +259,7 @@ fn script_bad_sig_function_dep() {
257259
parameters: SignatureIndex(0),
258260
return_: SignatureIndex(0),
259261
type_parameters: vec![],
262+
access_specifiers: None,
260263
};
261264
script.function_handles.push(fun_handle);
262265

@@ -465,6 +468,7 @@ fn forbid_script_emitting_events() {
465468
type_parameters: vec![
466469
AbilitySet::singleton(Ability::Store) | AbilitySet::singleton(Ability::Drop),
467470
],
471+
access_specifiers: None,
468472
});
469473
script.module_handles.push(ModuleHandle {
470474
address: AddressIdentifierIndex(0),

testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_code_unit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ fuzz_target!(|code_unit: CodeUnit| {
3232
parameters: SignatureIndex(0),
3333
return_: SignatureIndex(1),
3434
type_parameters: vec![],
35+
access_specifiers: None,
3536
};
3637

3738
module.function_handles.push(fun_handle);

testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_mixed.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ fuzz_target!(|mix: Mixed| {
4141
parameters: SignatureIndex(0),
4242
return_: SignatureIndex(1),
4343
type_parameters: vec![],
44+
access_specifiers: None,
4445
};
4546

4647
module.function_handles.push(fun_handle);

third_party/move/move-binary-format/serializer-tests/tests/serializer_tests.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44

55
use move_binary_format::{
66
access::ModuleAccess,
7+
deserializer::DeserializerConfig,
78
file_format::{
89
empty_module, AbilitySet, CompiledModule, FunctionHandle, IdentifierIndex, Signature,
910
SignatureIndex, SignatureToken,
1011
},
12+
file_format_common::{IDENTIFIER_SIZE_MAX, VERSION_MAX},
1113
};
1214
use move_core_types::identifier::Identifier;
1315
use proptest::prelude::*;
@@ -16,10 +18,13 @@ proptest! {
1618
#[test]
1719
fn serializer_roundtrip(module in CompiledModule::valid_strategy(20)) {
1820
let mut serialized = Vec::with_capacity(2048);
19-
module.serialize(&mut serialized).expect("serialization should work");
21+
module.serialize_for_version(Some(VERSION_MAX), &mut serialized).expect("serialization should work");
2022

21-
let deserialized_module = CompiledModule::deserialize(&serialized)
22-
.expect("deserialization should work");
23+
24+
let deserialized_module = CompiledModule::deserialize_with_config(
25+
&serialized,
26+
&DeserializerConfig::new(VERSION_MAX, IDENTIFIER_SIZE_MAX)
27+
).expect("deserialization should work");
2328

2429
prop_assert_eq!(module, deserialized_module);
2530
}
@@ -34,7 +39,7 @@ proptest! {
3439
#[test]
3540
fn garbage_inputs(module in any_with::<CompiledModule>(16)) {
3641
let mut serialized = Vec::with_capacity(65536);
37-
module.serialize(&mut serialized).expect("serialization should work");
42+
module.serialize_for_version(Some(VERSION_MAX), &mut serialized).expect("serialization should work");
3843

3944
let deserialized_module = CompiledModule::deserialize_no_check_bounds(&serialized)
4045
.expect("deserialization should work");
@@ -66,14 +71,18 @@ fn simple_generic_module_round_trip() {
6671
parameters: sig_t1_idx,
6772
return_: sig_unit_idx,
6873
type_parameters: vec![AbilitySet::EMPTY],
74+
access_specifiers: None,
6975
});
7076

7177
let mut serialized = Vec::with_capacity(2048);
72-
m.serialize(&mut serialized)
78+
m.serialize_for_version(Some(VERSION_MAX), &mut serialized)
7379
.expect("serialization should work");
7480

75-
let deserialized_m =
76-
CompiledModule::deserialize(&serialized).expect("deserialization should work");
81+
let deserialized_m = CompiledModule::deserialize_with_config(
82+
&serialized,
83+
&DeserializerConfig::new(VERSION_MAX, IDENTIFIER_SIZE_MAX),
84+
)
85+
.expect("deserialization should work");
7786

7887
assert_eq!(m, deserialized_m);
7988
}

third_party/move/move-binary-format/src/binary_views.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,16 @@ impl<'a> BinaryIndexedView<'a> {
299299
)
300300
}
301301

302+
pub fn safe_module_id_for_handle(&self, module_handle: &ModuleHandle) -> Option<ModuleId> {
303+
self.address_identifiers()
304+
.get(module_handle.address.0 as usize)
305+
.and_then(|a| {
306+
self.identifiers()
307+
.get(module_handle.name.0 as usize)
308+
.map(|id| ModuleId::new(*a, id.to_owned()))
309+
})
310+
}
311+
302312
pub fn self_id(&self) -> Option<ModuleId> {
303313
match self {
304314
BinaryIndexedView::Module(m) => Some(m.self_id()),

0 commit comments

Comments
 (0)