Merge pull request #3051 from ProvableHQ/feat/improve-register-index-error

[Refactor] Output record register check from V10

Author: vicsn

synthesizer/process/src/stack/helpers/check_upgrade.rs

@@ -36,6 +36,10 @@ impl<N: Network> Stack<N> {
     ///  | finalize          |   ❌   | ✅ (logic)   |  ✅   |
     ///  |-------------------|--------|--------------|-------|
     ///
+    /// There is one important caveat in that output register indices **MUST** remain the same.
+    /// For example, changing `output r10 as <NAME>.record` into `output r11 as <NAME>.record` would not be a valid upgrade.
+    /// This restriction is necessary because the output register index is instantiated as a constant in the caller circuit.
+    /// This check is enforced in `check_transaction` in `synthesizer/src/vm/verify.rs`.
     #[inline]
     pub fn check_upgrade_is_valid(old_program: &Program<N>, new_program: &Program<N>) -> Result<()> {
         // Get the new program ID.

synthesizer/src/vm/helpers/mod.rs

@@ -23,6 +23,9 @@ pub use history::*;
 
 mod macros;
 
+mod program;
+pub use program::*;
+
 mod rewards;
 pub use rewards::*;
 

synthesizer/src/vm/helpers/program.rs

@@ -0,0 +1,41 @@
+// Copyright (c) 2019-2025 Provable Inc.
+// This file is part of the snarkVM library.
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at:
+
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use console::{network::Network, program::ValueType};
+use snarkvm_synthesizer_program::Program;
+
+use anyhow::{Result, bail, ensure};
+
+/// Verifies that the existing output register indices are not changed in a new version of the program.
+// Note. This function is public so that depednent crates can cleanly surface this error to users.
+pub fn check_output_register_indices_unchanged<N: Network>(
+    old_program: &Program<N>,
+    new_program: &Program<N>,
+) -> Result<()> {
+    for (id, function) in old_program.functions() {
+        // Get the corresponding function in the new program.
+        let Ok(new_function) = new_program.get_function(id) else { bail!("Missing function '{id}'") };
+        // Ensure the record output registers match.
+        let existing_output_registers =
+            function.outputs().iter().filter(|output| matches!(output.value_type(), ValueType::Record(_)));
+        let new_output_registers =
+            new_function.outputs().iter().filter(|output| matches!(output.value_type(), ValueType::Record(_)));
+        ensure!(
+            existing_output_registers.eq(new_output_registers),
+            "Function '{id}' has mismatched record output registers"
+        );
+    }
+    Ok(())
+}

synthesizer/src/vm/mod.rs

@@ -29,19 +29,7 @@ use crate::{Restrictions, cast_mut_ref, cast_ref, convert, process};
 use console::{
     account::{Address, PrivateKey},
     network::prelude::*,
-    program::{
-        Argument,
-        Identifier,
-        Literal,
-        Locator,
-        Plaintext,
-        ProgramID,
-        ProgramOwner,
-        Record,
-        Response,
-        Value,
-        ValueType,
-    },
+    program::{Argument, Identifier, Literal, Locator, Plaintext, ProgramID, ProgramOwner, Record, Response, Value},
     types::{Field, Group, U16, U64},
 };
 use snarkvm_algorithms::snark::varuna::VarunaVersion;

synthesizer/src/vm/verify.rs

@@ -350,24 +350,10 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
                                 );
                                 // If the consensus version is V10 or greater, then check that each function's **record** output registers match the existing program.
                                 if consensus_version >= ConsensusVersion::V10 {
-                                    for (id, function) in existing_program.functions() {
-                                        // Get the corresponding function in the new program.
-                                        let Ok(new_function) = deployment.program().get_function(id) else {
-                                            bail!("Invalid deployment transaction '{id}' - missing function '{id}'")
-                                        };
-                                        // Ensure the record output registers match.
-                                        let existing_output_registers = function
-                                            .outputs()
-                                            .iter()
-                                            .filter(|output| matches!(output.value_type(), ValueType::Record(_)));
-                                        let new_output_registers = new_function
-                                            .outputs()
-                                            .iter()
-                                            .filter(|output| matches!(output.value_type(), ValueType::Record(_)));
-                                        ensure!(
-                                            existing_output_registers.eq(new_output_registers),
-                                            "Invalid deployment transaction '{id}' - function '{id}' has mismatched record output registers"
-                                        );
+                                    if let Err(e) =
+                                        check_output_register_indices_unchanged(existing_program, deployment.program())
+                                    {
+                                        bail!("Invalid deployment transaction '{id}' - {e}")
                                     }
                                 }
                             }