Resolve Error to user-defined UDT in client generation

Replace the spec-side `apply_error_udt_fallback` (which rewrote
`Udt { name: "Error" }` → `ScSpecTypeDef::Error` when no error enum
named `Error` was defined) with the inverse `apply_error_udt_override`
(which rewrites `ScSpecTypeDef::Error` → `Udt { name: "Error" }` when
the spec *does* define an error enum named `Error`).

This keeps the on-the-wire spec format identical to what every release
has produced: a contract function returning `Result<_, Error>` still
serializes its error type as the built-in `ScSpecTypeDef::Error`. The
override fires only at client generation, so already-deployed contracts
that define an `Error` error enum get a generated client referencing
that UDT instead of `soroban_sdk::Error` — without any redeploy.

Tests updated accordingly:
- `test_add_u64_spec_entries` now guards that the wasm spec still emits
  `ScSpecTypeDef::Error` (and `Udt { name: "MyError" }` for the
  differently-named enum).
- `test_missing_error_udt_falls_back_to_sdk_error` and
  `test_error_udt_overrides_sdk_error` (renamed) build hand-crafted
  specs whose function returns `Result<_, ScSpecTypeDef::Error>`,
  asserting the override only fires when a matching UDT is present.

Per review feedback on #1709:
#1709 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: willemneal

soroban-spec-rust/src/lib.rs

@@ -8,7 +8,7 @@ use proc_macro2::TokenStream;
 use quote::quote;
 use sha2::{Digest, Sha256};
 use stellar_xdr::curr as stellar_xdr;
-use stellar_xdr::{ScSpecEntry, ScSpecTypeDef, ScSpecUdtUnionCaseV0};
+use stellar_xdr::{ScSpecEntry, ScSpecTypeDef, ScSpecTypeUdt, ScSpecUdtUnionCaseV0};
 use syn::Error;
 
 use soroban_spec::read::{from_wasm, FromWasmError};
@@ -100,7 +100,7 @@ pub fn generate_without_file_with_options(
     specs: &[ScSpecEntry],
     opts: &GenerateOptions,
 ) -> TokenStream {
-    let specs = apply_error_udt_fallback(specs);
+    let specs = apply_error_udt_override(specs);
     let specs: &[ScSpecEntry] = &specs;
 
     let mut spec_fns = Vec::new();
@@ -152,40 +152,48 @@ pub fn generate_without_file_with_options(
     }
 }
 
-/// The `#[contractimpl]` macro emits any type named `Error` as a UDT reference
-/// in the spec, so a contract that uses `soroban_sdk::Error` directly (rather
-/// than defining its own error enum) produces a spec with
-/// `Udt { name: "Error" }` but no matching error-enum entry. When that
-/// happens, rewrite those references to the built-in `Error` type so the
-/// generated client code references `soroban_sdk::Error`.
+/// The `#[contractimpl]` macro emits any type named `Error` in a contract's
+/// function signatures as the built-in `ScSpecTypeDef::Error` in the spec,
+/// regardless of whether the contract defined its own error enum named `Error`
+/// or used `soroban_sdk::Error` directly. To let clients of contracts that
+/// define their own `Error` enum see the user-defined type instead of
+/// `soroban_sdk::Error`, this pass rewrites every `ScSpecTypeDef::Error`
+/// reference in the spec to `Udt { name: "Error" }` whenever the spec also
+/// contains a `UdtErrorEnumV0` named `Error`.
+///
+/// This keeps the on-the-wire spec format unchanged (so already-deployed
+/// contracts benefit without redeployment) and shifts the resolution to the
+/// client generator.
 ///
 /// Returns a borrowed slice when no rewrite is needed, otherwise a
 /// freshly-owned `Vec` with the rewrite applied.
-fn apply_error_udt_fallback(specs: &[ScSpecEntry]) -> Cow<'_, [ScSpecEntry]> {
+fn apply_error_udt_override(specs: &[ScSpecEntry]) -> Cow<'_, [ScSpecEntry]> {
     let has_error_udt = specs.iter().any(|e| {
         matches!(
             e,
             ScSpecEntry::UdtErrorEnumV0(err) if err.name.to_utf8_string_lossy() == "Error"
         )
     });
     if has_error_udt {
-        Cow::Borrowed(specs)
-    } else {
         let mut v = specs.to_vec();
-        rewrite_missing_error_udt(&mut v);
+        rewrite_error_to_udt(&mut v);
         Cow::Owned(v)
+    } else {
+        Cow::Borrowed(specs)
     }
 }
 
-/// Rewrites every `ScSpecTypeDef::Udt { name: "Error" }` reference in the
-/// given entries to `ScSpecTypeDef::Error`. Called only when the spec has no
-/// user-defined error enum named `Error`, so any such reference must refer to
-/// the built-in `soroban_sdk::Error` type.
-fn rewrite_missing_error_udt(entries: &mut [ScSpecEntry]) {
+/// Rewrites every `ScSpecTypeDef::Error` reference in the given entries to
+/// `ScSpecTypeDef::Udt { name: "Error" }`. Called only when the spec contains
+/// a user-defined error enum named `Error`, so the UDT reference resolves to
+/// that enum during code generation.
+fn rewrite_error_to_udt(entries: &mut [ScSpecEntry]) {
     fn rewrite_ty(t: &mut ScSpecTypeDef) {
         match t {
-            ScSpecTypeDef::Udt(u) if u.name.to_utf8_string_lossy() == "Error" => {
-                *t = ScSpecTypeDef::Error;
+            ScSpecTypeDef::Error => {
+                *t = ScSpecTypeDef::Udt(ScSpecTypeUdt {
+                    name: "Error".try_into().unwrap(),
+                });
             }
             ScSpecTypeDef::Option(o) => rewrite_ty(&mut o.value_type),
             ScSpecTypeDef::Result(r) => {
@@ -344,7 +352,11 @@ pub enum MyError {
     }
 
     /// Test that shows the raw spec entries from the wasm.
-    /// Verifies that both Error and MyError are correctly represented as UDTs.
+    /// Verifies that the on-the-wire spec format is unchanged: a contract
+    /// error enum named `Error` is still emitted as the built-in
+    /// `ScSpecTypeDef::Error` in function signatures (the user-defined-vs-SDK
+    /// disambiguation happens at client generation time, not here). A
+    /// differently-named error enum (`MyError`) is emitted as a UDT reference.
     #[test]
     fn test_add_u64_spec_entries() {
         use super::ScSpecEntry;
@@ -371,16 +383,10 @@ pub enum MyError {
             matches!(r.ok_type.as_ref(), ScSpecTypeDef::U64),
             "ok_type should be U64"
         );
-        let ScSpecTypeDef::Udt(u) = r.error_type.as_ref() else {
-            panic!(
-                "error_type should be a UDT for Error, got {:?}",
-                r.error_type
-            );
-        };
-        assert_eq!(
-            u.name.to_utf8_string().unwrap(),
-            "Error",
-            "error_type should be Error UDT"
+        assert!(
+            matches!(r.error_type.as_ref(), ScSpecTypeDef::Error),
+            "error_type should be the built-in Error in the wasm spec, got {:?}",
+            r.error_type
         );
 
         // Find the safe_add_two function spec
@@ -420,25 +426,23 @@ pub enum MyError {
         );
     }
 
-    /// If the spec references `Udt { name: "Error" }` but no error enum
-    /// named `Error` is defined, the generator must fall back to
-    /// `soroban_sdk::Error` so the generated client code still compiles.
-    /// This covers the (rare) case where a contract uses `soroban_sdk::Error`
-    /// directly as its Result error type instead of defining its own enum.
+    /// When the spec references `ScSpecTypeDef::Error` and contains no error
+    /// enum named `Error`, the generator must leave it as `soroban_sdk::Error`.
+    /// This covers contracts that use `soroban_sdk::Error` directly as their
+    /// Result error type, including every contract compiled before the
+    /// error-enum override was introduced.
     #[test]
     fn test_missing_error_udt_falls_back_to_sdk_error() {
         use super::ScSpecEntry;
-        use stellar_xdr::curr::{ScSpecFunctionV0, ScSpecTypeDef, ScSpecTypeResult, ScSpecTypeUdt};
+        use stellar_xdr::curr::{ScSpecFunctionV0, ScSpecTypeDef, ScSpecTypeResult};
 
         let func = ScSpecFunctionV0 {
             doc: "".try_into().unwrap(),
             name: "safe_add".try_into().unwrap(),
             inputs: [].try_into().unwrap(),
             outputs: [ScSpecTypeDef::Result(Box::new(ScSpecTypeResult {
                 ok_type: Box::new(ScSpecTypeDef::U64),
-                error_type: Box::new(ScSpecTypeDef::Udt(ScSpecTypeUdt {
-                    name: "Error".try_into().unwrap(),
-                })),
+                error_type: Box::new(ScSpecTypeDef::Error),
             }))]
             .try_into()
             .unwrap(),
@@ -459,14 +463,15 @@ pub trait Contract {
         );
     }
 
-    /// When the spec *does* contain a user-defined `Error` error enum, the
-    /// UDT reference must be preserved and not rewritten to `soroban_sdk::Error`.
+    /// When the spec contains a user-defined `Error` error enum, every
+    /// `ScSpecTypeDef::Error` reference in the spec must be rewritten to
+    /// reference that UDT instead of `soroban_sdk::Error`.
     #[test]
-    fn test_error_udt_preserved_when_defined() {
+    fn test_error_udt_overrides_sdk_error() {
         use super::ScSpecEntry;
         use stellar_xdr::curr::{
-            ScSpecFunctionV0, ScSpecTypeDef, ScSpecTypeResult, ScSpecTypeUdt,
-            ScSpecUdtErrorEnumCaseV0, ScSpecUdtErrorEnumV0,
+            ScSpecFunctionV0, ScSpecTypeDef, ScSpecTypeResult, ScSpecUdtErrorEnumCaseV0,
+            ScSpecUdtErrorEnumV0,
         };
 
         let func = ScSpecFunctionV0 {
@@ -475,9 +480,7 @@ pub trait Contract {
             inputs: [].try_into().unwrap(),
             outputs: [ScSpecTypeDef::Result(Box::new(ScSpecTypeResult {
                 ok_type: Box::new(ScSpecTypeDef::U64),
-                error_type: Box::new(ScSpecTypeDef::Udt(ScSpecTypeUdt {
-                    name: "Error".try_into().unwrap(),
-                })),
+                error_type: Box::new(ScSpecTypeDef::Error),
             }))]
             .try_into()
             .unwrap(),