Remove resource access control - #410
Open
fEst1ck wants to merge 11 commits into
Open
Conversation
Replace access specifiers with declared_acquires (legacy annotations) and acquired_structs (inferred); sourcifier prefers the inferred set so decompiled bytecode keeps its acquires. Adds regression tests for the v2 reference safety processor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete tests using the removed access specifier syntax along with the now-empty checking-lang-v2.5 config, and regenerate baselines for the new acquires dump/sourcified output and tighter error spans. The sourcifier no longer prints inferred acquires on inline functions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Matches the v3 processor and the bytecode verifier; previously only mutable borrows were flagged, letting unsound code reach bytecode verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deprecate `enable_resource_access_control` to `_enable_resource_access_control`, hardcoded false and no longer read: access specifiers are rejected regardless of the on-chain flag, which is now only used to warn that it is ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers the module and script rejection paths, with negative controls so the assertions cannot pass on an unrelated defect in the fixtures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resource access control has been removed, so the flag no longer enables anything; new chains should not turn it on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rename the flag to _DEPRECATED_ENABLE_RESOURCE_ACCESS_CONTROL, keeping discriminant 75 so the on-chain bitset is unaffected. The release builder's YAML-facing name is left unchanged so existing proposals still parse. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The assembler no longer copies specifiers from imported function handles, and the proptest generators no longer produce them, since the verifier now rejects any bytecode carrying them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fEst1ck
marked this pull request as ready for review
August 4, 2026 14:22
fEst1ck
requested review from
0xIcarus,
Primata,
ganymedio and
musitdev
as code owners
August 4, 2026 14:22
seanyoung
reviewed
Aug 4, 2026
| sig_checker_v2_fix_function_signatures, | ||
| enable_enum_types, | ||
| enable_resource_access_control, | ||
| _enable_resource_access_control: false, |
Collaborator
There was a problem hiding this comment.
Can we just remove this?
Author
There was a problem hiding this comment.
The comment of the structure says
/// Configuration for the bytecode verifier.
///
/// Always add new fields to the end, as we rely on the hash or serialized bytes of config to
/// detect if it has changed (e.g., new feature flag was enabled). Also, do not delete existing
/// fields, or change the type of existing field.
#[derive(Debug, Clone, Eq, PartialEq, Serialize)]
pub struct VerifierConfig {
Though it seems to me it's removable.
| verifier.verify_signatures()?; | ||
| verifier.verify_function_handles()?; | ||
| if !config.enable_resource_access_control && script.access_specifiers.is_some() { | ||
| if script.access_specifiers.is_some() { |
Collaborator
There was a problem hiding this comment.
can we remove script.access_specifiers?
Author
There was a problem hiding this comment.
This is part of the file format already enabled on-chain, so we'd better keep it but ignore the RAC.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Removes the Move VM resource access control (RAC) feature end to end, since it was never enabled on any Movement network (feature flag 75 is off on mainnet and testnet, so no on-chain module carries access specifiers):
reads/writes/pure), the expansion-AST and move-model representations, and file-format generation support.access_control.rs,runtime_access_specifier.rs, and the access-specifier loader. The dynamic reentrancy check onborrow_global/exists/move_from/move_tois retained viacheck_resource_access.enable_resource_access_control, now deprecated to_enable_resource_access_control). Rejection reusesStatusCode::FEATURE_NOT_ENABLED, matching the old flag-off behavior bit for bit.ENABLE_RESOURCE_ACCESS_CONTROLrenamed to_DEPRECATED_ENABLE_RESOURCE_ACCESS_CONTROL, keeping discriminant 75 so the on-chain bitset layout is unchanged; the release builder's YAML-facing name is preserved so existing proposals still parse.acquires-declaring functions are now rejected on any borrow of the global (previously only mutable borrows) — strictly stricter, and compiler-side only.How Has This Been Tested?
third_party/move/move-bytecode-verifier/bytecode-verifier-tests/src/unit_tests/access_specifier_tests.rs.expbaselines for access-specifier sources, which now report parse/check errors.transactional-tests/tests/no-v1-comparison/access_control/) and the runtime access-specifier proptests, which exercise deleted code.aptos-move/e2e-move-tests/src/tests/move_feature_gating.rsfor the removed gating.Key Areas to Review
third_party/move/move-bytecode-verifier/src/features.rs,verifier.rs):FeatureVerifierruns right after bounds checking in bothverify_module_with_configandverify_script_with_config, checking every function handle plus the script-level specifier field. This is the sole gate keeping specifier-carrying bytecode away from a runtime that no longer understands it.third_party/move/move-vm/runtime/src/interpreter.rs): the oldcheck_accesshelper mixed RAC enforcement with the resource reentrancy check; only the RAC half is removed. All four global-storage opcodes still callcheck_resource_access.types/src/on_chain_config/aptos_features.rs,aptos-move/aptos-release-builder/src/components/feature_flags.rs): discriminant 75 is kept explicitly and the release-builder mapping is preserved in both directions;VerifierConfigkeeps the (renamed) field so its serialized layout — used as a node-local cache key — is unchanged.third_party/move/move-compiler-v2/src/pipeline/reference_safety/reference_safety_processor_v2.rs,_v3.rs): confirm the any-borrow rejection matches the bytecode verifier's global-safety model.Type of Change
Which Components or Systems Does This Change Impact?
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.