[compiler][vm] add visibility modifier to structs/enums: step 5 by rahxephon89 · Pull Request #18489 · aptos-labs/aptos-core

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

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 and core layout representations, which are consensus-critical and can affect transaction acceptance, gas, and compatibility. Also introduces new parsing paths for enums/generics that could reject/accept transactions differently if edge cases are missed.

Overview
Adds end-to-end support for passing public copy Move structs/enums as transaction (and view) arguments, beyond the existing whitelist.

On the VM side, transaction argument validation now accepts public copy structs/enums by loading/caching pack$... functions, recursively validating field types (including generic instantiations), and constructing arguments by executing the appropriate pack function (including enum variant packs). The feature is gated via a new VM config flag enable_public_struct_args (enabled for VM_BINARY_FORMAT_V10).

On the API side, JSON-to-BCS conversion gains enum JSON parsing and handles generic entry functions by instantiating parameter types with provided type arguments before decoding. Core Move layout plumbing is updated so MoveStructLayout::WithVariants carries a StructTag, and enum layouts are produced by bytecode layout builders, with follow-on adjustments in BCS/string native utilities.

Adds extensive new API and e2e tests covering positive/negative cases (nested structs, vectors, Option, enums/variants, generic containers, and feature-flag on/off behavior).

^{Written by Cursor Bugbot for commit fb023bd. 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.

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

api/types/src/convert.rs

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

cursor · 2026-02-05T05:40:40Z

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

+                return false;
+            }
+        }
+    }


Duplicated loop logic in validate_fields_recursively function

Low Severity

The validate_fields_recursively function has nearly identical loop logic in both branches. Both iterate over fields and call is_valid_txn_arg, differing only in whether type substitution is applied. This could be consolidated into a single loop where the type to validate is conditionally substituted, reducing code duplication.

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?

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?

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(..)?

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?

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

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

aptos-move/framework/src/extended_checks.rs

cursor · 2026-02-07T03:38:32Z

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

+                Some(String::from("Pack function did not return value")),
+            )
+        })?
+        .0)


Pack function return count not validated, pop/index mismatch

Low Severity

In construct_public_copy_struct, the return type count check only verifies return_tys().is_empty() (line 630), unlike load_constructor_function which checks return_tys().len() != 1. Additionally, the type matching uses return_tys()[0] (first return type) while the result extraction uses ret_vals.pop() (last return value). For a hypothetical pack function returning multiple values, these would reference different items.

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/add-wellform-check-for-struct-apis branch from e0883c6 to 6d1d8a2 Compare February 4, 2026 04:48

rahxephon89 force-pushed the teng/public-struct-txn-args branch from de306aa to 15cf4ba Compare February 4, 2026 04:48

rahxephon89 mentioned this pull request Feb 4, 2026

[CLI][Move] support public structs/enums as txn args in CLI #18591

Open

22 tasks

cursor bot reviewed Feb 4, 2026

View reviewed changes

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

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

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 15cf4ba to fc1c26d Compare February 4, 2026 17:28

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 6d1d8a2 to a6b0c8f Compare February 4, 2026 17:28

cursor bot reviewed Feb 4, 2026

View reviewed changes

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

rahxephon89 force-pushed the teng/public-struct-txn-args branch from fc1c26d to d31e4d0 Compare February 5, 2026 01:06

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from a6b0c8f to 261f387 Compare February 5, 2026 01:06

cursor bot reviewed Feb 5, 2026

View reviewed changes

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

rahxephon89 force-pushed the teng/public-struct-txn-args branch 2 times, most recently from f37ad1b to b30d55a Compare February 5, 2026 05:31

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 261f387 to 41858a6 Compare February 5, 2026 05:31

cursor bot reviewed Feb 5, 2026

View reviewed changes

rahxephon89 force-pushed the teng/public-struct-txn-args branch from b30d55a to 8f27993 Compare February 5, 2026 07:00

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch from 41858a6 to e241d27 Compare February 5, 2026 07:00

rahxephon89 force-pushed the teng/public-struct-txn-args branch from 8f27993 to dd7a3d2 Compare February 5, 2026 18:45

rahxephon89 force-pushed the teng/add-wellform-check-for-struct-apis branch 2 times, most recently from 1732302 to 8ac52fd Compare February 5, 2026 22:35

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

txn args

fb023bd

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 from 810bb48 to fb023bd Compare February 9, 2026 01:24

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

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot Feb 5, 2026

Choose a reason for hiding this comment

Duplicated loop logic in validate_fields_recursively function

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!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor bot Feb 7, 2026

Choose a reason for hiding this comment

Pack function return count not validated, pop/index mismatch

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rahxephon89 commented Jan 22, 2026 •

edited by cursor bot

Loading

rahxephon89 commented Jan 22, 2026 •

edited

Loading