[compiler][vm] add visibility modifier to structs/enums: step 5 #18489

rahxephon89 · 2026-01-22T21:58:04Z

Description

This PR

Adds validation logic to verify that structs/enums used as transaction arguments are:
a. public (have public pack functions with #[struct_api(pack)] attribute)
b. have the copy ability
c. all field types are also valid transaction arguments
Adds the logic to construct values of public structs by finding and calling the pack API
Adds check to compiler for transaction arguments
Adds supports of enum in JSON representation for argument types
Adds a new feature flag PUBLIC_STRUCT_ENUM_ARGS for enabling public structs/enums as txn args.

Note that this PR does not contain code to support public structs in CLI because it requires encoding the type information.

How Has This Been Tested?

new e2e move tests and api tests.

Key Areas to Review

Whether the validation logic is correct and complete;
Whether the test cases give good coverage;
Whether this change may cause any compatibility issues.

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

Note

High Risk
Touches VM transaction argument validation/construction, type/layout plumbing, and on-chain feature gating, which can affect transaction acceptance and execution semantics. Although gated by PUBLIC_STRUCT_ENUM_ARGS, the change is broad and adds new parsing/construction paths (including generics and enums).

Overview
Enable public copy structs/enums as txn args behind PUBLIC_STRUCT_ENUM_ARGS. The VM verifier now treats non-whitelisted structs/enums as valid transaction arguments when they have copy ability, are public via generated pack$... functions, and all (recursively substituted) field types are themselves valid; it also constructs these arguments by invoking the appropriate cached pack function (including enum variants).

Expand API and tooling to encode/validate these types. The API JSON-to-BCS converter gains enum JSON parsing and generic parameter substitution for entry functions before argument conversion; Move layout/types plumbing is updated to carry type tags for WithVariants and to emit WithVariants layouts for general enums (while preserving Option’s legacy representation). A new on-chain feature flag is wired through release/prod configs, compiler extended checks are updated to match VM rules, and new API + e2e tests cover positive flows, malformed JSON/variants, generic/private type rejection, and feature-disable/enable behavior.

^{Written by Cursor Bugbot for commit 4f22f38. This will update automatically on new commits. Configure here.}

rahxephon89 · 2026-01-22T21:58:26Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

[CLI][Move] support public structs/enums as txn args in CLI #18591
[compiler][vm] add visibility modifier to structs/enums: step 5 #18489 👈 (View in Graphite)
[compiler][vm] add visibility modifier to structs/enums: step 4 #18165 : 1 other dependent PR (#18381 )
[compiler][vm] add visibility modifier to structs/enums: step 3 #18117
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

georgemitenkov

Why do we need a tag in WithVariants layout here?

georgemitenkov · 2026-02-06T12:14:15Z

third_party/move/move-core/types/src/value.rs

-    WithVariants(Vec<MoveVariantLayout>),
+    /// Like WithTypes, this carries the type tag for proper type identification.
+    WithVariants {
+        type_: StructTag,


Question: why this change? Looking through PR I do not see any usages of type_? But maybe I am missing something

georgemitenkov · 2026-02-06T12:15:17Z

third_party/move/move-core/types/src/value.rs

+                "Invalid MoveTypeLayout -> StructTag conversion--needed MoveLayoutType::WithTypes or WithVariants"
            ),
-            WithTypes { type_, .. } => Ok(type_.clone()),
+            WithTypes { type_, .. } | WithVariants { type_, .. } => Ok(type_.clone()),


I see, so it is used here. Do we need this for API? Or why exactly?

In build_from_definition, when we instantiate a generic with enum, we need to support MoveStructLayout for enums. If we don't support it here, error will be raised.

georgemitenkov · 2026-02-06T12:18:37Z

third_party/move/move-vm/runtime/src/config.rs

+    /// Whether this VM should support public copy structs/enums as transaction arguments.
+    /// When enabled, non-private structs and enums with the copy ability can be used as
+    /// transaction arguments if they have public pack functions with the Pack attribute.
+    pub enable_public_struct_args: bool,


I think having feature gate by v10 format lesehwere is better. For Move VM txn args are irrelevant, so I am not sure this is the best place to add this thing. Just gating in txn_arg_validation is good enough?

Added a new feature flag PUBLIC_STRUCT_ENUM_ARGS to control it instead of using V10.

georgemitenkov · 2026-02-06T12:19:31Z

third_party/move/tools/move-bytecode-utils/src/layout.rs

                    ),
                    LayoutType::WithTypes => {
                        let mid = m.self_id();
+                        // All type_arguments layouts can now be converted to TypeTags


nit: looks like useless comment?

georgemitenkov · 2026-02-06T12:19:35Z

third_party/move/tools/move-bytecode-utils/src/layout.rs

                    match layout_type {
                        LayoutType::WithTypes => {
                            let mid = m.self_id();
+                            // All type_arguments layouts can now be converted to TypeTags


nit: looks like useless comment?

georgemitenkov · 2026-02-06T12:21:41Z

third_party/move/tools/move-bytecode-utils/src/layout.rs

@@ -263,6 +265,7 @@ impl StructLayoutBuilder {
                    match layout_type {


Can we factor out his so that:

if m.self_id().is_option() { build_option_layout_from_definition(...) } else { build_any_variant_layout_from_definition(...) }

georgemitenkov · 2026-02-06T12:24:17Z

third_party/move/tools/move-bytecode-utils/src/layout.rs

+                    .collect::<anyhow::Result<Vec<_>>>()?;
+
+                Ok(match layout_type {
+                    LayoutType::Runtime => MoveStructLayout::RuntimeVariants(


Why we support it here, but not for option? I think we do not need this here and only WithTypes need to be supported, and if so, we can process all layouts here and do not even match on layout type: check it is with_types in advance.

georgemitenkov · 2026-02-06T12:31:55Z

aptos-move/framework/src/extended_checks.rs

+            Vector(ety) => self.is_valid_txn_arg_type(ety),
+            Struct(mid, sid, inst) => {
+                let qid = mid.qualified(*sid);
+                self.is_allowed_input_struct(qid) || self.is_public_copy_struct(qid, inst)


Is this correct: we would allow Option<NonPublicStruct> here. Don't we need to check inst for Option (because we know it is a field)? How did it work before could you construct None of non-public type, bit not Some(..)?

yes, because we don't know at the compile time whether T will be instantiated with a public or non-public values, which should be checked when running the entry function with full type instantiation.

georgemitenkov · 2026-02-06T14:04:07Z

...s-move/e2e-move-tests/src/tests/public_struct_args.data/pack/sources/public_struct_test.move

+    public entry fun test_option_point(sender: &signer, opt_point: std::option::Option<Point>) acquires TestResult {
+        let result = borrow_global_mut<TestResult>(std::signer::address_of(sender));
+        if (std::option::is_some(&opt_point)) {
+            let p = std::option::destroy_some(opt_point);


nit: imports and use . syntax?

georgemitenkov · 2026-02-06T14:08:58Z

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

+    );
+    assert_success!(result);
+
+    // Now disable VM_BINARY_FORMAT_V10 feature flag, which disables public struct/enum args


we should never disable it though, because if you published v10 bytecode it becomes unloadable? is it better to split feature gating and having a dedicated feature for controlling such args? in a sense that this scenario is not really possible? what can happen is v9 bytecode is published and Point is non-public. We fail to run functions/ Then we enable v10. Running this entry starts to work?

georgemitenkov · 2026-02-06T14:10:15Z

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

+
+    // Verify view function returns deserialization error as ErrorMessage
+    // (not MoveAbort, since CODE_DESERIALIZATION_ERROR is not an abort)
+    match view_result.values {


Do we need a match? can unwrap in tests

georgemitenkov · 2026-02-06T14:11:12Z

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

+            }
+
+            #[view]
+            public fun view_point(p: Point): u64 {


Q: can we return Point struct from this view function, does it work even for non-public structs?

currently view functions can already return a struct value

aptos-move/framework/src/extended_checks.rs

aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs

api/types/src/convert.rs

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

third_party/move/tools/move-bytecode-utils/src/layout.rs

cursor · 2026-02-10T08:00:17Z

api/src/tests/public_struct_enum_negative_test.rs

+    assert!(
+        !found_test_txn,
+        "Transaction with test_generic_container should NOT exist in ledger - validation should have rejected it"
+    );


Negative test incorrectly assumes transaction is discarded

Medium Severity

The test test_generic_container_with_non_public_type_arg_rejected calls api_execute_txn (which expects HTTP 202 and commits the transaction) then asserts the transaction is NOT in the ledger. However, INVALID_MAIN_FUNCTION_SIGNATURE (status code 1011) falls in the Verification status range, so keep_or_discard returns Ok(KeptVMStatus::MiscellaneousError) — the transaction is kept, not discarded. The transaction will be committed to the ledger with a failed execution status, and the assertion !found_test_txn will fail because the transaction IS present.

cursor · 2026-02-10T08:00:17Z

api/src/tests/public_struct_enum_negative_test.rs

+    assert!(
+        !found_test_txn,
+        "Transaction with test_generic_container should NOT exist in ledger - validation should have rejected it"
+    );


Negative test incorrectly assumes transaction is discarded

Medium Severity

The test test_generic_container_with_non_public_type_arg_rejected calls api_execute_txn (which expects HTTP 202 and commits the transaction) then asserts the transaction is NOT in the ledger. However, INVALID_MAIN_FUNCTION_SIGNATURE (status code 1011) falls in the Verification status range, so keep_or_discard returns Ok(KeptVMStatus::MiscellaneousError) — the transaction is kept, not discarded. The transaction will be committed to the ledger with a failed execution status, and the assertion !found_test_txn will fail because the transaction IS present.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

cursor · 2026-02-11T00:06:58Z

api/types/src/convert.rs

+                    variant_index as u16,
+                    field_results,
+                ),
+            ));


Variant index cast to u16 without bounds check

Low Severity

In try_into_vm_value_struct, variant_index from enumerate() (a usize) is cast to u16 with variant_index as u16 without a bounds check. If an enum has more than 65535 variants, this would silently truncate. The VM-side construct_public_copy_struct correctly validates variant_idx_usize > u16::MAX, but this JSON conversion path does not.

This was referenced Jan 22, 2026

[compiler][vm] add visibility modifier to structs/enums: step 3 #18117

Open

[compiler][vm] add visibility modifier to structs/enums: step 4 #18165

Open

rahxephon89 mentioned this pull request Jan 22, 2026

[compiler] add visibility modifier to structs/enums: step 5 #18381

Closed

22 tasks

rahxephon89 changed the title ~~txn args~~ [WIP] support public struct/enums as transaction arguments Jan 22, 2026

rahxephon89 changed the title ~~[WIP] support public struct/enums as transaction arguments~~ [compiler][vm] add visibility modifier to structs/enums: step 5 Jan 22, 2026

rahxephon89 changed the title ~~[compiler][vm] add visibility modifier to structs/enums: step 5~~ [WIP][compiler][vm] add visibility modifier to structs/enums: step 5 Jan 22, 2026

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from 06824ad to 41b73e1 Compare January 23, 2026 08:36

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch 2 times, most recently from 0bb5405 to 54ce6ce Compare January 23, 2026 23:36

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 41b73e1 to 2aed166 Compare January 23, 2026 23:36

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 54ce6ce to 12ef626 Compare January 24, 2026 10:04

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 2aed166 to 760ad44 Compare January 24, 2026 10:04

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 12ef626 to 2e6c361 Compare January 25, 2026 09:36

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from 42360a0 to ac79908 Compare January 25, 2026 18:33

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 2e6c361 to c253dbf Compare January 25, 2026 18:33

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from 9a7e33b to a6252ac Compare January 27, 2026 08:51

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from c253dbf to b0f9e72 Compare January 27, 2026 08:51

rahxephon89 force-pushed the teng/public-struct-txn-args branch from a6252ac to ce33809 Compare January 27, 2026 17:47

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from b0f9e72 to da3998a Compare January 28, 2026 00:09

rahxephon89 force-pushed the teng/public-struct-txn-args branch from ce33809 to 4b0a271 Compare January 28, 2026 00:09

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from da3998a to 35d544b Compare January 28, 2026 16:39

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from dd786c6 to beb910c Compare January 28, 2026 16:48

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 35d544b to 3442487 Compare January 28, 2026 16:48

rahxephon89 force-pushed the teng/public-struct-txn-args branch from beb910c to 1480203 Compare January 29, 2026 06:43

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 3442487 to 4731fc1 Compare January 29, 2026 06:43

rahxephon89 force-pushed the teng/public-struct-txn-args branch from dd7a3d2 to 966306b Compare February 5, 2026 22:35

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 8ac52fd to ca38b6b Compare February 6, 2026 03:09

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 966306b to a6c536f Compare February 6, 2026 03:09

georgemitenkov reviewed Feb 6, 2026

View reviewed changes

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from ca38b6b to 52495b2 Compare February 7, 2026 03:25

rahxephon89 force-pushed the teng/public-struct-txn-args branch from a6c536f to 810bb48 Compare February 7, 2026 03:25

cursor bot reviewed Feb 7, 2026

View reviewed changes

aptos-move/framework/src/extended_checks.rs Show resolved Hide resolved

aptos-move/aptos-vm/src/verifier/transaction_arg_validation.rs Show resolved Hide resolved

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 52495b2 to c07e5bd Compare February 9, 2026 01:24

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from fb023bd to f37f6c7 Compare February 9, 2026 08:54

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from c07e5bd to 14fb59d Compare February 9, 2026 08:54

cursor bot reviewed Feb 9, 2026

View reviewed changes

api/types/src/convert.rs Show resolved Hide resolved

aptos-move/e2e-move-tests/src/tests/public_structs_enums_upgrade.rs Outdated Show resolved Hide resolved

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from a354293 to 8fd8607 Compare February 9, 2026 09:51

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 14fb59d to 65a945b Compare February 9, 2026 09:51

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 8fd8607 to 05382f0 Compare February 9, 2026 10:20

cursor bot reviewed Feb 9, 2026

View reviewed changes

third_party/move/tools/move-bytecode-utils/src/layout.rs Outdated Show resolved Hide resolved

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 65a945b to 24787e4 Compare February 10, 2026 00:45

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 05382f0 to 12b69f7 Compare February 10, 2026 00:45

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 24787e4 to 8720ccb Compare February 10, 2026 07:45

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 12b69f7 to 82b0fc2 Compare February 10, 2026 07:45

rahxephon89 requested a review from georgemitenkov February 10, 2026 07:49

cursor bot reviewed Feb 10, 2026

View reviewed changes

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 8720ccb to c815eba Compare February 10, 2026 23:52

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 82b0fc2 to 428e114 Compare February 10, 2026 23:52

cursor bot reviewed Feb 11, 2026

View reviewed changes

txn args

4f22f38

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from c815eba to fc5f574 Compare February 11, 2026 09:18

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 428e114 to 4f22f38 Compare February 11, 2026 09:18

		@@ -263,6 +265,7 @@ impl StructLayoutBuilder {
		match layout_type {

[compiler][vm] add visibility modifier to structs/enums: step 5 #18489

Are you sure you want to change the base?

[compiler][vm] add visibility modifier to structs/enums: step 5 #18489

Conversation

rahxephon89 commented Jan 22, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

Uh oh!

rahxephon89 commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georgemitenkov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot Feb 10, 2026

Choose a reason for hiding this comment

Negative test incorrectly assumes transaction is discarded

Uh oh!

cursor bot Feb 10, 2026

Choose a reason for hiding this comment

Negative test incorrectly assumes transaction is discarded

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 11, 2026

Choose a reason for hiding this comment

Variant index cast to u16 without bounds check

Uh oh!

Reviewers

rahxephon89 commented Jan 22, 2026 •

edited by cursor bot

Loading

rahxephon89 commented Jan 22, 2026 •

edited

Loading