Abort inconsistent Ownable2Step update state

Author: Jim8y

src/Neo.SmartContract.Framework/Ownable2Step.cs

@@ -190,9 +190,10 @@ public static void RenounceOwnership()
         /// <remarks>
         /// On a contract update this preserves the current owner. If no owner is stored yet, it
         /// initializes one so contracts adopting <see cref="Ownable2Step"/> during an upgrade do not
-        /// remain ownerless. Note that an in-flight pending transfer is intentionally left untouched
-        /// across an update; operators should cancel any in-flight transfer before upgrading if the
-        /// upgrade changes trust assumptions.
+        /// remain ownerless. If an owner exists but the initialization marker is missing, the update
+        /// aborts instead of silently accepting inconsistent ownership state. Note that an in-flight
+        /// pending transfer is intentionally left untouched across an update; operators should cancel
+        /// any in-flight transfer before upgrading if the upgrade changes trust assumptions.
         /// </remarks>
         protected static void InitializeOwner(object? data, bool update)
         {
@@ -201,11 +202,7 @@ protected static void InitializeOwner(object? data, bool update)
                 if (OwnerInitializedStorage.GetBoolean(ByteString.Empty))
                     return;
 
-                if (GetOwner() is not null)
-                {
-                    OwnerInitializedStorage.Put(ByteString.Empty, true);
-                    return;
-                }
+                ExecutionEngine.Assert(GetOwner() is null, "owner initialization state is inconsistent");
             }
 
             data ??= Runtime.Transaction.Sender;

tests/Neo.SmartContract.Framework.UnitTests/Ownable2StepTest.cs

@@ -299,6 +299,19 @@ public void InitializeOwner_UpdateInitializesWhenOwnerMissing()
         Merge(contract);
     }
 
+    [TestMethod]
+    public void InitializeOwner_UpdateWithUnmarkedOwner_Aborts()
+    {
+        var engine = CreateEngine();
+        var contract = Deploy(engine, out _, out _);
+
+        contract.SetOwnerWithoutInitializedForTest(Bob.Account);
+
+        Assert.ThrowsException<TestException>(() => contract.InitializeForTest(Charlie.Account, true));
+        Assert.AreEqual(Bob.Account, contract.GetOwner());
+        Merge(contract);
+    }
+
     [TestMethod]
     public void InitializeOwner_UpdateDoesNotReinitializeAfterRenounce()
     {
@@ -438,6 +451,12 @@ public static void ClearOwnershipStateForTest()
         Storage.Delete(new byte[] { 0xFD });
         Storage.Delete(new byte[] { 0xFB });
     }
+
+    public static void SetOwnerWithoutInitializedForTest(UInt160 owner)
+    {
+        Storage.Put(new byte[] { 0xFD }, owner);
+        Storage.Delete(new byte[] { 0xFB });
+    }
 }";
 
         var tempFile = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid():N}.cs");
@@ -503,6 +522,9 @@ public abstract class OwnableTwoStepProxy(SmartContractInitialize initialize)
         [DisplayName("clearOwnershipStateForTest")]
         public abstract void ClearOwnershipStateForTest();
 
+        [DisplayName("setOwnerWithoutInitializedForTest")]
+        public abstract void SetOwnerWithoutInitializedForTest(UInt160 owner);
+
         [DisplayName("transferOwnership")]
         public abstract void TransferOwnership(UInt160 newOwner);