[move-vm][closures] Refactor: move abilities and closure mask into co…

…re types (#15668) * [move-vm][closures] Refactor: move abilities and closure mask into core types [PR 2/n vm closures] The type `AbilitySet` is required in `MoveTypeLayout`, and the type `ClosureMask` in `MoveValue`. This PRs moves those both types from file_format into the core types crate. * Document ClosureMask::MAX_ARGUMENTS
aptos-labs · Jan 25, 2025 · 84909ca · 84909ca
1 parent 9a06492
commit 84909ca
Show file tree

Hide file tree

Showing 76 changed files with 752 additions and 622 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/api/types/src/move_types.rs b/api/types/src/move_types.rs
@@ -8,11 +8,10 @@ use aptos_resource_viewer::{AnnotatedMoveStruct, AnnotatedMoveValue};
 use aptos_types::{account_config::CORE_CODE_ADDRESS, event::EventKey, transaction::Module};
 use move_binary_format::{
     access::ModuleAccess,
-    file_format::{
-        Ability, AbilitySet, CompiledModule, CompiledScript, StructTypeParameter, Visibility,
-    },
+    file_format::{CompiledModule, CompiledScript, StructTypeParameter, Visibility},
 };
 use move_core_types::{
+    ability::{Ability, AbilitySet},
     account_address::AccountAddress,
     identifier::Identifier,
     language_storage::{ModuleId, StructTag, TypeTag},
@@ -1225,8 +1224,8 @@ pub fn verify_identifier(identifier: &str) -> anyhow::Result<()> {
 mod tests {
     use super::*;
     use aptos_types::account_address::AccountAddress;
-    use move_binary_format::file_format::AbilitySet;
     use move_core_types::{
+        ability::AbilitySet,
         identifier::Identifier,
         language_storage::{StructTag, TypeTag},
     };

diff --git a/aptos-move/e2e-move-tests/src/tests/access_path_test.rs b/aptos-move/e2e-move-tests/src/tests/access_path_test.rs
@@ -7,7 +7,7 @@ use aptos_types::{
 };
 use move_binary_format::{
     file_format::{
-        AbilitySet, AddressIdentifierIndex, Bytecode::*, CodeUnit, Constant, ConstantPoolIndex,
+        AddressIdentifierIndex, Bytecode::*, CodeUnit, Constant, ConstantPoolIndex,
         FieldDefinition, FunctionDefinition, FunctionHandle, FunctionHandleIndex, IdentifierIndex,
         ModuleHandle, ModuleHandleIndex, Signature, SignatureIndex, SignatureToken,
         StructDefInstantiation, StructDefInstantiationIndex, StructDefinition,
@@ -16,7 +16,7 @@ use move_binary_format::{
     },
     CompiledModule,
 };
-use move_core_types::{identifier::Identifier, vm_status::StatusCode};
+use move_core_types::{ability::AbilitySet, identifier::Identifier, vm_status::StatusCode};
 
 #[test]
 fn access_path_panic() {

diff --git a/aptos-move/e2e-testsuite/src/tests/scripts.rs b/aptos-move/e2e-testsuite/src/tests/scripts.rs
@@ -9,11 +9,12 @@ use aptos_types::{
     transaction::{ExecutionStatus, Script, TransactionStatus},
 };
 use move_binary_format::file_format::{
-    empty_script, Ability, AbilitySet, AddressIdentifierIndex, Bytecode, FunctionHandle,
-    FunctionHandleIndex, FunctionInstantiation, FunctionInstantiationIndex, IdentifierIndex,
-    ModuleHandle, ModuleHandleIndex, Signature, SignatureIndex, SignatureToken,
+    empty_script, AddressIdentifierIndex, Bytecode, FunctionHandle, FunctionHandleIndex,
+    FunctionInstantiation, FunctionInstantiationIndex, IdentifierIndex, ModuleHandle,
+    ModuleHandleIndex, Signature, SignatureIndex, SignatureToken,
 };
 use move_core_types::{
+    ability::{Ability, AbilitySet},
     identifier::Identifier,
     language_storage::{StructTag, TypeTag},
     vm_status::{StatusCode, StatusCode::LINKER_ERROR},

diff --git a/aptos-move/framework/src/extended_checks.rs b/aptos-move/framework/src/extended_checks.rs
@@ -2,10 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{KnownAttribute, RandomnessAnnotation, RuntimeModuleMetadataV1};
-use move_binary_format::file_format::{Ability, AbilitySet, Visibility};
+use move_binary_format::file_format::Visibility;
 use move_cli::base::test_validation;
 use move_compiler::shared::known_attributes;
 use move_core_types::{
+    ability::{Ability, AbilitySet},
     account_address::AccountAddress,
     errmap::{ErrorDescription, ErrorMapping},
     identifier::Identifier,

diff --git a/aptos-move/framework/src/module_metadata.rs b/aptos-move/framework/src/module_metadata.rs
@@ -11,12 +11,13 @@ use lru::LruCache;
 use move_binary_format::{
     access::ModuleAccess,
     file_format::{
-        Ability, AbilitySet, CompiledScript, FunctionDefinition, FunctionHandle, IdentifierIndex,
-        SignatureToken, StructDefinition, StructFieldInformation, StructHandle, TableIndex,
+        CompiledScript, FunctionDefinition, FunctionHandle, IdentifierIndex, SignatureToken,
+        StructDefinition, StructFieldInformation, StructHandle, TableIndex,
     },
     CompiledModule,
 };
 use move_core_types::{
+    ability::{Ability, AbilitySet},
     errmap::ErrorDescription,
     identifier::{IdentStr, Identifier},
     language_storage::{ModuleId, StructTag},

diff --git a/ecosystem/indexer-grpc/indexer-grpc-fullnode/Cargo.toml b/ecosystem/indexer-grpc/indexer-grpc-fullnode/Cargo.toml
@@ -43,9 +43,7 @@ aptos-runtimes = { workspace = true }
 aptos-storage-interface = { workspace = true }
 aptos-types = { workspace = true }
 
-move-binary-format = { workspace = true }
 move-core-types = { workspace = true }
-move-package = { workspace = true }
 
 [dev-dependencies]
 goldenfile = { workspace = true }

diff --git a/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/convert.rs b/ecosystem/indexer-grpc/indexer-grpc-fullnode/src/convert.rs
@@ -26,7 +26,7 @@ use aptos_protos::{
 };
 use aptos_types::jwks::jwk::JWK;
 use hex;
-use move_binary_format::file_format::Ability;
+use move_core_types::ability::Ability;
 use std::time::Duration;
 
 pub fn convert_move_module_id(move_module_id: &MoveModuleId) -> transaction::MoveModuleId {

diff --git a/testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_code_unit.rs b/testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_code_unit.rs
@@ -5,14 +5,13 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
 use move_binary_format::file_format::{
-    empty_module, AbilitySet, CodeUnit, Constant, FieldDefinition, FunctionDefinition,
-    FunctionHandle, FunctionHandleIndex, IdentifierIndex, ModuleHandleIndex, Signature,
-    SignatureIndex,
+    empty_module, CodeUnit, Constant, FieldDefinition, FunctionDefinition, FunctionHandle,
+    FunctionHandleIndex, IdentifierIndex, ModuleHandleIndex, Signature, SignatureIndex,
     SignatureToken::{Address, Bool, U128, U64},
     StructDefinition, StructFieldInformation, StructHandle, StructHandleIndex, TypeSignature,
     Visibility,
 };
-use move_core_types::{account_address::AccountAddress, ident_str};
+use move_core_types::{ability::AbilitySet, account_address::AccountAddress, ident_str};
 use utils::helpers::compiled_module_serde;
 mod utils;
 

diff --git a/testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_mixed.rs b/testsuite/fuzzer/fuzz/fuzz_targets/move/bytecode_verifier_mixed.rs
@@ -6,14 +6,14 @@
 use arbitrary::Arbitrary;
 use libfuzzer_sys::fuzz_target;
 use move_binary_format::file_format::{
-    empty_module, AbilitySet, Bytecode, CodeUnit, Constant, FieldDefinition, FunctionDefinition,
+    empty_module, Bytecode, CodeUnit, Constant, FieldDefinition, FunctionDefinition,
     FunctionHandle, FunctionHandleIndex, IdentifierIndex, ModuleHandleIndex, Signature,
     SignatureIndex, SignatureToken,
     SignatureToken::{Address, Bool},
     StructDefinition, StructFieldInformation, StructHandle, StructHandleIndex, TypeSignature,
     Visibility,
 };
-use move_core_types::{account_address::AccountAddress, ident_str};
+use move_core_types::{ability::AbilitySet, account_address::AccountAddress, ident_str};
 use utils::helpers::compiled_module_serde;
 mod utils;
 

diff --git a/third_party/move/move-binary-format/serializer-tests/tests/serializer_tests.rs b/third_party/move/move-binary-format/serializer-tests/tests/serializer_tests.rs
@@ -6,12 +6,12 @@ use move_binary_format::{
     access::ModuleAccess,
     deserializer::DeserializerConfig,
     file_format::{
-        empty_module, AbilitySet, CompiledModule, FunctionHandle, IdentifierIndex, Signature,
-        SignatureIndex, SignatureToken,
+        empty_module, CompiledModule, FunctionHandle, IdentifierIndex, Signature, SignatureIndex,
+        SignatureToken,
     },
     file_format_common::{IDENTIFIER_SIZE_MAX, VERSION_MAX},
 };
-use move_core_types::identifier::Identifier;
+use move_core_types::{ability::AbilitySet, identifier::Identifier};
 use proptest::prelude::*;
 
 proptest! {

diff --git a/third_party/move/move-binary-format/src/binary_views.rs b/third_party/move/move-binary-format/src/binary_views.rs
@@ -7,19 +7,20 @@ use crate::{
     control_flow_graph::VMControlFlowGraph,
     errors::{PartialVMError, PartialVMResult},
     file_format::{
-        AbilitySet, AddressIdentifierIndex, CodeUnit, CompiledScript, Constant, ConstantPoolIndex,
-        FieldHandle, FieldHandleIndex, FieldInstantiation, FieldInstantiationIndex,
-        FunctionDefinition, FunctionDefinitionIndex, FunctionHandle, FunctionHandleIndex,
-        FunctionInstantiation, FunctionInstantiationIndex, IdentifierIndex, ModuleHandle,
-        ModuleHandleIndex, Signature, SignatureIndex, SignatureToken, StructDefInstantiation,
-        StructDefInstantiationIndex, StructDefinition, StructDefinitionIndex, StructHandle,
-        StructHandleIndex, StructVariantHandle, StructVariantHandleIndex,
-        StructVariantInstantiation, StructVariantInstantiationIndex, VariantFieldHandle,
-        VariantFieldHandleIndex, VariantFieldInstantiation, VariantFieldInstantiationIndex,
+        AddressIdentifierIndex, CodeUnit, CompiledScript, Constant, ConstantPoolIndex, FieldHandle,
+        FieldHandleIndex, FieldInstantiation, FieldInstantiationIndex, FunctionDefinition,
+        FunctionDefinitionIndex, FunctionHandle, FunctionHandleIndex, FunctionInstantiation,
+        FunctionInstantiationIndex, IdentifierIndex, ModuleHandle, ModuleHandleIndex, Signature,
+        SignatureIndex, SignatureToken, StructDefInstantiation, StructDefInstantiationIndex,
+        StructDefinition, StructDefinitionIndex, StructHandle, StructHandleIndex,
+        StructVariantHandle, StructVariantHandleIndex, StructVariantInstantiation,
+        StructVariantInstantiationIndex, VariantFieldHandle, VariantFieldHandleIndex,
+        VariantFieldInstantiation, VariantFieldInstantiationIndex,
     },
     CompiledModule,
 };
 use move_core_types::{
+    ability::AbilitySet,
     account_address::AccountAddress,
     identifier::{IdentStr, Identifier},
     language_storage::ModuleId,
@@ -342,7 +343,11 @@ impl<'a> BinaryIndexedView<'a> {
             TypeParameter(idx) => Ok(constraints[*idx as usize]),
             Vector(ty) => AbilitySet::polymorphic_abilities(AbilitySet::VECTOR, vec![false], vec![
                 self.abilities(ty, constraints)?,
-            ]),
+            ])
+            .map_err(|e| {
+                PartialVMError::new(StatusCode::VERIFIER_INVARIANT_VIOLATION)
+                    .with_message(e.to_string())
+            }),
             Function(_, _, abilities) => Ok(*abilities),
             Struct(idx) => {
                 let sh = self.struct_handle_at(*idx);
@@ -360,6 +365,10 @@ impl<'a> BinaryIndexedView<'a> {
                     sh.type_parameters.iter().map(|param| param.is_phantom),
                     type_arguments,
                 )
+                .map_err(|e| {
+                    PartialVMError::new(StatusCode::VERIFIER_INVARIANT_VIOLATION)
+                        .with_message(e.to_string())
+                })
             },
         }
     }

diff --git a/third_party/move/move-binary-format/src/check_bounds.rs b/third_party/move/move-binary-format/src/check_bounds.rs
@@ -9,18 +9,17 @@ use crate::{
         PartialVMError, PartialVMResult,
     },
     file_format::{
-        AbilitySet, Bytecode, CodeOffset, CodeUnit, CompiledModule, CompiledScript, Constant,
-        FieldDefinition, FieldHandle, FieldInstantiation, FunctionDefinition,
-        FunctionDefinitionIndex, FunctionHandle, FunctionInstantiation, LocalIndex, ModuleHandle,
-        Signature, SignatureIndex, SignatureToken, StructDefInstantiation, StructDefinition,
-        StructFieldInformation, StructHandle, StructVariantHandle, StructVariantInstantiation,
-        TableIndex, TypeParameterIndex, VariantFieldHandle, VariantFieldInstantiation,
-        VariantIndex,
+        Bytecode, CodeOffset, CodeUnit, CompiledModule, CompiledScript, Constant, FieldDefinition,
+        FieldHandle, FieldInstantiation, FunctionDefinition, FunctionDefinitionIndex,
+        FunctionHandle, FunctionInstantiation, LocalIndex, ModuleHandle, Signature, SignatureIndex,
+        SignatureToken, StructDefInstantiation, StructDefinition, StructFieldInformation,
+        StructHandle, StructVariantHandle, StructVariantInstantiation, TableIndex,
+        TypeParameterIndex, VariantFieldHandle, VariantFieldInstantiation, VariantIndex,
     },
     internals::ModuleIndex,
     IndexKind,
 };
-use move_core_types::vm_status::StatusCode;
+use move_core_types::{ability::AbilitySet, vm_status::StatusCode};
 use std::{
     cell::RefCell,
     collections::{btree_map, BTreeMap},

diff --git a/third_party/move/move-binary-format/src/compatibility.rs b/third_party/move/move-binary-format/src/compatibility.rs
@@ -6,16 +6,15 @@ use crate::{
     access::ModuleAccess,
     errors::{PartialVMError, PartialVMResult},
     file_format::{
-        AbilitySet, Signature, SignatureToken, StructHandleIndex, StructTypeParameter,
-        VariantIndex, Visibility,
+        Signature, SignatureToken, StructHandleIndex, StructTypeParameter, VariantIndex, Visibility,
     },
     file_format_common::VERSION_5,
     views::{
         FieldDefinitionView, ModuleView, StructDefinitionView, StructHandleView, ViewInternals,
     },
     CompiledModule,
 };
-use move_core_types::vm_status::StatusCode;
+use move_core_types::{ability::AbilitySet, vm_status::StatusCode};
 use std::collections::BTreeSet;
 
 /// The result of a linking and layout compatibility check. Here is what the different combinations. NOTE that if `check_struct_layout` is false, type safety over a series of upgrades cannot be guaranteed.

diff --git a/third_party/move/move-binary-format/src/compatibility_legacy.rs b/third_party/move/move-binary-format/src/compatibility_legacy.rs
@@ -7,12 +7,12 @@
 use crate::{
     compatibility,
     errors::{PartialVMError, PartialVMResult},
-    file_format::{AbilitySet, StructTypeParameter, Visibility},
+    file_format::{StructTypeParameter, Visibility},
     file_format_common::VERSION_5,
     normalized::Module,
 };
 use compatibility::Compatibility;
-use move_core_types::vm_status::StatusCode;
+use move_core_types::{ability::AbilitySet, vm_status::StatusCode};
 use std::collections::BTreeSet;
 
 impl Compatibility {

diff --git a/third_party/move/move-binary-format/src/deserializer.rs b/third_party/move/move-binary-format/src/deserializer.rs
@@ -4,7 +4,12 @@
 
 use crate::{check_bounds::BoundsChecker, errors::*, file_format::*, file_format_common::*};
 use move_core_types::{
-    account_address::AccountAddress, identifier::Identifier, metadata::Metadata, state::VMState,
+    ability::{Ability, AbilitySet},
+    account_address::AccountAddress,
+    function::ClosureMask,
+    identifier::Identifier,
+    metadata::Metadata,
+    state::VMState,
     vm_status::StatusCode,
 };
 use serde::Serialize;