Allow external structs. #3045

mohammadfawaz · 2025-11-27T20:58:49Z

Specifically,

Introduce a PlaintextType::ExternalStruct, and allow referring to such a type in .aleo code by a Locator.
Allow casting to external struct types.
Let two struct types be considered the same type as long as they are structurally the same and have the same local names. This funky ruleset is effectively here for backwards compatibility. In more detail:

/// Equivalence of structs means they have the same local names (regardless of whether
/// they're local or external), and their members have the same names and equivalent
/// types in the same order, recursively.
///
/// Equivalence of arrays means they have the same length and their element types are
/// equivalent.
///
/// This definition of equivalence was chosen to balance these concerns:
///
/// 1. All programs from before the existence of external structs will continue to work -
/// thus it's necessary for a struct created from another program to be considered equivalent
/// to a local one with the same name and structure, as in practice that was the behavior.
/// 2. We don't want to allow a fork. Thus we do need to check names, not just structural
/// equivalence - otherwise we could get a program deployable to a node which is using
/// this check, but not deployable to a node running an earlier SnarkVM.

Also note this other usecase from a user:

currently we got in an issue where one mainnet deployed contract imports tokenRegistry, and the contract we are writing for A imports that contract and A contract. but, A contract and tokenRegistryContract have same struct names so we cannot compile that.If we cannot change struct name in A contract, we need to wait for snarkVM update, just to compile the contracts

Co-authored-by: Michael Benfield <mike.benfield@gmail.com>

mohammadfawaz · 2025-11-27T21:00:14Z

Rebase and squash #2798 and opened a new PR with Michael as the co-author of the single commit. This will make it easier to keep rebasing as we review this.

mohammadfawaz · 2025-11-27T21:28:22Z

@vicsn I added a test that uses an external struct inside a record: 4fb2cd3

synthesizer/src/vm/tests/test_v13.rs

synthesizer/program/src/traits/stack_and_registers.rs

- Add a few more tests

mohammadfawaz · 2025-11-28T19:26:18Z

Review comments addressed in 3e0e03c

vicsn · 2025-11-28T20:11:18Z

Please no more force pushing after reviews have started

vicsn

LGTM, one test seems to fail. If you run with REWRITE_EXPECTATIONS=1, please still check the new output makes sense.

synthesizer/program/src/traits/stack_and_registers.rs

d0cd · 2025-12-13T19:59:46Z

console/program/src/data_types/plaintext_type/bytes.rs

            2 => Ok(Self::Array(ArrayType::read_le(&mut reader)?)),
-            3.. => Err(error(format!("Failed to deserialize annotation variant {variant}"))),
+            3 => Ok(Self::ExternalStruct(Locator::read_le(&mut reader)?)),
+            4.. => Err(error(format!("Failed to deserialize annotation variant {variant}"))),


Exists in current implementation, but it might be more clear to say

Suggested change

4.. => Err(error(format!("Failed to deserialize annotation variant {variant}"))),

4.. => Err(error(format!("Failed to deserialize plaintext type variant {variant}"))),

d0cd · 2025-12-13T20:00:31Z

console/program/src/data_types/plaintext_type/mod.rs

+use crate::{ArrayType, Identifier, LiteralType, Locator, ProgramID, StructType};
 use snarkvm_console_network::prelude::*;

 /// A `PlaintextType` defines the type parameter for a literal, struct, or array.


Suggested change

/// A `PlaintextType` defines the type parameter for a literal, struct, or array.

/// A `PlaintextType` defines the type parameter for a literal, struct, array, or external struct.

d0cd · 2025-12-13T20:08:49Z

console/program/src/data_types/plaintext_type/size_in_bits.rs

+                // Look up the struct
+                let struct_ = get_external_struct(locator)?;
+
+                // Account for the plaintext variant bits.


Could consider unifying with the struct logic in the variant above, for consistency.
Ditto for size_in_bits_raw.

d0cd · 2025-12-13T22:14:04Z

synthesizer/process/src/stack/mod.rs

            }
        }

+        for mapping in self.program.mappings().values() {


This check is performed in verify.rs gated by a consensus version. Does it also need to happen here?

d0cd · 2025-12-13T22:20:58Z

synthesizer/program/src/constructor/mod.rs


+    /// Returns whether this constructor refers to an external struct.
+    pub fn contains_external_struct(&self) -> bool {
+        self.commands.iter().any(|command| {


Should we also check the other command variants?

d0cd · 2025-12-13T22:21:28Z

synthesizer/program/src/finalize/mod.rs

+    pub fn contains_external_struct(&self) -> bool {
+        self.commands
+            .iter()
+            .any(|command| matches!(command, Command::Instruction(inst) if inst.contains_external_struct()))


Should we also check the other command variants?

d0cd · 2025-12-13T22:29:27Z

synthesizer/program/src/logic/instruction/mod.rs

+        // Only cast instructions may contain an explicit reference to an external struct.
+        // Calls may produce them, but they don't explicitly reference the type, and that's
+        // always been allowed.
+        matches!(self,


When a new instruction is added, we may forget to update this code.
For example, dynamic dispatch would in theory support get.dynamic.record r0.microcredits into r1 as foo.aleo/data, but it's easy for us to forget to update this rule.
This also holds true for some of the other checks that match on the instructions?
Is there a more reliable way we can enforce these properties?

d0cd · 2025-12-13T22:40:12Z

synthesizer/program/src/logic/instruction/operation/deserialize.rs

            // Cache the plaintext bits, and return the struct.
            Ok(circuit::Plaintext::Struct(members, Default::default()))
        }
+        PlaintextType::ExternalStruct(_identifier) => todo!(),


Bailing with an error message is fine if this is not supported.

d0cd · 2025-12-13T22:40:54Z

synthesizer/program/src/logic/instruction/operation/deserialize.rs

            // Cache the plaintext bits, and return the struct.
            Ok(Plaintext::Struct(members, Default::default()))
        }
+        PlaintextType::ExternalStruct(_identifier) => todo!(),


Bailing with an error message is fine if this is not supported.

d0cd · 2025-12-13T23:00:01Z

synthesizer/program/tests/types/equivalence.rs

+    Ok(stack)
+}
+
+// ---------- 1️⃣ Literal equivalence ----------


Nit. We should get rid of the numerical emojis in this file, to support our friends whose editors can't render :)

d0cd · 2025-12-13T23:05:19Z

synthesizer/src/vm/tests/test_v13.rs

+}
+
+// This test verifies that a program with external structs can be deployed on
+// consensus version 12.


Suggested change

// consensus version 12.

// consensus version 13.

d0cd · 2025-12-13T23:06:20Z

synthesizer/src/vm/tests/test_v13.rs

+    let caller_private_key = crate::vm::test_helpers::sample_genesis_private_key(rng);
+
+    // Initialize the VM at the correct height.
+    let v10_height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();


Suggested change

let v10_height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();

let height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();

d0cd · 2025-12-13T23:06:33Z

synthesizer/src/vm/tests/test_v13.rs

+
+    // Initialize the VM at the correct height.
+    let v10_height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();
+    let vm = crate::vm::test_helpers::sample_vm_at_height(v10_height, rng);


Suggested change

let vm = crate::vm::test_helpers::sample_vm_at_height(v10_height, rng);

let vm = crate::vm::test_helpers::sample_vm_at_height(height, rng);

d0cd · 2025-12-13T23:08:03Z

synthesizer/src/vm/verify.rs

+                if consensus_version < ConsensusVersion::V13 {
+                    ensure!(
+                        !deployment.program().contains_external_struct(),
+                        "Invalid deployment transaction '{id}' - external structs may only be used beginning with Consensus version 10"


Suggested change

"Invalid deployment transaction '{id}' - external structs may only be used beginning with Consensus version 10"

"Invalid deployment transaction '{id}' - external structs may only be used beginning with `ConsensusVersion::V13`"

d0cd · 2025-12-13T23:19:44Z

synthesizer/tests/tests/vm/execute_and_finalize/external_struct_in_array.aleo

+
+    // Build two identical committee_state structs
+    cast r0 r1 into r2 as credits.aleo/committee_state;
+    cast r0 r1 into r3 as credits.aleo/committee_state;


Do you think it would be worthwhile to test two arrays one with external and one with local?

d0cd

It's getting close, I'm aligned with this notion of partial nominal and structural equivalence. Left a few suggestions.

A couple points/questions:

What is the import rule? If a program A uses an external struct in B which itself uses an external struct in C, does A have to import B and C? It would be great to document and test the rule.
Before merging, we should refresh the program regressions and run it to make sure that the VM can still be initialized with all of the new checks.

- Fix a bug related to type equivalence for nested external structs

mohammadfawaz · 2025-12-16T17:57:09Z

@d0cd I believe I addressed all your comments in 93e4e48 except for

This check is performed in verify.rs gated by a consensus version. Does it also need to happen here?

I'm not sure why the check is gated by a consensus version check in verify.rs. Do you remember why Michael added this? Also, if we do want to added a consensus version check in synthesizer/process/src/stack/mod.rs too, how do we go about that? How do we get the consensus version in that file?

What is the import rule? If a program A uses an external struct in B which itself uses an external struct in C, does A have to import B and C? It would be great to document and test the rule.

I added a test that showcases that transitive dependencies are not required at the top level as long as the top level does not explicitly use the external struct name directly. I think this rule matches how import is meant to work for all symbols.

Before merging, we should refresh the program regressions and run it to make sure that the VM can still be initialized with all of the new checks.

What does this mean exactly?

vicsn · 2025-12-17T00:45:07Z

I'm not sure why the check is gated by a consensus version check in verify.rs. Do you remember why Michael added this? Also, if we do want to added a consensus version check in synthesizer/process/src/stack/mod.rs too, how do we go about that? How do we get the consensus version in that file?

The consensus version check in verify.rs was added to ensure all validators agree external structs are only enabled from the new consensus version onwards. I think this comment's point is that the logic in synthesizer/process/src/stack/mod.rs might be duplicate and superfluous.

d0cd · 2026-01-04T16:45:44Z

Before merging, we should refresh the program regressions and run it to make sure that the VM can still be initialized with all of the new checks.

What does this mean exactly?

The regression repo is here.
It contains a few utilities we can run to make sure changes are backwards compatible.

…ich is where we test consensus stuff

vicsn · 2026-01-07T21:08:58Z

Thank you everyone :)

External structs

9e71eed

Co-authored-by: Michael Benfield <mike.benfield@gmail.com>

mohammadfawaz mentioned this pull request Nov 27, 2025

Allow external structs. #2798

Closed

Add a test that uses an external struct in a record

4fb2cd3

vicsn reviewed Nov 28, 2025

View reviewed changes

synthesizer/src/vm/tests/test_v13.rs Show resolved Hide resolved

vicsn reviewed Nov 28, 2025

View reviewed changes

synthesizer/program/src/traits/stack_and_registers.rs Show resolved Hide resolved

- Move to consensus version 13

3e0e03c

- Add a few more tests

mohammadfawaz force-pushed the struct-namespace-squashed branch from 71adc3d to 3e0e03c Compare November 28, 2025 19:25

mohammadfawaz requested a review from vicsn November 28, 2025 19:27

vicsn approved these changes Nov 28, 2025

View reviewed changes

synthesizer/program/src/traits/stack_and_registers.rs Outdated Show resolved Hide resolved

vicsn requested a review from d0cd November 28, 2025 20:17

Mohammad Fawaz added 2 commits November 28, 2025 20:25

Remove dbg statements and update expectations of failing test

c795013

Merge branch 'staging' into struct-namespace-squashed

4743f23

d0cd reviewed Dec 13, 2025

View reviewed changes

- Address more review comments

93e4e48

- Fix a bug related to type equivalence for nested external structs

Mohammad Fawaz added 2 commits January 7, 2026 10:14

Merge branch 'staging' into struct-namespace-squashed

2091482

Remove redundant check in stack/mod.rs

2f8d46a

vicsn approved these changes Jan 7, 2026

View reviewed changes

Move testing of mapping with non-existent structs to test_v13.rs wh…

4c50a2c

…ich is where we test consensus stuff

vicsn merged commit 718183c into staging Jan 7, 2026
6 of 7 checks passed

vicsn deleted the struct-namespace-squashed branch January 7, 2026 21:08

vicsn mentioned this pull request Jan 8, 2026

Low priority: consider documenting external structs and records AleoNet/welcome#120

Open

	4.. => Err(error(format!("Failed to deserialize annotation variant {variant}"))),
	4.. => Err(error(format!("Failed to deserialize plaintext type variant {variant}"))),

	/// A `PlaintextType` defines the type parameter for a literal, struct, or array.
	/// A `PlaintextType` defines the type parameter for a literal, struct, array, or external struct.

	let v10_height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();
	let height = CurrentNetwork::CONSENSUS_HEIGHT(consensus_version).unwrap();

	let vm = crate::vm::test_helpers::sample_vm_at_height(v10_height, rng);
	let vm = crate::vm::test_helpers::sample_vm_at_height(height, rng);

	"Invalid deployment transaction '{id}' - external structs may only be used beginning with Consensus version 10"
	"Invalid deployment transaction '{id}' - external structs may only be used beginning with `ConsensusVersion::V13`"

Allow external structs. #3045

Allow external structs. #3045

Uh oh!

Conversation

mohammadfawaz commented Nov 27, 2025

Uh oh!

mohammadfawaz commented Nov 27, 2025

Uh oh!

mohammadfawaz commented Nov 27, 2025

Uh oh!

Uh oh!

Uh oh!

mohammadfawaz commented Nov 28, 2025

Uh oh!

vicsn commented Nov 28, 2025

Uh oh!

vicsn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d0cd Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

d0cd Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d0cd Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

d0cd left a comment

Choose a reason for hiding this comment

Uh oh!

mohammadfawaz commented Dec 16, 2025

Uh oh!

vicsn commented Dec 17, 2025

Uh oh!

d0cd commented Jan 4, 2026

Uh oh!

Uh oh!

vicsn commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

d0cd Dec 13, 2025 •

edited

Loading

d0cd Dec 13, 2025 •

edited

Loading

d0cd Dec 13, 2025 •

edited

Loading