From 1f8e36a7ea520dd7a6ed758276248b38f77dd1df Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 13:35:54 +0800 Subject: [PATCH 01/10] fix 3300 --- src/Neo.VM/ReferenceCounter.cs | 15 ++++++++++++--- .../SmartContract/ApplicationEngine.Runtime.cs | 12 +++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/Neo.VM/ReferenceCounter.cs b/src/Neo.VM/ReferenceCounter.cs index be7844dac8..4d025f89f6 100644 --- a/src/Neo.VM/ReferenceCounter.cs +++ b/src/Neo.VM/ReferenceCounter.cs @@ -11,9 +11,11 @@ using Neo.VM.StronglyConnectedComponents; using Neo.VM.Types; +using System; using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; +using Buffer = Neo.VM.Types.Buffer; namespace Neo.VM { @@ -115,7 +117,7 @@ internal int CheckZeroReferred() tracked_items.Remove(item); if (item is CompoundType compound) { - references_count -= compound.SubItemsCount; + DecreaseCounter(compound.SubItemsCount); foreach (StackItem subitem in compound.SubItems) { if (component.Contains(subitem)) continue; @@ -136,7 +138,7 @@ internal int CheckZeroReferred() internal void RemoveReference(StackItem item, CompoundType parent) { - references_count--; + DecreaseCounter(); if (!NeedTrack(item)) return; cached_components = null; item.ObjectReferences![parent].References--; @@ -146,10 +148,17 @@ internal void RemoveReference(StackItem item, CompoundType parent) internal void RemoveStackReference(StackItem item) { - references_count--; + DecreaseCounter(); if (!NeedTrack(item)) return; if (--item.StackReferences == 0) zero_referred.Add(item); } + + private void DecreaseCounter(int count = 1) + { + references_count-=count; + if(references_count<0) throw new InvalidOperationException("Reference counter is negative."); + + } } } diff --git a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs index b39fd55a01..a8ec6ea958 100644 --- a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs +++ b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs @@ -407,14 +407,20 @@ protected internal void SendNotification(UInt160 hash, string eventName, Array s /// /// The hash of the specified contract. It can be set to to get all notifications. /// The notifications sent during the execution. - protected internal NotifyEventArgs[] GetNotifications(UInt160 hash) + protected internal Array GetNotifications(UInt160 hash) { IEnumerable notifications = Notifications; if (hash != null) // must filter by scriptHash notifications = notifications.Where(p => p.ScriptHash == hash); - NotifyEventArgs[] array = notifications.ToArray(); + var array = notifications.ToArray(); if (array.Length > Limits.MaxStackSize) throw new InvalidOperationException(); - return array; + Array notifyArray = new(ReferenceCounter); + foreach (var notify in array) + { + notifyArray.Add(notify.ToStackItem(ReferenceCounter)); + } + + return notifyArray; } /// From c8d9322b8de2f825cf17689fe3e26cdfd37c039e Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 13:38:13 +0800 Subject: [PATCH 02/10] update format --- src/Neo.VM/ReferenceCounter.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Neo.VM/ReferenceCounter.cs b/src/Neo.VM/ReferenceCounter.cs index 4d025f89f6..33f44a2fb7 100644 --- a/src/Neo.VM/ReferenceCounter.cs +++ b/src/Neo.VM/ReferenceCounter.cs @@ -156,8 +156,8 @@ internal void RemoveStackReference(StackItem item) private void DecreaseCounter(int count = 1) { - references_count-=count; - if(references_count<0) throw new InvalidOperationException("Reference counter is negative."); + references_count -= count; + if (references_count < 0) throw new InvalidOperationException("Reference counter is negative."); } } From 81abdd359b9b494e0313398bc3c2cc2153e1c1b0 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 15:03:37 +0800 Subject: [PATCH 03/10] add state subitems to ref counter, with suggestion from DuSmart --- src/Neo/SmartContract/NotifyEventArgs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo/SmartContract/NotifyEventArgs.cs b/src/Neo/SmartContract/NotifyEventArgs.cs index 93c124ea88..6915e1e950 100644 --- a/src/Neo/SmartContract/NotifyEventArgs.cs +++ b/src/Neo/SmartContract/NotifyEventArgs.cs @@ -69,7 +69,7 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) { ScriptHash.ToArray(), EventName, - State + State.OnStack ? State : State.DeepCopy(true) }; } } From ee4c159de863fdaaf20c863c9c2741058ca46917 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 20:30:59 +0800 Subject: [PATCH 04/10] apply hardfork --- src/Neo/Hardfork.cs | 3 +- .../ApplicationEngine.Runtime.cs | 3 +- src/Neo/SmartContract/NotifyEventArgs.cs | 28 ++++++++++++++++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Neo/Hardfork.cs b/src/Neo/Hardfork.cs index 9ef6a63c1c..0214633c1e 100644 --- a/src/Neo/Hardfork.cs +++ b/src/Neo/Hardfork.cs @@ -15,6 +15,7 @@ public enum Hardfork : byte { HF_Aspidochelone, HF_Basilisk, - HF_Cockatrice + HF_Cockatrice, + HF_Domovoi // from https://github.com/neo-project/neo/pull/3290/files } } diff --git a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs index a8ec6ea958..e54529f6d1 100644 --- a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs +++ b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs @@ -417,9 +417,8 @@ protected internal Array GetNotifications(UInt160 hash) Array notifyArray = new(ReferenceCounter); foreach (var notify in array) { - notifyArray.Add(notify.ToStackItem(ReferenceCounter)); + notifyArray.Add(notify.ToStackItem(ReferenceCounter, this)); } - return notifyArray; } diff --git a/src/Neo/SmartContract/NotifyEventArgs.cs b/src/Neo/SmartContract/NotifyEventArgs.cs index 6915e1e950..74440e6639 100644 --- a/src/Neo/SmartContract/NotifyEventArgs.cs +++ b/src/Neo/SmartContract/NotifyEventArgs.cs @@ -66,11 +66,31 @@ public void FromStackItem(StackItem stackItem) public StackItem ToStackItem(ReferenceCounter referenceCounter) { return new Array(referenceCounter) + { + ScriptHash.ToArray(), + EventName, + State + }; + } + + public StackItem ToStackItem(ReferenceCounter referenceCounter, ApplicationEngine engine) + { + if (!engine.IsHardforkEnabled(Hardfork.HF_Domovoi)) { - ScriptHash.ToArray(), - EventName, - State.OnStack ? State : State.DeepCopy(true) - }; + return new Array(referenceCounter) + { + ScriptHash.ToArray(), + EventName, + State.OnStack ? State : State.DeepCopy(true) + }; + } + + return new Array(referenceCounter) + { + ScriptHash.ToArray(), + EventName, + State.OnStack ? State : State.DeepCopy(true) + }; } } } From c723ae16a6f8ab4e93674af68bc0b0079fc994ad Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 21:07:09 +0800 Subject: [PATCH 05/10] format --- src/Neo/SmartContract/NotifyEventArgs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo/SmartContract/NotifyEventArgs.cs b/src/Neo/SmartContract/NotifyEventArgs.cs index 74440e6639..deaa916be6 100644 --- a/src/Neo/SmartContract/NotifyEventArgs.cs +++ b/src/Neo/SmartContract/NotifyEventArgs.cs @@ -72,7 +72,7 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) State }; } - + public StackItem ToStackItem(ReferenceCounter referenceCounter, ApplicationEngine engine) { if (!engine.IsHardforkEnabled(Hardfork.HF_Domovoi)) From dca720b5886454601032a69bafc90b8ae7566d3d Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 22:31:25 +0800 Subject: [PATCH 06/10] my mistake --- src/Neo/SmartContract/NotifyEventArgs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo/SmartContract/NotifyEventArgs.cs b/src/Neo/SmartContract/NotifyEventArgs.cs index deaa916be6..eeb74b7f66 100644 --- a/src/Neo/SmartContract/NotifyEventArgs.cs +++ b/src/Neo/SmartContract/NotifyEventArgs.cs @@ -89,7 +89,7 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter, ApplicationEngin { ScriptHash.ToArray(), EventName, - State.OnStack ? State : State.DeepCopy(true) + State }; } } From 814d3342131cfcc7da0b39c3cce894f2c45a7c08 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 22:31:45 +0800 Subject: [PATCH 07/10] fix hardfork --- src/Neo/SmartContract/NotifyEventArgs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo/SmartContract/NotifyEventArgs.cs b/src/Neo/SmartContract/NotifyEventArgs.cs index eeb74b7f66..257efb3a66 100644 --- a/src/Neo/SmartContract/NotifyEventArgs.cs +++ b/src/Neo/SmartContract/NotifyEventArgs.cs @@ -75,7 +75,7 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) public StackItem ToStackItem(ReferenceCounter referenceCounter, ApplicationEngine engine) { - if (!engine.IsHardforkEnabled(Hardfork.HF_Domovoi)) + if (engine.IsHardforkEnabled(Hardfork.HF_Domovoi)) { return new Array(referenceCounter) { From 0bf9d117c517e7aaea7bf46ca63df9a5b5bf5261 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Fri, 7 Jun 2024 22:56:34 +0800 Subject: [PATCH 08/10] remove negative check --- src/Neo.VM/ReferenceCounter.cs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/Neo.VM/ReferenceCounter.cs b/src/Neo.VM/ReferenceCounter.cs index 33f44a2fb7..be7844dac8 100644 --- a/src/Neo.VM/ReferenceCounter.cs +++ b/src/Neo.VM/ReferenceCounter.cs @@ -11,11 +11,9 @@ using Neo.VM.StronglyConnectedComponents; using Neo.VM.Types; -using System; using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; -using Buffer = Neo.VM.Types.Buffer; namespace Neo.VM { @@ -117,7 +115,7 @@ internal int CheckZeroReferred() tracked_items.Remove(item); if (item is CompoundType compound) { - DecreaseCounter(compound.SubItemsCount); + references_count -= compound.SubItemsCount; foreach (StackItem subitem in compound.SubItems) { if (component.Contains(subitem)) continue; @@ -138,7 +136,7 @@ internal int CheckZeroReferred() internal void RemoveReference(StackItem item, CompoundType parent) { - DecreaseCounter(); + references_count--; if (!NeedTrack(item)) return; cached_components = null; item.ObjectReferences![parent].References--; @@ -148,17 +146,10 @@ internal void RemoveReference(StackItem item, CompoundType parent) internal void RemoveStackReference(StackItem item) { - DecreaseCounter(); + references_count--; if (!NeedTrack(item)) return; if (--item.StackReferences == 0) zero_referred.Add(item); } - - private void DecreaseCounter(int count = 1) - { - references_count -= count; - if (references_count < 0) throw new InvalidOperationException("Reference counter is negative."); - - } } } From 6afd38056678821c6509d1c3448dd5ed7afe8f17 Mon Sep 17 00:00:00 2001 From: Jimmy Date: Sat, 8 Jun 2024 18:46:22 +0800 Subject: [PATCH 09/10] add unit test --- .../ApplicationEngine.Runtime.cs | 5 +++- .../SmartContract/UT_NotifyEventArgs.cs | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs index e54529f6d1..b09035c5e4 100644 --- a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs +++ b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs @@ -398,7 +398,10 @@ protected internal void SendNotification(UInt160 hash, string eventName, Array s Notify?.Invoke(this, notification); notifications ??= new List(); notifications.Add(notification); - CurrentContext.GetState().NotificationCount++; + if (CurrentContext != null) + { + CurrentContext.GetState().NotificationCount++; + } } /// diff --git a/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs b/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs index fdaeb9106e..40af2387a6 100644 --- a/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs +++ b/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs @@ -13,6 +13,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Neo.Network.P2P.Payloads; using Neo.SmartContract; +using Neo.VM.Types; namespace Neo.UnitTests.SmartContract { @@ -27,5 +28,29 @@ public void TestGetScriptContainer() NotifyEventArgs args = new NotifyEventArgs(container, script_hash, "Test", null); args.ScriptContainer.Should().Be(container); } + + + [TestMethod] + public void TestIssue3300() // https://github.com/neo-project/neo/issues/3300 + { + using var engine = ApplicationEngine.Create(TriggerType.Application, null, null, settings: TestProtocolSettings.Default, gas: 1100_00000000); + var ns = new Array(engine.ReferenceCounter); + for (var i = 0; i < 500; i++) + { + ns.Add(""); + }; + + var hash = UInt160.Parse("0x179ab5d297fd34ecd48643894242fc3527f42853"); + engine.SendNotification(hash, "Test", ns); + // This should have being 0, but we have optimized the vm to not clean the reference counter + // unless it is necessary, so the reference counter will be 1000. + // Same reason why its 1504 instead of 504. + Assert.AreEqual(1000, engine.ReferenceCounter.Count); + // This will make a deepcopy for the notification, along with the 500 state items. + engine.GetNotifications(hash); + // With the fix of issue 3300, the reference counter calculates not only + // the notifaction items, but also the subitems of the notification state. + Assert.AreEqual(1504, engine.ReferenceCounter.Count); + } } } From 6529bfa87e88f6a481633fc8cdda0cd2dd6b37cc Mon Sep 17 00:00:00 2001 From: Jimmy Date: Mon, 10 Jun 2024 19:03:05 +0800 Subject: [PATCH 10/10] apply anna's suggestion --- src/Neo/SmartContract/ApplicationEngine.Runtime.cs | 5 +---- tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs | 9 +++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs index b09035c5e4..e54529f6d1 100644 --- a/src/Neo/SmartContract/ApplicationEngine.Runtime.cs +++ b/src/Neo/SmartContract/ApplicationEngine.Runtime.cs @@ -398,10 +398,7 @@ protected internal void SendNotification(UInt160 hash, string eventName, Array s Notify?.Invoke(this, notification); notifications ??= new List(); notifications.Add(notification); - if (CurrentContext != null) - { - CurrentContext.GetState().NotificationCount++; - } + CurrentContext.GetState().NotificationCount++; } /// diff --git a/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs b/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs index 40af2387a6..59afbbb760 100644 --- a/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs +++ b/tests/Neo.UnitTests/SmartContract/UT_NotifyEventArgs.cs @@ -13,6 +13,7 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Neo.Network.P2P.Payloads; using Neo.SmartContract; +using Neo.VM; using Neo.VM.Types; namespace Neo.UnitTests.SmartContract @@ -34,6 +35,14 @@ public void TestGetScriptContainer() public void TestIssue3300() // https://github.com/neo-project/neo/issues/3300 { using var engine = ApplicationEngine.Create(TriggerType.Application, null, null, settings: TestProtocolSettings.Default, gas: 1100_00000000); + using (var script = new ScriptBuilder()) + { + // Build call script calling disallowed method. + script.Emit(OpCode.NOP); + // Mock executing state to be a contract-based. + engine.LoadScript(script.ToArray()); + } + var ns = new Array(engine.ReferenceCounter); for (var i = 0; i < 500; i++) {