upgrade errors formatting positional fields, nested fields, named fie…

…lds on enums (MystenLabs#20653) ## Description Adds tests and formatting fixes for: - positional fields on structs and enums - more cases for named fields on enums - positional struct nested in struct - positional struct nested in enum ## Test plan How did you test the new or updated feature? snapshot testing --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
tnowacki · Dec 20, 2024 · a8b3ad3 · a8b3ad3
1 parent 958a891
commit a8b3ad3
Show file tree

Hide file tree

Showing 7 changed files with 576 additions and 123 deletions.
diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move
@@ -10,11 +10,11 @@ module upgrades::upgrades {
         A,
     }
 
-    public enum EnumRemoveAbility has copy, drop {
+    public enum EnumRemoveAbility has copy, drop { // remove drop
         A,
     }
 
-    public enum EnumAddAndRemoveAbility has copy, drop {
+    public enum EnumAddAndRemoveAbility has copy, drop { // change drop to store
         A,
     }
 
@@ -45,6 +45,7 @@ module upgrades::upgrades {
         C, // to be removed
     }
 
+    // with types
     public enum EnumAddAbilityWithTypes has copy { // add drop
         A { a: u8 },
     }
@@ -73,5 +74,38 @@ module upgrades::upgrades {
         B { b: u8 }, // to be changed to C
         // D { d: u8 }, to be added
     }
+
+    public enum EnumChangeAndRemoveVariantWithPositionalTypes {
+        A(u8),
+        B(u8), // to be changed to C
+        C(u8), // to be removed
+    }
+
+    public enum EnumChangePositionalType {
+        A, // add u8
+        B(u8), // to be changed to u16
+        C(u8, u8), // remove u8
+        D(u8) // remove last u8
+    }
+
+    public struct ChangeFieldA {
+        a: u32,
+    }
+
+    public struct ChangeFieldB {
+        b: u32,
+    }
+
+    public enum EnumWithPositionalChanged {
+        A(ChangeFieldA), // change to ChangeFieldB
+    }
+
+    public enum EnumWithNamedChanged {
+        A {
+            x: ChangeFieldA,
+            y: ChangeFieldA,
+            z: ChangeFieldA, // change to ChangeFieldB
+        },
+    }
 }
 
diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move
@@ -14,7 +14,7 @@ module upgrades::upgrades {
         A,
     }
 
-    public enum EnumAddAndRemoveAbility has copy, store {
+    public enum EnumAddAndRemoveAbility has copy, store { // change drop to store
         A,
     }
 
@@ -75,5 +75,38 @@ module upgrades::upgrades {
         D { d: u8 }, // added
     }
 
+    public enum EnumChangeAndRemoveVariantWithPositionalTypes {
+        A(u8),
+        C(u8), // changed to C
+        // C(u8) removed
+    }
+
+    public enum EnumChangePositionalType {
+        A(u8), // add u8
+        B(u16), // to be changed to u16
+        C(u8), // removed u8
+        D, // removed last u8
+    }
+
+    public struct ChangeFieldA {
+        a: u32,
+    }
+
+    public struct ChangeFieldB {
+        b: u32,
+    }
+
+    public enum EnumWithPositionalChanged {
+        A(ChangeFieldB), // changed to ChangeFieldB
+    }
+
+    public enum EnumWithNamedChanged {
+        A {
+            x: ChangeFieldA,
+            y: ChangeFieldA,
+            z: ChangeFieldB, // changed to ChangeFieldB
+        },
+    }
+
 }
 
diff --git a/...es/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move b/...es/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move
@@ -14,9 +14,23 @@ module upgrades::upgrades {
     public struct AddMultipleAbilities {}
 
 
-    // field mismatch
-    public struct AddField {}
-    // remove field
+    // add field to empty struct
+    public struct AddFieldToEmpty {
+        // add a,
+    }
+
+    // add fields
+    public struct AddField {
+        a: u32
+        // b
+    }
+
+    // remove field from struct with one field
+    public struct RemoveOnlyField {
+        a: u64,
+    }
+
+    // remove field from struct with multiple fields
     public struct RemoveField {
         a: u64,
         b: u64, // remove this field
@@ -33,5 +47,47 @@ module upgrades::upgrades {
         a: u64,
         b: u64, // change this field type to u32
     }
-}
 
+    // change field name and type
+    public struct ChangeFieldNameAndType {
+        a: u64,
+        b: u64, // change field name to c and type to u32
+    }
+
+    // add positional to empty positional struct
+    public struct EmptyPositionalAdd() // add u64
+
+    // struct new positional
+    public struct PositionalAdd(u64, u64) // add u64
+
+    // struct field missing
+    public struct PositionalRemove(u64, u64, u64) // remove u64
+
+    // struct field mismatch
+    public struct PositionalChange(u32, u32) // change second u32 to u64
+
+    // add named to empty positional struct
+    public struct PositionalAddNamed() // change to named { a: u64 }
+
+    // change positional to named
+    public struct PositionalToNamed(u64) // change to named { a: u64 }
+
+    // change positional to named and change type
+    public struct PositionalToNamedAndChangeType(u32) // change to named { a: u64 }
+
+    public struct ChangeFieldA {
+        a: u32,
+    }
+
+    public struct ChangeFieldB {
+        b: u32,
+    }
+
+    // change positional nested struct
+    public struct ChangePositionalStruct (ChangeFieldA) // change to ChangeFieldB
+
+    // change named nested struct
+    public struct ChangeNameNestedStruct {
+        a: ChangeFieldA, // change to ChangeFieldB
+    }
+}
diff --git a/...es/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move b/...es/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move
@@ -13,15 +13,27 @@ module upgrades::upgrades {
     public struct RemoveMultipleAbilities has drop {} // remove copy, store
     public struct AddMultipleAbilities has drop, copy {}
 
-    // field mismatch
+
+    // add field to empty struct
+    public struct AddFieldToEmpty {
+        a: u64,
+    }
+
+    // add field
     public struct AddField {
         a: u64,
-        b: u64,
+        b: u64, // added b
     }
-    // remove field
+
+    // remove field from struct with one field
+    public struct RemoveOnlyField {
+        // removed a: u64,
+    }
+
+    // remove field from struct with multiple fields
     public struct RemoveField {
         a: u64,
-        // b removed here
+        // removed b: u64,
     }
 
     // change field name
@@ -35,5 +47,47 @@ module upgrades::upgrades {
         a: u64,
         b: u32, // changed to u32
     }
-}
 
+    // change field name and type
+    public struct ChangeFieldNameAndType {
+        a: u64,
+        c: u32, // changed from b to c and u64 to u32
+    }
+
+    // add positional to empty positional struct
+    public struct EmptyPositionalAdd(u64) // removed the u64
+
+    // struct new positional
+    public struct PositionalAdd(u64, u64, u64) // added a u64
+
+    // struct field missing
+    public struct PositionalRemove(u64, u64) // removed a u64
+
+    // struct field mismatch
+    public struct PositionalChange(u32, u64) // changed second u32 to u64
+
+    // add named to empty positional struct
+    public struct PositionalAddNamed{ a: u64 } // changed to named from empty positional
+
+    // positional to named
+    public struct PositionalToNamed{ a: u64 } // changed to named from positional
+
+    // change positional to named and change type
+    public struct PositionalToNamedAndChangeType{ a: u64 } // changed to named from positional and changed type to u64
+
+    public struct ChangeFieldA {
+        a: u32,
+    }
+
+    public struct ChangeFieldB {
+        b: u32,
+    }
+
+    // change positional nested struct
+    public struct ChangePositionalStruct (ChangeFieldB) // changed to ChangeFieldB
+
+    // change named nested struct
+    public struct ChangeNameNestedStruct {
+        a: ChangeFieldB, // changed to ChangeFieldB
+    }
+}
diff --git a/...c/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap b/...c/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap
@@ -23,7 +23,7 @@ error[Compatibility E01003]: ability mismatch
 error[Compatibility E01003]: ability mismatch
    ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:17:17
    │
-17 │     public enum EnumAddAndRemoveAbility has copy, store {
+17 │     public enum EnumAddAndRemoveAbility has copy, store { // change drop to store
    │                 ^^^^^^^^^^^^^^^^^^^^^^^ Mismatched abilities: missing 'drop', unexpected 'store'
    │
    = Enums are part of a module's public interface and cannot be changed during an upgrade.
@@ -206,5 +206,99 @@ error[Compatibility E02001]: variant mismatch
    = Enums are part of a module's public interface and cannot be changed during an upgrade.
    = Restore the original enum's variants for enum 'EnumChangeAndAddVariantWithTypes' including the ordering.
 
+error[Compatibility E02001]: variant mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:78:17
+   │
+78 │     public enum EnumChangeAndRemoveVariantWithPositionalTypes {
+   │                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing variant 'C'.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variant 'C' for enum 'EnumChangeAndRemoveVariantWithPositionalTypes' including the ordering.
+
+error[Compatibility E02001]: variant mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:80:9
+   │
+78 │     public enum EnumChangeAndRemoveVariantWithPositionalTypes {
+   │                 --------------------------------------------- Enum definition
+79 │         A(u8),
+80 │         C(u8), // changed to C
+   │         ^^^^^ Mismatched variant name 'C', expected 'B'.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variants for enum 'EnumChangeAndRemoveVariantWithPositionalTypes' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:85:9
+   │
+84 │     public enum EnumChangePositionalType {
+   │                 ------------------------ Enum definition
+85 │         A(u8), // add u8
+   │         ^^^^^ Mismatched variant field count, expected 0, found 1.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:86:9
+   │
+84 │     public enum EnumChangePositionalType {
+   │                 ------------------------ Enum definition
+85 │         A(u8), // add u8
+86 │         B(u16), // to be changed to u16
+   │         ^^^^^^ Mismatched field 'u8' at position 0, expected 'u16'.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:87:9
+   │
+84 │     public enum EnumChangePositionalType {
+   │                 ------------------------ Enum definition
+   ·
+87 │         C(u8), // removed u8
+   │         ^^^^^ Mismatched variant field count, expected 2, found 1.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+   ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:88:9
+   │
+84 │     public enum EnumChangePositionalType {
+   │                 ------------------------ Enum definition
+   ·
+88 │         D, // removed last u8
+   │         ^ Mismatched variant field count, expected 1, found 0.
+   │
+   = Enums are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+    ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:100:9
+    │
+ 99 │     public enum EnumWithPositionalChanged {
+    │                 ------------------------- Enum definition
+100 │         A(ChangeFieldB), // changed to ChangeFieldB
+    │         ^^^^^^^^^^^^^^^ Mismatched field '0x0::upgrades::ChangeFieldA' at position 0, expected '0x0::upgrades::ChangeFieldB'.
+    │
+    = Enums are part of a module's public interface and cannot be changed during an upgrade.
+    = Restore the original enum's variant for enum 'EnumWithPositionalChanged' including the ordering.
+
+error[Compatibility E01004]: field mismatch
+    ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:104:9
+    │  
+103 │       public enum EnumWithNamedChanged {
+    │                   -------------------- Enum definition
+104 │ ╭         A {
+105 │ │             x: ChangeFieldA,
+106 │ │             y: ChangeFieldA,
+107 │ │             z: ChangeFieldB, // changed to ChangeFieldB
+108 │ │         },
+    │ ╰─────────^ Mismatched field 'z: 0x0::upgrades::ChangeFieldA', expected '0x0::upgrades::ChangeFieldB'.
+    │  
+    = Enums are part of a module's public interface and cannot be changed during an upgrade.
+    = Restore the original enum's variant for enum 'EnumWithNamedChanged' including the ordering.
+
 
 Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'compatible'.